Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Remember to cover the basics, that is, what you expected to happen and
what in fact did happen.  You don't know how to make a good report?  See

     https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

I recently installed Org 9.2.1 and encountered a few bugs / issues with
the new implementaion of Org tables:

1. columns do not correctly align when using a combined alignment and
   width cookie

2. shrinking to a specified column width indicates continuation /
   truncation where none exists

3. editing a cell narrower than its specified column width unnecessarily
   expands the entire column.

Several "Invalid function: org-table-with-shrunk-field" errors also show
up in *Messages* during these operations. (it's a macro)

Recipes:

Emacs -Q
(org-version) -> "9.2.1"

1.

Create a new table:

| one                  |
| two is a longer cell |

Add a right alignment override and realign with C-c C-c:

|                  <r> |
|                  one |
| two is a longer cell |

Specify a column width:

|                <r40> |
|                  one |
| two is a longer cell |

and shrink to size with C-c TAB:

| <r40>                                   …|
| one                                     …|
| two is a longer cell                    …|

The column is no longer right aligned. 


2.

In the last table above, continuation / truncation / shrunk cell
characters (…) display even though the column is the full specified
width (40 char in this case) and no cell text is truncated. I expect
continuation to only show when text is actually truncated.

In the example above, something like this (mockup):

|                                    <r40> |
|                                      one |
|                     two is a longer cell |

And in the case of a narrower column, where a cell contains truncated /
hidden text, something like this (mockup):

|           <r15> |
|             one |
| two is a longer…|


3.

Create a new table and shrink to a specified column width with 
C-u C-c TAB:

| <15>           …|
| one            …|
| two is a longer…|

Move point to the second cell and delete the "e" in "one":

| <15>                 |
| on                   |
| two is a longer cell |

The entire column expands, even though this action is not necessary to
perform the edit (annoying if the column contains other cells with text
longer than the window width).


Emacs  : GNU Emacs 26.1 (build 1, x86_64-apple-darwin18.2.0, Carbon Version 158 AppKit 1671.2)
 of 2019-02-08
Package: Org mode version 9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nicolas Goaziou]

Hello,

Nick Helm <nick@tenpoint.co.nz> writes:

> Create a new table:

[...]

> |                <r40> |
> |                  one |
> | two is a longer cell |
>
> and shrink to size with C-c TAB:
>
> | <r40>                                   …|
> | one                                     …|
> | two is a longer cell                    …|
>
> The column is no longer right aligned. 

This is by design, so you can often edit the field without expanding the
column.

> In the last table above, continuation / truncation / shrunk cell
> characters (…) display even though the column is the full specified
> width (40 char in this case) and no cell text is truncated. I expect
> continuation to only show when text is actually truncated.
>
> In the example above, something like this (mockup):
>
> |                                    <r40> |
> |                                      one |
> |                     two is a longer cell |
>
> And in the case of a narrower column, where a cell contains truncated /
> hidden text, something like this (mockup):
>
> |           <r15> |
> |             one |
> | two is a longer…|

I think this is a matter of taste. 

Of course, this is slightly more informative, but I prefer a more
visible "…" character. It might be confusing otherwise, e.g., if you
edit a narrow column, which suddenly expands because a very large column
below.

> Create a new table and shrink to a specified column width with 
> C-u C-c TAB:
>
> | <15>           …|
> | one            …|
> | two is a longer…|
>
> Move point to the second cell and delete the "e" in "one":
>
> | <15>                 |
> | on                   |
> | two is a longer cell |
>
> The entire column expands, even though this action is not necessary to
> perform the edit (annoying if the column contains other cells with text
> longer than the window width).

I cannot reproduce it. It used to be that way in an old implementation
of shrunk columns, but I don't think it is the case anymore.

Regards,

-- 
Nicolas Goaziou

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Thank you for your response.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Nick Helm <nick@tenpoint.co.nz> writes:
>>
>> The column is no longer right aligned. 
>
> This is by design, so you can often edit the field without expanding the
> column.

