emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)]
@ 2018-05-19 18:54 Piyush Srivastava
  2018-05-22  9:14 ` Nicolas Goaziou
  0 siblings, 1 reply; 3+ messages in thread
From: Piyush Srivastava @ 2018-05-19 18:54 UTC (permalink / raw)
  To: emacs-orgmode


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)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)]
  2018-05-19 18:54 Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)] Piyush Srivastava
@ 2018-05-22  9:14 ` Nicolas Goaziou
  2018-05-23 14:35   ` Piyush Srivastava
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Goaziou @ 2018-05-22  9:14 UTC (permalink / raw)
  To: Piyush Srivastava; +Cc: emacs-orgmode

Hello,

Piyush Srivastava <piyush.srivastava@tifr.res.in> 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(@-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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)]
  2018-05-22  9:14 ` Nicolas Goaziou
@ 2018-05-23 14:35   ` Piyush Srivastava
  0 siblings, 0 replies; 3+ messages in thread
From: Piyush Srivastava @ 2018-05-23 14:35 UTC (permalink / raw)
  Cc: emacs-orgmode


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 <piyush.srivastava@tifr.res.in> 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(@-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,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-23 14:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-19 18:54 Bug: Bug in handling of named fields in the org spreadsheet [9.1.13 (release_9.1.13)] Piyush Srivastava
2018-05-22  9:14 ` Nicolas Goaziou
2018-05-23 14:35   ` Piyush Srivastava

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).