all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] lisp/org-table.el: Allow named columns on lhs
@ 2023-07-19  2:36 Gavin Downard
  2023-07-19  7:44 ` Ihor Radchenko
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Gavin Downard @ 2023-07-19  2:36 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

This patch does prioritize named columns over named fields, which can
break compatibility in tables with a named column and named field with
the same name. Alternatively, we could prioritize named fields to
preserve compatibility, but since named columns are prioritized on the
rhs, it could be pretty confusing.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-org-table.el-Allow-named-columns-on-lhs.patch --]
[-- Type: text/x-patch, Size: 3840 bytes --]

From c6ebbf02e0cb89839606338e8bbc4032810ea398 Mon Sep 17 00:00:00 2001
From: Gavin Downard <gavin.downard@runbox.com>
Date: Sat, 1 Jul 2023 13:26:46 -0700
Subject: [PATCH] lisp/org-table.el: Allow named columns on lhs

* lisp/org-table.el (org-table-recalculate): Add support for named
columns on the lhs of spreadsheet formulas, prioritizing named columns
over named fields if there is a conflict.
(org-table-edit-formulas): Modify category name to include column
formulas with field formulas.
(org-table-get-stored-formulas): Remove comment mentioning lack of named
columns in the lhs.
* testing/lisp/test-org-table.el (test-org-table/named-column): Add test
case for named columns.
* etc/ORG-NEWS (Spreadsheets now support named columns on the lhs):
Document the change

This change breaks compatibility in tables with a named field and named
column with the same name, when that name is used on the lhs of a formula.
---
 etc/ORG-NEWS                   | 4 ++++
 lisp/org-table.el              | 9 +++++----
 testing/lisp/test-org-table.el | 9 +++++++++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index a4725ae8c..42f39fd45 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -534,6 +534,10 @@ special repeaters ~++~ and ~.+~ are skipped.
 A capture template can target ~(here)~ which is the equivalent of
 invoking a capture template with a zero prefix.
 
+*** Spreadsheets now support named columns on the lhs
+
+Spreadsheet formulas can now use named column references on the lhs.
+
 ** New functions and changes in function arguments
 *** =TYPES= argument in ~org-element-lineage~ can now be a symbol
 
diff --git a/lisp/org-table.el b/lisp/org-table.el
index c5efe8f0c..34b0a562e 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -2253,8 +2253,7 @@ LOCATION is a buffer position, consider the formulas there."
 			((not (match-end 2)) m)
 			;; Is it a column reference?
 			((string-match-p "\\`\\$\\([0-9]+\\|[<>]+\\)\\'" m) m)
-			;; Since named columns are not possible in
-			;; LHS, assume this is a named field.
+			;; This is either a named field or column.
 			(t (match-string 2 string)))))
 		    (rhs (match-string 3 string)))
 		(push (cons lhs rhs) eq-alist)
@@ -2963,7 +2962,9 @@ existing formula for column %s"
 		      (t old-lhs)))))
 	      (if (string-match-p "\\`\\$[0-9]+\\'" lhs)
 		  (push (cons lhs rhs) eqlcol)
-		(push (cons lhs rhs) eqlfield))))
+                (if-let ((named-column (assoc lhs org-table-column-names)))
+                    (push (cons (concat "$" (cdr named-column)) rhs) eqlcol)
+                  (push (cons lhs rhs) eqlfield)))))
 	  (setq eqlcol (nreverse eqlcol))
 	  ;; Expand ranges in lhs of formulas
 	  (setq eqlfield (org-table-expand-lhs-ranges (nreverse eqlfield)))
@@ -3355,7 +3356,7 @@ Parameters get priority."
 	  (sel-win (selected-window))
 	  (titles '((column . "# Column Formulas\n")
 		    (field . "# Field and Range Formulas\n")
-		    (named . "# Named Field Formulas\n"))))
+		    (named . "# Named Field and Named Column Formulas\n"))))
       (org-switch-to-buffer-other-window "*Edit Formulas*")
       (erase-buffer)
       ;; Keep global-font-lock-mode from turning on font-lock-mode
