Re: [O] Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)]

[Piyush Srivastava]

Thank you.  I will get back to creating the patch this weekend and will
include your suggested change.

Best regards,
-- Piyush.



Nicolas Goaziou writes:

> Hello,
>
> Piyush Srivastava <address@hidden> writes:
>
>> In the current org mode version (release_9.1.13), when a named cell
>> formula is stored using C-u C-c =, the #+TBLFM line stores the name of
>> the cell *without* a prefix $ sign, as in the following table (notice
>> that in the #+TBLFM row, there is no $ sign before the 'sum' variable):
>>
>> |   |   c |
>> |---+-----|
>> |   |   3 |
>> |   |  10 |
>> |---+-----|
>> | # |  14 |
>> | ^ | sum |
>> |---+-----|
>>
>> #+TBLFM: sum=vsum(@address@hidden)
>>
>>
>> With this convention, pressing TAB in the row after making a change in
>> the table does not update the field referred to by $sum, contrary to
>> what is said in the manual.
>>
>>
>> However, if I manually add a prefix $ to the variable sum in the #+TBLFM
>> row (as in the table below), then TAB update work as described in the
>> manual (notice the $sum variable in the #+TBLFM row):
>>
>>
>> |   |   c |
>> |---+-----|
>> |   |   3 |
>> |   |  12 |
>> |---+-----|
>> | # |  15 |
>> | ^ | sum |
>> |---+-----|
>>
>> #+TBLFM: $sum=vsum(@address@hidden)
>>
>>
>> A fix is to ensure that if the LHS of a field formula being inserted is
>> a field name rather than a numerical reference, then a '$' should be
>> prefixed to it.  I have the following patch that implements this:
>>
>>
>> diff --git a/lisp/org-table.el b/lisp/org-table.el
>> --- a/lisp/org-table.el
>> +++ b/lisp/org-table.el
>> @@ -2301,7 +2301,13 @@ LOCATION instead."
>>         (org-indent-line)
>>         (insert (or (match-string 2) "#+TBLFM:")))
>>        (insert " "
>> -             (mapconcat (lambda (x) (concat (car x) "=" (cdr x)))
>> +             (mapconcat (lambda (x)
>> +                          (let* ((name (car x))
>> +                                 (first (substring-no-properties name 0 1))
>> +                                 (test (or (string= first "@") (string= 
>> first "$")))
>> +                                 )
>> +                            (concat (if test name (concat "$" name))
>> +                                    "=" (cdr x))))
>>                          (sort alist #'org-table-formula-less-p)
>>                          "::")
>>               "\n"))))
>
> You're right.
>
> I suggest the following, though:
>
>               (mapconcat (lambda (x)
>                          (pcase-let ((`(,lhs . ,rhs) x))
>                            (format "%s=%s"
>                                    (if (string-match-p "address@hidden" lhs) 
> lhs
>                                      ;; Named fields are stored
>                                      ;; without the "$" prefix.
>                                      (concat "$" lhs))
>                                    rhs)))
>                        (sort alist #'org-table-formula-less-p)
>                        "::")
>
>> I am happy to assign copyright for the patch, if that is needed and if
>> the patch if found suitable, for it to be included in org-mode code.
>
> This is not needed because this patch is a TINYCHANGE. However, starting
> the copyright assignment process is still valuable if you intend to
> provide more patches.
>
> In any case, would you mind providing a patch incorporating the changes
> above with a proper commit message? Also, it might be interesting to add
> a test for it in "test-org-table.el".
>
> Thank you.
>
> Regards,