bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Robert Pluim]

`global-set-key' allows you to bind keys to strings (or vectors). The
following all succeed without error (although only [1] and [3] match the
expectations of users unfamiliar with Emacs' key representation).

1. (global-set-key (kbd "C-c h") "hello")
2. (global-set-key (kbd "C-c h") "olá")
3. (global-set-key (kbd "C-c h") (kbd "olá"))

Trying to map this to emacs-29's `keymap-global-set', we get the
following issues

(keymap-global-set "C-c h" "hello") => error
(keymap-global-set "C-c h" "olá") => error

OK, maybe we need to wrap the definitions in `kbd' like [3] above.

(keymap-global-set "C-c h" (kbd "olá")) => definition is a vector -> ok

but
4. (keymap-global-set "C-c h" (kbd "hello")) => defn is a string -> error

After reading up on `key-valid-p':

5. (keymap-global-set "C-c h" "h e l l o") => success!

or alternatively

6. (keymap-global-set "C-c h" [?h ?e ?l ?l ?o]) => success!

Whilst not strictly a regression, this behaviour is confusing and
unhelpful, and the solution is not easily found. I can think of two
solutions:

1. Change `kbd' to always return a vector even if the input is
ascii-only, which makes [4] work
2. Change `keymap-set' to convert ascii-only strings to the format in
[5] or [6]. Probably just a call to `string-to-vector' is enough.

Thanks

Robert

In GNU Emacs 29.0.50 (build 43, x86_64-pc-linux-gnu, GTK+ Version
 3.24.24, cairo version 1.16.0) of 2022-11-16 built on rltb
Repository revision: 60f2bb862f834fcb580d839ac79b30a8b4cd4167
Repository branch: master
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure -C'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Robert Pluim]

>>>>> On Wed, 16 Nov 2022 09:47:31 +0100, Robert Pluim <rpluim@gmail.com> said:

    Robert> 1. Change `kbd' to always return a vector even if the input is
    Robert> ascii-only, which makes [4] work

I tried that, and it seems to work, but I guess itʼs kind of
risky. Just what we need to see if people are testing the pre-test
properly 😸

    Robert> 2. Change `keymap-set' to convert ascii-only strings to the format in
    Robert> [5] or [6]. Probably just a call to `string-to-vector' is enough.

This is less invasive. Something like this:

diff --git a/lisp/keymap.el b/lisp/keymap.el
index eaeba96644..deb44844fb 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -61,7 +61,23 @@ keymap-set
   ;; If we're binding this key to another key, then parse that other
   ;; key, too.
   (when (stringp definition)
-    (keymap--check definition)
+    ;; For backwards compatibility, we want people to be able to say
+    ;; "hello" instead of forcing them to say "h e l l o", and to fix
+    ;; the common mistake of specifying a string with non-ascii
+    ;; characters, we convert to a vector instead.
+    (cond
+     ((let ((case-fold-search nil)) ;; no special chars or keywords
+             (not (string-match-p
+                   (rx (or " " "^" "<" ">" "-" "\\"
+                           "NUL" "RET" "TAB" "LFD"
+                           "ESC" "SPC" "DEL")))))
+      (setq definition (string-join (seq-split definition 1) " "))
+      (keymap--check definition))
+     ((string-match-p ;; non-ascii characters
+       "[[:nonascii:]]" definition)
+      (setq definition (string-to-vector definition)))
+     (t ;; kbd syntax
+      (keymap--check definition)))
     (setq definition (key-parse definition)))
   (define-key keymap (key-parse key) definition))


Robert
-- 

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Stefan Kangas]

Robert Pluim <rpluim@gmail.com> writes:

> After reading up on `key-valid-p':
>
> 5. (keymap-global-set "C-c h" "h e l l o") => success!
>
> or alternatively
>
> 6. (keymap-global-set "C-c h" [?h ?e ?l ?l ?o]) => success!
>
> Whilst not strictly a regression, this behaviour is confusing and
> unhelpful, and the solution is not easily found. I can think of two
> solutions:
>
> 1. Change `kbd' to always return a vector even if the input is
> ascii-only, which makes [4] work
> 2. Change `keymap-set' to convert ascii-only strings to the format in
> [5] or [6]. Probably just a call to `string-to-vector' is enough.

I feel like the second alternative goes against the design of
`keymap-global-set', where the idea explicitly was to only support a KEY
argument that is `key-valid-p'.

