unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key
@ 2022-11-16  8:47 Robert Pluim
  2022-11-22 16:58 ` Robert Pluim
  2022-11-25  0:14 ` Stefan Kangas
  0 siblings, 2 replies; 5+ messages in thread
From: Robert Pluim @ 2022-11-16  8:47 UTC (permalink / raw)
  To: 59305

`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






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

* bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key
  2022-11-16  8:47 bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key Robert Pluim
@ 2022-11-22 16:58 ` Robert Pluim
  2022-11-25  0:14 ` Stefan Kangas
  1 sibling, 0 replies; 5+ messages in thread
From: Robert Pluim @ 2022-11-22 16:58 UTC (permalink / raw)
  To: 59305

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





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

* bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key
  2022-11-16  8:47 bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key Robert Pluim
  2022-11-22 16:58 ` Robert Pluim
@ 2022-11-25  0:14 ` Stefan Kangas
  2022-11-25  8:01   ` Robert Pluim
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Kangas @ 2022-11-25  0:14 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Lars Ingebrigtsen, 59305

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





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

* bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key
  2022-11-25  0:14 ` Stefan Kangas
@ 2022-11-25  8:01   ` Robert Pluim
  2022-11-25  8:25     ` Stefan Kangas
  0 siblings, 1 reply; 5+ messages in thread
From: Robert Pluim @ 2022-11-25  8:01 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Lars Ingebrigtsen, 59305

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





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

* bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key
  2022-11-25  8:01   ` Robert Pluim
@ 2022-11-25  8:25     ` Stefan Kangas
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Kangas @ 2022-11-25  8:25 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Lars Ingebrigtsen, 59305

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





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

end of thread, other threads:[~2022-11-25  8:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-16  8:47 bug#59305: 29.0.50; keymap-global-set handling of string bindings different from global-set-key Robert Pluim
2022-11-22 16:58 ` Robert Pluim
2022-11-25  0:14 ` Stefan Kangas
2022-11-25  8:01   ` Robert Pluim
2022-11-25  8:25     ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).