Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Yu Han Quek]

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

Calling org-table-insert-column in a table with formulas wrongly increments
the column number left of the newly inserted column.

Minimal example:

| 1 | 2 | 3 |
#+TBLFM: $3=$1+$2

With cursor in the `1` cell, call `M-x org-table-insert-column`.

Expected output:

| 1 |   | 2 | 3 |
#+TBLFM: $4=$1+$3

Actual output ($1 wrongly changed to $2):

| 1 |   | 2 | 3 |
#+TBLFM: $4=$2+$3


The bug seems to be in the last 2 lines of the
function org-table-insert-column:

      (org-table-fix-formulas "$" nil (1- col) 1)
      (org-table-fix-formulas "$LR" nil (1- col) 1)

Changing `(1- col)` to `col` solves the issue.


------------------------------------------------------------------------

Emacs  : GNU Emacs 28.0.50 (build 2, x86_64-apple-darwin18.7.0, NS
appkit-1671.60 Version 10.14.6 (Build 18G1012))
 of 2020-01-05
Package: Org mode version 9.3 (release_9.3 @
/usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)

current state:
==============
(setq
 org-src-mode-hook '(org-src-babel-configure-edit-buffer
    org-src-mode-configure-edit-buffer)
 org-link-shell-confirm-function 'yes-or-no-p
 org-metadown-hook '(org-babel-pop-to-session-maybe)
 org-clock-out-hook '(org-clock-remove-empty-clock-drawer)
 org-mode-hook '(#[0 "\300\301\302\303\304$\207"
  [add-hook change-major-mode-hook org-show-all append local]
  5]
#[0 "\300\301\302\303\304$\207"
  [add-hook change-major-mode-hook org-babel-show-result-all
   append local]
  5]
org-babel-result-hide-spec org-babel-hide-all-hashes)
 org-archive-hook '(org-attach-archive-delete-maybe)
 org-confirm-elisp-link-function 'yes-or-no-p
 org-agenda-before-write-hook '(org-agenda-add-entry-text)
 org-metaup-hook '(org-babel-load-in-session-maybe)
 org-bibtex-headline-format-function #[257 "\300 \236A\207" [:title] 3
"\n\n(fn ENTRY)"]
 org-babel-pre-tangle-hook '(save-buffer)
 org-tab-first-hook '(org-babel-hide-result-toggle-maybe
     org-babel-header-arg-expand)
 org-occur-hook '(org-first-headline-recenter)
 org-cycle-hook '(org-cycle-hide-archived-subtrees
org-cycle-show-empty-lines
 org-optimize-window-after-visibility-change)
 org-speed-command-hook '(org-speed-command-activate
 org-babel-speed-command-activate)
 org-confirm-shell-link-function 'yes-or-no-p
 org-link-parameters '(("attachment" :follow org-attach-open-link :export
org-attach-export-link :complete
org-attach-complete-link)
      ("id" :follow org-id-open)
      ("eww" :follow eww :store org-eww-store-link)
      ("rmail" :follow org-rmail-open :store
org-rmail-store-link)
      ("mhe" :follow org-mhe-open :store org-mhe-store-link)
      ("irc" :follow org-irc-visit :store org-irc-store-link
:export org-irc-export)
      ("info" :follow org-info-open :export org-info-export
:store org-info-store-link)
      ("gnus" :follow org-gnus-open :store
org-gnus-store-link)
      ("docview" :follow org-docview-open :export
org-docview-export :store org-docview-store-link)
      ("bibtex" :follow org-bibtex-open :store
org-bibtex-store-link)
      ("bbdb" :follow org-bbdb-open :export org-bbdb-export
:complete org-bbdb-complete-link :store
org-bbdb-store-link)
      ("w3m" :store org-w3m-store-link) ("file+sys")
      ("file+emacs") ("shell" :follow org-link--open-shell)
      ("news" :follow
#[257 "\301\300\302 Q!\207" ["news" browse-url ":"] 5
 "\n\n(fn URL)"]
)
      ("mailto" :follow
#[257 "\301\300\302 Q!\207" ["mailto" browse-url ":"]
 5 "\n\n(fn URL)"]
)
      ("https" :follow
#[257 "\301\300\302 Q!\207" ["https" browse-url ":"]
 5 "\n\n(fn URL)"]
)
      ("http" :follow
#[257 "\301\300\302 Q!\207" ["http" browse-url ":"] 5
 "\n\n(fn URL)"]
)
      ("ftp" :follow
#[257 "\301\300\302 Q!\207" ["ftp" browse-url ":"] 5
 "\n\n(fn URL)"]
)
      ("help" :follow org-link--open-help)
      ("file" :complete org-link-complete-file)
      ("elisp" :follow org-link--open-elisp)
      ("doi" :follow org-link--open-doi))
 org-link-elisp-confirm-function 'yes-or-no-p
 )

