Subject: bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize

From: Eli Zaretskii <[email-masked]>
Date: Sat, 12 Oct 2024 17:36:05 +0300

> From: Stefan Monnier <[email-masked]>
> Cc: Eduardo Ochs <[email-masked]>,  [email-masked]
> Date: Sat, 12 Oct 2024 09:33:59 -0400
> 
> >>   (keymap-lookup global-map "M-C-h")
> >>     ;;-> "M-C-h" is not a valid key definition; see `key-valid-p'
> [...]
> > Should we fix key-valid-p to be more lenient?
> 
> AFAIK the strictness was a conscious choice, so maybe we should simply
> improve the error message to say something like "M-C-h uses an invalid
> modifier ordering, maybe you meant C-M-h".

The general problem is much wider, so it is not easy to intuit "what
you meant".  E.g., what do you say if the key is "S-s-M-H-A-C-b"?  Do
you want to have a function that reorders the modifiers in canonical
order?  And if we have such a function, why not reorder silently and
accept the key, like we do with the order of BEG..END in functions
that accept the region?

> >> if we run this
> >>
> >>   (format-kbd-macro (read-key-sequence-vector "Type C-M-h:"))
> >>
> >> we get "M-C-h".
> 
> I guess we also need to make sure `format-kbd-macro` generates
> something that `key-valid-p` accepts.

AFAICS, that's impossible without completely rewriting its code.  It
proceeds from left to right (i.e. from MSB to LSB).

> > Or maybe just remove the call to keymap--check from keymap-lookup?
> 
> IIRC we wanted to use `keymap--check` on all input coming from the user,
> so I guess the question is whether the second arg of `keymap-lookup` is
> expected to be an immediate constant.

But keymap-look up is not an interactive function.

> That function is still young, so it's hard to tell how it'll turn up,
> but my `grep` says that indeed many (most) calls take a literal string
> as argument, so `keymap--check` seems appropriate.

If we remove the call to keymap--check, won't the other APIs signal an
error instead?  If they will, why do we need to protect them?