Subject: Re: bug#59580: 29.0.50; [PATCH] Add new choices to bookmark-sort-flag

From: Eli Zaretskii <[email-masked]>
Date: Thu, 01 Dec 2022 10:23:41 +0200

> From: Gabriel <[email-masked]>
> Date: Fri, 25 Nov 2022 15:26:49 -0300
> 
> 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.

Is it really a good idea to use string-collate-lessp here?  Its effect
depends on the underlying OS and the user's locale, so it could produce
different and sometimes surprising results.  Why isn't string-lessp
sufficient?  IMO, at the very least, the fact that the sorting is
locale-dependent should be prominently stated in the doc string.  Also, see
the notes in the string-collate-lessp's doc string about MS-Windows, and
maybe do what it suggests (if we decide to keep string-collate-lessp here).

Karl, any comments?

Thanks.