Re: [PATCH] Fix bookmark-bmenu-list sorting.

[Karl Fogel <kfogel@red-bean.com>]

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

On 07 Mar 2022, Manuel Giraud wrote:
>Karl Fogel <kfogel@red-bean.com> writes:
>
>>>Thanks.
>>>
>>>Let's install this on master.  We can consider backporting to 
>>>the
>>>emacs-28 branch after Emacs 28.1 is released.
>>
>> Installed in commit a6abd06c73.
>
>Thanks. I've tested both of your patch and they are working as 
>intended:
>the order is modified between two `bookmark-bmenu-list' call if I 
>change
>`bookmark-sort-flag'.
>
>There is one minor thing left: the tabulated-list bar (I don't 
>how it is
>called) keep the old information « sorted by bookmark name » when 
>I have
>revert to `bookmark-sort-flag' as nil.

Manuel, could you please give me a more verbose reproduction 
recipe for that last paragraph above?  Maybe even include a 
screenshot?

I've attached an updated patch here (please use this version), and 
when I test it, I don't see anything that says "sorted by bookmark 
name".  I don't know what the tabulated-list bar you're referring 
to is, but I don't see anything that would obviously be it.

(I have tried things like turning `menu-bar-mode' on and off. 
I've also tried looking in the code of 
"lisp/emacs-lisp/tabulated-list.el", but I don't see any place 
where a message "sorted by bookmark name" would be set up.)

Now, there is another related UI issue:

In the tabulated-list header line at the top of the buffer, we 
have the expected three columns: "Bookmark", "Type", and "File". 
The first column, "Bookmark", has a down-triangle / up-triangle 
toggle for switching between descending and ascending sort by 
bookmark name.  But if `bookmark-sort-flag' is nil, then when the 
buffer is regenerated the bookmarks will be listed in 
creation-date order -- and the triangular toggle will just keep 
whichever value the triangle had already.

Maybe the right fix for that is for the buffer to somehow indicate 
*which* property the bookmarks are sorted by -- name or 
creation-date -- and make sure that the triangle always reflects 
the sorting direction of whichever thing is currently being sorted 
on.  However, before I go down that road, I'd like to fully 
understand your initial "one minor thing" bug report above first.

Best regards,
-Karl


[-- Attachment #2: 0001-Ensure-bookmark-bmenu-buffer-sorts-when-it-should.patch --]
[-- Type: text/x-diff, Size: 1913 bytes --]

From c8c00cac4bc16e7fccb228756b825ae022b315a0 Mon Sep 17 00:00:00 2001
From: Karl Fogel <kfogel@red-bean.com>
Date: Thu, 17 Mar 2022 22:47:16 -0500
Subject: [PATCH] Ensure bookmark bmenu buffer sorts when it should
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch is still WIP, as, as Manuel Giraud has pointed out in his
post "Re: [PATCH] Fix bookmark-bmenu-list sorting." of 7 March 2022
(Message-ID: <87zgm25d9o.fsf@elite.giraud>) the following:

  > There is one minor thing left: the tabulated-list bar (I don't how
  > it is called) keep the old information « sorted by bookmark name »
  > when I have revert to `bookmark-sort-flag' as nil.

I'm finding out more details from him; once we have it all sorted out,
I will update this change.
---
 lisp/bookmark.el | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git lisp/bookmark.el lisp/bookmark.el
index 80fb1cdfc7..400a673447 100644
--- lisp/bookmark.el
+++ lisp/bookmark.el
@@ -1819,7 +1819,17 @@ bookmark-bmenu--revert
                        (list location))])
               entries)))
     (tabulated-list-init-header)
-    (setq tabulated-list-entries (reverse entries)))
+    ;; The value of `bookmark-sort-flag' might have changed since the
+    ;; last time the buffer contents were generated, so re-check it.
+    (if bookmark-sort-flag
+        (progn
+          (setq tabulated-list-sort-key '("Bookmark" . nil))
+          (setq tabulated-list-entries entries))
+      (setq tabulated-list-sort-key nil)
+      ;; And since we're not sorting by bookmark name, show bookmarks
+      ;; in reverse order of creation: most recently created at the
+      ;; top, least recently created at the bottom.
+      (setq tabulated-list-entries (reverse entries))))
   (tabulated-list-print t))
 
 ;;;###autoload
-- 
2.35.1