[PATCH] Improve sorting in the bookmark list buffer.

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

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

On 07 Mar 2022, Manuel Giraud wrote:
>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 looked more closely at the situation and came up with the 
attached patch.  The new behavior is a little complex, but I think 
it's the right thing for most users:

If `bookmark-sort-flag' is non-nil, which is the default, then the 
bookmark list [1] will be sorted lexically (case-insensitively, 
and in locale-dependent collation order).

If `bookmark-sort-flag' is nil, then the list starts out sorted by 
bookmark creation order, *but* clicking on the sort-control toggle 
for the Bookmark Name column will sort the list lexically (and 
toggling it changes the direction of that lexical sort each time). 
However, you can use "g" (`revert-buffer') to refresh and thus go 
right back to creation-order sort.  Also, if you change the value 
of `bookmark-sort-flag' and regenerate the buffer, via `g' or 
otherwise, the sort will change accordingly, as one would expect.

Note that there is no option to reverse the direction of creation 
order sort: when sorting by creation order, it's always most 
recent bookmark on top to least recent at bottom.  I didn't see 
any clear need for the reverse direction, and having the column 
sort toggle behave in a familiar (lexical) way seemed much more 
important.

Review / feedback welcome.  This is based against current 'master' 
branch (commit 25308a95f8869c), but once it's been through review 
I'll ask Eli which branch it should go on (or Eli if you want to 
just say now that's fine too).

Best regards,
-Karl

[1] That is, the buffer generated by `bookmark-bmenu-list'.


[-- Attachment #2: 0001-Improve-sorting-in-the-bookmark-list-buffer.patch --]
[-- Type: text/plain, Size: 6228 bytes --]

From dd203d5bd936c1771e068a14f4d24a3921be4396 Mon Sep 17 00:00:00 2001
From: Karl Fogel <kfogel@red-bean.com>
Date: Wed, 20 Apr 2022 12:46:26 -0500
Subject: [PATCH] Improve sorting in the bookmark list buffer

  - Ensure that the bookmark bmenu buffer sorts when it should.
  - Sort case-insensitively and by locale-dependent collation order.
  - Rename "Bookmark" column to "Bookmark Name".
  - Coordinate that column's sort toggle and `bookmark-sort-flag'.
  - Document the new behavior.

* lisp/bookmark.el (bookmark-bmenu--name-predicate,
bookmark-bmenu--type-predicate, bookmark-bmenu--file-predicate): Use
`string-collate-lessp' with IGNORE-CASE argument, instead of plain
`string<'.
(bookmark-bmenu--revert): Sort based on `bookmark-sort-flag'.
(bookmark-bmenu-mode): Document the new behavior.   Rename the
"Bookmark" column to "Bookmark Name" for clarity & documentabilty.
---
 lisp/bookmark.el | 53 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 7 deletions(-)

diff --git lisp/bookmark.el lisp/bookmark.el
index 31876c83a2..6c46268a34 100644
--- lisp/bookmark.el
+++ lisp/bookmark.el
@@ -1824,7 +1824,29 @@ 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 Name" . nil))
+          (setq tabulated-list-entries entries))
+      (setq tabulated-list-sort-key nil)
+      ;; And since we're not sorting by bookmark name, show bookmarks
+      ;; according to order of creation, with the most recently
+      ;; created bookmarks at the top and the least recently created
+      ;; at the bottom.
+      ;;
+      ;; Note that clicking the column sort toggle for the bookmark
+      ;; name column will invoke the `tabulated-list-mode' sort, which
+      ;; uses `bookmark-bmenu--name-predicate' to sort lexically by
+      ;; bookmark name instead of by (reverse) creation order.
+      ;; Clicking the toggle again will reverse the lexical sort, but
+      ;; the sort will still be lexical not creation-order.  However,
+      ;; if the user reverts the buffer, then the above check of
+      ;; `bookmark-sort-flag' will happen again and the buffer will
+      ;; go back to a creation-order sort.  This is all expected
+      ;; behavior, as documented in `bookmark-bmenu-mode'.
+      (setq tabulated-list-entries (reverse entries))))
   (tabulated-list-print t))
 
 ;;;###autoload
@@ -1868,6 +1890,18 @@ bookmark-bmenu-mode
 Each line describes one of the bookmarks in Emacs.
 Letters do not insert themselves; instead, they are commands.
 Bookmark names preceded by a \"*\" have annotations.
+
+If `bookmark-sort-flag' is non-nil, then sort the list by
+bookmark name (case-insensitively, in collation order); the
+direction of that sort can be reversed by using the column sort
+toggle for the bookmark name column.
+
+If `bookmark-sort-flag' is nil, then sort the list by bookmark
+creation order, with most recently created bookmarks on top.
+However, the column sort toggle will still activate (and
+thereafter toggle the direction of) lexical sorting by bookmark name.
+At any time you may use \\[revert-buffer] to go back to sorting by creation order.
+
 \\<bookmark-bmenu-mode-map>
 \\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
 \\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
@@ -1900,20 +1934,23 @@ bookmark-bmenu-mode
   in another buffer.
 \\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
 \\[bookmark-bmenu-edit-annotation] -- edit the annotation for the current bookmark.
-\\[bookmark-bmenu-search] -- incrementally search for bookmarks."
+\\[bookmark-bmenu-search] -- incrementally search for bookmarks.
+\\[revert-buffer] -- refresh the buffer, and thus refresh the sort order (useful
+  if `bookmark-sort-flag' is nil)."
   (setq truncate-lines t)
   (setq buffer-read-only t)
   ;; FIXME: The header could also display the current default bookmark file
   ;; according to `bookmark-bookmarks-timestamp'.
   (setq tabulated-list-format
         `[("" 1) ;; Space to add "*" for bookmark with annotation
-          ("Bookmark" ,bookmark-bmenu-file-column bookmark-bmenu--name-predicate)
+          ("Bookmark Name"
+           ,bookmark-bmenu-file-column bookmark-bmenu--name-predicate)
           ("Type" 8 bookmark-bmenu--type-predicate)
           ,@(if bookmark-bmenu-toggle-filenames
                 '(("File" 0 bookmark-bmenu--file-predicate)))])
   (setq tabulated-list-padding bookmark-bmenu-marks-width)
   (when bookmark-sort-flag
-    (setq tabulated-list-sort-key '("Bookmark" . nil)))
+    (setq tabulated-list-sort-key '("Bookmark Name" . nil)))
   (add-hook 'tabulated-list-revert-hook #'bookmark-bmenu--revert nil t)'
   (setq revert-buffer-function 'bookmark-bmenu--revert)
   (tabulated-list-init-header))
@@ -1922,17 +1959,19 @@ bookmark-bmenu-mode
 (defun bookmark-bmenu--name-predicate (a b)
   "Predicate to sort \"*Bookmark List*\" buffer by the name column.
 This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
-  (string< (caar a) (caar b)))
+  (string-collate-lessp (caar a) (caar b) nil t))
 
 (defun bookmark-bmenu--type-predicate (a b)
   "Predicate to sort \"*Bookmark List*\" buffer by the type column.
 This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
-  (string< (elt (cadr a) 2) (elt (cadr b) 2)))
+  (string-collate-lessp (elt (cadr a) 2) (elt (cadr b) 2) nil t))
 
 (defun bookmark-bmenu--file-predicate (a b)
   "Predicate to sort \"*Bookmark List*\" buffer by the file column.
 This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
-  (string< (bookmark-location (car a)) (bookmark-location (car b))))
+  (string-collate-lessp (bookmark-location (car a))
+                        (bookmark-location (car b))
+                        nil t))
 
 
 (defun bookmark-bmenu-toggle-filenames (&optional show)
-- 
2.35.1