Tabulated list recenter issue

Tabulated list recenter issue

[Ian Dunn]

When sorting a tabulated list, tabulated-list-mode will move the window to keep the current entry at the same line in the window.

If a user has their cursor on the top entry with sort active, then reverses the sort, the other entries seemingly disappear.

You can see this in the package menu if you've got a small archive (I've been using the org archive) or have only a few new packages.  Sort the package list by status (for few new packages) or archive (for a small archive), then reverse the sort.

This isn't a big problem for something like the package menu where there are enough entries to fill a few screens, but I'm using tabulated-list-mode for ENWC, and I don't think I've ever filled a screen with access points.

The code causing the behavior in question is in tabulated-list-print, at lines 400 and 401:

> (when window-line
>   (recenter window-line))

window-line is set at lines 343 and 344:

> (setq window-line
>   (count-screen-lines (window-start) (point)))

My proposed solution is this:

  If moving the window to keep the current entry at the same line would leave blank space at the end of the window, don't move the window, but keep point on the current entry.

Does anyone else have any thoughts on this?

-- 
Ian Dunn

Re: Tabulated list recenter issue

[Ian Dunn]

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


>   If moving the window to keep the current entry at the same line would leave
> blank space at the end of the window, don't move the window, but keep point on
> the current entry.

I made this modification and tried it out on ENWC and the Package List.

I'd like to apply this to master, but, since it will affect the Package List, I want to get feedback on it first.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tabulated-list.el.diff --]
[-- Type: text/x-diff, Size: 1355 bytes --]

diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index b6b49b1bfa..57dca910a8 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -395,10 +395,21 @@ changing `tabulated-list-sort-key'."
     (set-buffer-modified-p nil)
     ;; If REMEMBER-POS was specified, move to the "old" location.
     (if saved-pt
-	(progn (goto-char saved-pt)
-	       (move-to-column saved-col)
-	       (when window-line
-                 (recenter window-line)))
+	(let* ((lines (window-screen-lines))
+               (id (tabulated-list-get-id saved-pt))
+               (entries (if (functionp tabulated-list-entries)
+                            (funcall tabulated-list-entries)
+                          tabulated-list-entries))
+               (position (seq-position (map-keys entries) id))
+               (num-entries (length entries))
+               (remaining-entries (- num-entries position)))
+          (goto-char saved-pt)
+          (move-to-column saved-col)
+          ;; Only recenter if there are enough lines so that there's no empty
+          ;; space below the end of the text
+          (when (and window-line
+                     (> remaining-entries lines))
+            (recenter window-line)))
       (goto-char (point-min)))))
 
 (defun tabulated-list-print-entry (id cols)

[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


--
Ian Dunn

Re: Tabulated list recenter issue

[Eli Zaretskii]

> From: Ian Dunn <dunni@gnu.org>
> Date: Tue, 11 Apr 2017 20:56:31 -0400
> 
> >   If moving the window to keep the current entry at the same line would leave
> > blank space at the end of the window, don't move the window, but keep point on
> > the current entry.
> 
> I made this modification and tried it out on ENWC and the Package List.
> 
> I'd like to apply this to master, but, since it will affect the Package List, I want to get feedback on it first.

This will affect all the features that use tabulated-list, including,
for example, buffer-menu.  Since the use case where this behavior is
somewhat problematic is quite special, I don't think changing the
behavior globally would be a good idea.  Perhaps there should be a
special value of the optional argument REMEMBER-POS that would avoid
recentering under the conditions you describe, and ENWC (and perhaps
package.el as well, if users of package.el would like) should use that
special value?

Re: Tabulated list recenter issue

[Clément Pit-Claudel]

On 2017-04-12 02:03, Eli Zaretskii wrote:
> This will affect all the features that use tabulated-list, including,
> for example, buffer-menu.  Since the use case where this behavior is
> somewhat problematic is quite special, I don't think changing the
> behavior globally would be a good idea. 

Hi Eli,

Are there any cases in which the current behavior is desirable? If not, I think it should be changed ("fixed") globally.

Clément.

Re: Tabulated list recenter issue

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Wed, 12 Apr 2017 02:25:33 -0400
> 
> On 2017-04-12 02:03, Eli Zaretskii wrote:
> > This will affect all the features that use tabulated-list, including,
> > for example, buffer-menu.  Since the use case where this behavior is
> > somewhat problematic is quite special, I don't think changing the
> > behavior globally would be a good idea. 
> 
> Hi Eli,
> 
> Are there any cases in which the current behavior is desirable?

All of them I'm familiar with, IMO.  Having the window-position move
when you change sorting is an annoyance IME.

IOW, I personally like the current behavior a lot.  Of course, all of
my use cases are with lists longer than a single window.

Re: Tabulated list recenter issue

[Eli Zaretskii]

> From: Ian Dunn <dunni@gnu.org>
> Date: Tue, 11 Apr 2017 20:56:31 -0400
> 
> diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
> index b6b49b1bfa..57dca910a8 100644
> --- a/lisp/emacs-lisp/tabulated-list.el
> +++ b/lisp/emacs-lisp/tabulated-list.el
> @@ -395,10 +395,21 @@ changing `tabulated-list-sort-key'."
>      (set-buffer-modified-p nil)
>      ;; If REMEMBER-POS was specified, move to the "old" location.
>      (if saved-pt
> -	(progn (goto-char saved-pt)
> -	       (move-to-column saved-col)
> -	       (when window-line
> -                 (recenter window-line)))
> +	(let* ((lines (window-screen-lines))
> +               (id (tabulated-list-get-id saved-pt))
> +               (entries (if (functionp tabulated-list-entries)
> +                            (funcall tabulated-list-entries)
> +                          tabulated-list-entries))
> +               (position (seq-position (map-keys entries) id))
> +               (num-entries (length entries))
> +               (remaining-entries (- num-entries position)))
> +          (goto-char saved-pt)
> +          (move-to-column saved-col)
> +          ;; Only recenter if there are enough lines so that there's no empty
> +          ;; space below the end of the text
> +          (when (and window-line
> +                     (> remaining-entries lines))
> +            (recenter window-line)))
>        (goto-char (point-min)))))
>  
>  (defun tabulated-list-print-entry (id cols)

Regardless of whether this will be installed by default or as an
option, this code might behave unexpectedly when the entries are not
all of the same height, because AFAIU it counts the entries
disregarding their height.  Also, window-screen-lines might return a
fractional value, and I wonder whether the behavior will be correct
when the number of entries differs from the window height by less than
a whole line.

Re: Tabulated list recenter issue

[Clément Pit-Claudel]

On 2017-04-12 03:15, Eli Zaretskii wrote:
>> Cc: emacs-devel@gnu.org
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Wed, 12 Apr 2017 02:25:33 -0400
>>
>> On 2017-04-12 02:03, Eli Zaretskii wrote:
>>> This will affect all the features that use tabulated-list, including,
>>> for example, buffer-menu.  Since the use case where this behavior is
>>> somewhat problematic is quite special, I don't think changing the
>>> behavior globally would be a good idea. 
>>
>> Hi Eli,
>>
>> Are there any cases in which the current behavior is desirable?
> 
> All of them I'm familiar with, IMO.  Having the window-position move
> when you change sorting is an annoyance IME.
> 
> IOW, I personally like the current behavior a lot.  Of course, all of
> my use cases are with lists longer than a single window.

Wait, but in the very large majority of these cases the patch won't change anything, will it? The only case in which it changes things is when sorting puts your current entry towards the bottom of the list.

Re: Tabulated list recenter issue

[Eli Zaretskii]

> Cc: dunni@gnu.org, emacs-devel@gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Wed, 12 Apr 2017 08:28:40 -0400
> 
> > IOW, I personally like the current behavior a lot.  Of course, all of
> > my use cases are with lists longer than a single window.
> 
> Wait, but in the very large majority of these cases the patch won't change anything, will it? The only case in which it changes things is when sorting puts your current entry towards the bottom of the list.

Yes.  But I also like it when I'm in the last window-full of text.

Re: Tabulated list recenter issue

[Clément Pit-Claudel]

On 2017-04-12 08:44, Eli Zaretskii wrote:
>> Cc: dunni@gnu.org, emacs-devel@gnu.org
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Wed, 12 Apr 2017 08:28:40 -0400
>>
>>> IOW, I personally like the current behavior a lot.  Of course, all of
>>> my use cases are with lists longer than a single window.
>>
>> Wait, but in the very large majority of these cases the patch won't change anything, will it? The only case in which it changes things is when sorting puts your current entry towards the bottom of the list.
> 
> Yes.  But I also like it when I'm in the last window-full of text.

Got it, thanks.

Re: Tabulated list recenter issue

[Eli Zaretskii]

> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Wed, 12 Apr 2017 09:11:28 -0400
> Cc: dunni@gnu.org, emacs-devel@gnu.org
> 
> On 2017-04-12 08:44, Eli Zaretskii wrote:
> >> Cc: dunni@gnu.org, emacs-devel@gnu.org
> >> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> >> Date: Wed, 12 Apr 2017 08:28:40 -0400
> >>
> >>> IOW, I personally like the current behavior a lot.  Of course, all of
> >>> my use cases are with lists longer than a single window.
> >>
> >> Wait, but in the very large majority of these cases the patch won't change anything, will it? The only case in which it changes things is when sorting puts your current entry towards the bottom of the list.
> > 
> > Yes.  But I also like it when I'm in the last window-full of text.
> 
> Got it, thanks.

Btw, in addition to what I proposed, I'm also okay with changing the
default behavior globally, as long as there's an option to get the old
behavior back.  The current behavior exists for the last 6 years, so
if we change it, there should be a way to get it back if someone wants
or needs that.

Re: Tabulated list recenter issue

[Clément Pit-Claudel]

On 2017-04-12 10:06, Eli Zaretskii wrote:
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Wed, 12 Apr 2017 09:11:28 -0400
>> Cc: dunni@gnu.org, emacs-devel@gnu.org
>>
>> On 2017-04-12 08:44, Eli Zaretskii wrote:
>>>> Cc: dunni@gnu.org, emacs-devel@gnu.org
>>>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>>>> Date: Wed, 12 Apr 2017 08:28:40 -0400
>>>>
>>>>> IOW, I personally like the current behavior a lot.  Of course, all of
>>>>> my use cases are with lists longer than a single window.
>>>>
>>>> Wait, but in the very large majority of these cases the patch won't change anything, will it? The only case in which it changes things is when sorting puts your current entry towards the bottom of the list.
>>>
>>> Yes.  But I also like it when I'm in the last window-full of text.
>>
>> Got it, thanks.
> 
> Btw, in addition to what I proposed, I'm also okay with changing the
> default behavior globally, as long as there's an option to get the old
> behavior back.  The current behavior exists for the last 6 years, so
> if we change it, there should be a way to get it back if someone wants
> or needs that.

Great, thanks Eli! This makes complete sense.

Re: Tabulated list recenter issue

[Ian Dunn]

Eli Zaretskii writes:

>
> Btw, in addition to what I proposed, I'm also okay with changing the
> default behavior globally, as long as there's an option to get the old
> behavior back.  The current behavior exists for the last 6 years, so
> if we change it, there should be a way to get it back if someone wants
> or needs that.

If there are people that like the current behavior (obviously you're one
of them), then I agree that my proposed changes should only be optional.

My problem with the current behavior is that when sorting networks in
ENWC, it appears as if many of them disappear when reversing the sort.
It might be obvious that they didn't vanish, but I think of it similar
to narrowing; even though it's relatively easy to notice and reverse, it
may confuse new users.

It makes sense to me to use a local variable set by the inheriting mode
of tabulated-list-mode, similar to tabulated-list-printer or
tabulated-list-format.  That way, uses like ENWC with few expected
entries can use the new behavior that works better for fewer entries,
and uses like the Package List can retain the old behavior if desired.

--
Ian Dunn

Re: Tabulated list recenter issue

[Eli Zaretskii]

> From: Ian Dunn <dunni@gnu.org>
> Cc: Clément Pit-Claudel <cpitclaudel@gmail.com>,
>  emacs-devel@gnu.org
> Date: Wed, 12 Apr 2017 21:10:33 -0400
> 
> If there are people that like the current behavior (obviously you're one
> of them), then I agree that my proposed changes should only be optional.

IME, after enough years have passed over a certain behavior, we have
no good ways of predicting whether someone or something out there
depend on it.  In our "arrogance" (pardon my French) we tried from
time to time to pretend that we do know, and changed the long-standing
behavior in backward-incompatible ways, only to discover later that
someone got hurt or that subtle hard-to-solve bugs emerged.

In general, I think it makes sense, in such a veteran package, to
always leave a "fire escape" for those who must have the old behavior,
as a matter of principle.  We should realize that we can no longer
change Emacs in arbitrary incompatible ways, not after 30-odd years of
history and many veteran users who have certain habits burnt into
their muscle memory.  We should make this part of our development
practice.

Thanks.

Re: Tabulated list recenter issue

[Ian Dunn]

Eli Zaretskii writes:

>> From: Ian Dunn <dunni@gnu.org>
>> Cc: Clément Pit-Claudel <cpitclaudel@gmail.com>,
>>  emacs-devel@gnu.org
>> Date: Wed, 12 Apr 2017 21:10:33 -0400
>>
>> If there are people that like the current behavior (obviously you're one
>> of them), then I agree that my proposed changes should only be optional.
>
> IME, after enough years have passed over a certain behavior, we have
> no good ways of predicting whether someone or something out there
> depend on it.  In our "arrogance" (pardon my French) we tried from
> time to time to pretend that we do know, and changed the long-standing
> behavior in backward-incompatible ways, only to discover later that
> someone got hurt or that subtle hard-to-solve bugs emerged.
>
> In general, I think it makes sense, in such a veteran package, to
> always leave a "fire escape" for those who must have the old behavior,
> as a matter of principle.  We should realize that we can no longer
> change Emacs in arbitrary incompatible ways, not after 30-odd years of
> history and many veteran users who have certain habits burnt into
> their muscle memory.  We should make this part of our development
> practice.
>
> Thanks.

Having been on the receiving end of such changes, I wholeheartedly agree.

--
Ian Dunn