[-- Attachment #2: Type: text/html, Size: 5402 bytes --]

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Marco Wahl]

Yu Han Quek <qythium@gmail.com> writes:

> Calling org-table-insert-column in a table with formulas wrongly increments
> the column number left of the newly inserted column.
>
> Minimal example:
>
> | 1 | 2 | 3 |
>
> #+TBLFM: $3=$1+$2
>
> With cursor in the `1` cell, call `M-x org-table-insert-column`.
>
> Expected output:
>
> | 1 |   | 2 | 3 |
>
> #+TBLFM: $4=$1+$3
>
> Actual output ($1 wrongly changed to $2):
>
> | 1 |   | 2 | 3 |
>
> #+TBLFM: $4=$2+$3
>
> The bug seems to be in the last 2 lines of the
> function org-table-insert-column:
>
>       (org-table-fix-formulas "$" nil (1- col) 1)
>       (org-table-fix-formulas "$LR" nil (1- col) 1)
>
> Changing `(1- col)` to `col` solves the issue.

Thanks.  I committed your fix with the TINYCHANGE marker.

I find it slightly suspicious that the documentation says

#v+
‘M-S-<RIGHT>’ (‘org-table-insert-column’)
     Insert a new column to the left of point position.
#v-

But actually the new column goes to the right and this is also fused by
the tests.  Has there been a stealth shift to the right?  Anyway, I
guess the documentation text should be updated as well.

Any objections or comments about this?


Ciao,
-- Marco

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Kyle Meyer]

Marco Wahl <marcowahlsoft@gmail.com> writes:

> I find it slightly suspicious that the documentation says
>
> #v+
> ‘M-S-<RIGHT>’ (‘org-table-insert-column’)
>      Insert a new column to the left of point position.
> #v-
>
> But actually the new column goes to the right and this is also fused by
> the tests.  Has there been a stealth shift to the right?

It looks like the documentation became stale with b8d473a04 (org-table:
Insert new column to the right instead of the left, 2017-11-19).  Thanks
for updating it.

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Eric S Fraga]

On Monday, 30 Mar 2020 at 23:22, Kyle Meyer wrote:
> It looks like the documentation became stale with b8d473a04 (org-table:
> Insert new column to the right instead of the left, 2017-11-19).  Thanks
> for updating it.

Annoyingly, I would have preferred to have the column inserted to the
left of the current column as this would be consistent with the
insertion of rows.  It gets me every time I try to insert a column as I
then have to shift columns.  And there's no way to insert a column at
the beginning of the row easily.  Whereas adding a new column to the end
is trivial: type anything after the last |.

Could we leave the documentation as it is and change the behaviour?

Thank you.
-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-438-g5b9698

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Nicolas Goaziou]

Hello,

Eric S Fraga <e.fraga@ucl.ac.uk> writes:

> On Monday, 30 Mar 2020 at 23:22, Kyle Meyer wrote:
>> It looks like the documentation became stale with b8d473a04 (org-table:
>> Insert new column to the right instead of the left, 2017-11-19).  Thanks
>> for updating it.
>
> Annoyingly, I would have preferred to have the column inserted to the
> left of the current column as this would be consistent with the
> insertion of rows.  It gets me every time I try to insert a column as I
> then have to shift columns.  And there's no way to insert a column at
> the beginning of the row easily.  Whereas adding a new column to the end
> is trivial: type anything after the last |.
>
> Could we leave the documentation as it is and change the behaviour?

Well, since I apparently made the change, I have to speak for it. 

When one presses M-S-Right in order to create a new column, I think
a reasonable expectation is :

1. to move point to the right,
2. to move point in the newly created column.

Consequently, a reasonable expectation is to create the column to the
right.

Note that adding a column to the beginning of the row is easy: just type
"|" near the beginning of the first cell.

Does that make sense?

Regards,

-- 
Nicolas Goaziou

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Julius Dittmar]

Am 31.03.20 um 16:05 schrieb Nicolas Goaziou:
> Note that adding a column to the beginning of the row is easy: just type
> "|" near the beginning of the first cell.

Does not seem correct.