diff --git a/testing/lisp/test-org-table.el b/testing/lisp/test-org-table.el
index 27aeb5ab3..8cd01049a 100644
--- a/testing/lisp/test-org-table.el
+++ b/testing/lisp/test-org-table.el
@@ -2158,6 +2158,15 @@ See also `test-org-table/copy-field'."
 | ! | name |   |
 |   |    1 |   |
 <point>#+TBLFM: @2$3=$name"
+      (org-table-calc-current-TBLFM)
+      (buffer-string))))
+  (should
+   (string-match-p
+    "| +# +| +1 +| +1 +|"
+    (org-test-with-temp-text "
+| ! | lhs | rhs |
+| # |     |   1 |
+<point>#+TBLFM: $lhs=$rhs"
       (org-table-calc-current-TBLFM)
       (buffer-string)))))
 
-- 
2.40.1


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-19  2:36 [PATCH] lisp/org-table.el: Allow named columns on lhs Gavin Downard
@ 2023-07-19  7:44 ` Ihor Radchenko
  2023-07-21 15:15   ` Max Nikulin
  2023-07-22 12:45 ` Max Nikulin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Ihor Radchenko @ 2023-07-19  7:44 UTC (permalink / raw)
  To: Gavin Downard; +Cc: emacs-orgmode

Gavin Downard <gavin.downard@runbox.com> writes:

> This patch does prioritize named columns over named fields, which can
> break compatibility in tables with a named column and named field with
> the same name. Alternatively, we could prioritize named fields to
> preserve compatibility, but since named columns are prioritized on the
> rhs, it could be pretty confusing.

Makes sense.
Do I understand correctly the named columns where not allowed at all in
the left side of the formulas previously?

> This change breaks compatibility in tables with a named field and named
> column with the same name, when that name is used on the lhs of a formula.

Please, document this in the new as well.

> +*** Spreadsheets now support named columns on the lhs
> +
> +Spreadsheet formulas can now use named column references on the lhs.

lhs is not a commonly known abbreviation.

Also, may you document the priority of named columns in 3.5.10 Advanced
features section of the manual?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-19  7:44 ` Ihor Radchenko
@ 2023-07-21 15:15   ` Max Nikulin
  2023-07-21 18:33     ` Gavin Downard
  0 siblings, 1 reply; 20+ messages in thread
From: Max Nikulin @ 2023-07-21 15:15 UTC (permalink / raw)
  To: Gavin Downard; +Cc: emacs-orgmode

On 19/07/2023 14:44, Ihor Radchenko wrote:
> 
> Do I understand correctly the named columns where not allowed at all in
> the left side of the formulas previously?

When I tried table formulas a couple of years ago I was surprised that 
it is not possible to use column names to specify target cells. At first 
glance it should be a great feature. Have you searched the mailing list 
archive whether it was requested earlier? There are might be reasons why 
it has not been implemented. Org spreadsheets are quite complicated.



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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-21 15:15   ` Max Nikulin
@ 2023-07-21 18:33     ` Gavin Downard
  2023-07-22  2:12       ` Max Nikulin
  0 siblings, 1 reply; 20+ messages in thread
From: Gavin Downard @ 2023-07-21 18:33 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode


Max Nikulin <manikulin@gmail.com> writes:
> When I tried table formulas a couple of years ago I was surprised that it is not
> possible to use column names to specify target cells. At first glance it should
> be a great feature. Have you searched the mailing list archive whether it was
> requested earlier? There are might be reasons why it has not been implemented.
> Org spreadsheets are quite complicated.

