GNU bug report logs - #29349
read_key_sequence is only partially recursive. This is a bug.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29349 in the body.
You can then email your comments to 29349 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: bug-gnu-emacs <at> gnu.org
Subject: read_key_sequence is only partially recursive.  This is a bug.
Date: Sat, 18 Nov 2017 09:38:43 +0000

Hello, Emacs.

In branch emacs-26.

I came across this bug whilst working on bug #29272 ("C-h k C-mouse-3"
followed by menu selection asks for more keys).

In a Linux tty using the GPM mouse package, doing read_key_sequence (the
C function in keyboard.c), when a menu action is activated,
read_key_sequence calls itself recursively to handle all the mouse
manipulation.

Unfortunately, the variable raw_keybuf_count is initialised to 0 in
r_k_s.  This includes in the recursive call.  This variable indexes the
global buffer raw_keybuf, which accumulates the raw events which make up
the key sequence.

The result of this is that the events in the recursive call overwrite
the stored events of the outer r_k_s call, leaving a mess.

r_k_s is static in keyboard.c and is called from three functions:
command_loop_1, read_menu_command (the one that gives the trouble), and
read_key_sequence_vs.

So I propose as a solution that raw_keybuf_count be initialised to zero
in two of these three functions, but not in read_menu_command (and no
longer in read_key_sequence).  I've tried this, and it seems to work.
It has the disadvantage of being ugly, and it makes read_menu_command
only callable as a subfunction of r_k_s.

Has anybody any thoughts on this?

#########################################################################

As a second problem, is there any way, preferably at the Lisp level, to
determine that a key sequence is a menu key sequence?

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #8 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: 29349 <at> debbugs.gnu.org
Subject: Re: read_key_sequence is only partially recursive.  This is a bug.
Date: Sun, 19 Nov 2017 12:34:56 +0000

Hello, Emacs.

On Sat, Nov 18, 2017 at 09:38:43 +0000, Alan Mackenzie wrote:
> In branch emacs-26.

> I came across this bug whilst working on bug #29272 ("C-h k C-mouse-3"
> followed by menu selection asks for more keys).

> In a Linux tty using the GPM mouse package, doing read_key_sequence (the
> C function in keyboard.c), when a menu action is activated,
> read_key_sequence calls itself recursively to handle all the mouse
> manipulation.

> Unfortunately, the variable raw_keybuf_count is initialised to 0 in
> r_k_s.  This includes in the recursive call.  This variable indexes the
> global buffer raw_keybuf, which accumulates the raw events which make up
> the key sequence.

> The result of this is that the events in the recursive call overwrite
> the stored events of the outer r_k_s call, leaving a mess.

> r_k_s is static in keyboard.c and is called from three functions:
> command_loop_1, read_menu_command (the one that gives the trouble), and
> read_key_sequence_vs.

> So I propose as a solution that raw_keybuf_count be initialised to zero
> in two of these three functions, but not in read_menu_command (and no
> longer in read_key_sequence).  I've tried this, and it seems to work.
> It has the disadvantage of being ugly, and it makes read_menu_command
> only callable as a subfunction of r_k_s.

> Has anybody any thoughts on this?

Here is how I propose to solve this:

(i) In keyboard.c, the static variables raw_keybuf and raw_keybuf_count
will become pointers.  They will initially point to a static buffer and
a static integer.  For safety's sake, they will be reinitialised to
those static variables in command_loop_1(), just before the invocation
of read_key_sequence().

(ii) read_key_sequence() will get a Lisp_Object buffer and a count
variable as local variables.  Around the call to read_char(),
raw_keybuf{,_count} will be set to point to these locals, so that should
read_char encounter a menu, its events will be stored in the local copy
of the Lisp_Object buffer.

(iii) On return from read_char, if any events are in the local buffer,
they will be appended to the buffer in the enclosing scope.  The global
pointers raw_keybuf{,_count} will be restored to their previous values.

