Re: bug#58799: Make winner key sequences repeatable in repeat-mode

Re: bug#58799: Make winner key sequences repeatable in repeat-mode

[Robert Pluim]

(redirecting to emacs-devel)
>>>>> On Mon, 31 Oct 2022 07:47:36 -0700, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> close 58799 29.1
    Stefan> thanks

    Stefan> Robert Pluim <rpluim@gmail.com> writes:

    >> In any case, I think we can close this particular bug.

    Stefan> Agreed.  Now done.

So the minimal change I think is this:

diff --git c/lisp/keymap.el i/lisp/keymap.el
index 107565590c..750095fab4 100644
--- c/lisp/keymap.el
+++ i/lisp/keymap.el
@@ -545,6 +545,10 @@ define-keymap
           (let ((def (pop definitions)))
             (if (eq key :menu)
                 (easy-menu-define nil keymap "" def)
+              (unless (key-valid-p key)
+                (error
+                 "Key '%s' in keymap '%s' is not `key-valid-p'"
+                 key keymap))
               (if (member key seen-keys)
                   (error "Duplicate definition for key: %S %s" key keymap)
                 (push key seen-keys))

which will give a run-time error for things like "[left]" and
'[left]', but thatʼs redundant with the compile time warning:

    make[3]: Entering directory '/home/rpluim/repos/emacs/lispʼ
      ELC      winner.elc

    In toplevel form:
    winner.el:314:15: Warning: Invalid ‘kbd’ syntax: "[left]"

Eli? Lars? Is this overkill for such a rare case?

Robert
-- 

Re: bug#58799: Make winner key sequences repeatable in repeat-mode

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  emacs-devel@gnu.org
> Date: Mon, 31 Oct 2022 16:09:54 +0100
> 
> diff --git c/lisp/keymap.el i/lisp/keymap.el
> index 107565590c..750095fab4 100644
> --- c/lisp/keymap.el
> +++ i/lisp/keymap.el
> @@ -545,6 +545,10 @@ define-keymap
>            (let ((def (pop definitions)))
>              (if (eq key :menu)
>                  (easy-menu-define nil keymap "" def)
> +              (unless (key-valid-p key)
> +                (error
> +                 "Key '%s' in keymap '%s' is not `key-valid-p'"
> +                 key keymap))
>                (if (member key seen-keys)
>                    (error "Duplicate definition for key: %S %s" key keymap)
>                  (push key seen-keys))
> 
> which will give a run-time error for things like "[left]" and
> '[left]', but thatʼs redundant with the compile time warning:
> 
>     make[3]: Entering directory '/home/rpluim/repos/emacs/lispʼ
>       ELC      winner.elc
> 
>     In toplevel form:
>     winner.el:314:15: Warning: Invalid ‘kbd’ syntax: "[left]"
> 
> Eli? Lars? Is this overkill for such a rare case?

I don't know.  I guess it depends on how many people will be tripped
by this.  The change itself is small enough, I think.

Re: bug#58799: Make winner key sequences repeatable in repeat-mode

[Robert Pluim]

>>>>> On Mon, 31 Oct 2022 17:28:18 +0200, Eli Zaretskii <eliz@gnu.org> said:
    >> make[3]: Entering directory '/home/rpluim/repos/emacs/lispʼ
    >> ELC      winner.elc
    >> 
    >> In toplevel form:
    >> winner.el:314:15: Warning: Invalid ‘kbd’ syntax: "[left]"
    >> 
    >> Eli? Lars? Is this overkill for such a rare case?

    Eli> I don't know.  I guess it depends on how many people will be tripped
    Eli> by this.  The change itself is small enough, I think.

0, it turns out, as thereʼs already a runtime check for this case 😀
Motto: test the existing code before proposing changes

Robert
--