I had table
#+begin_src org
| 1 | 2 |
| 3 | 4 |
#+end_src

Adding a | at the beginning of the first cell changed that to:
#+begin_src org
|   | 1 | 2 |
| 3 | 4 |   |
#+end_src

That's not adding a column, just a cell, and breaking columns.

What would I have to type to have that add a column?

TIA,

Julius

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Eric S Fraga]

On Tuesday, 31 Mar 2020 at 16:05, Nicolas Goaziou wrote:
> Hello,
>
> Well, since I apparently made the change, I have to speak for it. 

Thank you.  It's good to discuss!

> When one presses M-S-Right in order to create a new column, I think
> a reasonable expectation is :
>
> 1. to move point to the right,
> 2. to move point in the newly created column.

And I believe that a reasonable expectation is symmetry with inserting a
row.  The way I think of it is the arrow indicates to make space by
moving columns/rows in that direction.  Most spreadsheets work in this
way, in my experience (but I could be wrong).

> Note that adding a column to the beginning of the row is easy: just type
> "|" near the beginning of the first cell.

Unfortunately, this only inserts a cell, not a column.

-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-438-g5b9698

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Nicolas Goaziou]

Eric S Fraga <e.fraga@ucl.ac.uk> writes:

> And I believe that a reasonable expectation is symmetry with inserting a
> row.  The way I think of it is the arrow indicates to make space by
> moving columns/rows in that direction.  Most spreadsheets work in this
> way, in my experience (but I could be wrong).

I understand now. Thanks.

Then I agree with you, it would be better to change the behaviour
instead of the documentation.

> Unfortunately, this only inserts a cell, not a column.

Indeed.

Regards,

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Marco Wahl]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Eric S Fraga <e.fraga@ucl.ac.uk> writes:
>
>> And I believe that a reasonable expectation is symmetry with inserting a
>> row.  The way I think of it is the arrow indicates to make space by
>> moving columns/rows in that direction.  Most spreadsheets work in this
>> way, in my experience (but I could be wrong).
>
> I understand now. Thanks.
>
> Then I agree with you, it would be better to change the behaviour
> instead of the documentation.

Okay, this is all understandable.

For the record: A few days ago I changed the documentation so that it's
in a consistent state with the current code.

Since the change has been in place since 2017 what's the right thing to
do?

- Wait a while for someone speaking out to keep the state as it is now?
  If nobody shows up, just change back to inserting to the left?
  
- Introduce a customizable variable for inserting to the left?

- Add a parameter to org-table-insert-column to control insertion
  left/right and possibly even the number of columns?

- Something else?


Best regards,
-- Marco

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Nicolas Goaziou]

Hello,

Marco Wahl <marcowahlsoft@gmail.com> writes:

> Since the change has been in place since 2017 what's the right thing to
> do?
>
> - Wait a while for someone speaking out to keep the state as it is now?
>   If nobody shows up, just change back to inserting to the left?

That, and documenting it in ORG-NEWS, sounds good.

> - Introduce a customizable variable for inserting to the left?

Please don't.

> - Add a parameter to org-table-insert-column to control insertion
>   left/right and possibly even the number of columns?

`org-table-insert-row' has indeed a parameter to control side of
insertion. It would make sense to have it in `org-table-insert-column',
too. I still think the default should be changed, tho.

Regards,

-- 
Nicolas Goaziou

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Carsten Dominik]

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

Hi,

I just pulled the lates master, and I think the creation of a new column
has not been set back to the way it used to be, even though Nicolas agreed
to do so.  Am I missing something?

Thanks.

Carsten

On Tue, Mar 31, 2020 at 10:10 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Eric S Fraga <e.fraga@ucl.ac.uk> writes:
>
> > And I believe that a reasonable expectation is symmetry with inserting a
> > row.  The way I think of it is the arrow indicates to make space by
> > moving columns/rows in that direction.  Most spreadsheets work in this
> > way, in my experience (but I could be wrong).
>
> I understand now. Thanks.
>
> Then I agree with you, it would be better to change the behaviour
> instead of the documentation.
>
> > Unfortunately, this only inserts a cell, not a column.
>
> Indeed.
>
> Regards,
>
>

[-- Attachment #2: Type: text/html, Size: 1322 bytes --]

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Marco Wahl]

Hi Carsten,

> I just pulled the lates master, and I think the creation of a new column
> has not been set back to the way it used to be, even though Nicolas agreed
> to do so.  Am I missing something?

