all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
@ 2024-10-12  4:46 Eduardo Ochs
  2024-10-12  9:18 ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Eduardo Ochs @ 2024-10-12  4:46 UTC (permalink / raw)
  To: 73764

Hi list,

if we run this

  (format-kbd-macro (read-key-sequence-vector "Type C-M-h:"))

we get "M-C-h". But try:

  (keymap-lookup global-map "M-C-h")
    ;;-> "M-C-h" is not a valid key definition; see `key-valid-p'

  (keymap-lookup global-map "C-M-h")
    ;;-> mark-defun

Tested on GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu,
GTK+ Version 3.24.24, cairo version 1.16.0) of 2024-10-11,
git-pulled and compiled a few hours ago.

  Cheers,
    Eduardo Ochs
    http://anggtwu.net/





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-12  4:46 bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize Eduardo Ochs
@ 2024-10-12  9:18 ` Eli Zaretskii
  2024-10-12 13:33   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-10-12  9:18 UTC (permalink / raw)
  To: Eduardo Ochs, Stefan Monnier; +Cc: 73764

> From: Eduardo Ochs <eduardoochs@gmail.com>
> Date: Sat, 12 Oct 2024 01:46:26 -0300
> 
> if we run this
> 
>   (format-kbd-macro (read-key-sequence-vector "Type C-M-h:"))
> 
> we get "M-C-h". But try:
> 
>   (keymap-lookup global-map "M-C-h")
>     ;;-> "M-C-h" is not a valid key definition; see `key-valid-p'
> 
>   (keymap-lookup global-map "C-M-h")
>     ;;-> mark-defun

This is because key-valid-p is too restrictive: it only accepts
modifiers in the canonical order A-C-H-M-S-s.  But key-parse, which is
called by keymap-lookup, accepts and correctly processes the modifiers
in any order:

  (key-parse "M-C-h")
   => [134217736]

Should we fix key-valid-p to be more lenient?  Or maybe just remove
the call to keymap--check from keymap-look up?

Stefan, WDYT?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-12  9:18 ` Eli Zaretskii
@ 2024-10-12 13:33   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-10-12 14:36     ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-10-12 13:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 73764, Eduardo Ochs

>>   (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".

>> 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.

> 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.
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.


        Stefan






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-12 13:33   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-10-12 14:36     ` Eli Zaretskii
  2024-10-12 14:51       ` Andreas Schwab
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-10-12 14:36 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 73764, eduardoochs

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eduardo Ochs <eduardoochs@gmail.com>,  73764@debbugs.gnu.org
> 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?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-12 14:36     ` Eli Zaretskii
@ 2024-10-12 14:51       ` Andreas Schwab
  2024-10-12 15:18         ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Schwab @ 2024-10-12 14:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 73764, Stefan Monnier, eduardoochs

On Okt 12 2024, Eli Zaretskii wrote:

>> 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).

The issue here is that C-h is a character on its own, unlike, say, C-+.

ELISP> (format-kbd-macro [?\M-\C-+])
"C-M-+"

Maybe key-valid-p should accept those control characters as the last
component as a special case.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-12 14:51       ` Andreas Schwab
@ 2024-10-12 15:18         ` Eli Zaretskii
  2024-10-14 15:06           ` Robert Pluim
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-10-12 15:18 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 73764, monnier, eduardoochs

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  73764@debbugs.gnu.org,
>   eduardoochs@gmail.com
> Date: Sat, 12 Oct 2024 16:51:49 +0200
> 
> On Okt 12 2024, Eli Zaretskii wrote:
> 
> >> 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).
> 
> The issue here is that C-h is a character on its own, unlike, say, C-+.

Right.  And in that case, the C- prefix is generated when a control
character is seen, which is at the end.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-12 15:18         ` Eli Zaretskii
@ 2024-10-14 15:06           ` Robert Pluim
  2024-10-14 15:54             ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Robert Pluim @ 2024-10-14 15:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 73764, Andreas Schwab, monnier, eduardoochs

>>>>> On Sat, 12 Oct 2024 18:18:18 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Andreas Schwab <schwab@linux-m68k.org>
    >> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  73764@debbugs.gnu.org,
    >> eduardoochs@gmail.com
    >> Date: Sat, 12 Oct 2024 16:51:49 +0200
    >> 
    >> On Okt 12 2024, Eli Zaretskii wrote:
    >> 
    >> >> 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).
    >> 
    >> The issue here is that C-h is a character on its own, unlike, say, C-+.

    Eli> Right.  And in that case, the C- prefix is generated when a control
    Eli> character is seen, which is at the end.

So if format-kbd-macro generates something with 'C-' in the "wrong"
place, whatʼs to stop us re-ordering the modifiers as a
post-processing step (inside `format-kbd-macro')?