Surprisingly, I wasn't able to find much. Generally all I could find
talked about hline references , which are similarly unsupported.


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-21 18:33     ` Gavin Downard
@ 2023-07-22  2:12       ` Max Nikulin
  2023-07-22  7:25         ` Ihor Radchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Max Nikulin @ 2023-07-22  2:12 UTC (permalink / raw)
  To: Gavin Downard; +Cc: emacs-orgmode

On 22/07/2023 01:33, Gavin Downard wrote:
> 
> Max Nikulin writes:
>> When I tried table formulas a couple of years ago I was surprised that it is not
>> possible to use column names to specify target cells. At first glance it should
>> be a great feature. Have you searched the mailing list archive whether it was
>> requested earlier? There are might be reasons why it has not been implemented.
>> Org spreadsheets are quite complicated.
> 
> Surprisingly, I wasn't able to find much. Generally all I could find
> talked about hline references , which are similarly unsupported.

Quick search revealed some messages where the feature was mentioned. 
However there are no objections from the developers and maintainers:

https://list.orgmode.org/CAA3095_PFhoSFDZFQSFYB3DEe-PbmRbnHkWzpEfx4ChxLHuz-g@mail.gmail.com/T/#u
Bug: assignment to named column - not working as expected 2020-02-29

https://list.orgmode.org/CAJcAo8sF=2UG0ONvq4SSpRLyKfo-0VJfhsFMUerOJokovCLC2g@mail.gmail.com/T/#u
is this spreadsheet correct? 2012-12-01

https://list.orgmode.org/CALPHr6yH_etZa0=GajRMbu7x-oEJGdAP=3XEveLJnpEf0tQeLw@mail.gmail.com/
Bug: column names in tables cannot be used as target in table formula 
2011-08-19

Perhaps git blame for the manual may give some hints. If the change is 
really old then org.texi (removed at certain moment) should be inspected 
as well.

By the way, the manual needs an update if the suggested patch is applied.



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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-22  2:12       ` Max Nikulin
@ 2023-07-22  7:25         ` Ihor Radchenko
  2023-07-22 13:16           ` Max Nikulin
  0 siblings, 1 reply; 20+ messages in thread
From: Ihor Radchenko @ 2023-07-22  7:25 UTC (permalink / raw)
  To: Max Nikulin; +Cc: Gavin Downard, emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:

> Perhaps git blame for the manual may give some hints. If the change is 
> really old then org.texi (removed at certain moment) should be inspected 
> as well.

This has been added very, very long time ago. org-table has undergone
through major transformations since that time.

I cannot find anything blocking the proposed change.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-19  2:36 [PATCH] lisp/org-table.el: Allow named columns on lhs Gavin Downard
  2023-07-19  7:44 ` Ihor Radchenko
@ 2023-07-22 12:45 ` Max Nikulin
  2023-07-22 18:21   ` Gavin Downard
  2023-07-24 15:17 ` Max Nikulin
  2023-08-29  9:58 ` Ihor Radchenko
  3 siblings, 1 reply; 20+ messages in thread
From: Max Nikulin @ 2023-07-22 12:45 UTC (permalink / raw)
  To: Gavin Downard, emacs-orgmode

On 19/07/2023 09:36, Gavin Downard wrote:
> +++ b/lisp/org-table.el
> @@ -2253,8 +2253,7 @@ LOCATION is a buffer position, consider the formulas there."
>   			((not (match-end 2)) m)
>   			;; Is it a column reference?
>   			((string-match-p "\\`\\$\\([0-9]+\\|[<>]+\\)\\'" m) m)
> -			;; Since named columns are not possible in
> -			;; LHS, assume this is a named field.
> +			;; This is either a named field or column.
>   			(t (match-string 2 string)))))
>   		    (rhs (match-string 3 string)))
>   		(push (cons lhs rhs) eq-alist)

Notice
		   "Double definition `%s=' in TBLFM line, please fix by hand"

below. A bit more code is required to keep this sanity check for named 
columns.