Could we perhaps introduce a new optional argument to treat the argument
as a literal string?  Or if really want it to not be `key-valid-p', to
at least require it to be something like '(literal "foo") ?

It would be useful to here what Lars thinks (in Cc).

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Robert Pluim]

>>>>> On Thu, 24 Nov 2022 16:14:05 -0800, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> Robert Pluim <rpluim@gmail.com> writes:
    >> After reading up on `key-valid-p':
    >> 
    >> 5. (keymap-global-set "C-c h" "h e l l o") => success!
    >> 
    >> or alternatively
    >> 
    >> 6. (keymap-global-set "C-c h" [?h ?e ?l ?l ?o]) => success!
    >> 
    >> Whilst not strictly a regression, this behaviour is confusing and
    >> unhelpful, and the solution is not easily found. I can think of two
    >> solutions:
    >> 
    >> 1. Change `kbd' to always return a vector even if the input is
    >> ascii-only, which makes [4] work
    >> 2. Change `keymap-set' to convert ascii-only strings to the format in
    >> [5] or [6]. Probably just a call to `string-to-vector' is enough.

    Stefan> I feel like the second alternative goes against the design of
    Stefan> `keymap-global-set', where the idea explicitly was to only support a KEY
    Stefan> argument that is `key-valid-p'.

It was? If so, then it should not have been touted as the replacement
for `global-set-key' everywhere without a big warning sign.

    Stefan> Could we perhaps introduce a new optional argument to treat the argument
    Stefan> as a literal string?  Or if really want it to not be `key-valid-p', to
    Stefan> at least require it to be something like '(literal "foo") ?

(string-to-vector "foo") will do. But that just highlights the problem
even more: if itʼs that simple, why canʼt `keymap-global-set' do that
internally instead of forcing users to jump through hoops?

Robert
-- 

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Stefan Kangas]

Robert Pluim <rpluim@gmail.com> writes:

>     Stefan> I feel like the second alternative goes against the design of
>     Stefan> `keymap-global-set', where the idea explicitly was to only support a KEY
>     Stefan> argument that is `key-valid-p'.
>
> It was? If so, then it should not have been touted as the replacement
> for `global-set-key' everywhere without a big warning sign.

Yes, see the discussions about it at the time.  The same idea is there
in all the new keybinding functions.

>     Stefan> Could we perhaps introduce a new optional argument to treat the argument
>     Stefan> as a literal string?  Or if really want it to not be `key-valid-p', to
>     Stefan> at least require it to be something like '(literal "foo") ?
>
> (string-to-vector "foo") will do. But that just highlights the problem
> even more: if itʼs that simple, why canʼt `keymap-global-set' do that
> internally instead of forcing users to jump through hoops?

AFAIU, because otherwise we can't have error handling for common typos,
such as:

    (keymap-global-set "xo") ; bad, I actually want "x o"

I think catching this is more important than supporting the somewhat
nicer syntax for inserting strings with a key binding (which you can
still do by other means, as you point out).

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Robert Pluim]

>>>>> On Fri, 25 Nov 2022 00:25:34 -0800, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> AFAIU, because otherwise we can't have error handling for common typos,
    Stefan> such as:

    Stefan>     (keymap-global-set "xo") ; bad, I actually want "x o"

    Stefan> I think catching this is more important than supporting the somewhat
    Stefan> nicer syntax for inserting strings with a key binding (which you can
    Stefan> still do by other means, as you point out).

Then perhaps we should just change the docstrings and manual to
explain that binding a key to a string of characters should use
`kmacro' (since `kbd' is not the right thing for ASCII-only entry)

Robert
-- 

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Robert Pluim]

>>>>> On Thu, 15 Dec 2022 09:42:46 +0100, Robert Pluim <rpluim@gmail.com> said:


    Robert> Then perhaps we should just change the docstrings and manual to
    Robert> explain that binding a key to a string of characters should use
    Robert> `kmacro' (since `kbd' is not the right thing for ASCII-only entry)

I was going to write 'Stefan why did you overwrite my patch', but itʼs
me who didnʼt follow up :-)

Changing `kbd' to always return a vector, attractive as it is to the
purist in me, falls foul of existing code that expects eg `(kbd
"RET")' to return a string (and thatʼs just in Emacsʼ code). I guess
we could add an 'always produce a vector' argument to `kbd', but
perhaps just recommending `kmacro' is best, since itʼs a fairly niche
usage.

This is against emacs-29

Robert
-- 