Robert
-- 





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-14 15:06           ` Robert Pluim
@ 2024-10-14 15:54             ` Eli Zaretskii
  2024-10-14 16:25               ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-10-14 15:54 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 73764, schwab, monnier, eduardoochs

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Andreas Schwab <schwab@linux-m68k.org>,  73764@debbugs.gnu.org,
>   monnier@iro.umontreal.ca,  eduardoochs@gmail.com
> Date: Mon, 14 Oct 2024 17:06:05 +0200
> 
> >>>>> On Sat, 12 Oct 2024 18:18:18 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> From: Andreas Schwab <schwab@linux-m68k.org>
>     >> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  73764@debbugs.gnu.org,
>     >> eduardoochs@gmail.com
>     >> Date: Sat, 12 Oct 2024 16:51:49 +0200
>     >> 
>     >> On Okt 12 2024, Eli Zaretskii wrote:
>     >> 
>     >> >> 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).
>     >> 
>     >> The issue here is that C-h is a character on its own, unlike, say, C-+.
> 
>     Eli> Right.  And in that case, the C- prefix is generated when a control
>     Eli> character is seen, which is at the end.
> 
> So if format-kbd-macro generates something with 'C-' in the "wrong"
> place, whatʼs to stop us re-ordering the modifiers as a
> post-processing step (inside `format-kbd-macro')?

If that's the only case where we put C- in the wrong place, then
nothing stops us.  Is it?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-14 15:54             ` Eli Zaretskii
@ 2024-10-14 16:25               ` Eli Zaretskii
  2024-10-14 21:48                 ` Stefan Kangas
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-10-14 16:25 UTC (permalink / raw)
  To: rpluim; +Cc: 73764, schwab, monnier, eduardoochs

> Cc: 73764@debbugs.gnu.org, schwab@linux-m68k.org, monnier@iro.umontreal.ca,
>  eduardoochs@gmail.com
> Date: Mon, 14 Oct 2024 18:54:27 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Robert Pluim <rpluim@gmail.com>
> > Cc: Andreas Schwab <schwab@linux-m68k.org>,  73764@debbugs.gnu.org,
> >   monnier@iro.umontreal.ca,  eduardoochs@gmail.com
> > Date: Mon, 14 Oct 2024 17:06:05 +0200
> > 
> > So if format-kbd-macro generates something with 'C-' in the "wrong"
> > place, whatʼs to stop us re-ordering the modifiers as a
> > post-processing step (inside `format-kbd-macro')?
> 
> If that's the only case where we put C- in the wrong place, then
> nothing stops us.  Is it?

In any case, there's the more general issue at hand here:
keymap--check is (at least in this case) unnecessarily restrictive:
where key-parse will gladly accept and correctly process a key whose
modifiers are in any order, keymap--check insist on the canonical
order.  I'm not sure I like this, at least in non-interactive
functions.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize
  2024-10-14 16:25               ` Eli Zaretskii
@ 2024-10-14 21:48                 ` Stefan Kangas
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Kangas @ 2024-10-14 21:48 UTC (permalink / raw)
  To: Eli Zaretskii, rpluim; +Cc: 73764, schwab, monnier, eduardoochs

Eli Zaretskii <eliz@gnu.org> writes:

> In any case, there's the more general issue at hand here:
> keymap--check is (at least in this case) unnecessarily restrictive:
> where key-parse will gladly accept and correctly process a key whose
> modifiers are in any order, keymap--check insist on the canonical
> order.  I'm not sure I like this, at least in non-interactive
> functions.

One issue that these new functions tried to resolve was that the old
functions were too lenient.  See, for example, this passage in the
manual, and its docstring:

     The ‘kbd’ function is very permissive, and will try to return
     something sensible even if the syntax used isn't completely
     conforming.  To check whether the syntax is actually valid, use the
     ‘key-valid-p’ function.

So this strict behaviour is not by accident, and we should probably
think twice before going back on this design decision.

At least, we should review the old arguments to make sure we don't bring
back any undesired/buggy behaviour that this was supposed to remedy.
I didn't have the time to do that fully, as there are many relevant
threads from around that period, both on emacs-devel and bug-gnu-emacs.





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-10-14 21:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-12  4:46 bug#73764: format-kbd-macro returns a key name that keymap-lookup doesn't recognize Eduardo Ochs
2024-10-12  9:18 ` Eli Zaretskii
2024-10-12 13:33   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-10-12 14:36     ` Eli Zaretskii
2024-10-12 14:51       ` Andreas Schwab
2024-10-12 15:18         ` Eli Zaretskii
2024-10-14 15:06           ` Robert Pluim
2024-10-14 15:54             ` Eli Zaretskii
2024-10-14 16:25               ` Eli Zaretskii
2024-10-14 21:48                 ` Stefan Kangas

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.