I'm not sure I follow. Are you saying the ability align cell contents in
a shrunk column has been purposefully removed from 9.2?

If that's the case, it's a significant loss of functionality. This would
mean, for instance, that it's no longer possible to format financial
data with a uniform column width.

>> In the last table above, continuation / truncation / shrunk cell
>> characters (…) display even though the column is the full specified
>> width (40 char in this case) and no cell text is truncated. I expect
>> continuation to only show when text is actually truncated.
>
> I think this is a matter of taste. 
>
> Of course, this is slightly more informative, but I prefer a more
> visible "…" character. It might be confusing otherwise, e.g., if you
> edit a narrow column, which suddenly expands because a very large column
> below.

The choice of continuation character is indeed personal preference, but
the character's presence on non-truncated cells is not. It's misleading
and ambiguous.

Let me try to illustrate with another example. If you shrink this table
with C-c TAB:

| <5>                      |
| one                  two |
| one                      |

you get the following:

| <5> …|
| one …|
| one …|

This is misleading - cell 3 contains no additional content yet the
indicator says it does. It's also ambiguous - it's impossible to
determine whether cell 2 or 3 contains the longer field.

Compare with this, where such information is clearly conveyed:

| <5>  |
| one …|
| one  |

I wouldn't describe this difference as a matter of taste. This is a
feature that previously existed (substitute "=>" for "…" in the table
above and you have the result in 9.1). Has this been removed from 9.2 as
well?

Nick

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nicolas Goaziou]

Hello,

Nick Helm <nick@tenpoint.co.nz> writes:

> If that's the case, it's a significant loss of functionality. This would
> mean, for instance, that it's no longer possible to format financial
> data with a uniform column width.

"significant" may be relative. The feature has been available on master
for months, and this went unnoticed.

Anyway, I changed the algorithm, so shrinking should now obey to
alignment. Thank you for the feedback.

> Let me try to illustrate with another example. If you shrink this table
> with C-c TAB:
>
> | <5>                      |
> | one                  two |
> | one                      |
>
> you get the following:
>
> | <5> …|
> | one …|
> | one …|
>
> This is misleading - cell 3 contains no additional content yet the
> indicator says it does. It's also ambiguous - it's impossible to
> determine whether cell 2 or 3 contains the longer field.
>
> Compare with this, where such information is clearly conveyed:
>
> | <5>  |
> | one …|
> | one  |

This is not misleading. The "…" characters means the /column/ as a whole
is shrunk, not that an individual field is. Luckily, you can easily use,
for example, `C-TAB` for the rare case the situation requires
disambiguation.

> Has this been removed from 9.2 as well?

Yes, it has.

Regards,

-- 
Nicolas Goaziou

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Anyway, I changed the algorithm, so shrinking should now obey to
> alignment. Thank you for the feedback.

Thank you – this is a welcome improvement. 

>> | <5> …|
>> | one …|
>> | one …|
>>
>> This is misleading - cell 3 contains no additional content yet the
>> indicator says it does. It's also ambiguous - it's impossible to
>> determine whether cell 2 or 3 contains the longer field.
>>
>> Compare with this, where such information is clearly conveyed:
>>
>> | <5>  |
>> | one …|
>> | one  |
>
> This is not misleading. The "…" characters means the /column/ as a whole
> is shrunk, not that an individual field is. 

OK, I see the value of indicating the column state like this when it's
shrunk to a width of 1 character and no cell content is visible (which,
BTW, is an great new feature).

But, the rest of the time, can't the indicator serve both purposes -
indicate a shrunken column and the presence of truncated cells - when it
is limited to places where text is hidden?

Granted, it might be less visible and it wouldn't appear on cells that
truncate only whitespace when shrunk, but IMO those are minor downsides.

>> Has this been removed from 9.2 as well?
>
> Yes, it has.

If I can't convince you to restore the default, would you consider
adding the previous behaviour as a configurable option?

Nick

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nicolas Goaziou]

Hello,

