all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
@ 2023-03-15 16:07 Jonas Bernoulli
  2023-03-15 16:51 ` Jonas Bernoulli
  2023-03-15 17:13 ` Eli Zaretskii
  0 siblings, 2 replies; 28+ messages in thread
From: Jonas Bernoulli @ 2023-03-15 16:07 UTC (permalink / raw)
  To: 62207

(keymap-unset map key t) is supposed to remove a binding completely,
and that works if there actually is a binding:

  (let ((map (make-sparse-keymap)))
    (keymap-set map "i" 'bound)
    (keymap-unset map "i" t)
    map)
  => (keymap)

However if you try to remove a binding that does not actually exist,
then the opposite happens, a "nil binding" is *added*:

  (let ((map (make-sparse-keymap)))
    (keymap-unset map "i" t)
    map)
  => (keymap (105))





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 16:07 bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding Jonas Bernoulli
@ 2023-03-15 16:51 ` Jonas Bernoulli
  2023-03-15 17:36   ` Robert Pluim
  2023-03-15 17:13 ` Eli Zaretskii
  1 sibling, 1 reply; 28+ messages in thread
From: Jonas Bernoulli @ 2023-03-15 16:51 UTC (permalink / raw)
  To: 62207

As a side-note, it would be nice if it were possible to lookup a
key in a keymap only, while ignoring bindings in its parent keymap.

I was reminded of this because I had to resurrect some old code of
mine to work around this bug.  The workaround is:

(defun my--keymap-unset (keymap key &optional remove)
  (if remove
      (when (kmu-lookup-local-key keymap key nil t)
        (keymap-unset keymap key t))
    (keymap-unset keymap key)))

And below is the code I had to resurrect in order to make that happen.
If `lookup-key', and similar, took a NOPARENT argument, all of that
could be avoided.

(defun kmu-lookup-local-key ( keymap key
                              &optional accept-default no-remap position)
  "In KEYMAP, look up key sequence KEY.  Return the definition.

Unlike `keymap-lookup' (which see) this doesn't consider bindings
made in KEYMAP's parent keymap."
  (keymap-lookup (kmu--strip-keymap keymap)
                 key accept-default no-remap position))

(defun kmu--strip-keymap (keymap)
  "Return a copy of KEYMAP with all parent keymaps removed.

This not only removes the parent keymap of KEYMAP but also recursively
the parent keymap of any keymap a key in KEYMAP is bound to."
  (cl-labels ((strip-keymap (keymap)
                (set-keymap-parent keymap nil)
                (cl-loop for _key being the key-code of keymap
                         using (key-binding binding) do
                         (and (keymapp binding)
                              (not (kmu-prefix-command-p binding))
                              (strip-keymap binding)))
                keymap))
    (strip-keymap (copy-keymap keymap))))

(defun kmu-prefix-command-p (object)
  "Return non-nil if OBJECT is a symbol whose function definition is a keymap.
The value returned is the keymap stored as OBJECT's variable
definition or else the variable which holds the keymap."
  (and (symbolp object)
       (fboundp object)
       (keymapp (symbol-function object))
       (if (and (boundp  object)
                (keymapp (symbol-value object)))
           (symbol-value object)
         (kmu-keymap-variable (symbol-function object)))))

(defun kmu-keymap-variable-p (object)
  "Return t if OBJECT is a symbol whose variable definition is a keymap."
  (and (symbolp object)
       (boundp  object)
       (keymapp (symbol-value object))))





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 16:07 bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding Jonas Bernoulli
  2023-03-15 16:51 ` Jonas Bernoulli
@ 2023-03-15 17:13 ` Eli Zaretskii
  2023-03-15 17:39   ` Robert Pluim
  2023-03-15 18:02   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 2 replies; 28+ messages in thread
From: Eli Zaretskii @ 2023-03-15 17:13 UTC (permalink / raw)
  To: Jonas Bernoulli, Stefan Monnier, Lars Ingebrigtsen; +Cc: 62207

> From: Jonas Bernoulli <jonas@bernoul.li>
> Date: Wed, 15 Mar 2023 17:07:42 +0100
> 
> However if you try to remove a binding that does not actually exist,
> then the opposite happens, a "nil binding" is *added*:
> 
>   (let ((map (make-sparse-keymap)))
>     (keymap-unset map "i" t)
>     map)
>   => (keymap (105))

The same happens when you call define-key with REMOVE non-nil.
keymap-unset just calls define-key, and does little else.

Stefan, it sounds like the part of store_in_keymap after the label
keymap_end should do nothing if REMOVE is non-zero, am I right?





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 16:51 ` Jonas Bernoulli
@ 2023-03-15 17:36   ` Robert Pluim
  2023-03-15 18:12     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 28+ messages in thread
From: Robert Pluim @ 2023-03-15 17:36 UTC (permalink / raw)
  To: Jonas Bernoulli; +Cc: 62207

>>>>> On Wed, 15 Mar 2023 17:51:27 +0100, Jonas Bernoulli <jonas@bernoul.li> said:

    Jonas> As a side-note, it would be nice if it were possible to lookup a
    Jonas> key in a keymap only, while ignoring bindings in its parent keymap.

A feature request and a bug report? Tsk ;-) Luckily the infrastructure
is actually there already.

The following passes my admittedly quick testing for both.

diff --git c/lisp/keymap.el i/lisp/keymap.el
index 4f02639ffe2..706da70d360 100644
--- c/lisp/keymap.el
+++ i/lisp/keymap.el
@@ -370,7 +370,7 @@ key-translate
 	    (make-char-table 'keyboard-translate-table nil)))
   (aset keyboard-translate-table (key-parse from) (key-parse to)))
 
-(defun keymap-lookup (keymap key &optional accept-default no-remap position)
+(defun keymap-lookup (keymap key &optional accept-default no-remap position noparent)
   "Return the binding for command KEY in KEYMAP.
 KEY is a string that satisfies `key-valid-p'.
 
@@ -406,8 +406,10 @@ keymap-lookup
   (keymap--check key)
   (when (and keymap position)
     (error "Can't pass in both keymap and position"))
+  (when (and (not keymap) noparent)
+    (error "Must specify keymap when noparent is t"))
   (if keymap
-      (let ((value (lookup-key keymap (key-parse key) accept-default)))
+      (let ((value (lookup-key keymap (key-parse key) accept-default noparent)))
         (if (and (not no-remap)
                    (symbolp value))
             (or (command-remapping value) value)
diff --git c/src/keymap.c i/src/keymap.c
index 23453eaa9a6..a660a687994 100644
--- c/src/keymap.c
+++ i/src/keymap.c
@@ -887,22 +887,23 @@ store_in_keymap (Lisp_Object keymap, register Lisp_Object idx,
   keymap_end:
     /* We have scanned the entire keymap, and not found a binding for
        IDX.  Let's add one.  */
-    {
-      Lisp_Object elt;
+    if (!remove)
+      {
+	Lisp_Object elt;
 
-      if (CONSP (idx) && CHARACTERP (XCAR (idx)))
-	{
-	  /* IDX specifies a range of characters, and not all of them
-	     were handled yet, which means this keymap doesn't have a
-	     char-table.  So, we insert a char-table now.  */
-	  elt = Fmake_char_table (Qkeymap, Qnil);
-	  Fset_char_table_range (elt, idx, NILP (def) ? Qt : def);
-	}
-      else
-	elt = Fcons (idx, def);
-      CHECK_IMPURE (insertion_point, XCONS (insertion_point));
-      XSETCDR (insertion_point, Fcons (elt, XCDR (insertion_point)));
-    }
+	if (CONSP (idx) && CHARACTERP (XCAR (idx)))
+	  {
+	    /* IDX specifies a range of characters, and not all of them
+	       were handled yet, which means this keymap doesn't have a
+	       char-table.  So, we insert a char-table now.  */
+	    elt = Fmake_char_table (Qkeymap, Qnil);
+	    Fset_char_table_range (elt, idx, NILP (def) ? Qt : def);
+	  }
+	else
+	  elt = Fcons (idx, def);
+	CHECK_IMPURE (insertion_point, XCONS (insertion_point));
+	XSETCDR (insertion_point, Fcons (elt, XCDR (insertion_point)));
+      }
   }
 
   return def;
@@ -1240,14 +1241,15 @@ DEFUN ("command-remapping", Fcommand_remapping, Scommand_remapping, 1, 3, 0,
   if (NILP (keymaps))
     command = Fkey_binding (command_remapping_vector, Qnil, Qt, position);
   else
-    command = Flookup_key (keymaps, command_remapping_vector, Qnil);
+    command = Flookup_key (keymaps, command_remapping_vector, Qnil, Qnil);
   return FIXNUMP (command) ? Qnil : command;
 }
 
 static Lisp_Object
-lookup_key_1 (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default)
+lookup_key_1 (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default, Lisp_Object noparent)
 {
   bool t_ok = !NILP (accept_default);
+  bool noinherit = !NILP (noparent);
 
   if (!CONSP (keymap) && !NILP (keymap))
     keymap = get_keymap (keymap, true, true);
@@ -1275,7 +1277,7 @@ lookup_key_1 (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default)
       if (!FIXNUMP (c) && !SYMBOLP (c) && !CONSP (c) && !STRINGP (c))
 	message_with_string ("Key sequence contains invalid event %s", c, 1);
 
-      Lisp_Object cmd = access_keymap (keymap, c, t_ok, 0, 1);
+      Lisp_Object cmd = access_keymap (keymap, c, t_ok, noinherit, 1);
       if (idx == length)
 	return cmd;
 
@@ -1290,7 +1292,7 @@ lookup_key_1 (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default)
 /* Value is number if KEY is too long; nil if valid but has no definition.  */
 /* GC is possible in this function.  */
 
-DEFUN ("lookup-key", Flookup_key, Slookup_key, 2, 3, 0,
+DEFUN ("lookup-key", Flookup_key, Slookup_key, 2, 4, 0,
        doc: /* Look up key sequence KEY in KEYMAP.  Return the definition.
 This is a legacy function; see `keymap-lookup' for the recommended
 function to use instead.
@@ -1310,9 +1312,9 @@ DEFUN ("lookup-key", Flookup_key, Slookup_key, 2, 3, 0,
 usable as a general function for probing keymaps.  However, if the
 third optional argument ACCEPT-DEFAULT is non-nil, `lookup-key' will
 recognize the default bindings, just as `read-key-sequence' does.  */)
-  (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default)
+  (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default, Lisp_Object noparent)
 {
-  Lisp_Object found = lookup_key_1 (keymap, key, accept_default);
+  Lisp_Object found = lookup_key_1 (keymap, key, accept_default, noparent);
   if (!NILP (found) && !NUMBERP (found))
     return found;
 
@@ -1390,7 +1392,7 @@ DEFUN ("lookup-key", Flookup_key, Slookup_key, 2, 3, 0,
 	}
 
       /* Check for match.  */
-      found = lookup_key_1 (keymap, new_key, accept_default);
+      found = lookup_key_1 (keymap, new_key, accept_default, noparent);
       if (!NILP (found) && !NUMBERP (found))
 	break;
 
@@ -1432,7 +1434,7 @@ DEFUN ("lookup-key", Flookup_key, Slookup_key, 2, 3, 0,
 	}
 
       /* Check for match.  */
-      found = lookup_key_1 (keymap, new_key, accept_default);
+      found = lookup_key_1 (keymap, new_key, accept_default, noparent);
       if (!NILP (found) && !NUMBERP (found))
 	break;
     }
@@ -1823,7 +1825,7 @@ DEFUN ("key-binding", Fkey_binding, Skey_binding, 1, 4, 0,
     }
 
   Lisp_Object value = Flookup_key (Fcurrent_active_maps (Qt, position),
-				   key, accept_default);
+				   key, accept_default, Qnil);
 
   if (NILP (value) || FIXNUMP (value))
     return Qnil;
@@ -1864,7 +1866,7 @@ DEFUN ("minor-mode-key-binding", Fminor_mode_key_binding, Sminor_mode_key_bindin
   int j;
   for (int i = j = 0; i < nmaps; i++)
     if (!NILP (maps[i])
-	&& !NILP (binding = Flookup_key (maps[i], key, accept_default))
+	&& !NILP (binding = Flookup_key (maps[i], key, accept_default, Qnil))
 	&& !FIXNUMP (binding))
       {
 	if (KEYMAPP (binding))
@@ -2013,7 +2015,7 @@ DEFUN ("accessible-keymaps", Faccessible_keymaps, Saccessible_keymaps,
     {
       /* If a prefix was specified, start with the keymap (if any) for
 	 that prefix, so we don't waste time considering other prefixes.  */
-      Lisp_Object tem = Flookup_key (keymap, prefix, Qt);
+      Lisp_Object tem = Flookup_key (keymap, prefix, Qt, Qnil);
       /* Flookup_key may give us nil, or a number,
 	 if the prefix is not defined in this particular map.
 	 It might even give us a list that isn't a keymap.  */
@@ -2453,7 +2455,7 @@ preferred_sequence_p (Lisp_Object seq)
 shadow_lookup (Lisp_Object keymap, Lisp_Object key, Lisp_Object accept_default,
 	       bool remap)
 {
-  Lisp_Object value = Flookup_key (keymap, key, accept_default);
+  Lisp_Object value = Flookup_key (keymap, key, accept_default, Qnil);
 
   if (FIXNATP (value))          /* `key' is too long!  */
     return Qnil;
@@ -3237,7 +3239,7 @@ describe_vector (Lisp_Object vector, Lisp_Object prefix, Lisp_Object args,
 	 one in the same keymap.  */
       if (!NILP (entire_map))
 	{
-	  Lisp_Object tem = Flookup_key (entire_map, kludge, Qt);
+	  Lisp_Object tem = Flookup_key (entire_map, kludge, Qt, Qnil);
 
 	  if (!EQ (tem, definition))
 	    continue;





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 17:13 ` Eli Zaretskii
@ 2023-03-15 17:39   ` Robert Pluim
  2023-03-15 18:02   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 28+ messages in thread
From: Robert Pluim @ 2023-03-15 17:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62207, Lars Ingebrigtsen, Jonas Bernoulli, Stefan Monnier

>>>>> On Wed, 15 Mar 2023 19:13:20 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Jonas Bernoulli <jonas@bernoul.li>
    >> Date: Wed, 15 Mar 2023 17:07:42 +0100
    >> 
    >> However if you try to remove a binding that does not actually exist,
    >> then the opposite happens, a "nil binding" is *added*:
    >> 
    >> (let ((map (make-sparse-keymap)))
    >> (keymap-unset map "i" t)
    >> map)
    >> => (keymap (105))

    Eli> The same happens when you call define-key with REMOVE non-nil.
    Eli> keymap-unset just calls define-key, and does little else.

    Eli> Stefan, it sounds like the part of store_in_keymap after the label
    Eli> keymap_end should do nothing if REMOVE is non-zero, am I right?

Thatʼs the conclusion I came to. See the patch in my other message.

Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 17:13 ` Eli Zaretskii
  2023-03-15 17:39   ` Robert Pluim
@ 2023-03-15 18:02   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-17  8:23     ` Eli Zaretskii
  1 sibling, 1 reply; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-15 18:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Lars Ingebrigtsen, Jonas Bernoulli, 62207

> Stefan, it sounds like the part of store_in_keymap after the label
> keymap_end should do nothing if REMOVE is non-zero, am I right?

Sounds right, yes.  This said, the semantics of REMOVE a bit murky, so
I'd rather tell people not to use it.


        Stefan






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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 17:36   ` Robert Pluim
@ 2023-03-15 18:12     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-15 22:26       ` Jonas Bernoulli
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-15 18:12 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62207, Jonas Bernoulli

>     Jonas> As a side-note, it would be nice if it were possible to lookup a
>     Jonas> key in a keymap only, while ignoring bindings in its parent keymap.

Could you explain why you need `keymap-unset-key` and why you need to
lookup "all but the ignore the parent"?

I'm curious because the meaning of these can be subtly more complex than
meets the eye once you consider things like `make-compose-keymap`.

> A feature request and a bug report? Tsk ;-) Luckily the infrastructure
> is actually there already.

Hmm... if you need to add yet-another-arg to `Flookup_key`, than I'm not
sure it qualifies as "the infrastructure is actually there already."

BTW, `map-keymap-internal` could be another way to attack the problem
(it has other downsides, but it stops before entering the parents).


        Stefan






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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 18:12     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-03-15 22:26       ` Jonas Bernoulli
  2023-03-17 21:09         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 28+ messages in thread
From: Jonas Bernoulli @ 2023-03-15 22:26 UTC (permalink / raw)
  To: Stefan Monnier, Robert Pluim; +Cc: 62207

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>     Jonas> As a side-note, it would be nice if it were possible to lookup a
>>     Jonas> key in a keymap only, while ignoring bindings in its parent keymap.
>
> Could you explain why you need `keymap-unset-key`

Directional commands usually involve the keys "b", "p", "n" and "f".
These keys are mnemonic but are also scattered all across the keyboard.
So in my own Emacs I have chosen to use "physically-mnemonic" bindings
instead:     "i"
         "j" "k" "l"

[ Maybe that was a stupid decision.  But I made it very early in my
  readline/Emacs career.  There is no way back. ]

That makes it necessary to edit many keymaps.  Using keymap-unser with
non-nil REMOVE makes that much less painful.  It's still painful but any
relieve is welcome.

In magit-section-mode-map, for example, I make these changes

  default             custom  
  "p"     replaced by "i"     magit-section-backward
  "n"     replaced by "k"     magit-section-forward

magit-mode-map has the above keymap as parent.  It also binds "i"
to magit-gitignore.  I could just override that binding using

  (keymap-set magit-mode-map "i" #'magit-section-backward)

but it is much clean to

  (keymap-unset magit-mode-map "i" t)

It is also more future-proof.  Imagine the author of Magit decides
that magit-section-{forward,backward}-sibling are actually more
important than magit-section-{forward,backward}, and thus exchanges
the respective key bindings.

If I previously removed the conflicting binding in magit-mode-map,
my bindings continue to be consistent; "i"/"k" continue to do the
inverse of one another.  But if I was forced to explicitly double
down on bindings in derived keymaps, then that would be less robust.
In this case I would end up with this in magit-mode-map:

  "i" magit-section-backward
  "k" magit-section-forward-sibling

> lookup "all but the ignore the parent"?

kmu-lookup-local-key
    In KEYMAP, look up key sequence KEY.  Return the definition.
    Unlike `keymap-lookup' (which see) this doesn't consider bindings
    made in KEYMAP's parent keymap.

A long time ago I tried to automate the process of moving directional
commands to different keys.  When mapping through bindings of a keymap
in order to do that, it was important to ignore bindings made in the
parent keymap.  So I used a variant of map-keymap that did that, and it
made sense to implement a variant of lookup-key along the same line.  I
have not used these functions in a major way since I stopped trying to
automate the remapping.

But they just came in handy.  When using keymap-unset to remove a
binding but there isn't actually a binding, then that ends up adding
a binding.  To work around that bug I have to first check if there is
a binding.  But it must be a binding in the keymap itself, not a parent
keymap.

> I'm curious because the meaning of these can be subtly more complex than
> meets the eye once you consider things like `make-compose-keymap`.

My current implementation seems to treat later keymaps in composed
keymaps like parent keymaps.  Maybe that makes sense, maybe a proper
implementation should make a distinctions between parent keymaps and
"tail" keymaps.

>> A feature request and a bug report? Tsk ;-) Luckily the infrastructure
>> is actually there already.

I had a hunch the two might require changes in the same part of the
code. ;D

> Hmm... if you need to add yet-another-arg to `Flookup_key`, than I'm not
> sure it qualifies as "the infrastructure is actually there already."

What I care about the most is that the bug is fixed (instead of users
being told to not use the REMOVE argument, as you seem to suggest
elsewhere in this thread).

Key lookup that ignores parent keymaps would be nice, but not nearly as
important as the bugfix.

> BTW, `map-keymap-internal` could be another way to attack the problem
> (it has other downsides, but it stops before entering the parents).
>
>
>         Stefan





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 18:02   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-03-17  8:23     ` Eli Zaretskii
  2023-03-17  8:54       ` Robert Pluim
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2023-03-17  8:23 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: larsi, jonas, 62207

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Jonas Bernoulli <jonas@bernoul.li>,  Lars Ingebrigtsen <larsi@gnus.org>,
>   62207@debbugs.gnu.org
> Date: Wed, 15 Mar 2023 14:02:59 -0400
> 
> > Stefan, it sounds like the part of store_in_keymap after the label
> > keymap_end should do nothing if REMOVE is non-zero, am I right?
> 
> Sounds right, yes.

Then Robert, please install on the emacs-29 branch the part of your
suggested patch which fixes the problem with REMOVE.  The other part
should go to master, I think.

> This said, the semantics of REMOVE a bit murky, so I'd rather tell
> people not to use it.

Robert, could you please say something to this effect in the doc
string?

Thanks.





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17  8:23     ` Eli Zaretskii
@ 2023-03-17  8:54       ` Robert Pluim
  2023-03-17  9:55         ` Robert Pluim
  2023-03-17 11:32         ` Eli Zaretskii
  0 siblings, 2 replies; 28+ messages in thread
From: Robert Pluim @ 2023-03-17  8:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62207, larsi, jonas, Stefan Monnier

>>>>> On Fri, 17 Mar 2023 10:23:37 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Stefan Monnier <monnier@iro.umontreal.ca>
    >> Cc: Jonas Bernoulli <jonas@bernoul.li>,  Lars Ingebrigtsen <larsi@gnus.org>,
    >> 62207@debbugs.gnu.org
    >> Date: Wed, 15 Mar 2023 14:02:59 -0400
    >> 
    >> > Stefan, it sounds like the part of store_in_keymap after the label
    >> > keymap_end should do nothing if REMOVE is non-zero, am I right?
    >> 
    >> Sounds right, yes.

    Eli> Then Robert, please install on the emacs-29 branch the part of your
    Eli> suggested patch which fixes the problem with REMOVE.  The other part
    Eli> should go to master, I think.

The former I can do. Stefan seemed to object to the other part. Itʼs
also not been tested a great deal 😀

    >> This said, the semantics of REMOVE a bit murky, so I'd rather tell
    >> people not to use it.

    Eli> Robert, could you please say something to this effect in the doc
    Eli> string?

Sorry to be difficult, but why? I think the semantics are clear,
although Iʼd prefer it if the first line of the docstring for
`keymap-unset' said

    Unset key sequence KEY in KEYMAP.

to reduce confusion with the REMOVE argument.

Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17  8:54       ` Robert Pluim
@ 2023-03-17  9:55         ` Robert Pluim
  2023-03-17 11:36           ` Eli Zaretskii
  2023-03-17 11:32         ` Eli Zaretskii
  1 sibling, 1 reply; 28+ messages in thread
From: Robert Pluim @ 2023-03-17  9:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62207, jonas, larsi, Stefan Monnier

>>>>> On Fri, 17 Mar 2023 09:54:05 +0100, Robert Pluim <rpluim@gmail.com> said:
    Robert> Sorry to be difficult, but why? I think the semantics are clear,
    Robert> although Iʼd prefer it if the first line of the docstring for
    Robert> `keymap-unset' said

    Robert>     Unset key sequence KEY in KEYMAP.

    Robert> to reduce confusion with the REMOVE argument.

`keymap-unset' is also not documented in the lisp reference
manual. How about this?

@findex keymap-unset
@defun keymap-unset keymap key &optional remove
This function is the inverse of @code{keymap-set}, it unsets the
binding for @var{key} in @var{keymap}, which is the same as setting
the binding to @code{nil}.  In order to instead remove the binding
completely, specify @var{remove} as non-nil.  This only makes a
difference if @var{keymap} has a parent keymap.  When unsetting a key
in a child map, it will still shadow the same key in the parent
keymap.  Removing the binding will allow the key in the parent keymap
to be used.



Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17  8:54       ` Robert Pluim
  2023-03-17  9:55         ` Robert Pluim
@ 2023-03-17 11:32         ` Eli Zaretskii
  2023-03-17 13:20           ` Robert Pluim
  2023-03-17 20:51           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 2 replies; 28+ messages in thread
From: Eli Zaretskii @ 2023-03-17 11:32 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62207, larsi, jonas, monnier

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  larsi@gnus.org,
>   jonas@bernoul.li,  62207@debbugs.gnu.org
> Date: Fri, 17 Mar 2023 09:54:05 +0100
> 
> >>>>> On Fri, 17 Mar 2023 10:23:37 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> From: Stefan Monnier <monnier@iro.umontreal.ca>
>     >> Cc: Jonas Bernoulli <jonas@bernoul.li>,  Lars Ingebrigtsen <larsi@gnus.org>,
>     >> 62207@debbugs.gnu.org
>     >> Date: Wed, 15 Mar 2023 14:02:59 -0400
>     >> 
>     >> > Stefan, it sounds like the part of store_in_keymap after the label
>     >> > keymap_end should do nothing if REMOVE is non-zero, am I right?
>     >> 
>     >> Sounds right, yes.
> 
>     Eli> Then Robert, please install on the emacs-29 branch the part of your
>     Eli> suggested patch which fixes the problem with REMOVE.  The other part
>     Eli> should go to master, I think.
> 
> The former I can do. Stefan seemed to object to the other part. Itʼs
> also not been tested a great deal 😀

OK, then just the emacs-29 part for now.  The rest is not urgent
anyway.

>     >> This said, the semantics of REMOVE a bit murky, so I'd rather tell
>     >> people not to use it.
> 
>     Eli> Robert, could you please say something to this effect in the doc
>     Eli> string?
> 
> Sorry to be difficult, but why? I think the semantics are clear,
> although Iʼd prefer it if the first line of the docstring for
> `keymap-unset' said
> 
>     Unset key sequence KEY in KEYMAP.
> 
> to reduce confusion with the REMOVE argument.

If this part is still controversial, I'm okay with only fixing the bug
itself.  As for REMOVE, let's see what Stefan has to say in defense of
his request, and take it from there.

Thanks.





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17  9:55         ` Robert Pluim
@ 2023-03-17 11:36           ` Eli Zaretskii
  2023-03-17 13:20             ` Robert Pluim
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2023-03-17 11:36 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62207, jonas, larsi, monnier

> From: Robert Pluim <rpluim@gmail.com>
> Cc: 62207@debbugs.gnu.org,  larsi@gnus.org,  jonas@bernoul.li,  Stefan
>  Monnier <monnier@iro.umontreal.ca>
> Date: Fri, 17 Mar 2023 10:55:23 +0100
> 
> `keymap-unset' is also not documented in the lisp reference
> manual. How about this?
> 
> @findex keymap-unset
> @defun keymap-unset keymap key &optional remove
> This function is the inverse of @code{keymap-set}, it unsets the
> binding for @var{key} in @var{keymap}, which is the same as setting
> the binding to @code{nil}.  In order to instead remove the binding
> completely, specify @var{remove} as non-nil.  This only makes a
> difference if @var{keymap} has a parent keymap.  When unsetting a key
> in a child map, it will still shadow the same key in the parent
> keymap.  Removing the binding will allow the key in the parent keymap
> to be used.

LGTM, thanks.





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17 11:32         ` Eli Zaretskii
@ 2023-03-17 13:20           ` Robert Pluim
  2023-03-20 18:14             ` Jonas Bernoulli
  2023-03-17 20:51           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 28+ messages in thread
From: Robert Pluim @ 2023-03-17 13:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62207, larsi, jonas, monnier

tags 62207 fixed
close 62207 29.1
quit

>>>>> On Fri, 17 Mar 2023 13:32:48 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> The former I can do. Stefan seemed to object to the other part. Itʼs
    >> also not been tested a great deal 😀

    Eli> OK, then just the emacs-29 part for now.  The rest is not urgent
    Eli> anyway.

done

    Eli> If this part is still controversial, I'm okay with only fixing the bug
    Eli> itself.  As for REMOVE, let's see what Stefan has to say in defense of
    Eli> his request, and take it from there.

ok

Closing.
Committed as bb3e0ded9eb

Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17 11:36           ` Eli Zaretskii
@ 2023-03-17 13:20             ` Robert Pluim
  0 siblings, 0 replies; 28+ messages in thread
From: Robert Pluim @ 2023-03-17 13:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62207, jonas, larsi, monnier

>>>>> On Fri, 17 Mar 2023 13:36:34 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> LGTM, thanks.

Done

Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17 11:32         ` Eli Zaretskii
  2023-03-17 13:20           ` Robert Pluim
@ 2023-03-17 20:51           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-18  5:51             ` Eli Zaretskii
  2023-03-18  9:43             ` Robert Pluim
  1 sibling, 2 replies; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-17 20:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62207, Robert Pluim, jonas, larsi

> If this part is still controversial, I'm okay with only fixing the bug
> itself.  As for REMOVE, let's see what Stefan has to say in defense of
> his request, and take it from there.

I can't remember off-hand all the subtleties that can show up in corner
cases, but I'll simply note that the current removal code is already the
result of my pointing out several not-so-corner cases that the original
coder missed (even though he was not a beginner at the keymap game), and
this very bug report illustrates that the code is still buggy.

[ Which reminds me that we need a regression test for this.  ]

I think the removal code is "good enough" for uses in a user's init
code, but I'd rather add some recommendation to avoid on it in
ELisp packages.


        Stefan






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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-15 22:26       ` Jonas Bernoulli
@ 2023-03-17 21:09         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-20 18:46           ` Jonas Bernoulli
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-17 21:09 UTC (permalink / raw)
  To: Jonas Bernoulli; +Cc: Robert Pluim, 62207

>>>     Jonas> As a side-note, it would be nice if it were possible to lookup a
>>>     Jonas> key in a keymap only, while ignoring bindings in its parent keymap.
>>
>> Could you explain why you need `keymap-unset-key`
>
> Directional commands usually involve the keys "b", "p", "n" and "f".
> These keys are mnemonic but are also scattered all across the keyboard.
> So in my own Emacs I have chosen to use "physically-mnemonic" bindings
> instead:     "i"
>          "j" "k" "l"
>
> [ Maybe that was a stupid decision.  But I made it very early in my
>   readline/Emacs career.  There is no way back. ]
>
> That makes it necessary to edit many keymaps.  Using keymap-unser with
> non-nil REMOVE makes that much less painful.  It's still painful but any
> relieve is welcome.
>
> In magit-section-mode-map, for example, I make these changes
>
>   default             custom  
>   "p"     replaced by "i"     magit-section-backward
>   "n"     replaced by "k"     magit-section-forward

[ Indeed, Emacs's keymap system doesn't support this very well.
  For the same reason Evil/god-mode/boon/... need a lot of
  package-specific tweaks to "patch things up".  :-(  ]

> magit-mode-map has the above keymap as parent.  It also binds "i"
> to magit-gitignore.  I could just override that binding using
>
>   (keymap-set magit-mode-map "i" #'magit-section-backward)
>
> but it is much clean to
>
>   (keymap-unset magit-mode-map "i" t)

Agreed, this is a good use case, thanks.

> But they just came in handy.  When using keymap-unset to remove a
> binding but there isn't actually a binding, then that ends up adding
> a binding.

This is the bug we're discussing, right?  AFAIC it's a plain bug with an
easy fix (provided by Robert).

> To work around that bug I have to first check if there is a binding.
> But it must be a binding in the keymap itself, not a parent keymap.

Right, but this need is only brought about by the bug, so once the bug
is fixed, this need will disappear.  I hoped you had other needs for
that functionality.

>> I'm curious because the meaning of these can be subtly more complex than
>> meets the eye once you consider things like `make-compose-keymap`.
> My current implementation seems to treat later keymaps in composed
> keymaps like parent keymaps.  Maybe that makes sense, maybe a proper
> implementation should make a distinctions between parent keymaps and
> "tail" keymaps.

`map-keymap-internal` stops at both a parent keymap (i.e. a keymap in
the `cdr`) and a "subkeymap" (i.e. a keymap in the `car`).

> What I care about the most is that the bug is fixed

Agreed.

> (instead of users being told to not use the REMOVE argument, as you
> seem to suggest elsewhere in this thread).

I'm mostly worried about people using it in ELisp packages where it's
more likely to interact with many more situations.
For uses in an init file I think we can make it work well enough.


        Stefan






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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17 20:51           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-03-18  5:51             ` Eli Zaretskii
  2023-03-18 14:05               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-18  9:43             ` Robert Pluim
  1 sibling, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2023-03-18  5:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 62207, rpluim, jonas, larsi

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Robert Pluim <rpluim@gmail.com>,  larsi@gnus.org,  jonas@bernoul.li,
>   62207@debbugs.gnu.org
> Date: Fri, 17 Mar 2023 16:51:56 -0400
> 
> I think the removal code is "good enough" for uses in a user's init
> code, but I'd rather add some recommendation to avoid on it in
> ELisp packages.

If we want to recommend that, we need to come up with a rationale and
a couple of alternative ways of achieving goals that commonly lead to
using this option.  Can you tell what to say to that effect?





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17 20:51           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-18  5:51             ` Eli Zaretskii
@ 2023-03-18  9:43             ` Robert Pluim
  2023-03-18 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 28+ messages in thread
From: Robert Pluim @ 2023-03-18  9:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: larsi, Eli Zaretskii, jonas, 62207

>>>>> On Fri, 17 Mar 2023 16:51:56 -0400, Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> said:

    >> If this part is still controversial, I'm okay with only fixing the bug
    >> itself.  As for REMOVE, let's see what Stefan has to say in defense of
    >> his request, and take it from there.

    Stefan> I can't remember off-hand all the subtleties that can show up in corner
    Stefan> cases, but I'll simply note that the current removal code is already the
    Stefan> result of my pointing out several not-so-corner cases that the original
    Stefan> coder missed (even though he was not a beginner at the keymap game), and
    Stefan> this very bug report illustrates that the code is still buggy.

    Stefan> [ Which reminds me that we need a regression test for this.  ]

Done. Let me know if there are corner cases I missed.

    Stefan> I think the removal code is "good enough" for uses in a user's init
    Stefan> code, but I'd rather add some recommendation to avoid on it in
    Stefan> ELisp packages.

We can put that in the elisp manual, no?

Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-18  5:51             ` Eli Zaretskii
@ 2023-03-18 14:05               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-18 14:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62207, rpluim, jonas, larsi

>> I think the removal code is "good enough" for uses in a user's init
>> code, but I'd rather add some recommendation to avoid on it in
>> ELisp packages.
>
> If we want to recommend that, we need to come up with a rationale and
> a couple of alternative ways of achieving goals that commonly lead to
> using this option.  Can you tell what to say to that effect?

I don't know of "goals that commonly lead to using this option", sorry.
AFAIK it's not used commonly (after all, it's brand new in Emacs-30).

Most of the requests for it I have seen were either of the "make the API
complete" kind of argument, or examples like Jonas' (which are
inherently tightly tied to a particular keymap and a particular binding
in that map, so the chance of bumping into an odd corner case is
significantly reduced).


        Stefan






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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-18  9:43             ` Robert Pluim
@ 2023-03-18 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-20  9:09                 ` Robert Pluim
  0 siblings, 1 reply; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-18 14:07 UTC (permalink / raw)
  To: Robert Pluim; +Cc: larsi, Eli Zaretskii, jonas, 62207

> Done.

Thanks.

> Let me know if there are corner cases I missed.

I will if/when I can think of a new one.

>     Stefan> I think the removal code is "good enough" for uses in a user's init
>     Stefan> code, but I'd rather add some recommendation to avoid on it in
>     Stefan> ELisp packages.
> We can put that in the elisp manual, no?

No objection on my side :-)


        Stefan






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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-18 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-03-20  9:09                 ` Robert Pluim
  2023-03-20 12:17                   ` Eli Zaretskii
  2023-03-20 15:03                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 28+ messages in thread
From: Robert Pluim @ 2023-03-20  9:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 62207, larsi, jonas, Eli Zaretskii

>>>>> On Sat, 18 Mar 2023 10:07:02 -0400, Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> said:

    >> Done.
    Bug> Thanks.

    >> Let me know if there are corner cases I missed.

    Bug> I will if/when I can think of a new one.

    Stefan> I think the removal code is "good enough" for uses in a user's init
    Stefan> code, but I'd rather add some recommendation to avoid on it in
    Stefan> ELisp packages.
    >> We can put that in the elisp manual, no?

    Bug> No objection on my side :-)

Something like this?

+Note: using @code{keymap-unset} with @var{remove} non-@code{nil} is
+intended for users to put in their init file; Emacs packages should
+avoid using it if possible, since they have complete control over
+their own keymaps anyway, and they should not be altering other
+packages' keymaps.
+

Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-20  9:09                 ` Robert Pluim
@ 2023-03-20 12:17                   ` Eli Zaretskii
  2023-03-20 15:03                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 28+ messages in thread
From: Eli Zaretskii @ 2023-03-20 12:17 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62207, larsi, jonas, monnier

> From: Robert Pluim <rpluim@gmail.com>
> Cc: larsi@gnus.org,  Eli Zaretskii <eliz@gnu.org>,  jonas@bernoul.li,
>   62207@debbugs.gnu.org
> Date: Mon, 20 Mar 2023 10:09:03 +0100
> 
> +Note: using @code{keymap-unset} with @var{remove} non-@code{nil} is
> +intended for users to put in their init file; Emacs packages should
> +avoid using it if possible, since they have complete control over
> +their own keymaps anyway, and they should not be altering other
> +packages' keymaps.

Fine by me, but please wait for Stefan to respond.





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-20  9:09                 ` Robert Pluim
  2023-03-20 12:17                   ` Eli Zaretskii
@ 2023-03-20 15:03                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-03-20 15:27                     ` Robert Pluim
  1 sibling, 1 reply; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-20 15:03 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62207, larsi, jonas, Eli Zaretskii

> Something like this?
>
> +Note: using @code{keymap-unset} with @var{remove} non-@code{nil} is
> +intended for users to put in their init file; Emacs packages should
> +avoid using it if possible, since they have complete control over
> +their own keymaps anyway, and they should not be altering other
> +packages' keymaps.

Fine by me, thanks,


        Stefan






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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-20 15:03                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-03-20 15:27                     ` Robert Pluim
  0 siblings, 0 replies; 28+ messages in thread
From: Robert Pluim @ 2023-03-20 15:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 62207, jonas, Eli Zaretskii, larsi

>>>>> On Mon, 20 Mar 2023 11:03:37 -0400, Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> said:

    >> Something like this?
    >> 
    >> +Note: using @code{keymap-unset} with @var{remove} non-@code{nil} is
    >> +intended for users to put in their init file; Emacs packages should
    >> +avoid using it if possible, since they have complete control over
    >> +their own keymaps anyway, and they should not be altering other
    >> +packages' keymaps.

    Stefan> Fine by me, thanks,

Done

Robert
-- 





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17 13:20           ` Robert Pluim
@ 2023-03-20 18:14             ` Jonas Bernoulli
  0 siblings, 0 replies; 28+ messages in thread
From: Jonas Bernoulli @ 2023-03-20 18:14 UTC (permalink / raw)
  To: Robert Pluim, Eli Zaretskii; +Cc: 62207, larsi, monnier

Robert Pluim <rpluim@gmail.com> writes:

> tags 62207 fixed
> close 62207 29.1
> quit
>
>>>>>> On Fri, 17 Mar 2023 13:32:48 +0200, Eli Zaretskii <eliz@gnu.org> said:
>
>     >> The former I can do. Stefan seemed to object to the other part. Itʼs
>     >> also not been tested a great deal 😀
>
>     Eli> OK, then just the emacs-29 part for now.  The rest is not urgent
>     Eli> anyway.
>
> done

Thanks a lot Robert!

     Jonas





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-17 21:09         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-03-20 18:46           ` Jonas Bernoulli
  2023-03-20 21:25             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 28+ messages in thread
From: Jonas Bernoulli @ 2023-03-20 18:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Robert Pluim, 62207

Eli Zaretskii <eliz@gnu.org> writes:

> Then Robert, please install on the emacs-29 branch the part of your
> suggested patch which fixes the problem with REMOVE.  The other part
> should go to master, I think.

The other part hasn't landed on master yet.  I agree it shouldn't go on
emacs-29, but would like to see it in master.  Stefan seems to have some
objections, which I believe boil down to "nobody has demonstrated a real
need yet, beside 'making the api complete'".

I don't have an immediate need, but I am fairly sure I would make use
of it eventually.  I haven't bothered with improving my personal key
bindings much lately, but plan to get back into the game eventually.

From the top of my head I can think of the following use-case.

The which-key package displays available bindings in some sort of popup
buffer, not unlike "C-h m", but more dynamic and less verbose.  I would
like to (eventually) create a package to display a more curated list of
bindings.

A minimal viable version of that, for my own personal, use would display
the keys I personally reserve for "directional commands", namely
[MOD]-{j,i,k,l}, along with the commands they are bound to in the
current buffer/mode.  When I start using a new mode, I would bring up
that bindings buffer, and if it showed me that, for example, "k" was
bound to sacrifice-goat, instead of some *-previous, then I would know
I had to remove that binding for my own use and possibly make other
adjustments.

At this point it would be nice if there were some indication in what
keymap the default binding, this-mode-map or parent-mode-map.  The UI
could even allow directly jumping to the definition of the appropriate
keymap.  Eventually it might even be possible to make the changes
directly from the popup buffer.

If that is to vague or obscure, I am fine with the
ignore-bindings-in-parent feature not being installed on master just
yet.  But in that case we should probably open a new issue to track it.

     Cheers,
     Jonas





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

* bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding
  2023-03-20 18:46           ` Jonas Bernoulli
@ 2023-03-20 21:25             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 28+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-03-20 21:25 UTC (permalink / raw)
  To: Jonas Bernoulli; +Cc: Robert Pluim, 62207

> The other part hasn't landed on master yet.  I agree it shouldn't go on
> emacs-29, but would like to see it in master.  Stefan seems to have some
> objections, which I believe boil down to "nobody has demonstrated a real
> need yet, beside 'making the api complete'".

I can't remember exactly what "the other part" entails, but w.r.t
providing an optional arg that makes lookup-key ignores parents, I don't
think it would make the API complete (I can still see needs for yet more
optional arguments, most importantly one that doesn't look inside
`menu-item`s).

So I'd prefer to have a new lower-level function which does "the
minimum" but returns enough info that we can build something like
`lookup-key` on top of it.

I guess it would return 2 pieces of info: the binding that was found (if
any, potentially annotated with whether it was a default binding or
not), and the rest of the keymap.  Or maybe return the binding together
with the `cons` cell where that binding was found (i.e. along the lines
of what `member/memq` do).

> At this point it would be nice if there were some indication in what
> keymap the default binding, this-mode-map or parent-mode-map.  The UI
> could even allow directly jumping to the definition of the appropriate
> keymap.  Eventually it might even be possible to make the changes
> directly from the popup buffer.

`C-h k` tries to identify the keymap, but indeed it doesn't distinguish
between the keymap and its parent(s).  Patch welcome to improve this.


        Stefan






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

end of thread, other threads:[~2023-03-20 21:25 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-15 16:07 bug#62207: 29.0.60; Trying to remove non-existent key binding instead adds a binding Jonas Bernoulli
2023-03-15 16:51 ` Jonas Bernoulli
2023-03-15 17:36   ` Robert Pluim
2023-03-15 18:12     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-15 22:26       ` Jonas Bernoulli
2023-03-17 21:09         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-20 18:46           ` Jonas Bernoulli
2023-03-20 21:25             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-15 17:13 ` Eli Zaretskii
2023-03-15 17:39   ` Robert Pluim
2023-03-15 18:02   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-17  8:23     ` Eli Zaretskii
2023-03-17  8:54       ` Robert Pluim
2023-03-17  9:55         ` Robert Pluim
2023-03-17 11:36           ` Eli Zaretskii
2023-03-17 13:20             ` Robert Pluim
2023-03-17 11:32         ` Eli Zaretskii
2023-03-17 13:20           ` Robert Pluim
2023-03-20 18:14             ` Jonas Bernoulli
2023-03-17 20:51           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-18  5:51             ` Eli Zaretskii
2023-03-18 14:05               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-18  9:43             ` Robert Pluim
2023-03-18 14:07               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-20  9:09                 ` Robert Pluim
2023-03-20 12:17                   ` Eli Zaretskii
2023-03-20 15:03                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-20 15:27                     ` Robert Pluim

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.