* Re: master f3aa648: Make `lookup-key' understand the new key sequence syntax [not found] ` <20211019030802.83DE621120@vcs0.savannah.gnu.org> @ 2021-10-19 12:45 ` Stefan Monnier 2021-10-19 13:40 ` Lars Ingebrigtsen 0 siblings, 1 reply; 5+ messages in thread From: Stefan Monnier @ 2021-10-19 12:45 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: emacs-devel I think your `possibly_translate_key_sequence` should be extended to cover the XEmacs format as well, And it should be exported to ELisp. And I'd argue it should also convert strings to vectors. Oh and also convert [M-C-next] to [C-M-next]. WDYT? Stefan > +static Lisp_Object > +possibly_translate_key_sequence (Lisp_Object key, ptrdiff_t *length) > +{ > + if (VECTORP (key) && ASIZE (key) == 1 && STRINGP (AREF (key, 0))) > + { > + /* KEY is on the ["C-c"] format, so translate to internal > + format. */ > + if (NILP (Ffboundp (Qkbd_valid_p))) > + xsignal2 (Qerror, > + build_string ("`kbd-valid-p' is not defined, so this syntax can't be used: %s"), > + key); > + if (NILP (call1 (Qkbd_valid_p, AREF (key, 0)))) > + xsignal2 (Qerror, build_string ("Invalid `kbd' syntax: %S"), key); > + key = call1 (Qkbd, AREF (key, 0)); > + *length = CHECK_VECTOR_OR_STRING (key); > + if (*length == 0) > + xsignal2 (Qerror, build_string ("Invalid `kbd' syntax: %S"), key); > + } > + > + return key; > +} > + > /* GC is possible in this function if it autoloads a keymap. */ > > DEFUN ("define-key", Fdefine_key, Sdefine_key, 3, 3, 0, > @@ -1084,21 +1106,7 @@ binding KEY to DEF is added at the front of KEYMAP. */) > def = tmp; > } > > - if (VECTORP (key) && ASIZE (key) == 1 && STRINGP (AREF (key, 0))) > - { > - /* KEY is on the ["C-c"] format, so translate to internal > - format. */ > - if (NILP (Ffboundp (Qkbd_valid_p))) > - xsignal2 (Qerror, > - build_string ("`kbd-valid-p' is not defined, so this syntax can't be used: %s"), > - key); > - if (NILP (call1 (Qkbd_valid_p, AREF (key, 0)))) > - xsignal2 (Qerror, build_string ("Invalid `kbd' syntax: %S"), key); > - key = call1 (Qkbd, AREF (key, 0)); > - length = CHECK_VECTOR_OR_STRING (key); > - if (length == 0) > - xsignal2 (Qerror, build_string ("Invalid `kbd' syntax: %S"), key); > - } > + key = possibly_translate_key_sequence (key, &length); > > ptrdiff_t idx = 0; > while (1) > @@ -1229,6 +1237,8 @@ recognize the default bindings, just as `read-key-sequence' does. */) > if (length == 0) > return keymap; > > + key = possibly_translate_key_sequence (key, &length); > + > ptrdiff_t idx = 0; > while (1) > { > diff --git a/test/src/keymap-tests.el b/test/src/keymap-tests.el > index 68b42c3..13f47b4 100644 > --- a/test/src/keymap-tests.el > +++ b/test/src/keymap-tests.el > @@ -317,6 +317,13 @@ g .. h foo > (should (equal (single-key-description 'C-s-home) > "C-s-<home>"))) > > +(ert-deftest keymap-test-lookups () > + (should (eq (lookup-key (current-global-map) "\C-x\C-f") 'find-file)) > + (should (eq (lookup-key (current-global-map) [(control x) (control f)]) > + 'find-file)) > + (should (eq (lookup-key (current-global-map) ["C-x C-f"]) 'find-file)) > + (should (eq (lookup-key (current-global-map) [?\C-x ?\C-f]) 'find-file))) > + > (provide 'keymap-tests) > > ;;; keymap-tests.el ends here ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: master f3aa648: Make `lookup-key' understand the new key sequence syntax 2021-10-19 12:45 ` master f3aa648: Make `lookup-key' understand the new key sequence syntax Stefan Monnier @ 2021-10-19 13:40 ` Lars Ingebrigtsen 2021-10-19 14:13 ` Stefan Monnier 0 siblings, 1 reply; 5+ messages in thread From: Lars Ingebrigtsen @ 2021-10-19 13:40 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > I think your `possibly_translate_key_sequence` should be extended to > cover the XEmacs format as well, And it should be exported to ELisp. lookup-key/define-key already understands that format, but mixes the interpretation into the code itself. (lookup-key (current-global-map) [(control x) (control f)]) => find-file Pulling it out of the code and putting it into possibly_translate_key_sequence would be fine, I think. > And I'd argue it should also convert strings to vectors. Sure. > Oh and also convert [M-C-next] to [C-M-next]. Uhm... I'm not sure what the impact would be... -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: master f3aa648: Make `lookup-key' understand the new key sequence syntax 2021-10-19 13:40 ` Lars Ingebrigtsen @ 2021-10-19 14:13 ` Stefan Monnier 2021-10-19 14:17 ` Lars Ingebrigtsen 0 siblings, 1 reply; 5+ messages in thread From: Stefan Monnier @ 2021-10-19 14:13 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: emacs-devel Lars Ingebrigtsen [2021-10-19 15:40:57] wrote: > Stefan Monnier <monnier@iro.umontreal.ca> writes: >> I think your `possibly_translate_key_sequence` should be extended to >> cover the XEmacs format as well, And it should be exported to ELisp. > > lookup-key/define-key already understands that format, but mixes the > interpretation into the code itself. > > (lookup-key (current-global-map) [(control x) (control f)]) > => find-file > > Pulling it out of the code and putting it into > possibly_translate_key_sequence would be fine, I think. > >> And I'd argue it should also convert strings to vectors. > > Sure. > >> Oh and also convert [M-C-next] to [C-M-next]. > > Uhm... I'm not sure what the impact would be... In terms of behavior, no impact: ELISP> (let ((map (make-sparse-keymap))) (define-key map [C-M-next] 'dummy) map) (keymap (C-M-next . dummy)) ELISP> (let ((map (make-sparse-keymap))) (define-key map [M-C-next] 'dummy) map) (keymap (C-M-next . dummy)) ELISP> -- Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: master f3aa648: Make `lookup-key' understand the new key sequence syntax 2021-10-19 14:13 ` Stefan Monnier @ 2021-10-19 14:17 ` Lars Ingebrigtsen 2021-10-19 14:47 ` Stefan Monnier 0 siblings, 1 reply; 5+ messages in thread From: Lars Ingebrigtsen @ 2021-10-19 14:17 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > In terms of behavior, no impact: > > ELISP> (let ((map (make-sparse-keymap))) (define-key map > ELISP> [C-M-next] 'dummy) map) > (keymap (C-M-next . dummy)) > > ELISP> (let ((map (make-sparse-keymap))) (define-key map > ELISP> [M-C-next] 'dummy) map) > (keymap (C-M-next . dummy)) Would this just be a simplification of the innards of `define-key', then? -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: master f3aa648: Make `lookup-key' understand the new key sequence syntax 2021-10-19 14:17 ` Lars Ingebrigtsen @ 2021-10-19 14:47 ` Stefan Monnier 0 siblings, 0 replies; 5+ messages in thread From: Stefan Monnier @ 2021-10-19 14:47 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: emacs-devel Lars Ingebrigtsen [2021-10-19 16:17:50] wrote: > Stefan Monnier <monnier@iro.umontreal.ca> writes: >> In terms of behavior, no impact: >> >> ELISP> (let ((map (make-sparse-keymap))) (define-key map >> ELISP> [C-M-next] 'dummy) map) >> (keymap (C-M-next . dummy)) >> >> ELISP> (let ((map (make-sparse-keymap))) (define-key map >> ELISP> [M-C-next] 'dummy) map) >> (keymap (C-M-next . dummy)) > > Would this just be a simplification of the innards of `define-key', then? It would move all the canonicalization to a separate function, yes. Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-19 14:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20211019030801.23534.79581@vcs0.savannah.gnu.org> [not found] ` <20211019030802.83DE621120@vcs0.savannah.gnu.org> 2021-10-19 12:45 ` master f3aa648: Make `lookup-key' understand the new key sequence syntax Stefan Monnier 2021-10-19 13:40 ` Lars Ingebrigtsen 2021-10-19 14:13 ` Stefan Monnier 2021-10-19 14:17 ` Lars Ingebrigtsen 2021-10-19 14:47 ` Stefan Monnier
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.