In short, raw_keybuf and raw_keybuf_count will be "bound" to local
variables around the call to read_char().

Comments?

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #11 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: 29349 <at> debbugs.gnu.org
Subject: Re: [Patch] Bug 29349: read_key_sequence is only partially
 recursive.  This is a bug.
Date: Sun, 19 Nov 2017 15:59:08 +0000

Hello, Emacs.

On Sun, Nov 19, 2017 at 12:34:56 +0000, Alan Mackenzie wrote:
> On Sat, Nov 18, 2017 at 09:38:43 +0000, Alan Mackenzie wrote:
> > In branch emacs-26.

> > I came across this bug whilst working on bug #29272 ("C-h k C-mouse-3"
> > followed by menu selection asks for more keys).

> > In a Linux tty using the GPM mouse package, doing read_key_sequence (the
> > C function in keyboard.c), when a menu action is activated,
> > read_key_sequence calls itself recursively to handle all the mouse
> > manipulation.

> > Unfortunately, the variable raw_keybuf_count is initialised to 0 in
> > r_k_s.  This includes in the recursive call.  This variable indexes the
> > global buffer raw_keybuf, which accumulates the raw events which make up
> > the key sequence.

> > The result of this is that the events in the recursive call overwrite
> > the stored events of the outer r_k_s call, leaving a mess.

> > r_k_s is static in keyboard.c and is called from three functions:
> > command_loop_1, read_menu_command (the one that gives the trouble), and
> > read_key_sequence_vs.

> > So I propose as a solution that raw_keybuf_count be initialised to zero
> > in two of these three functions, but not in read_menu_command (and no
> > longer in read_key_sequence).  I've tried this, and it seems to work.
> > It has the disadvantage of being ugly, and it makes read_menu_command
> > only callable as a subfunction of r_k_s.

> > Has anybody any thoughts on this?

> Here is how I propose to solve this:

> (i) In keyboard.c, the static variables raw_keybuf and raw_keybuf_count
> will become pointers.  They will initially point to a static buffer and
> a static integer.  For safety's sake, they will be reinitialised to
> those static variables in command_loop_1(), just before the invocation
> of read_key_sequence().

> (ii) read_key_sequence() will get a Lisp_Object buffer and a count
> variable as local variables.  Around the call to read_char(),
> raw_keybuf{,_count} will be set to point to these locals, so that should
> read_char encounter a menu, its events will be stored in the local copy
> of the Lisp_Object buffer.

> (iii) On return from read_char, if any events are in the local buffer,
> they will be appended to the buffer in the enclosing scope.  The global
> pointers raw_keybuf{,_count} will be restored to their previous values.

> In short, raw_keybuf and raw_keybuf_count will be "bound" to local
> variables around the call to read_char().

> Comments?

Here's a patch which implements the above:



diff --git a/src/keyboard.c b/src/keyboard.c
index 57757cf211..ae687aad89 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -121,12 +121,17 @@ ptrdiff_t this_command_key_count;
 
 /* This vector is used as a buffer to record the events that were actually read
    by read_key_sequence.  */
-static Lisp_Object raw_keybuf;
-static int raw_keybuf_count;
-
-#define GROW_RAW_KEYBUF							\
- if (raw_keybuf_count == ASIZE (raw_keybuf))				\
-   raw_keybuf = larger_vector (raw_keybuf, 1, -1)
+static Lisp_Object raw_keybuf_buffer;
+static int raw_keybuf_count_buffer;
+static Lisp_Object *raw_keybuf = &raw_keybuf_buffer;
+static int *raw_keybuf_count = &raw_keybuf_count_buffer;
+
+#define GROW_RAW_KEYBUF(inc)                                            \
+  if (*raw_keybuf_count > ASIZE (*raw_keybuf) - (inc))                  \
+    *raw_keybuf =                                                       \
+      larger_vector (*raw_keybuf,                                       \
+                     (inc) + *raw_keybuf_count - ASIZE (*raw_keybuf),   \
+                     -1)
 
 /* Number of elements of this_command_keys
    that precede this key sequence.  */
