Subject: 29.0.50; [PATCH] Add new choices to bookmark-sort-flag

From: Gabriel <[email-masked]>
Date: Fri, 25 Nov 2022 15:26:49 -0300

Severity: wishlist

Description:

Add new choices 'type and 'file to `bookmark-sort-flag'.  See [1].

Notes:

1) It feels strange to have value t meaning 'name and value nil meaning
'creation-time, but it's better to not change how things have been
working for many years.  A more flexible approach could offer a custom
function as a choice of `bookmark-sort-flag'.

2) I took the liberty to update some comments and docstrings, please
review.

3) I took the liberty to remove some code in `bookmark-bmenu-mode' that
seems to be unnecessary, please review.  Everything seems to be working
as expected according to my tests.  I can also write some tests.

4) Used `string-collate-lessp' in `bookmark-maybe-sort-alist' to match
`bookmark-bmenu--*-predicate' functions.

[1] https://lists.gnu.org/archive/html/emacs-devel/2022-11/msg00995.html

>From c41d0990774a39a942e645aeaaec1126de419db1 Mon Sep 17 00:00:00 2001
From: Gabriel do Nascimento Ribeiro <[email-masked]>
Date: Fri, 25 Nov 2022 15:06:58 -0300
Subject: [PATCH 1/1] Add more choices to bookmark-sort-flag

* lisp/bookmark.el (bookmark-sort-flag): Add new choices 'type and
'file.
(bookmark-maybe-sort-alist, bookmark-bmenu-mode): Handle new choices
of `bookmark-sort-flag'.
(bookmark-bmenu-mode): Update docstring and remove unnecessary code.

* etc/NEWS: Announce the changes.
---
 etc/NEWS         |  5 ++--
 lisp/bookmark.el | 69 +++++++++++++++++++++++-------------------------
 2 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 0ffc849fec..acd0ed1d6e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2554,9 +2554,8 @@ Types are registered via a 'bookmark-handler-type' symbol property on
 the jumping function.
 
 +++
-*** 'bookmark-sort-flag' can now be set to 'last-modified'.
-This will display bookmark list from most recently set to least
-recently set.
+*** 'bookmark-sort-flag' now accepts more choices.
+It now accepts the values 'type', 'file' and 'last-modified'.
 
 ---
 *** When editing a bookmark annotation, 'C-c C-k' will now cancel.
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 7f3a264f53..084f7450fe 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -38,13 +38,6 @@
 (require 'fringe) ; for builds --without-x
 (eval-when-compile (require 'cl-lib))
 
-;;; Misc comments:
-;;
-;; The bookmark list is sorted lexically by default, but you can turn
-;; this off by setting bookmark-sort-flag to nil.  If it is nil, then
-;; the list will be presented in the order it is recorded
-;; (chronologically), which is actually fairly useful as well.
-
 ;;; User Variables
 
 (defgroup bookmark nil
@@ -116,16 +109,16 @@ bookmark-completion-ignore-case

 (defcustom bookmark-sort-flag t
-  "This controls the bookmark display sorting.
-nil means they will be displayed in LIFO order (that is, most
-recently created ones come first, oldest ones come last).
-
-`last-modified' means that bookmarks will be displayed sorted
-from most recently modified to least recently modified.
+  "Controls the bookmark display sorting.
 
-Other values means that bookmarks will be displayed sorted by
-bookmark name."
+A value of t means to display bookmarks sorted by name.  A value
+of `type' means to display bookmarks sorted by type.  A value of
+`file' means to display bookmarks sorted by file.  A value of
+`last-modified' means to display bookmarks by modification time.
+A value of nil means to display bookmarks by creation time."
   :type '(choice (const :tag "By name" t)
+                 (const :tag "By type" type)
+                 (const :tag "By file" file)
                  (const :tag "By modified time" last-modified)
                  (const :tag "By creation time" nil)))
 
@@ -524,14 +517,22 @@ bookmark--remove-fringe-mark
                 (delete-overlay (setq found temp))))))))))
 
 (defun bookmark-maybe-sort-alist ()
-  "Return `bookmark-alist' for display.
-If `bookmark-sort-flag' is T, then return a sorted by name copy of the alist.
-If `bookmark-sort-flag' is LAST-MODIFIED, then return a sorted by last modified
-copy of the alist.  Otherwise, just return `bookmark-alist', which by default
-is ordered from most recently created to least recently created bookmark."
+  "Return a copy of `bookmark-alist' for display.
+
+The sort order is controlled by `bookmark-sort-flag'."
   (let ((copy (copy-alist bookmark-alist)))
     (cond ((eq bookmark-sort-flag t)
-           (sort copy (lambda (x y) (string-lessp (car x) (car y)))))
+           (sort copy (lambda (x y)
+                        (string-collate-lessp  (bookmark-name-from-full-record x)
+                                               (bookmark-name-from-full-record y)))))
+          ((eq bookmark-sort-flag 'type)
+           (sort copy (lambda (x y)
+                        (string-collate-lessp (bookmark-type-from-full-record x)
+                                              (bookmark-type-from-full-record y)))))
+          ((eq bookmark-sort-flag 'file)
+           (sort copy (lambda (x y)
+                        (string-collate-lessp (bookmark-location x)
+                                              (bookmark-location y)))))
           ((eq bookmark-sort-flag 'last-modified)
            (sort copy (lambda (x y)
                         (let ((tx (bookmark-get-last-modified x))
@@ -1903,6 +1904,12 @@ bookmark-bmenu--revert
     (cond ((eq bookmark-sort-flag t)
            (setq tabulated-list-sort-key '("Bookmark Name" . nil)
                  tabulated-list-entries entries))
+          ((eq bookmark-sort-flag 'type)
+           (setq tabulated-list-sort-key '("Type" . nil)
+                 tabulated-list-entries entries))
+          ((eq bookmark-sort-flag 'file)
+           (setq tabulated-list-sort-key '("File" . nil)
+                 tabulated-list-entries entries))
           ((or (null bookmark-sort-flag)
                (eq bookmark-sort-flag 'last-modified))
            (setq tabulated-list-sort-key nil)
@@ -1971,16 +1978,10 @@ bookmark-bmenu-mode
 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.
+The sort order can be controlled with `bookmark-sort-flag'.  The
+sort direction can be reversed by using the column sort toggle
+for the bookmark name column.  At any time you may use
+\\[revert-buffer] to go back to the original sort order.
 
 \\<bookmark-bmenu-mode-map>
 \\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
@@ -2015,8 +2016,7 @@ bookmark-bmenu-mode
 \\[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.
-\\[revert-buffer] -- refresh the buffer, and thus refresh the sort order (useful
-  if `bookmark-sort-flag' is nil)."
+\\[revert-buffer] -- refresh the buffer, and thus refresh the sort order."
   (setq truncate-lines t)
   (setq buffer-read-only t)
   ;; FIXME: The header could also display the current default bookmark file
@@ -2029,9 +2029,6 @@ bookmark-bmenu-mode
           ,@(if bookmark-bmenu-toggle-filenames
                 '(("File" 0 bookmark-bmenu--file-predicate)))])
   (setq tabulated-list-padding bookmark-bmenu-marks-width)
-  (when (and bookmark-sort-flag
-             (not (eq bookmark-sort-flag 'last-modified)))
-    (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))
-- 
2.34.1

---
Gabriel