Re: [WIP PATCH] Adding keys to keymaps in alphabetical order (for use with `mode-line-mode-menu')

[Stefan Monnier]

>> Performance-wise, the cost is negligible for the default value of
>> `mode-line-mode-menu': ~10ms for the first time and 0.02ms for
>> subsequent calls.
> Thanks; looks good, so I've applied it to Emacs 28 (with some trivial
> changes).

It might want to clarify that the sorting is based on the menu item
names (rather than, say, the name of the key sequence).
Maybe it should have "menu" somewhere in its name ;-)

>> +         (sort (cdr keymap)
>> +               (lambda (a b)
>> +                 (string< (bindings--menu-item-string (cdr-safe a))
>> +                          (bindings--menu-item-string (cdr-safe b)))))))
>> +    (setcdr keymap menu-items)
>
> I'm slightly worried about this, though -- will altering the keymap
> structure have any adverse side effects?  I don't think so, and testing
> a bit doesn't seem to reveal anything obvious, but we should be on the
> lookup.

Altering the keymap by side-effect is not dangerous in and of itself
(`define-key` does it as well).
But the above code will mess things up when applied to a keymap that has
a parent keymap.

It will also fail to do its job on menu keymaps which use a vector
rather or on composed keymaps (which can be
created "implicitly", e.g. when looking up `menu-bar` in a keymap which
has a `menu-bar` binding and which additionally inherits from a keymap
that also has a `menu-bar` binding).

To solve those issues I think one would have to `map-keymap` and return
a brand new keymap rather than modify it in-situ, AFAICT.

Note that in my example above of a composed keymap created implicitly by
`access_keymap`, the menu.c code currently deals very poorly with such
situations (the two composed keymaps get just "concatenated"; worse,
even though entries from the first keymap can hide/override elements
from the second, menu.c will show them all, so you can end up with menu
entries which can't actually be used).  The merging of the composed
keymaps would best be done according to directions embedded directly in
the keymaps themselves so a child keymap can insert new menu entries in
the middle of a parent menu.


        Stefan