bug#81462: [PATCH] Skip non-key events in `where-is-internal' with FIRSTONLY
Aaron Zeng via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <[email protected]>
| Newsgroups | gmane.emacs.bugs |
|---|---|
| Message-ID | <CAB7SQMF-8O52KrxERHT096eDJU5JDqopEXYdadwfraaBx5X8BQ@mail.gmail.com> |
On Thu, Aug 13, 2026 at 1:48 PM Aaron Zeng <[email protected]> wrote: > > On Wed, Aug 12, 2026 at 4:04 PM Stefan Monnier <[email protected]> wrote: > > > > > FWIW, here's a totally different idea that might solve the same > > > problem. What if where-is-internal had a SHORTEST argument where you > > > could ask it for the shortest binding rather than the first one? > > > > I thought that's already what it does. At least if you ask for all > > bindings, they should be sorted shortest to longest. > > It doesn't seem to. Digging into this further, I see that Fwhere_is_internal says that the `sequences' variable should contain: /* Potentially relevant bindings in "shortest to longest" order. */ Lisp_Object sequences = Qnil; This is calculated by where_is_internal, which has this comment preceding it: /* Return the list of bindings found. This list is ordered "longest to shortest". It may include bindings that are actually shadowed by others, as well as duplicate bindings and remapping bindings. The list returned is potentially shared with where_is_cache, so be careful not to modify it via side-effects. */ static Lisp_Object where_is_internal (Lisp_Object definition, Lisp_Object keymaps, bool noindirect, bool nomenus) However, I think this ordering claim might only be true for keys within a single keymap from the keymaps argument. It is not true across multiple keymaps. I think maybe the thing that would fix my problem most elegantly is if where_is_internal's return value were reliably sorted shortest-to-longest as the comment says. The new attached patch does the above, and adds new tests. I confirmed that the tests fail without the changes to keymap.c. By the way, I suppose this bug should be renamed, or maybe I should open a new bug to reframe the conversation on the underlying problem I'm trying to solve. Sorry, not yet familiar with what the convention is in this case. > Also, setting where-is-preferred-modifier to `ctrl' doesn't seem to > matter to any of the above function calls. It seems this is because preferred_sequence_p only returns a match if the event includes the ctrl_modifier bit but not for ASCII control characters like C-c.
0001-Fix-where_is_internal-to-sort-shortest-to-longest-as.patch
(text/x-patch, 4.9 KB)
From e218b84e564d1c7ee29ed7bd92c42fcf8ae3a4d4 Mon Sep 17 00:00:00 2001 From: "Aaron L. Zeng" <[email protected]> Date: Thu, 20 Aug 2026 17:51:00 -0400 Subject: [PATCH] Fix where_is_internal to sort shortest-to-longest as advertised * src/keymap.c (insert_sequence_by_len_desc): New function. (where_is_internal_1): Keep sequences sorted by length. (Fwhere_is_internal): Document shortest-first semantics and maintain ordering across remapped sequences. * test/src/keymap-tests.el (keymap-where-is-internal/shortest-first-across-keymaps): New test. --- src/keymap.c | 42 ++++++++++++++++++++++++++++++++++++---- test/src/keymap-tests.el | 16 +++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/keymap.c b/src/keymap.c index 9c2aa7634fc..eb5bb05856c 100644 --- a/src/keymap.c +++ b/src/keymap.c @@ -2480,6 +2480,28 @@ shadow_lookup (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default, static Lisp_Object Vmouse_events; +/* Insert SEQUENCE into SEQUENCES, which is sorted descending by length. + Ties are resolved by inserting SEQUENCE earliest. */ +static Lisp_Object +insert_sequence_by_len_desc (Lisp_Object sequence, Lisp_Object sequences) +{ + ptrdiff_t len = ASIZE (sequence); + Lisp_Object prev = Qnil, tail = sequences; + + while (CONSP (tail) && ASIZE (XCAR (tail)) > len) + { + prev = tail; + tail = XCDR (tail); + } + + Lisp_Object cell = Fcons (sequence, tail); + if (NILP (prev)) + return cell; + + XSETCDR (prev, cell); + return sequences; +} + struct where_is_internal_data { Lisp_Object definition, this, last; bool last_is_meta, noindirect; @@ -2601,6 +2623,10 @@ DEFUN ("where-is-internal", Fwhere_is_internal, Swhere_is_internal, 1, 5, 0, that use the modifier key specified in `where-is-preferred-modifier' \(or their meta variants) and entirely reject menu bindings. +The key sequences are searched in increasing order of length, so the +first key sequence found is typically the shortest (but may not be, +depending on a preferred modifier key or advertised bindings). + If optional 4th arg NOINDIRECT is non-nil, don't extract the commands inside menu-items. This makes it possible to search for a menu-item itself. @@ -2690,7 +2716,7 @@ DEFUN ("where-is-internal", Fwhere_is_internal, Swhere_is_internal, 1, 5, 0, /* If we're at the end of the `sequences' list and we haven't considered remapped sequences yet, copy them over and process them. */ - || (!remapped && (sequences = remapped_sequences, + || (!remapped && (sequences = Fnreverse (remapped_sequences), remapped = true, CONSP (sequences)))) { @@ -2722,7 +2748,12 @@ DEFUN ("where-is-internal", Fwhere_is_internal, Swhere_is_internal, 1, 5, 0, { Lisp_Object seqs = where_is_internal (function, keymaps, !NILP (noindirect), nomenus); - remapped_sequences = nconc2 (Freverse (seqs), remapped_sequences); + while (CONSP (seqs)) + { + remapped_sequences = + insert_sequence_by_len_desc (XCAR (seqs), remapped_sequences); + seqs = XCDR (seqs); + } continue; } @@ -2824,10 +2855,13 @@ where_is_internal_1 (Lisp_Object key, Lisp_Object binding, Lisp_Object args, voi if (!NILP (where_is_cache)) { Lisp_Object sequences = Fgethash (binding, where_is_cache, Qnil); - Fputhash (binding, Fcons (sequence, sequences), where_is_cache); + /* During cache filling, it is okay to mutate the lists. */ + Fputhash (binding, + insert_sequence_by_len_desc (sequence, sequences), + where_is_cache); } else - d->sequences = Fcons (sequence, d->sequences); + d->sequences = insert_sequence_by_len_desc (sequence, d->sequences); } /* describe-bindings - summarizing all the bindings in a set of keymaps. */ diff --git a/test/src/keymap-tests.el b/test/src/keymap-tests.el index c53ab7871c3..bb279720576 100644 --- a/test/src/keymap-tests.el +++ b/test/src/keymap-tests.el @@ -302,6 +302,22 @@ keymap-where-is-internal/preferred-modifier-is-a-string (where-is-internal 'execute-extended-command global-map t)) [#x8000078]))) +(ert-deftest keymap-where-is-internal/shortest-first-across-keymaps () + (let ((map1 (define-keymap "x" 'foo)) + (map2 (define-keymap "y z" 'foo))) + (should (equal (where-is-internal 'foo (list map1 map2)) + (list [?x] [?y ?z]))) + (should (equal (where-is-internal 'foo (list map2 map1)) + (list [?x] [?y ?z]))) + (should (equal (where-is-internal 'foo (list map1 map2) t) [?x])) + (should (equal (where-is-internal 'foo (list map2 map1) t) [?x])) + + ;; `where-is-preferred-modifier' is more important than length + (keymap-set map1 "s-f s-o s-o" 'foo) + (let ((where-is-preferred-modifier 'super)) + (should (equal (where-is-internal 'foo (list map1 map2) t) (kbd "s-f s-o s-o"))) + (should (equal (where-is-internal 'foo (list map2 map1) t) (kbd "s-f s-o s-o")))))) + ;;;; describe_vector -- 2.43.7