diff --git c/doc/emacs/custom.texi i/doc/emacs/custom.texi
index 4bd78f3ce83..d59440e203d 100644
--- c/doc/emacs/custom.texi
+++ i/doc/emacs/custom.texi
@@ -1933,8 +1933,38 @@ Init Rebinding
 (keymap-global-set "<mouse-2>" 'mouse-save-then-kill)
 @end example
 
-  Language and coding systems may cause problems with key bindings for
-non-@acronym{ASCII} characters.  @xref{Init Non-ASCII}.
+@cindex binding key to string
+  Key sequences can also be bound directly to Lisp strings rather than
+commands.  Such strings are written using the same syntax as key
+sequences.  For example, to bind @kbd{C-c h} to the string
+@samp{hello}:
+
+@example
+(keymap-global-set "C-c h" "h e l l o")
+@end example
+
+  Since this is somewhat cumbersome to write, the convenience function
+@code{kmacro} can be used instead:
+
+@example
+(keymap-global-set "C-c h" (kmacro "hello"))
+@end example
+
+  Non-@acronym{ASCII} characters can be specified directly in the
+string. To bind to  e.g. @samp{ol@U{00E1}}, use:
+
+@example
+(keymap-global-set "C-c h" (kmacro "ol@U{00E1}"))
+@end example
+
+  However, be aware that language and coding systems may cause
+problems with key bindings for non-@acronym{ASCII} characters.
+@xref{Init Non-ASCII}.  It may thus be better to use the following
+instead:
+
+@example
+(keymap-global-set "C-c h" (kmacro "ol\u00E1"))
+@end example
 
 @findex global-set-key
 @findex define-key
diff --git c/lisp/keymap.el i/lisp/keymap.el
index 4bdf65d39fa..592a9fd89ab 100644
--- c/lisp/keymap.el
+++ i/lisp/keymap.el
@@ -84,6 +84,9 @@ keymap-global-set
 If COMMAND is a string (which can only happen when this function is
 called from Lisp), it must satisfy `key-valid-p'.
 
+The `kmacro' convenience function converts a simple string of
+characters to an equivalent form that is acceptable for COMMAND.
+
 Note that if KEY has a local binding in the current buffer,
 that local binding will continue to shadow any global binding
 that you make with this function."
@@ -108,6 +111,9 @@ keymap-local-set
 If COMMAND is a string (which can only happen when this function is
 called from Lisp), it must satisfy `key-valid-p'.
 
+The `kmacro' convenience function converts a simple string of
+characters to an equivalent form that is acceptable for COMMAND.
+
 The binding goes in the current buffer's local keymap, 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))

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Eli Zaretskii]

> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 59305@debbugs.gnu.org
> From: Robert Pluim <rpluim@gmail.com>
> Date: Fri, 26 Apr 2024 16:34:29 +0200
> 
> >>>>> On Thu, 15 Dec 2022 09:42:46 +0100, Robert Pluim <rpluim@gmail.com> said:
> 
> 
>     Robert> Then perhaps we should just change the docstrings and manual to
>     Robert> explain that binding a key to a string of characters should use
>     Robert> `kmacro' (since `kbd' is not the right thing for ASCII-only entry)
> 
> I was going to write 'Stefan why did you overwrite my patch', but itʼs
> me who didnʼt follow up :-)
> 
> Changing `kbd' to always return a vector, attractive as it is to the
> purist in me, falls foul of existing code that expects eg `(kbd
> "RET")' to return a string (and thatʼs just in Emacsʼ code). I guess
> we could add an 'always produce a vector' argument to `kbd', but
> perhaps just recommending `kmacro' is best, since itʼs a fairly niche
> usage.
> 
> This is against emacs-29

Stefan, any comments?

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Whilst not strictly a regression, this behaviour is confusing and
>> unhelpful, and the solution is not easily found. I can think of two
>> solutions:
>>
>> 1. Change `kbd' to always return a vector even if the input is
>> ascii-only, which makes [4] work

`key-parse` always returns a vector.

>> 2. Change `keymap-set' to convert ascii-only strings to the format in
>> [5] or [6]. Probably just a call to `string-to-vector' is enough.
>
> I feel like the second alternative goes against the design of
> `keymap-global-set', where the idea explicitly was to only support a KEY
> argument that is `key-valid-p'.

The problem with 2 is that it requires a heuristic that tries to guess
which kind of string it is (either the good "new" kbd-style or the old
bad event-sequence).  This *will* misfire sooner or later.

>> (string-to-vector "foo") will do. But that just highlights the problem
>> even more: if itʼs that simple, why canʼt `keymap-global-set' do that
>> internally instead of forcing users to jump through hoops?
> AFAIU, because otherwise we can't have error handling for common typos,
> such as:

It's worse than that: we cannot reliably know whether the user meant for
the string to be a kbd-style description of events or a "raw" sequence
of events.

E.g. should

    (keymap-global-set <foo> "C-a")

remap that <foo> sequence to [?\C-a] or to [?C ?- ?a]?

> Then perhaps we should just change the docstrings and manual to
> explain that binding a key to a string of characters should use
> `kmacro' (since `kbd' is not the right thing for ASCII-only entry)

I fully support this: I think we should phase out the
use of strings/vectors as commands.


        Stefan

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Robert Pluim]

