Small bug, small fix. Suppose we have a table embedded in a begin-end block. #+begin: aaa :param value | a | b | | a | b | #+end: Suppose we want to add a formula, with C-c = We end up with an incorrect result: #+begin: aaa :param value | a | 33 | | a | b | :param value $2=33 #+end: The fix: in org-table.el, line 2177, change (insert (or (match-string 2) "#+TBLFM:"))) to (insert "#+TBLFM:")) Then we get the correct result: #+begin: aaa :param value | a | 33 | | a | b | #+TBLFM: $2=33 #+end: Why? Because (match-string 2) is supposed to refer to the (looking-at) instruction 7 lines above. But (match-string 2) is in the else branch, which means that (looking-at) failed. Therefore (match-string 2) returns garbage. Thanks to Uwe Brauer for pointing to this bug.
Hi Thierry,
Thanks for this! Looking at the change you suggest, do you know why the
(match-string 2) bit might have been added in the first place? I'm just
wondering if there might be some edge-case adversely affected by this ---
hence trading one bug for another :P
--
Timothy
tbanelwebmin <tbanelwebmin@free.fr> writes:
> Small bug, small fix.
>
> Suppose we have a table embedded in a begin-end block.
>
> #+begin: aaa :param value
> | a | b |
> | a | b |
> #+end:
>
> Suppose we want to add a formula, with C-c =
> We end up with an incorrect result:
>
> #+begin: aaa :param value
> | a | 33 |
> | a | b |
> :param value $2=33
> #+end:
>
> The fix: in org-table.el, line 2177, change
> (insert (or (match-string 2) "#+TBLFM:")))
> to
> (insert "#+TBLFM:"))
>
> Then we get the correct result:
>
> #+begin: aaa :param value
> | a | 33 |
> | a | b |
> #+TBLFM: $2=33
> #+end:
>
> Why? Because (match-string 2) is supposed to refer to the (looking-at)
> instruction 7 lines above. But (match-string 2) is in the else branch,
> which means that (looking-at) failed. Therefore (match-string 2) returns
> garbage.
>
> Thanks to Uwe Brauer for pointing to this bug.
Hi Timothy
I don't know the intention. But the answer may lie in the comment 4
lines above:
;; Don't overwrite TBLFM, we might use text properties to
;; store stuff.
In this case, the intention would be to keep the original "#+TBLFM:"
instead of inserting a fresh new one.
But we are in the else branch of (if (looking-at ...)), which means
there was no "#+TBLFM:". And no text properties to save. Therefore we
may safely remove this (match-string 2).
Thanks Timothy for taking this into account so carefully!
Best
Thierry
Le 21/07/2021 à 12:50, Timothy a écrit :
> Hi Thierry,
>
> Thanks for this! Looking at the change you suggest, do you know why the
> (match-string 2) bit might have been added in the first place? I'm just
> wondering if there might be some edge-case adversely affected by this ---
> hence trading one bug for another :P
>
> --
> Timothy
>
> tbanelwebmin <tbanelwebmin@free.fr> writes:
>
>> Small bug, small fix.
>>
>> Suppose we have a table embedded in a begin-end block.
>>
>> #+begin: aaa :param value
>> | a | b |
>> | a | b |
>> #+end:
>>
>> Suppose we want to add a formula, with C-c =
>> We end up with an incorrect result:
>>
>> #+begin: aaa :param value
>> | a | 33 |
>> | a | b |
>> :param value $2=33
>> #+end:
>>
>> The fix: in org-table.el, line 2177, change
>> (insert (or (match-string 2) "#+TBLFM:")))
>> to
>> (insert "#+TBLFM:"))
>>
>> Then we get the correct result:
>>
>> #+begin: aaa :param value
>> | a | 33 |
>> | a | b |
>> #+TBLFM: $2=33
>> #+end:
>>
>> Why? Because (match-string 2) is supposed to refer to the (looking-at)
>> instruction 7 lines above. But (match-string 2) is in the else branch,
>> which means that (looking-at) failed. Therefore (match-string 2) returns
>> garbage.
>>
>> Thanks to Uwe Brauer for pointing to this bug.
Hi Thierry, tbanelwebmin <tbanelwebmin@free.fr> writes: > I don't know the intention. But the answer may lie in the comment 4 > lines above: > ;; Don't overwrite TBLFM, we might use text properties to > ;; store stuff. > > In this case, the intention would be to keep the original "#+TBLFM:" > instead of inserting a fresh new one. > > But we are in the else branch of (if (looking-at ...)), which means > there was no "#+TBLFM:". And no text properties to save. Therefore we > may safely remove this (match-string 2). Thank you for looking into this, I'm reassured by your inference that this change is safe to make. I'm not really one of the main contribution-acceptors/pushers though, so I'd rather leave this for someone like Nicolas to sign off on. Would you mind bumping this thread in a few weeks if nothing happens? Hope that's not too much of an inconvenience, Timothy. > Le 21/07/2021 à 12:50, Timothy a écrit : >> Hi Thierry, >> >> Thanks for this! Looking at the change you suggest, do you know why the >> (match-string 2) bit might have been added in the first place? I'm just >> wondering if there might be some edge-case adversely affected by this --- >> hence trading one bug for another :P >> >> -- >> Timothy >> >> tbanelwebmin <tbanelwebmin@free.fr> writes: >> >>> Small bug, small fix. >>> >>> Suppose we have a table embedded in a begin-end block. >>> >>> #+begin: aaa :param value >>> | a | b | >>> | a | b | >>> #+end: >>> >>> Suppose we want to add a formula, with C-c = >>> We end up with an incorrect result: >>> >>> #+begin: aaa :param value >>> | a | 33 | >>> | a | b | >>> :param value $2=33 >>> #+end: >>> >>> The fix: in org-table.el, line 2177, change >>> (insert (or (match-string 2) "#+TBLFM:"))) >>> to >>> (insert "#+TBLFM:")) >>> >>> Then we get the correct result: >>> >>> #+begin: aaa :param value >>> | a | 33 | >>> | a | b | >>> #+TBLFM: $2=33 >>> #+end: >>> >>> Why? Because (match-string 2) is supposed to refer to the (looking-at) >>> instruction 7 lines above. But (match-string 2) is in the else branch, >>> which means that (looking-at) failed. Therefore (match-string 2) returns >>> garbage. >>> >>> Thanks to Uwe Brauer for pointing to this bug.
Ok, Timothy, fair enough
Le 21/07/2021 à 17:07, Timothy a écrit :
> Hi Thierry,
>
> tbanelwebmin <tbanelwebmin@free.fr> writes:
>> I don't know the intention. But the answer may lie in the comment 4
>> lines above:
>> ;; Don't overwrite TBLFM, we might use text properties to
>> ;; store stuff.
>>
>> In this case, the intention would be to keep the original "#+TBLFM:"
>> instead of inserting a fresh new one.
>>
>> But we are in the else branch of (if (looking-at ...)), which means
>> there was no "#+TBLFM:". And no text properties to save. Therefore we
>> may safely remove this (match-string 2).
> Thank you for looking into this, I'm reassured by your inference that
> this change is safe to make. I'm not really one of the main
> contribution-acceptors/pushers though, so I'd rather leave this for
> someone like Nicolas to sign off on.
>
> Would you mind bumping this thread in a few weeks if nothing happens?
>
> Hope that's not too much of an inconvenience,
>
> Timothy.
>
>> Le 21/07/2021 à 12:50, Timothy a écrit :
>>> Hi Thierry,
>>>
>>> Thanks for this! Looking at the change you suggest, do you know why the
>>> (match-string 2) bit might have been added in the first place? I'm just
>>> wondering if there might be some edge-case adversely affected by this ---
>>> hence trading one bug for another :P
>>>
>>> --
>>> Timothy
>>>
>>> tbanelwebmin <tbanelwebmin@free.fr> writes:
>>>
>>>> Small bug, small fix.
>>>>
>>>> Suppose we have a table embedded in a begin-end block.
>>>>
>>>> #+begin: aaa :param value
>>>> | a | b |
>>>> | a | b |
>>>> #+end:
>>>>
>>>> Suppose we want to add a formula, with C-c =
>>>> We end up with an incorrect result:
>>>>
>>>> #+begin: aaa :param value
>>>> | a | 33 |
>>>> | a | b |
>>>> :param value $2=33
>>>> #+end:
>>>>
>>>> The fix: in org-table.el, line 2177, change
>>>> (insert (or (match-string 2) "#+TBLFM:")))
>>>> to
>>>> (insert "#+TBLFM:"))
>>>>
>>>> Then we get the correct result:
>>>>
>>>> #+begin: aaa :param value
>>>> | a | 33 |
>>>> | a | b |
>>>> #+TBLFM: $2=33
>>>> #+end:
>>>>
>>>> Why? Because (match-string 2) is supposed to refer to the (looking-at)
>>>> instruction 7 lines above. But (match-string 2) is in the else branch,
>>>> which means that (looking-at) failed. Therefore (match-string 2) returns
>>>> garbage.
>>>>
>>>> Thanks to Uwe Brauer for pointing to this bug.
Hi Thierry,
tbanelwebmin <tbanelwebmin@free.fr> writes:
> Thanks to Uwe Brauer for pointing to this bug.
... and thanks for the analysis and solution, fixed now.
PS: If you can share the solution as a patch, that's easier to track,
test and apply. TIA!
--
Bastien