Re: [O] Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)]
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 Srivastavawrites: > >> 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(@-II$0..@-I$0) >> >> >> 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(@-II$0..@-I$0) >> >> >> 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 "\\`[$@]" 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,
Re: [O] Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)]
Hello, Piyush Srivastavawrites: > 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(@-II$0..@-I$0) > > > 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(@-II$0..@-I$0) > > > 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 "\\`[$@]" 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, -- Nicolas Goaziou
[O] Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)]
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(@-II$0..@-I$0) 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(@-II$0..@-I$0) 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" What this patch does is the following: if the reference being inserted does not begin with an '@' or a '$', then it is assumed to be a named reference, and a '$' is prefixed to it. This will work *provided* no named references themselves start with a '$'. 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. Thanks, -- Piyush. Emacs : GNU Emacs 25.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.26) of 2018-02-09 Package: Org mode version 9.1.13 (release_9.1.13)