Nick Helm <nick@tenpoint.co.nz> writes:

> But, the rest of the time, can't the indicator serve both purposes -
> indicate a shrunken column and the presence of truncated cells - when it
> is limited to places where text is hidden?

I find it confusing -- you may actually miss the information that the
current column is shrunk -- and not terribly useful. In particular, you
can tweak the column width, and, more importantly, expand, and shrink
again, the column quickly.

> If I can't convince you to restore the default, would you consider
> adding the previous behaviour as a configurable option?

I'm not even convinced there's a real use-case for this behaviour that
cannot be solved otherwise. Therefore, I don't think such a variable is
needed. However, I suggest to try the new behaviour first.

Regards,

-- 
Nicolas Goaziou

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Nick Helm <nick@tenpoint.co.nz> writes:
>
>> But, the rest of the time, can't the indicator serve both purposes -
>> indicate a shrunken column and the presence of truncated cells - when it
>> is limited to places where text is hidden?
>
> I find it confusing -- you may actually miss the information that the
> current column is shrunk -- and not terribly useful. In particular, you
> can tweak the column width, and, more importantly, expand, and shrink
> again, the column quickly.

I guess we use org in quite different ways. 

>> If I can't convince you to restore the default, would you consider
>> adding the previous behaviour as a configurable option?
>
> I'm not even convinced there's a real use-case for this behaviour that
> cannot be solved otherwise. Therefore, I don't think such a variable is
> needed. 

That's a shame.

Could you tell me what functions govern this feature? 

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nicolas Goaziou]

Nick Helm <nick@tenpoint.co.nz> writes:

> That's a shame.

Feel free to demonstrate a practical use-case, if you want to.

> Could you tell me what functions govern this feature? 

You may want to have a look into `org-table--shrink-field' and
`org-table--make-shrinking-overlay'.

Regards,

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Nick Helm <nick@tenpoint.co.nz> writes:
>
>> Could you tell me what functions govern this feature? 
>
> You may want to have a look into `org-table--shrink-field' and
> `org-table--make-shrinking-overlay'.

Great, got it sorted now. Thanks again for your time.

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Eric S Fraga]

On Tuesday, 19 Feb 2019 at 03:16, Nick Helm wrote:

[...]

> Great, got it sorted now. Thanks again for your time.

It would be great, for others that may be interested, if you could post
your solution to this list.

thanks,
-- 
Eric S Fraga via Emacs 27.0.50, Org release_9.2-193-ge7901c

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Eric S Fraga <esflists@gmail.com> writes:

> On Tuesday, 19 Feb 2019 at 03:16, Nick Helm wrote:
>
> [...]
>
>> Great, got it sorted now. Thanks again for your time.
>
> It would be great, for others that may be interested, if you could post
> your solution to this list.

Sure, patch below. It's a bit crude, but it works for 9.2.1-dist (needs
changes for today's master):

--- a/lisp/org-table.el	2019-02-19 14:06:13.000000000 +1300
+++ b/lisp/org-table.el	2019-02-19 14:07:58.000000000 +1300
@@ -3938,7 +3938,7 @@
     (list (org-table--make-shrinking-overlay
 	   start end
 	   (concat (make-string (max 0 (1+ width)) ?-)
-		   org-table-shrunk-column-indicator)
+		   "-")
 	   "")))
    (t
     ;; If the field is not empty, consider using two overlays: one for
@@ -3962,7 +3962,7 @@
 		  ;; white space characters to the right overlay.
 		  (org-table--make-shrinking-overlay
 		   (1- end) end (concat (make-string (- width w) ?\s)
-					org-table-shrunk-column-indicator)
+					" ")
 		   contents)
 		;; Find cut location so that WIDTH characters are visible.
 		(org-table--make-shrinking-overlay
@@ -3979,7 +3979,10 @@
 			   ((pred (< width)) (setq upper mean))
 			   (_ (setq lower mean)))))
 		     upper))