> @@ -2963,7 +2962,9 @@ existing formula for column %s"
>   		      (t old-lhs)))))
>   	      (if (string-match-p "\\`\\$[0-9]+\\'" lhs)
>   		  (push (cons lhs rhs) eqlcol)
> -		(push (cons lhs rhs) eqlfield))))
> +                (if-let ((named-column (assoc lhs org-table-column-names)))

`if-let' is not available in Emacs-26

> +                    (push (cons (concat "$" (cdr named-column)) rhs) eqlcol)
> +                  (push (cons lhs rhs) eqlfield)))))
>   	  (setq eqlcol (nreverse eqlcol))
>   	  ;; Expand ranges in lhs of formulas
>   	  (setq eqlfield (org-table-expand-lhs-ranges (nreverse eqlfield)))



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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-22  7:25         ` Ihor Radchenko
@ 2023-07-22 13:16           ` Max Nikulin
  2023-07-22 13:25             ` Ihor Radchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Max Nikulin @ 2023-07-22 13:16 UTC (permalink / raw)
  To: Org Mode List; +Cc: Gavin Downard

On 22/07/2023 14:25, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>> Perhaps git blame for the manual may give some hints. If the change is
>> really old then org.texi (removed at certain moment) should be inspected
>> as well.
> 
> This has been added very, very long time ago. org-table has undergone
> through major transformations since that time.
> 
> I cannot find anything blocking the proposed change.

I have not found anything valuable as well. So the only conflict is 
named cell vs. named column.

The comment in the code was added in
d85ddfba0 2015-11-26 22:41:14 +0100 Nicolas Goaziou: org-table: Fix 
c651e150cc8fb230fca99dfff27caedfddced8ff

in response to
https://list.orgmode.org/20151126195059.GC13848@scotty.home/T/#u

that was a fix for
c651e150c 2015-11-12 23:12:18 +0100 Nicolas Goaziou: org-table: Fix 
`org-table-get-range' with column formulas

https://list.orgmode.org/CAKzohg+2eU474+rcAzKwjYk2Vcj1p-9zvbYkMqwDhta8WAa02Q@mail.gmail.com/T/#u

I am not familiar with table code enough to track earlier changes.

There are were experiments with last row references as $LR12

dadc9a1af 2008-12-19 18:28:58 +0100 Carsten Dominik: Tables:  Implement 
last-row references.

later replaced by @>

https://list.orgmode.org/A8BC38BE-FAD6-4834-ACD9-8C0D372B4B08@gmail.com/T/#u
Carsten Dominik. Re: Table rows and ranges as LHS of formulas. Wed, 2 
Mar 2011 23:57:32 +0100

3dd474575 2011-03-08 14:49:53 +0100 Carsten Dominik: Tables:  Make @< 
and $< point to row/column 1 in a stable way

Note to the manual was added by
6761dcbbd 2009-02-12 07:12:38 +0100 Carsten Dominik: Minor fixes

The following commit

8237c9ae6 2011-03-01 09:05:56 +0100 Carsten Dominik: Implement table 
formulas that apply to field ranges, fix minor issues

added support of @L last row reference, $LR12 is mentioned as backward 
compatible way to specify a last row column.

So names after $ were used in the past, but only named fields are 
currently supported, so it is not serious reason to disallow column names.


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-22 13:16           ` Max Nikulin
@ 2023-07-22 13:25             ` Ihor Radchenko
  0 siblings, 0 replies; 20+ messages in thread
From: Ihor Radchenko @ 2023-07-22 13:25 UTC (permalink / raw)
  To: Max Nikulin; +Cc: Org Mode List, Gavin Downard

Max Nikulin <manikulin@gmail.com> writes:

>> I cannot find anything blocking the proposed change.
>
> I have not found anything valuable as well. So the only conflict is 
> named cell vs. named column.
> ...

I followed similar trace.

> So names after $ were used in the past, but only named fields are 
> currently supported, so it is not serious reason to disallow column names.

Agree.
Just need to make sure that we document this clearly in the manual.
Some parts of the manual are a ambiguous about column names vs. field
names.

3.5.1 References -> Named references only talks about column names, but
not field named.

3.5.5 Field and range formulas has

‘$NAME=’
     Named field, see *note Advanced features::.

only mentioning filed.

3.5.6 Column formulas says
    The left-hand
    side of a column formula can not be the name of column, it must be the
    numeric column reference or ‘$>’.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-22 12:45 ` Max Nikulin
@ 2023-07-22 18:21   ` Gavin Downard
  2023-07-23  6:50     ` Ihor Radchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Gavin Downard @ 2023-07-22 18:21 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:
> On 19/07/2023 09:36, Gavin Downard wrote:
>> +++ b/lisp/org-table.el
>> @@ -2253,8 +2253,7 @@ LOCATION is a buffer position, consider the formulas there."
>>   			((not (match-end 2)) m)
>>   			;; Is it a column reference?
>>   			((string-match-p "\\`\\$\\([0-9]+\\|[<>]+\\)\\'" m) m)
>> -			;; Since named columns are not possible in
>> -			;; LHS, assume this is a named field.
>> +			;; This is either a named field or column.
>>   			(t (match-string 2 string)))))
>>   		    (rhs (match-string 3 string)))
>>   		(push (cons lhs rhs) eq-alist)
>
> Notice
> 		   "Double definition `%s=' in TBLFM line, please fix by hand"
>
> below. A bit more code is required to keep this sanity check for named columns.
>

