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

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

On 04 Mar 2022, Eli Zaretskii wrote:
>The original message didn't describe any bugs, so I concluded it 
>was a
>new feature.  Sorry about that.
>
>However, if this is a bug, please tell:
>
>  . what is the bug and how to reproduce it?
>  . if this is a regression, what was the last Emacs version 
>  where the
>    reproduction recipe worked correctly?

The variable `bookmark-sort-flag' is not obeyed in 
`bookmark-bmenu-mode' (that's the Dired-like mode entered 
interactively via `edit-bookmarks').

The behavior is supposed to be that when `bookmark-sort-flag' is 
non-nil, bookmarks are listed in lexical order by bookmark name, 
and when the variable is nil, they are listed in order from 
most-recently-created down to least-recently-created

(As an aside: the latter case relies on the assumption that the 
order of elements in `bookmark-alist' reflects the order in which 
the bookmarks were created.  This happens to be true, and is even 
documented in a couple of places in bookmark.el, but is not 
well-documented at the code site where it is actually implemented. 
That's something I'll take care of separately after handling 
Manuel's patch.)

The bug Manuel found is that if you set `bookmark-sort-flag' to 
nil, the list is still displayed in lexically sorted order, as 
though the flag were still non-nil.  His patch fixes that 
(although there is still a related pre-existing buglet, which I 
discuss farther down in this email).

Manuel's original post said this in a compact and less detailed 
way: "Here is a patch that I think fix the default 
`bookmark-bmenu-list' sorting when `bookmark-sort-flag' is nil."

>How do you know whether the current pretest of 28.1 is or isn't 
>"in
>the very latest stages of its pretest"?  The text says to ask if 
>you
>aren't sure.  As things are, we are, I hope, in the very latest
>stages.

Oh, I see now.  There's an ambiguity (between CONTRIBUTE and how 
version numbers are recorded in the release branch's README), but 
from what you say, I now see that where CONTRIBUTE says "when it 
is for Emacs version NN.1 that is in the very last stages of its 
pretest", that means that the version number in the README on the 
branch will say NN.0.X.  That isn't obvious from CONTRIBUTE, but 
it's a perfectly reasonable interpretation.  (The other 
interpretation -- the one I made -- is that README would contain 
NN.1[.x] already; some projects do release branches one way, some 
do it the other.)

>> I'm happy to put this change on whatever branch you prefer, of
>> course.
>
>I will make up my mind after I know the answers to the above
>questions.

In addition to the two changes described above (Manuel's patch, 
and a separate internal comment fix), there is another buglet I am 
chasing, which I found when I was testing Manuel's patch:

Even after Manuel's fix, the buffer generated by `edit-bookmarks' 
will not reorder the bookmarks after one toggles 
`bookmark-sort-flag' and types `g' (which runs `revert-buffer', 
which in this case invokes a `revert-buffer-function' that 
regenerates the buffer).  But this problem is only for the first 
incarnation of that buffer!  If one kills the buffer and rebuilds 
it from scratch, after that, the toggle-flag-and-revert routine 
*will* correctly reorder the bookmarks each time. 

This is a very minor buglet, in terms of its effects on users, 
since most people set `bookmark-sort-flag' in their init files 
anyway and aren't likely to toggle it during an Emacs session. 
However, it still bothers me, and it certainly interferes with 
testing `bookmark-sort-flag', so I'm planning to fix it too.

So in summary, there are three small changes coming:

1) Manuel's original bugfix

2) Internal comment fix, motivated by (1)

3) Fix for the toggle-regenerate buglet.

I would mildly prefer to put them all on the same branch, but I 
don't care which branch that is; let me know what you'd like.

If you would like them on different branches, I can do that, just 
let me know which change goes to which branch.

Best regards,
-Karl