-		 end org-table-shrunk-column-indicator contents)))))
+		 end (if (> (length contents) width)
+               org-table-shrunk-column-indicator
+             " ")
+       contents)))))
       (delq nil (list pre-overlay post-overlay))))))
 
 (defun org-table--read-column-selection (select max)

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Anyway, I changed the algorithm, so shrinking should now obey to
> alignment. 

By the way, I think this change may have introduced a new bug. 

When the specified column width is one or two characters wider than the
longest cell in the column (26 and 27 char in the example below), the
shrunk column indicator and right-hand table borders do not draw
correctly.

For example, shrink the following with C-c TAB:

| <26>                      |
| this cell is 25 char long |
|                           |

and you get:

| <26>                      …|
| this cell is 25 char long …|
|                           |

One char wider gives:

| <27>                       …|
| this cell is 25 char long  …|
|                           …

org-version -> org-9.2.1-252-g3a106a

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nicolas Goaziou]

Hello,

Nick Helm <nick@tenpoint.co.nz> writes:

> When the specified column width is one or two characters wider than the
> longest cell in the column (26 and 27 char in the example below), the
> shrunk column indicator and right-hand table borders do not draw
> correctly.
>
> For example, shrink the following with C-c TAB:
>
> | <26>                      |
> | this cell is 25 char long |
> |                           |
>
> and you get:
>
> | <26>                      …|
> | this cell is 25 char long …|
> |                           |
>
> One char wider gives:
>
> | <27>                       …|
> | this cell is 25 char long  …|
> |                           …

Fixed. Thank you.

Regards,

-- 
Nicolas Goaziou

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Nick Helm]

Eric S Fraga <esflists@gmail.com> writes:

> It would be great, for others that may be interested, if you could post
> your solution to this list.

Here's an alternative way to achieve this with today's master.

--- a/lisp/org-table.el	2019-02-21 21:28:34.000000000 +1300
+++ b/lisp/org-table.el	2019-02-21 22:17:04.000000000 +1300
@@ -3945,7 +3945,8 @@
 	(when (org-table--shrunk-field) (push column shrunk)))
       (nreverse shrunk))))
 