Oh, good catch. Specifically, I think this should be caught inside of
`org-table-recalculate', where it catches conflicting direct column
references (eg "$1") and end-relative column references ("$<").
> 	    let* ((rhs (org-table-formula-substitute-names
> 			 (org-table-formula-handle-first/last-rc (cdr eq))))
> 		   (old-lhs (car eq))
> 		   (lhs
> 		    (org-table-formula-handle-first/last-rc
> 		     (cond
> 		      ((string-match "\\`@-?I+" old-lhs)
> 		       (user-error "Can't assign to hline relative reference"))
> 		      ((string-match "\\`\\$[<>]" old-lhs)
> 		       (let ((new (org-table-formula-handle-first/last-rc
> 				   old-lhs)))
> 			 (when (assoc new eqlist)
> 			   (user-error "\"%s=\" formula tries to overwrite \
> existing formula for column %s"
^ right here
> 				       old-lhs
> 				       new))
> 			 new))
> 		      (t old-lhs)))))

Also, this should probably be addressed in a different patch, but the
above code doesn't catch two different end-relative column references
that refer to the same column, such as "$<" and "$>>" in a two-column table.

I have also modified the manual to reflect the addition of named
columns; I'll send an updated patch after I add the check for double definitions.

Max Nikulin <manikulin@gmail.com> writes:
>
> `if-let' is not available in Emacs-26
>

Are you sure? It looks like `if-let' was introduced in Emacs 25.


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-22 18:21   ` Gavin Downard
@ 2023-07-23  6:50     ` Ihor Radchenko
  2023-07-24  3:25       ` Gavin Downard
  0 siblings, 1 reply; 20+ messages in thread
From: Ihor Radchenko @ 2023-07-23  6:50 UTC (permalink / raw)
  To: Gavin Downard; +Cc: Max Nikulin, emacs-orgmode

Gavin Downard <gavin.downard@runbox.com> writes:

>> Notice
>> 		   "Double definition `%s=' in TBLFM line, please fix by hand"
>>
>> below. A bit more code is required to keep this sanity check for named columns.
>>
>
> Oh, good catch. Specifically, I think this should be caught inside of
> `org-table-recalculate', where it catches conflicting direct column
> references (eg "$1") and end-relative column references ("$<").

Do we have any tests covering this part of the code?

>> `if-let' is not available in Emacs-26
>>
>
> Are you sure? It looks like `if-let' was introduced in Emacs 25.

In Emacs 26, we will need (require 'subr-x), but otherwise it is not a
problem to use `if-let'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-23  6:50     ` Ihor Radchenko
@ 2023-07-24  3:25       ` Gavin Downard
  2023-07-24  7:23         ` Ihor Radchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Gavin Downard @ 2023-07-24  3:25 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:
> Do we have any tests covering this part of the code?

Not that I know of. I can add a test case for this specific instance,
but I'm not sure if I should add more comprehensive tests in this patch.
What do you think?

> In Emacs 26, we will need (require 'subr-x), but otherwise it is not a
> problem to use `if-let'.

Oh, I didn't realize that. Sorry about that.


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-24  3:25       ` Gavin Downard
@ 2023-07-24  7:23         ` Ihor Radchenko
  0 siblings, 0 replies; 20+ messages in thread
From: Ihor Radchenko @ 2023-07-24  7:23 UTC (permalink / raw)
  To: Gavin Downard; +Cc: Max Nikulin, emacs-orgmode

Gavin Downard <gavin.downard@runbox.com> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>> Do we have any tests covering this part of the code?
>
> Not that I know of. I can add a test case for this specific instance,
> but I'm not sure if I should add more comprehensive tests in this patch.
> What do you think?

I'd prefer to see tests for the aspects of org-table that are modified
by the patch (if there are no tests available).

org-table is an old, not very readable code. So, we should better ensure
test coverage to avoid breakage. Eyeballing the code is not very
reliable in this case.

>> In Emacs 26, we will need (require 'subr-x), but otherwise it is not a
>> problem to use `if-let'.
>
> Oh, I didn't realize that. Sorry about that.

Nothing to sorry about. You do not even need to do anything.
org-compat is already loaded by org-table and org-compat loads subr-x.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-19  2:36 [PATCH] lisp/org-table.el: Allow named columns on lhs Gavin Downard
  2023-07-19  7:44 ` Ihor Radchenko
  2023-07-22 12:45 ` Max Nikulin
@ 2023-07-24 15:17 ` Max Nikulin
  2023-07-24 20:29   ` Gavin Downard
  2023-08-29  9:58 ` Ihor Radchenko
  3 siblings, 1 reply; 20+ messages in thread
From: Max Nikulin @ 2023-07-24 15:17 UTC (permalink / raw)
  To: Gavin Downard, emacs-orgmode

On 19/07/2023 09:36, Gavin Downard wrote:
> This patch does prioritize named columns over named fields, which can
> break compatibility in tables with a named column and named field with
> the same name.

I have tried the patch. The formula for the named column "$three=" does 
not work.

| ! | one | two | three | four |
|---+-----+-----+-------+------|
| # |   1 |   2 |       |    3 |
#+tblfm: @>$5=$one+$two::@>$three=$one+$two

P.S. Due to @II-like references it would be harder to consistently 
implement named rows for symmetry with named columns...


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-24 15:17 ` Max Nikulin
@ 2023-07-24 20:29   ` Gavin Downard
  2023-07-25 15:01     ` Max Nikulin
  0 siblings, 1 reply; 20+ messages in thread
From: Gavin Downard @ 2023-07-24 20:29 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:
> I have tried the patch. The formula for the named column "$three=" does
> not work.
>
> | ! | one | two | three | four |
> |---+-----+-----+-------+------|
> | # |   1 |   2 |       |    3 |
>
> #+tblfm: @>$5=$one+$two::@>$three=$one+$two

Yeah, I guess my current patch only supports column names in column
formulas. Allowing column names inside of arbitrary references will take
some more comprehensive changes. I'll see what I can do.


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-24 20:29   ` Gavin Downard
@ 2023-07-25 15:01     ` Max Nikulin
  2023-07-26 21:50       ` Gavin Downard
  0 siblings, 1 reply; 20+ messages in thread
From: Max Nikulin @ 2023-07-25 15:01 UTC (permalink / raw)
  To: Gavin Downard; +Cc: emacs-orgmode

On 25/07/2023 03:29, Gavin Downard wrote:
> Max Nikulin writes:
>> I have tried the patch. The formula for the named column "$three=" does
>> not work.
>>
>> | ! | one | two | three | four |
>> |---+-----+-----+-------+------|
>> | # |   1 |   2 |       |    3 |
>> #+tblfm: @>$5=$one+$two::@>$three=$one+$two
> 
> Yeah, I guess my current patch only supports column names in column
> formulas. Allowing column names inside of arbitrary references will take
> some more comprehensive changes. I'll see what I can do.

The regexp for parsing formulas does not allow named references after @. 
I have no idea if other code should be modified as well. I consider it 
as more important than detection of duplicated definitions.

A crazy idea: several columns may have the same name:

|   | Jun sum | Jun count | Jun avg | Jul sum | Jul count | Jul avg |
| ! |         |           | average |         |           | average |
|---+---------+-----------+---------+---------+-----------+---------|
|   |  150    | 10        |         | 200     |        14 |         |
#+tblfm: $average=$-2/$-1

It has no sense for numeric references but with names it can help to 
avoid repeated expressions.


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-25 15:01     ` Max Nikulin
@ 2023-07-26 21:50       ` Gavin Downard
  2023-07-27  7:36         ` Ihor Radchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Gavin Downard @ 2023-07-26 21:50 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:
> The regexp for parsing formulas does not allow named references after @.
> I have no idea if other code should be modified as well. I consider it
> as more important than detection of duplicated definitions.

Yeah, besides changing the regexp, the main thing that will need to be
changed is the interface of `org-table-get-stored-formulas'. Currently,
a field name on the lhs can only exist by itself (that is, it can't be a
part of any other expression, such as a range), so the leading '$' is
removed. So, '$name=1+2' is parsed as '("name" . "1+2")'. This is at
odds with field/column names in the middle of an expression.

I think the best way to do this is to change
`org-table-get-stored-formulas' to not strip the leading '$', and
instead of using '(assoc FIELD-NAME org-table-named-field-locations)' or
'(assoc COL-NAME org-table-column-names)', we can a modified* version of
`org-table-formula-substitute-names' (which is used for field and column
names on the rhs of a formula). That would work as a replacement for the
existent handling of field names that would also work for column names
as we want them.

As a side note, this would allow ranges on the lhs that contain field
names in them:

|   |     1 | 2 |   3 |
| # |       |   |     |
| ^ | begin |   | end |
#+TBLFM: $begin..$end=@1*@1

(*): the issue with `org-table-formula-substitute-names' as it is, is
that it will replace field names with the field /value/, not the field
index. Hopefully modifying the function to replace field names with
their indices won't break anything, so we can use the same function for
both sides of the formula.

> A crazy idea: several columns may have the same name:
>
> |   | Jun sum | Jun count | Jun avg | Jul sum | Jul count | Jul avg |
> | ! |         |           | average |         |           | average |
> |---+---------+-----------+---------+---------+-----------+---------|
> |   |  150    | 10        |         | 200     |        14 |         |
>
> #+tblfm: $average=$-2/$-1
>
> It has no sense for numeric references but with names it can help to
> avoid repeated expressions.

Or, if you want to get really crazy, we can let fields and columns with
the same name share a formula. :)