@@ -1365,6 +1370,8 @@ command_loop_1 (void)
       Vthis_command_keys_shift_translated = Qnil;
 
       /* Read next key sequence; i gets its length.  */
+      raw_keybuf_count = &raw_keybuf_count_buffer; /* For safety */
+      raw_keybuf = &raw_keybuf_buffer;             /* Ditto */
       i = read_key_sequence (keybuf, ARRAYELTS (keybuf),
 			     Qnil, 0, 1, 1, 0);
 
@@ -8910,6 +8917,11 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
   /* How many keys there are in the current key sequence.  */
   int t;
 
+  int *outer_raw_keybuf_count;
+  Lisp_Object *outer_raw_keybuf;
+  int inner_raw_keybuf_count_buffer;
+  Lisp_Object inner_raw_keybuf_buffer = Fmake_vector (make_number (30), Qnil);
+
   /* The length of the echo buffer when we started reading, and
      the length of this_command_keys when we started reading.  */
   ptrdiff_t echo_start UNINIT;
@@ -8971,7 +8983,7 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
   /* List of events for which a fake prefix key has been generated.  */
   Lisp_Object fake_prefixed_keys = Qnil;
 
-  raw_keybuf_count = 0;
+  *raw_keybuf_count = 0;
 
   last_nonmenu_event = Qnil;
 
