GNU bug report logs - #61149
29.0.60; keymap-local-set rejects key sequences

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 61149 in the body.
You can then email your comments to 61149 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Cc: Robert Pluim <rpluim <at> gmail.com>
Subject: 29.0.60; keymap-local-set rejects key sequences
Date: Sun, 29 Jan 2023 15:44:44 +0200

To reproduce:

  emacs -Q
  M-x keymap-local-set
  C-c .
  forward-line RET

Observe the error message:

  [3 46] is not a valid key definition; see ‘key-valid-p’

(The error message comes from keymap--check.)

To type the key sequence, I actually pressed Ctrl and typed 'c', then
released Ctrl and typed '.'.  Isn't that what one is supposed to use?

So the above basically makes keymap-local-set useless, or am I missing
something?  The following does work:

  M-: (keymap-local-set "C-c ." 'forward-line) RET

However, I could find no way of typing the string "C-c ." at the
prompt of keymap-local-set.


In GNU Emacs 29.0.60 (build 259, i686-pc-mingw32) of 2023-01-29 built on
 HOME-C4E4A596F7
Repository revision: b73539832d9c4e802925cb8f261a13473da383b3
Repository branch: emacs-29
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
System Description: Microsoft Windows XP Service Pack 3 (v5.1.0.2600)

Configured using:
 'configure -C --prefix=/d/usr --with-wide-int
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -gdwarf-4 -g3''

Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
W32NOTIFY PDUMPER PNG RSVG SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XPM ZLIB

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel dos-w32
ls-lisp disp-table term/w32-win w32-win w32-vars term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads
w32notify w32 lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 42883 10204)
 (symbols 48 6278 0)
 (strings 16 16568 2962)
 (string-bytes 1 399068)
 (vectors 16 9325)
 (vector-slots 8 146723 13332)
 (floats 8 24 24)
 (intervals 40 293 123)
 (buffers 888 11))

Message #8 received at 61149 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61149 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>
Subject: Re: bug#61149: 29.0.60; keymap-local-set rejects key sequences
Date: Sun, 29 Jan 2023 15:35:22 +0100

[Message part 1 (text/plain, inline)]

On Sun, 29 Jan 2023 15:44:44 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:

