Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Gavin Downard]

Ihor Radchenko  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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Ihor Radchenko]

Gavin Downard  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Ihor Radchenko]

Gavin Downard  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Gavin Downard]

Max Nikulin  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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Max Nikulin]

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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Gavin Downard]

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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Max Nikulin]

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...

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Ihor Radchenko]

Gavin Downard  writes:

> Ihor Radchenko  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Gavin Downard]

Ihor Radchenko  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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Ihor Radchenko]

Gavin Downard  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Gavin Downard]

Max Nikulin  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  writes:
>
> `if-let' is not available in Emacs-26
>

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

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Ihor Radchenko]

Max Nikulin  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Max Nikulin]

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.gc13...@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-9zvbykmqwdhta8waa...@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-8c0d372b4...@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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Max Nikulin]

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)))

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Ihor Radchenko]

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.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Max Nikulin]

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-pbmrbnhkwzpefx4chxlhu...@mail.gmail.com/T/#u
Bug: assignment to named column - not working as expected 2020-02-29

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

https://list.orgmode.org/CALPHr6yH_etZa0=GajRMbu7x-oEJGdAP=3xeveljnpef0tq...@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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Gavin Downard]

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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Max Nikulin]

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.

Re: [PATCH] lisp/org-table.el: Allow named columns on lhs

[Ihor Radchenko]

Gavin Downard  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 .
Support Org development at ,
or support my work at 

[PATCH] lisp/org-table.el: Allow named columns on lhs

[Gavin Downard]

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.

>From c6ebbf02e0cb89839606338e8bbc4032810ea398 Mon Sep 17 00:00:00 2001
From: Gavin Downard 
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 |   |
 #+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 |
+#+TBLFM: $lhs=$rhs"
   (org-table-calc-current-TBLFM)
   (buffer-string)
 
-- 
2.40.1