>>>>> On Thu, 02 May 2024 09:13:41 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    >>> Whilst not strictly a regression, this behaviour is confusing and
    >>> unhelpful, and the solution is not easily found. I can think of two
    >>> solutions:
    >>> 
    >>> 1. Change `kbd' to always return a vector even if the input is
    >>> ascii-only, which makes [4] work

    Stefan> `key-parse` always returns a vector.

You want us to recommend this?
 
(keymap-global-set "C-a" (key-parse "hello"))

That works, but the documentation about the new format accepted
there says you should put spaces between the chars, and

(key-valid-p "hello") => nil

so Iʼd still worry about future regressions caused by `key-parse'
being changed to enforce `key-valid-p'

    >>> 2. Change `keymap-set' to convert ascii-only strings to the format in
    >>> [5] or [6]. Probably just a call to `string-to-vector' is enough.
    >> 
    >> I feel like the second alternative goes against the design of
    >> `keymap-global-set', where the idea explicitly was to only support a KEY
    >> argument that is `key-valid-p'.

    Stefan> The problem with 2 is that it requires a heuristic that tries to guess
    Stefan> which kind of string it is (either the good "new" kbd-style or the old
    Stefan> bad event-sequence).  This *will* misfire sooner or later.

ok

    >>> (string-to-vector "foo") will do. But that just highlights the problem
    >>> even more: if itʼs that simple, why canʼt `keymap-global-set' do that
    >>> internally instead of forcing users to jump through hoops?
    >> AFAIU, because otherwise we can't have error handling for common typos,
    >> such as:

    Stefan> It's worse than that: we cannot reliably know whether the user meant for
    Stefan> the string to be a kbd-style description of events or a "raw" sequence
    Stefan> of events.

ok

    Stefan> E.g. should

    Stefan>     (keymap-global-set <foo> "C-a")

    Stefan> remap that <foo> sequence to [?\C-a] or to [?C ?- ?a]?

    >> Then perhaps we should just change the docstrings and manual to
    >> explain that binding a key to a string of characters should use
    >> `kmacro' (since `kbd' is not the right thing for ASCII-only entry)

    Stefan> I fully support this: I think we should phase out the
    Stefan> use of strings/vectors as commands.

I wrote some doc changes in a previous message for this bug,
recommending `kmacro', although we could recommend `key-parse'
instead. Comments welcome

Robert
-- 

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>     Stefan> `key-parse` always returns a vector.
> You want us to recommend this?
> (keymap-global-set "C-a" (key-parse "hello"))

Sorry, I was not thinking right.  The function you'd need to use to
convert "hello" into the proper form for `keymap-global-set` is either:

- `key-description`: does the "reverse" of `kbd` and `key-parse`.
- `kmacro--to-vector`: turns an old-style string of events to a vector
  of events.

> so Iʼd still worry about future regressions caused by `key-parse'
> being changed to enforce `key-valid-p'

Indeed.  Not only that, but it'll mis-interpret `C-a` to mean a single
event instead of 3 events.


        Stefan

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Robert Pluim]

>>>>> On Mon, 06 May 2024 09:24:56 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    Stefan> `key-parse` always returns a vector.
    >> You want us to recommend this?
    >> (keymap-global-set "C-a" (key-parse "hello"))

    Stefan> Sorry, I was not thinking right.  The function you'd need to use to
    Stefan> convert "hello" into the proper form for `keymap-global-set` is either:

    Stefan> - `key-description`: does the "reverse" of `kbd` and `key-parse`.
    Stefan> - `kmacro--to-vector`: turns an old-style string of events to a vector
    Stefan>   of events.

`key-description' does the wrong thing with multibyte strings:

(key-description "olá") => "o l M-a"

`kmacro--to-vector' does the right thing:

(kmacro--to-vector "hello") => [104 101 108 108 111]
(kmacro--to-vector "olá") => [111 108 225]

We could invent 'YetAnotherName'™ or we could just use `kmacro'

(kmacro "hello") => #[256 "\301\242\302%1b\x1c\305\300%2\306#\210\301\f\240*\207" [[104 101 108 108 111] (0) "%d" kmacro-counter-format-start kmacro-counter execute-kbd-macro kmacro-loop-setup-function] 5 kmacro]
(kmacro "olá") => #[256 "\301\242\302%1b\x1c\305\300%2\306#\210\301\f\240*\207" [[111 108 225] (0) "%d" kmacro-counter-format-start kmacro-counter execute-kbd-macro kmacro-loop-setup-function] 5 kmacro]

although that *also* has the caveat about `key-valid-p'

Thatʼs the thing about Emacs: so many options to choose from :-)

    >> so Iʼd still worry about future regressions caused by `key-parse'
    >> being changed to enforce `key-valid-p'

    Stefan> Indeed.  Not only that, but it'll mis-interpret `C-a` to mean a single
    Stefan> event instead of 3 events.

Right.

Robert
-- 

bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> `key-description' does the wrong thing with multibyte strings:
> (key-description "olá") => "o l M-a"

Indeed!  I just pushed a fix for that to `master`.


        Stefan