> To reproduce:
>
>   emacs -Q
>   M-x keymap-local-set
>   C-c .
>   forward-line RET
>
> Observe the error message:
>
>   [3 46] is not a valid key definition; see ‘key-valid-p’
>
> (The error message comes from keymap--check.)
>
> To type the key sequence, I actually pressed Ctrl and typed 'c', then
> released Ctrl and typed '.'.  Isn't that what one is supposed to use?
>
> So the above basically makes keymap-local-set useless, or am I missing
> something?  The following does work:
>
>   M-: (keymap-local-set "C-c ." 'forward-line) RET
>
> However, I could find no way of typing the string "C-c ." at the
> prompt of keymap-local-set.

This patch seems to DTRT:

[Message part 2 (text/x-patch, inline)]

diff --git a/lisp/keymap.el b/lisp/keymap.el
index 791221f2459..9643dce5f9c 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -95,7 +95,8 @@ keymap-local-set
 cases is shared with all other buffers in the same major mode."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
   (interactive "KSet key locally: \nCSet key %s locally to command: ")
-  (let ((map (current-local-map)))
+  (let ((map (current-local-map))
+	(key (key-description key)))
     (unless map
       (use-local-map (setq map (make-sparse-keymap))))
     (keymap-set map key command)))

[Message part 3 (text/plain, inline)]

Steve Berman

Message #11 received at 61149 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61149 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>
Subject: Re: bug#61149: 29.0.60; keymap-local-set rejects key sequences
Date: Sun, 29 Jan 2023 16:28:44 +0100

[Message part 1 (text/plain, inline)]

On Sun, 29 Jan 2023 15:35:22 +0100 Stephen Berman <stephen.berman <at> gmx.net> wrote:

> On Sun, 29 Jan 2023 15:44:44 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>> To reproduce:
>>
>>   emacs -Q
>>   M-x keymap-local-set
>>   C-c .
>>   forward-line RET
>>
>> Observe the error message:
>>
>>   [3 46] is not a valid key definition; see ‘key-valid-p’
>>
>> (The error message comes from keymap--check.)
>>
>> To type the key sequence, I actually pressed Ctrl and typed 'c', then
>> released Ctrl and typed '.'.  Isn't that what one is supposed to use?
>>
>> So the above basically makes keymap-local-set useless, or am I missing
>> something?  The following does work:
>>
>>   M-: (keymap-local-set "C-c ." 'forward-line) RET
>>
>> However, I could find no way of typing the string "C-c ." at the
>> prompt of keymap-local-set.
>
> This patch seems to DTRT:
>
> diff --git a/lisp/keymap.el b/lisp/keymap.el
> index 791221f2459..9643dce5f9c 100644
> --- a/lisp/keymap.el
> +++ b/lisp/keymap.el
> @@ -95,7 +95,8 @@ keymap-local-set
>  cases is shared with all other buffers in the same major mode."
>    (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
>    (interactive "KSet key locally: \nCSet key %s locally to command: ")
> -  (let ((map (current-local-map)))
> +  (let ((map (current-local-map))
> +	(key (key-description key)))
>      (unless map
>        (use-local-map (setq map (make-sparse-keymap))))
>      (keymap-set map key command)))

... but not in your non-interactive case; so maybe this instead:

[Message part 2 (text/x-patch, inline)]

diff --git a/lisp/keymap.el b/lisp/keymap.el
index 791221f2459..48ec91d03c8 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -98,6 +98,8 @@ keymap-local-set
   (let ((map (current-local-map)))
     (unless map
       (use-local-map (setq map (make-sparse-keymap))))
+    (unless (stringp key)
+      (setq key (key-description key)))
     (keymap-set map key command)))

 (defun keymap-global-unset (key &optional remove)

[Message part 3 (text/plain, inline)]

Steve Berman

Message #14 received at 61149 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61149 <at> debbugs.gnu.org
Subject: Re: bug#61149: 29.0.60; keymap-local-set rejects key sequences
Date: Mon, 30 Jan 2023 08:59:03 +0100

>>>>> On Sun, 29 Jan 2023 16:28:44 +0100, Stephen Berman <stephen.berman <at> gmx.net> said:

    Stephen> ... but not in your non-interactive case; so maybe this instead:

    Stephen> diff --git a/lisp/keymap.el b/lisp/keymap.el
    Stephen> index 791221f2459..48ec91d03c8 100644
    Stephen> --- a/lisp/keymap.el
    Stephen> +++ b/lisp/keymap.el
    Stephen> @@ -98,6 +98,8 @@ keymap-local-set
    Stephen>    (let ((map (current-local-map)))
    Stephen>      (unless map
    Stephen>        (use-local-map (setq map (make-sparse-keymap))))
    Stephen> +    (unless (stringp key)
    Stephen> +      (setq key (key-description key)))
    Stephen>      (keymap-set map key command)))

`keymap-global-set' has the same type of issue. How about this
instead:

diff --git a/lisp/keymap.el b/lisp/keymap.el
index 791221f2459..1a339598f0c 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -79,7 +79,7 @@ keymap-global-set
   (interactive
    (let* ((menu-prompting nil)
           (key (read-key-sequence "Set key globally: " nil t)))
-     (list key
+     (list (key-description key)
            (read-command (format "Set key %s to command: "
                                  (key-description key))))))
   (keymap-set (current-global-map) key command))
@@ -94,7 +94,12 @@ keymap-local-set
 The binding goes in the current buffer's local map, which in most
 cases is shared with all other buffers in the same major mode."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key locally: \nCSet key %s locally to command: ")
+  (interactive
+   (let* ((menu-prompting nil)
+          (key (read-key-sequence "Set key locally: " nil t)))
+     (list (key-description key)
+           (read-command (format "Set key %s to command: "
+                                 (key-description key))))))
   (let ((map (current-local-map)))
     (unless map
       (use-local-map (setq map (make-sparse-keymap))))

Robert
-- 

Message #17 received at 61149 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61149 <at> debbugs.gnu.org
Subject: Re: bug#61149: 29.0.60; keymap-local-set rejects key sequences
Date: Mon, 30 Jan 2023 09:46:49 +0100

On Mon, 30 Jan 2023 08:59:03 +0100 Robert Pluim <rpluim <at> gmail.com> wrote:

>>>>>> On Sun, 29 Jan 2023 16:28:44 +0100, Stephen Berman
> <stephen.berman <at> gmx.net> said:
>
>     Stephen> ... but not in your non-interactive case; so maybe this instead:
>
>     Stephen> diff --git a/lisp/keymap.el b/lisp/keymap.el
>     Stephen> index 791221f2459..48ec91d03c8 100644
>     Stephen> --- a/lisp/keymap.el
>     Stephen> +++ b/lisp/keymap.el
>     Stephen> @@ -98,6 +98,8 @@ keymap-local-set
>     Stephen>    (let ((map (current-local-map)))
>     Stephen>      (unless map
>     Stephen>        (use-local-map (setq map (make-sparse-keymap))))
>     Stephen> +    (unless (stringp key)
>     Stephen> +      (setq key (key-description key)))
>     Stephen>      (keymap-set map key command)))
>
> `keymap-global-set' has the same type of issue.

Thanks, I failed to check that.

>                                                 How about this
> instead:
>
> diff --git a/lisp/keymap.el b/lisp/keymap.el
> index 791221f2459..1a339598f0c 100644
> --- a/lisp/keymap.el
> +++ b/lisp/keymap.el
> @@ -79,7 +79,7 @@ keymap-global-set
>    (interactive
>     (let* ((menu-prompting nil)
>            (key (read-key-sequence "Set key globally: " nil t)))
> -     (list key
> +     (list (key-description key)
>             (read-command (format "Set key %s to command: "
>                                   (key-description key))))))
>    (keymap-set (current-global-map) key command))
> @@ -94,7 +94,12 @@ keymap-local-set
>  The binding goes in the current buffer's local map, which in most
>  cases is shared with all other buffers in the same major mode."
>    (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
> -  (interactive "KSet key locally: \nCSet key %s locally to command: ")
> +  (interactive
> +   (let* ((menu-prompting nil)
> +          (key (read-key-sequence "Set key locally: " nil t)))
> +     (list (key-description key)
> +           (read-command (format "Set key %s to command: "
> +                                 (key-description key))))))
>    (let ((map (current-local-map)))
>      (unless map
>        (use-local-map (setq map (make-sparse-keymap))))

This also DTRT, but it also gives keymap-local-set the same UI as that
of keymap-global-set, which is that at the first prompt the cursor jumps
out of the minibuffer (due to read-key-sequence), and then jumps back at
the second prompt.  I find this quite jarring (and the first time I was
quite surprised and thought I had either mistyped or there was a bug).
I would prefer it if both commands had keymap-local-set's current UI.

Steve Berman

Message #20 received at 61149 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61149 <at> debbugs.gnu.org
Subject: Re: bug#61149: 29.0.60; keymap-local-set rejects key sequences
Date: Mon, 30 Jan 2023 10:47:35 +0100

>>>>> On Mon, 30 Jan 2023 09:46:49 +0100, Stephen Berman <stephen.berman <at> gmx.net> said:

    Stephen> This also DTRT, but it also gives keymap-local-set the same UI as that
    Stephen> of keymap-global-set, which is that at the first prompt the cursor jumps
    Stephen> out of the minibuffer (due to read-key-sequence), and then jumps back at
    Stephen> the second prompt.  I find this quite jarring (and the first time I was
    Stephen> quite surprised and thought I had either mistyped or there was a bug).
    Stephen> I would prefer it if both commands had keymap-local-set's current UI.

Yes, that is quite annoying. So letʼs do it your way (with an
incidental spacing fix :-) )

Eli, emacs-29, I think, if youʼre ok with the fix.

diff --git a/lisp/keymap.el b/lisp/keymap.el
index 791221f2459..caabedd5aec 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -76,12 +76,9 @@ keymap-global-set
 that local binding will continue to shadow any global binding
 that you make with this function."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive
-   (let* ((menu-prompting nil)
-          (key (read-key-sequence "Set key globally: " nil t)))
-     (list key
-           (read-command (format "Set key %s to command: "
-                                 (key-description key))))))
+  (interactive "KSet key globally:\nCSet key %s globally to command: ")
+  (unless (stringp key)
+    (setq key (key-description key)))
   (keymap-set (current-global-map) key command))
 
 (defun keymap-local-set (key command)
@@ -94,10 +91,12 @@ keymap-local-set
 The binding goes in the current buffer's local map, which in most
 cases is shared with all other buffers in the same major mode."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key locally: \nCSet key %s locally to command: ")
+  (interactive "KSet key locally:\nCSet key %s locally to command: ")
   (let ((map (current-local-map)))
     (unless map
       (use-local-map (setq map (make-sparse-keymap))))
+    (unless (stringp key)
+      (setq key (key-description key)))
     (keymap-set map key command)))
 
 (defun keymap-global-unset (key &optional remove)

Message #23 received at 61149 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: 61149 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#61149: 29.0.60; keymap-local-set rejects key sequences
Date: Mon, 30 Jan 2023 15:14:24 +0200

> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  61149 <at> debbugs.gnu.org
> Date: Mon, 30 Jan 2023 10:47:35 +0100
> 
> Yes, that is quite annoying. So letʼs do it your way (with an
> incidental spacing fix :-) )
> 
> Eli, emacs-29, I think, if youʼre ok with the fix.

Definitely emacs-29, since these commands are new in Emacs 29, and we
cannot possibly release broken commands.

Thanks.

Message #26 received at 61149 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61149 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#61149: 29.0.60; keymap-local-set rejects key sequences
Date: Mon, 30 Jan 2023 14:25:54 +0100

tags 61149 fixed
close 61149 29.1
quit

>>>>> On Mon, 30 Jan 2023 15:14:24 +0200, Eli Zaretskii <eliz <at> gnu.org> said:

    >> From: Robert Pluim <rpluim <at> gmail.com>
    >> Cc: Eli Zaretskii <eliz <at> gnu.org>,  61149 <at> debbugs.gnu.org
    >> Date: Mon, 30 Jan 2023 10:47:35 +0100
    >> 
    >> Yes, that is quite annoying. So letʼs do it your way (with an
    >> incidental spacing fix :-) )
    >> 
    >> Eli, emacs-29, I think, if youʼre ok with the fix.

    Eli> Definitely emacs-29, since these commands are new in Emacs 29, and we
    Eli> cannot possibly release broken commands.

    Eli> Thanks.

Closing, pushed as f67a9a12b7b

Robert
-- 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.