-(defun org-table--make-shrinking-overlay (start end display field &optional pre)
+(defun org-table--make-shrinking-overlay (start end display field
+                                          &optional pre indicator)
   "Create an overlay to shrink text between START and END.
 
 Use string DISPLAY instead of the real text between the two
@@ -3955,8 +3956,9 @@
 
 When optional argument PRE is non-nil, assume the overlay is
 located at the beginning of the field, and prepend
-`org-table-separator-space' to it.  Otherwise, concatenate
-`org-table-shrunk-column-indicator' at its end.
+`org-table-separator-space' to it. Otherwise, concatenate
+optional string INDICATOR at its end. If INDICATOR is omitted or
+nil, `org-table-shrunk-column-indicator' is used instead.
 
 Return the overlay."
   (let ((show-before-edit
@@ -3965,7 +3967,8 @@
 	   ;; same column.
 	   (mapc #'delete-overlay
 		 (cdr (overlay-get o 'org-table-column-overlays)))))
-	(o (make-overlay start end)))
+	(o (make-overlay start end))
+   (indicator (or indicator org-table-shrunk-column-indicator)))
     (overlay-put o 'insert-behind-hooks (list show-before-edit))
     (overlay-put o 'insert-in-front-hooks (list show-before-edit))
     (overlay-put o 'modification-hooks (list show-before-edit))
@@ -3975,7 +3978,7 @@
     ;; See `org-table-overlay-coordinates'.
     (overlay-put o 'priority 1)
     (let ((d (if pre (concat org-table-separator-space display)
-	       (concat display org-table-shrunk-column-indicator))))
+	       (concat display indicator))))
       (org-overlay-display o d 'org-table t))
     o))
 
@@ -4014,10 +4017,11 @@
    ((org-table--shrunk-field) nil)	;already shrunk
    ((= 0 width)				;shrink to one character
     (list (org-table--make-shrinking-overlay
-	   start end "" (if (eq 'hline contents) "" contents))))
+	   start end "" (if (eq 'hline contents) "" contents)
+      nil org-table-shrunk-column-indicator)))
    ((eq contents 'hline)
     (list (org-table--make-shrinking-overlay
-	   start end (make-string (1+ width) ?-) "")))
+	   start end (make-string (1+ width) ?-) ""  nil "-")))
    ((equal contents "")			;no contents to hide
     (list
      (let ((w (org-string-width (buffer-substring start end)))
@@ -4026,8 +4030,11 @@
 	   (full (+ 2 width)))
        (if (<= w full)
 	   (org-table--make-shrinking-overlay
-	    (1- end) end (make-string (- full w) ?\s) "")
-	 (org-table--make-shrinking-overlay (- end (- w full) 1) end "" "")))))
+	    (1- end) end (make-string (- full w) ?\s)
+       "" nil org-table-separator-space)
+	 (org-table--make-shrinking-overlay
+     (- end (- w full) 1) end "" ""
+     nil org-table-separator-space)))))
    (t
     ;; If the field is not empty, display exactly WIDTH characters.
     ;; It can mean to partly hide the field, or extend it with virtual
@@ -4048,7 +4055,8 @@
 	(let ((pre
 	       (and (> lead 0)
 		    (org-table--make-shrinking-overlay
-		     start (+ start lead) "" contents t)))
+		     start (+ start lead) "" contents
+           t org-table-shrunk-column-indicator)))
 	      (post
 	       (org-table--make-shrinking-overlay
 		;; Find cut location so that WIDTH characters are
@@ -4069,7 +4077,7 @@
 			  ((pred (< width)) (setq upper mean))
 			  (_ (setq lower mean)))))
 		    upper))
-		end "" contents)))
+		end "" contents nil org-table-shrunk-column-indicator)))
 	  (if pre (list pre post) (list post))))
        ;; Contents fit it WIDTH characters.  First compute number of
        ;; white spaces needed on each side of contents, then expand or
@@ -4091,24 +4099,28 @@
 		  ((or (guard (= lead 0)) (pred (= before))) nil)
 		  ((pred (< before))
 		   (org-table--make-shrinking-overlay
-		    start (+ start (- lead before)) "" contents t))
+		    start (+ start (- lead before))
+          "" contents t org-table-separator-space))
 		  (_
 		   (org-table--make-shrinking-overlay
 		    start (1+ start)
 		    (make-string (- before (1- lead)) ?\s)
-		    contents t))))
+		    contents t org-table-separator-space))))
 	       (post
 		(pcase (1- trail)
 		  ((pred (= after))
-		   (org-table--make-shrinking-overlay (1- end) end "" contents))
+		   (org-table--make-shrinking-overlay
+          (1- end) end "" contents
+          nil org-table-separator-space))
 		  ((pred (< after))
 		   (org-table--make-shrinking-overlay
-		    (+ after (- end trail)) end "" contents))
+		    (+ after (- end trail)) end "" contents
+          nil org-table-separator-space))
 		  (_
 		   (org-table--make-shrinking-overlay
 		    (1- end) end
 		    (make-string (- after (1- trail)) ?\s)
-		    contents)))))
+		    contents nil org-table-separator-space)))))
 	  (if pre (list pre post) (list post)))))))))
 
 (defun org-table--read-column-selection (select max)

Re: Bug: Org 9.2.1 table issues [9.2.1 (9.2.1-dist @ /Users/nick/.emacs.d/lisp/org-9.2.1/)]

[Eric S Fraga]

On Thursday, 21 Feb 2019 at 20:42, Nick Helm wrote:
> Eric S Fraga <esflists@gmail.com> writes:
>
>> It would be great, for others that may be interested, if you could post
>> your solution to this list.
>
> Here's an alternative way to achieve this with today's master.

Thank you.
-- 
Eric S Fraga via Emacs 27.0.50, Org release_9.2.1-258-g02eea8