As I understood, we practice some patience now to see if someone votes
for keeping the current behavior.  And then act according to the
discussion (or non-discussion.)


Best regards,
-- Marco

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Carsten Dominik]

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

Hi Marco,

thank you for the reply.  For the record, I am in favor of the old
workings, as described by Eric.  It is more consistent in several ways.

Carsten

On Wed, Apr 1, 2020 at 7:52 PM Marco Wahl <marcowahlsoft@gmail.com> wrote:

> Hi Carsten,
>
> > I just pulled the lates master, and I think the creation of a new column
> > has not been set back to the way it used to be, even though Nicolas
> agreed
> > to do so.  Am I missing something?
>
> As I understood, we practice some patience now to see if someone votes
> for keeping the current behavior.  And then act according to the
> discussion (or non-discussion.)
>
>
> Best regards,
> -- Marco
>

[-- Attachment #2: Type: text/html, Size: 1045 bytes --]

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Marco Wahl]

Hi,

Carsten Dominik <dominik@uva.nl> writes:

>> > I just pulled the lates master, and I think the creation of a new
>> > column has not been set back to the way it used to be, even though
>> > Nicolas agreed to do so.  Am I missing something?

>> As I understood, we practice some patience now to see if someone votes
>> for keeping the current behavior.  And then act according to the
>> discussion (or non-discussion.)

> For the record, I am in favor of the old workings, as described
> by Eric.  It is more consistent in several ways.

Hereby I declare patience to be over!  AFAICT no one spoke up to keep
the insertion of a new column with org-table-insert-column to the right
of the current column.

I had a look at the issue and I think I can take over the
implementation.  It's not a big deal AFAICS.

Where shall the change go?  maint (and master) or just to master?


Best wishes,
-- Marco

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Nicolas Goaziou]

Hello,

Marco Wahl <marcowahlsoft@gmail.com> writes:

> I had a look at the issue and I think I can take over the
> implementation.  It's not a big deal AFAICS.

Thank you.

> Where shall the change go?  maint (and master) or just to master?

I think maint is fine.

Regards,

-- 
Nicolas Goaziou

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Marco Wahl]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> Where shall the change go?  maint (and master) or just to master?
>
> I think maint is fine.

Okay, pushed.


Thanks,
-- Marco

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Eric S Fraga]

On Saturday, 18 Apr 2020 at 00:51, Marco Wahl wrote:
> Okay, pushed.

Thank you!  Seems to work very well.

Now, I might be pushing my luck so apologies but: it would be nice if
the kill row/column behaviour were also consistent.  Right now, if you
kill a row, it leaves point where it was; if you kill a column, it moves
point to the previous column.  That is, for instance, if point is in row
2 column 2, deleting a row will leave point in row 2, column 2. Deleting
a column moves point to row 2 column 1.  The row killing behaviour seems
more usable so I wonder whether we could have the column behaviour
changed as well?

Thanks again,
eric

-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-492-gc990d4

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Marco Wahl]

Hi,

And thanks, Eric!

Eric S Fraga <e.fraga@ucl.ac.uk> writes:

> Now, I might be pushing my luck so apologies but: it would be nice if
> the kill row/column behaviour were also consistent.  Right now, if you
> kill a row, it leaves point where it was; if you kill a column, it moves
> point to the previous column.  That is, for instance, if point is in row
> 2 column 2, deleting a row will leave point in row 2, column 2. Deleting
> a column moves point to row 2 column 1.  The row killing behaviour seems
> more usable so I wonder whether we could have the column behaviour
> changed as well?

Indeed, why not?  I pushed a respective change.  I introduced a subtlety
when deleting from the rightmost column or from immediately right of the
table.  That was not needed with point moving to the left, as it has
been before.

The change is small, only two lines.  So this could be easily reverted.

Please check it out.


Thanks and best regards,
-- Marco

Re: Bug: org-table-insert-column edits formulas wrongly [9.3 (release_9.3 @ /usr/local/Cellar/emacs-plus/HEAD-9d38564/share/emacs/28.0.50/lisp/org/)]

[Eric S Fraga]

On Saturday, 18 Apr 2020 at 16:30, Marco Wahl wrote:
> Indeed, why not?  I pushed a respective change.  I introduced a subtlety
> when deleting from the rightmost column or from immediately right of the
> table.  That was not needed with point moving to the left, as it has
> been before.

This works very well.  Thank you.  I hope others are okay with this
change.

-- 
: Eric S Fraga via Emacs 28.0.50, Org release_9.3.6-520-ge69937