@@ -9142,11 +9154,23 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
 	  {
 	    KBOARD *interrupted_kboard = current_kboard;
 	    struct frame *interrupted_frame = SELECTED_FRAME ();
+            int i;
+            outer_raw_keybuf_count = raw_keybuf_count;
+            outer_raw_keybuf = raw_keybuf;
+            inner_raw_keybuf_count_buffer = 0;
+            raw_keybuf_count = &inner_raw_keybuf_count_buffer;
+            raw_keybuf = &inner_raw_keybuf_buffer;
 	    /* Calling read_char with COMMANDFLAG = -2 avoids
 	       redisplay in read_char and its subroutines.  */
 	    key = read_char (prevent_redisplay ? -2 : NILP (prompt),
 		             current_binding, last_nonmenu_event,
                              &used_mouse_menu, NULL);
+            raw_keybuf_count = outer_raw_keybuf_count;
+            raw_keybuf = outer_raw_keybuf;
+            GROW_RAW_KEYBUF (inner_raw_keybuf_count_buffer);
+            for (i = 0; i < inner_raw_keybuf_count_buffer; i++)
+              ASET (*raw_keybuf, (*raw_keybuf_count)++,
+                    AREF (inner_raw_keybuf_buffer, i));
 	    if ((INTEGERP (key) && XINT (key) == -2) /* wrong_kboard_jmpbuf */
 		/* When switching to a new tty (with a new keyboard),
 		   read_char returns the new buffer, rather than -2
@@ -9254,9 +9278,9 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
 	      && XINT (key) == quit_char
 	      && current_buffer != starting_buffer)
 	    {
-	      GROW_RAW_KEYBUF;
-	      ASET (raw_keybuf, raw_keybuf_count, key);
-	      raw_keybuf_count++;
+	      GROW_RAW_KEYBUF (1);
+	      ASET (*raw_keybuf, *raw_keybuf_count, key);
+	      (*raw_keybuf_count)++;
 	      keybuf[t++] = key;
 	      mock_input = t;
 	      Vquit_flag = Qnil;
@@ -9295,9 +9319,9 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
 	      current_binding = active_maps (first_event);
 	    }
 
-	  GROW_RAW_KEYBUF;
-	  ASET (raw_keybuf, raw_keybuf_count, key);
-	  raw_keybuf_count++;
+	  GROW_RAW_KEYBUF (1);
+	  ASET (*raw_keybuf, *raw_keybuf_count, key);
+	  (*raw_keybuf_count)++;
 	}
 
       /* Clicks in non-text areas get prefixed by the symbol
@@ -9343,8 +9367,9 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt,
 		      && BUFFERP (XWINDOW (window)->contents)
 		      && XBUFFER (XWINDOW (window)->contents) != current_buffer)
 		    {
-		      ASET (raw_keybuf, raw_keybuf_count, key);
-		      raw_keybuf_count++;
+		      GROW_RAW_KEYBUF (1);
+                      ASET (*raw_keybuf, *raw_keybuf_count, key);
+		      (*raw_keybuf_count)++;
 		      keybuf[t] = key;
 		      mock_input = t + 1;
 
@@ -10117,7 +10142,7 @@ shows the events before all translations (except for input methods).
 The value is always a vector.  */)
   (void)
 {
-  return Fvector (raw_keybuf_count, XVECTOR (raw_keybuf)->contents);
+  return Fvector (*raw_keybuf_count, XVECTOR (*raw_keybuf)->contents);
 }
 
 DEFUN ("clear-this-command-keys", Fclear_this_command_keys,
@@ -11262,8 +11287,8 @@ syms_of_keyboard (void)
   this_command_keys = Fmake_vector (make_number (40), Qnil);
   staticpro (&this_command_keys);
 
-  raw_keybuf = Fmake_vector (make_number (30), Qnil);
-  staticpro (&raw_keybuf);
+  raw_keybuf_buffer = Fmake_vector (make_number (30), Qnil);
+  staticpro (raw_keybuf);
 
   DEFSYM (Qcommand_execute, "command-execute");
   DEFSYM (Qinternal_echo_keystrokes_prefix, "internal-echo-keystrokes-prefix");


-- 
Alan Mackenzie (Nuremberg, Germany).

Message #14 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29349 <at> debbugs.gnu.org
Subject: Re: bug#29349: [Patch] Bug 29349: read_key_sequence is only partially
 recursive. This is a bug.
Date: Sun, 19 Nov 2017 19:01:09 +0200

> Date: Sun, 19 Nov 2017 15:59:08 +0000
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > > In a Linux tty using the GPM mouse package, doing read_key_sequence (the
> > > C function in keyboard.c), when a menu action is activated,
> > > read_key_sequence calls itself recursively to handle all the mouse
> > > manipulation.
> 
> > > Unfortunately, the variable raw_keybuf_count is initialised to 0 in
> > > r_k_s.  This includes in the recursive call.  This variable indexes the
> > > global buffer raw_keybuf, which accumulates the raw events which make up
> > > the key sequence.
> 
> > > The result of this is that the events in the recursive call overwrite
> > > the stored events of the outer r_k_s call, leaving a mess.
> 
> > > r_k_s is static in keyboard.c and is called from three functions:
> > > command_loop_1, read_menu_command (the one that gives the trouble), and
> > > read_key_sequence_vs.
> 
> > > So I propose as a solution that raw_keybuf_count be initialised to zero
> > > in two of these three functions, but not in read_menu_command (and no
> > > longer in read_key_sequence).  I've tried this, and it seems to work.
> > > It has the disadvantage of being ugly, and it makes read_menu_command
> > > only callable as a subfunction of r_k_s.
> 
> > > Has anybody any thoughts on this?
> 
> > Here is how I propose to solve this:

Can you please show a recipe where the current code misbehaves?  I've
re-read the thread, and found myself confused wrt the practical
implications of the problem you describe.

Thanks.

Message #17 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29349 <at> debbugs.gnu.org
Subject: Re: bug#29349: [Patch] Bug 29349: read_key_sequence is only
 partially recursive. This is a bug.
Date: Sun, 19 Nov 2017 17:45:22 +0000

Hello, Eli.

On Sun, Nov 19, 2017 at 19:01:09 +0200, Eli Zaretskii wrote:
> > Date: Sun, 19 Nov 2017 15:59:08 +0000
> > From: Alan Mackenzie <acm <at> muc.de>

> > > > In a Linux tty using the GPM mouse package, doing read_key_sequence (the
> > > > C function in keyboard.c), when a menu action is activated,
> > > > read_key_sequence calls itself recursively to handle all the mouse
> > > > manipulation.

> > > > Unfortunately, the variable raw_keybuf_count is initialised to 0 in
> > > > r_k_s.  This includes in the recursive call.  This variable indexes the
> > > > global buffer raw_keybuf, which accumulates the raw events which make up
> > > > the key sequence.

> > > > The result of this is that the events in the recursive call overwrite
> > > > the stored events of the outer r_k_s call, leaving a mess.

> > > > r_k_s is static in keyboard.c and is called from three functions:
> > > > command_loop_1, read_menu_command (the one that gives the trouble), and
> > > > read_key_sequence_vs.

> > > > So I propose as a solution that raw_keybuf_count be initialised to zero
> > > > in two of these three functions, but not in read_menu_command (and no
> > > > longer in read_key_sequence).  I've tried this, and it seems to work.
> > > > It has the disadvantage of being ugly, and it makes read_menu_command
> > > > only callable as a subfunction of r_k_s.

> > > > Has anybody any thoughts on this?

> > > Here is how I propose to solve this:

> Can you please show a recipe where the current code misbehaves?  I've
> re-read the thread, and found myself confused wrt the practical
> implications of the problem you describe.

In the emacs-26 branch, in a Linux tty with GPM configured and working,
type:

    C-h c  C-mouse-3 mouse-1 mouse-1

, without moving the mouse.  This will end up clicking on
"emacs-tutorial".  The message printed in the message area is then:

    <C-down-mouse-3> <help-menu> <emacs-tutorial> (translated from
    <mouse-1> <emacs-tutorial>) at that spot runs the command
    help-with-tutorial

.  In the "translated from <mouse-1> <emacs-tutorial>", the first event,
C-mouse-3 has been overwritten by mouse-1.  This mouse-1 is a mouse-click
from the menu processing.  `describe-key-briefly' can then do nothing
other than printing a spurious "translated from" message.

With the patch applied, the C-down-mouse-3 survives in the raw key
buffer, enabling `describe-key-briefly' to do the Right Thing.  It does
this by collecting the menu processing's mouse events in a separate
buffer, then copying that buffer to the main one afterwards.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #20 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29349 <at> debbugs.gnu.org
Subject: Re: bug#29349: [Patch] Bug 29349: read_key_sequence is only
 partially recursive. This is a bug.
Date: Sun, 19 Nov 2017 20:02:23 +0200

> Date: Sun, 19 Nov 2017 17:45:22 +0000
> Cc: 29349 <at> debbugs.gnu.org
> From: Alan Mackenzie <acm <at> muc.de>
> 
> In the emacs-26 branch, in a Linux tty with GPM configured and working,
> type:
> 
>     C-h c  C-mouse-3 mouse-1 mouse-1
> 
> , without moving the mouse.  This will end up clicking on
> "emacs-tutorial".  The message printed in the message area is then:
> 
>     <C-down-mouse-3> <help-menu> <emacs-tutorial> (translated from
>     <mouse-1> <emacs-tutorial>) at that spot runs the command
>     help-with-tutorial
> 
> .  In the "translated from <mouse-1> <emacs-tutorial>", the first event,
> C-mouse-3 has been overwritten by mouse-1.  This mouse-1 is a mouse-click
> from the menu processing.  `describe-key-briefly' can then do nothing
> other than printing a spurious "translated from" message.
> 
> With the patch applied, the C-down-mouse-3 survives in the raw key
> buffer, enabling `describe-key-briefly' to do the Right Thing.  It does
> this by collecting the menu processing's mouse events in a separate
> buffer, then copying that buffer to the main one afterwards.

OK, but then (a) please install the patch on master, not on the
release branch, and (b) why do we need the followup patch -- with the
mouse-1 events injected into the sequence the "translation" looks
correct and even educational.

Thanks.

Message #23 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29349 <at> debbugs.gnu.org
Subject: Re: bug#29349: [Patch] Bug 29349: read_key_sequence is only
 partially recursive. This is a bug.
Date: Sun, 19 Nov 2017 20:41:21 +0000

Hello, Eli.

On Sun, Nov 19, 2017 at 20:02:23 +0200, Eli Zaretskii wrote:
> > Date: Sun, 19 Nov 2017 17:45:22 +0000
> > Cc: 29349 <at> debbugs.gnu.org
> > From: Alan Mackenzie <acm <at> muc.de>

> > In the emacs-26 branch, in a Linux tty with GPM configured and working,
> > type:

> >     C-h c  C-mouse-3 mouse-1 mouse-1

> > , without moving the mouse.  This will end up clicking on
> > "emacs-tutorial".  The message printed in the message area is then:

> >     <C-down-mouse-3> <help-menu> <emacs-tutorial> (translated from
> >     <mouse-1> <emacs-tutorial>) at that spot runs the command
> >     help-with-tutorial

> > .  In the "translated from <mouse-1> <emacs-tutorial>", the first event,
> > C-mouse-3 has been overwritten by mouse-1.  This mouse-1 is a mouse-click
> > from the menu processing.  `describe-key-briefly' can then do nothing
> > other than printing a spurious "translated from" message.

> > With the patch applied, the C-down-mouse-3 survives in the raw key
> > buffer, enabling `describe-key-briefly' to do the Right Thing.  It does
> > this by collecting the menu processing's mouse events in a separate
> > buffer, then copying that buffer to the main one afterwards.

> OK, but then (a) please install the patch on master, not on the
> release branch, ....

I'll do that, but probably not tonight.

> .... and (b) why do we need the followup patch -- with the mouse-1
> events injected into the sequence the "translation" looks correct and
> even educational.

I don't think it looks correct.  The C-down-mouse-3 which exists as an
essential part of the key sequence has been overwritten in the
"translation".

The other thing is that if mouse-movements get into the raw event buffer
(which I've seen, but for some reason amn't seeing any more) the
"translated from" could become objectionably long.

I think the "translated from" bit is intended to document a sequence the
user is aware of (such as a double click) being translated into a
different sequence she's aware of (such as a single click).  The mouse-1,
I believe, is more part of the user's subconsciousness rather than
awareness.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #26 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29349 <at> debbugs.gnu.org
Subject: Re: bug#29349: [Patch] Bug 29349: read_key_sequence is only
 partially recursive. This is a bug.
Date: Mon, 20 Nov 2017 05:33:28 +0200

> Date: Sun, 19 Nov 2017 20:41:21 +0000
> Cc: 29349 <at> debbugs.gnu.org
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > > With the patch applied, the C-down-mouse-3 survives in the raw key
> > > buffer, enabling `describe-key-briefly' to do the Right Thing.  It does
> > > this by collecting the menu processing's mouse events in a separate
> > > buffer, then copying that buffer to the main one afterwards.
> 
> > OK, but then (a) please install the patch on master, not on the
> > release branch, ....
> 
> I'll do that, but probably not tonight.
> 
> > .... and (b) why do we need the followup patch -- with the mouse-1
> > events injected into the sequence the "translation" looks correct and
> > even educational.
> 
> I don't think it looks correct.  The C-down-mouse-3 which exists as an
> essential part of the key sequence has been overwritten in the
> "translation".

That's not what I see here, with your patch applied in keyboard.c,  I
see this:

  <C-down-mouse-3> <indent-pp-sexp> (translated from <C-down-mouse-3>
  <mouse-1> <indent-pp-sexp>) at that spot runs the command
  indent-pp-sexp (found in global-map), which is an interactive compiled
  Lisp function in `lisp-mode.el'.

So C-down-mouse-3 is still there, we just have each click in the menus
injected into the sequence.  What did you see after applying that
patch.

> The other thing is that if mouse-movements get into the raw event buffer
> (which I've seen, but for some reason amn't seeing any more) the
> "translated from" could become objectionably long.

I don't see that as a problem.

> I think the "translated from" bit is intended to document a sequence the
> user is aware of (such as a double click) being translated into a
> different sequence she's aware of (such as a single click).

And that's exactly what happens in this case.

> The mouse-1, I believe, is more part of the user's subconsciousness
> rather than awareness.

But those mouse-1 clicks are real.

Message #29 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29349 <at> debbugs.gnu.org
Subject: Re: bug#29349: [Patch] Bug 29349: read_key_sequence is only
 partially recursive. This is a bug.
Date: Mon, 20 Nov 2017 17:27:26 +0000

Hello, Eli.

On Mon, Nov 20, 2017 at 05:33:28 +0200, Eli Zaretskii wrote:
> > Date: Sun, 19 Nov 2017 20:41:21 +0000
> > Cc: 29349 <at> debbugs.gnu.org
> > From: Alan Mackenzie <acm <at> muc.de>

[ .... ]

> > > .... and (b) why do we need the followup patch -- with the mouse-1
> > > events injected into the sequence the "translation" looks correct and
> > > even educational.

> > I don't think it looks correct.  The C-down-mouse-3 which exists as an
> > essential part of the key sequence has been overwritten in the
> > "translation".

> That's not what I see here, with your patch applied in keyboard.c,  I
> see this:

>   <C-down-mouse-3> <indent-pp-sexp> (translated from <C-down-mouse-3>
>   <mouse-1> <indent-pp-sexp>) at that spot runs the command
>   indent-pp-sexp (found in global-map), which is an interactive compiled
>   Lisp function in `lisp-mode.el'.

> So C-down-mouse-3 is still there, we just have each click in the menus
> injected into the sequence.  What did you see after applying that
> patch.

Apologies.  I wasn't paying enough attention to your post, and I was a
little confused.

> > The other thing is that if mouse-movements get into the raw event buffer
> > (which I've seen, but for some reason amn't seeing any more) the
> > "translated from" could become objectionably long.

> I don't see that as a problem.

OK.

> > I think the "translated from" bit is intended to document a sequence the
> > user is aware of (such as a double click) being translated into a
> > different sequence she's aware of (such as a single click).

> And that's exactly what happens in this case.

> > The mouse-1, I believe, is more part of the user's subconsciousness
> > rather than awareness.

> But those mouse-1 clicks are real.

OK, again.  I think I would still prefer to suppress that "translated
from" message, but it's not a strong preference, and not a terribly
important point.

So, I'll get on and commit my patch to keyboard.c (to master), but leave
help.el alone.  If those "translated from"s ever do get to be
objectionable, as measured by user response, we can always do something
about them later.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #34 received at 29349-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: 29349-done <at> debbugs.gnu.org
Subject: Re: [Patch] Bug 29349: read_key_sequence is only partially
 recursive.  This is a bug.
Date: Mon, 20 Nov 2017 18:12:27 +0000

Bug fixed in master.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #37 received at 29349 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29349 <at> debbugs.gnu.org
Subject: Re: bug#29349: [Patch] Bug 29349: read_key_sequence is only
 partially recursive. This is a bug.
Date: Mon, 20 Nov 2017 20:24:41 +0200

> Date: Mon, 20 Nov 2017 17:27:26 +0000
> Cc: 29349 <at> debbugs.gnu.org
> From: Alan Mackenzie <acm <at> muc.de>
> 
> So, I'll get on and commit my patch to keyboard.c (to master), but leave
> help.el alone.  If those "translated from"s ever do get to be
> objectionable, as measured by user response, we can always do something
> about them later.

Agreed.

Thanks for working on this tricky issue.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.