[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#59580: 29.0.50; [PATCH] Add new choices to bookmark-sort-flag
From: |
Eli Zaretskii |
Subject: |
bug#59580: 29.0.50; [PATCH] Add new choices to bookmark-sort-flag |
Date: |
Thu, 01 Dec 2022 10:23:41 +0200 |
> From: Gabriel <gabriel376@hotmail.com>
> 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.
- bug#59580: 29.0.50; [PATCH] Add new choices to bookmark-sort-flag,
Eli Zaretskii <=