| ! | Apple budget |  Pear Budget | Orange Budget |         total |
|---+--------------+--------------+---------------+---------------|
| # |            1 |            2 |             3 |             6 |
| # |            4 |            5 |             6 |            15 |
|---+--------------+--------------+---------------+---------------|
| # |            2 |            3 |             5 |            26 |
| ^ | extra-cost-1 | extra-cost-2 |         total | ultimatetotal |
#+TBLFM: $total=vsum($<<..$-1) :: $ultimatetotal=vsum(@I..II) + $-1

But in all seriousness, that does seem like it could be useful. I guess
on the rhs we could give each of those "shared names" the value of an array of
that name's connected columns (or fields). But I think support for that should
probably be added in a separate patch, since it's a bit separate from
the main issue.


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-26 21:50       ` Gavin Downard
@ 2023-07-27  7:36         ` Ihor Radchenko
  0 siblings, 0 replies; 20+ messages in thread
From: Ihor Radchenko @ 2023-07-27  7:36 UTC (permalink / raw)
  To: Gavin Downard; +Cc: Max Nikulin, emacs-orgmode

Gavin Downard <gavin.downard@runbox.com> writes:

> (*): the issue with `org-table-formula-substitute-names' as it is, is
> that it will replace field names with the field /value/, not the field
> index. Hopefully modifying the function to replace field names with
> their indices won't break anything, so we can use the same function for
> both sides of the formula.

This sounds reasonable, but please proceed with care.
If you can, please factor out the common parts into a separate helper
function that we can carefully test (first, without adding new
features).

What I have in mind is a preparatory "refactoring" patch that we will
later supply with new tests.

> Or, if you want to get really crazy, we can let fields and columns with
> the same name share a formula. :)
> ...
> But in all seriousness, that does seem like it could be useful. I guess
> on the rhs we could give each of those "shared names" the value of an array of
> that name's connected columns (or fields). But I think support for that should
> probably be added in a separate patch, since it's a bit separate from
> the main issue.

Yes, please move the discussion of "shared names" to a separate thread.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-07-19  2:36 [PATCH] lisp/org-table.el: Allow named columns on lhs Gavin Downard
                   ` (2 preceding siblings ...)
  2023-07-24 15:17 ` Max Nikulin
@ 2023-08-29  9:58 ` Ihor Radchenko
  2023-08-31 14:19   ` Gavin Downard
  3 siblings, 1 reply; 20+ messages in thread
From: Ihor Radchenko @ 2023-08-29  9:58 UTC (permalink / raw)
  To: Gavin Downard; +Cc: emacs-orgmode

Gavin Downard <gavin.downard@runbox.com> writes:

> This patch does prioritize named columns over named fields, which can
> break compatibility in tables with a named column and named field with
> the same name. Alternatively, we could prioritize named fields to
> preserve compatibility, but since named columns are prioritized on the
> rhs, it could be pretty confusing.

It has been over one month since the last message in this thread.
Gavin, may I know if you are still interested to work on the patch?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


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

* Re: [PATCH] lisp/org-table.el: Allow named columns on lhs
  2023-08-29  9:58 ` Ihor Radchenko
