bug#53133: 28.0.90; tabulated-list-sort doesn't work as expected

bug#53133: 28.0.90; tabulated-list-sort doesn't work as expected

[Shingo Tanaka]

Hi,

tabulated-list-sort sometimes doesn't work as expected due to the change between
28.0.90 and 28.0.50.

Here is how to reproduce:
1. Open Emacs from terminal with `emacs --no-init-file'
2. Type M-x list-buffers
3. Type C-x o (go to Buffer Menu)
4. Type C-M-Right 5 times to move on to "Fundamental" in "Mode" column
5. Type S 2 times or more to toggle sorting order
6. You will be able to see that "Size" column is selected as the sorting column
   unexpectedly

Looks like below change from 28.0.50 is releated to this issue, because it
disappears when I just rewind this change.

--- tabulated-list.el.org	2022-01-06 22:32:13.936852142 +0900
+++ tabulated-list.el	2022-01-07 15:15:42.000000000 +0900
@@ -574,7 +581,8 @@
       (when not-last-col
         (when (> pad-right 0) (insert (make-string pad-right ?\s)))
         (insert (propertize
-                 (make-string (- width (min width label-width)) ?\s)
+                 ;; We need at least one space to align correctly.
+                 (make-string (- width (min 1 width label-width)) ?\s)
                  'display `(space :align-to ,next-x))))
       (put-text-property opoint (point) 'tabulated-list-column-name name)
       next-x)))

Hope this will be fixed appropriately in the coming major release.

Regards,
Shingo

bug#53133: 28.0.90; tabulated-list-sort doesn't work as expected

[Eli Zaretskii]

unarchive 48639
reopen 48639
thanks

> Date: Sun, 09 Jan 2022 11:03:27 +0900
> From: Shingo Tanaka <shingo.fg8@gmail.com>
> 
> Here is how to reproduce:
> 1. Open Emacs from terminal with `emacs --no-init-file'
> 2. Type M-x list-buffers
> 3. Type C-x o (go to Buffer Menu)
> 4. Type C-M-Right 5 times to move on to "Fundamental" in "Mode" column
> 5. Type S 2 times or more to toggle sorting order
> 6. You will be able to see that "Size" column is selected as the sorting column
>    unexpectedly
> 
> Looks like below change from 28.0.50 is releated to this issue, because it
> disappears when I just rewind this change.
> 
> --- tabulated-list.el.org	2022-01-06 22:32:13.936852142 +0900
> +++ tabulated-list.el	2022-01-07 15:15:42.000000000 +0900
> @@ -574,7 +581,8 @@
>        (when not-last-col
>          (when (> pad-right 0) (insert (make-string pad-right ?\s)))
>          (insert (propertize
> -                 (make-string (- width (min width label-width)) ?\s)
> +                 ;; We need at least one space to align correctly.
> +                 (make-string (- width (min 1 width label-width)) ?\s)
>                   'display `(space :align-to ,next-x))))
>        (put-text-property opoint (point) 'tabulated-list-column-name name)
>        next-x)))

You are right.

Stefan, I reverted that change and I'm reopening bug#48639.  This
problem is a much more serious regression than what happens sometimes
when text-size is scaled.  We will need to find another fix for that
bug, most probably in one of the hooks used by tabulated-list.el, by
adding the display property only when actually needed.

bug#53133: 28.0.90; tabulated-list-sort doesn't work as expected

[Eli Zaretskii]

> Date: Sun, 09 Jan 2022 12:44:15 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 53133@debbugs.gnu.org
> 
> > --- tabulated-list.el.org	2022-01-06 22:32:13.936852142 +0900
> > +++ tabulated-list.el	2022-01-07 15:15:42.000000000 +0900
> > @@ -574,7 +581,8 @@
> >        (when not-last-col
> >          (when (> pad-right 0) (insert (make-string pad-right ?\s)))
> >          (insert (propertize
> > -                 (make-string (- width (min width label-width)) ?\s)
> > +                 ;; We need at least one space to align correctly.
> > +                 (make-string (- width (min 1 width label-width)) ?\s)
> >                   'display `(space :align-to ,next-x))))
> >        (put-text-property opoint (point) 'tabulated-list-column-name name)
> >        next-x)))
> 
> You are right.
> 
> Stefan, I reverted that change and I'm reopening bug#48639.  This
> problem is a much more serious regression than what happens sometimes
> when text-size is scaled.  We will need to find another fix for that
> bug, most probably in one of the hooks used by tabulated-list.el, by
> adding the display property only when actually needed.

To help debugging the original problem: the 'display' property that
was always present caused move-to-column in tabulated-list-print to
stop at the wrong position, where the tabulated-list-column-name
property had the wrong value.  So either the 'display' property should
be used only when actually needed, or the tabulated-list-column-name
property should be also put on the text covered by the 'display'
property.