@ 2023-08-31 14:19   ` Gavin Downard
  0 siblings, 0 replies; 20+ messages in thread
From: Gavin Downard @ 2023-08-31 14:19 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:
> It has been over one month since the last message in this thread.
> Gavin, may I know if you are still interested to work on the patch?

Apologies for the lack of communication. I do plan on working on this
patch in the near future.


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

end of thread, other threads:[~2023-08-31 17:18 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-19  2:36 [PATCH] lisp/org-table.el: Allow named columns on lhs Gavin Downard
2023-07-19  7:44 ` Ihor Radchenko
2023-07-21 15:15   ` Max Nikulin
2023-07-21 18:33     ` Gavin Downard
2023-07-22  2:12       ` Max Nikulin
2023-07-22  7:25         ` Ihor Radchenko
2023-07-22 13:16           ` Max Nikulin
2023-07-22 13:25             ` Ihor Radchenko
2023-07-22 12:45 ` Max Nikulin
2023-07-22 18:21   ` Gavin Downard
2023-07-23  6:50     ` Ihor Radchenko
2023-07-24  3:25       ` Gavin Downard
2023-07-24  7:23         ` Ihor Radchenko
2023-07-24 15:17 ` Max Nikulin
2023-07-24 20:29   ` Gavin Downard
2023-07-25 15:01     ` Max Nikulin
2023-07-26 21:50       ` Gavin Downard
2023-07-27  7:36         ` Ihor Radchenko
2023-08-29  9:58 ` Ihor Radchenko
2023-08-31 14:19   ` Gavin Downard

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.