bug#46243: [External] : Re: bug#46243: 26.3; If invoke menu item that reads a char, get keystrokes echo

[Stefan Monnier <monnier@iro.umontreal.ca>]

>> Do you happen to remember how you came to think that
>> echo_message_buffer was involved in that old bug report's scenario?
> Sadly, no.

But I do have some relevant indirect info (it may seem unrelated at
first, but bear with me):

I was trying to write a test case for bug#15332.
Here's what I have so far:

    (ert-deftest keyboard-tests--echo-keystrokes-bug15332 ()
      (let ((msgs '())
            (unread-command-events nil)
            (redisplay--interactive t)
            (echo-keystrokes 2))
        (setq unread-command-events '(?\C-u))
        (let* ((timer1
                (run-with-timer 3 1
                                (lambda ()
                                  (setq unread-command-events '(?5)))))
               (timer2
                (run-with-timer 2.5 1
                                (lambda ()
                                  (push (current-message) msgs)))))
          (run-with-timer 5 nil
                          (lambda ()
                            (cancel-timer timer1)
                            (cancel-timer timer2)
                            (throw 'exit msgs)))
          (recursive-edit)
          (should (equal msgs '("C-u 55-" "C-u 5-" "C-u-"))))))

It works when Emacs is run interactively: when I revert the offending
patch, the test signals an error because the collected messages are only
`(nil "C-u 5-" "C-u-")` since the timer that collects the messages runs
just when the second `?5` event causes the echo to be erroneously
temporarily erased (until the next `echo-keystrokes` timeout).

Now I tried to make it work in batch mode, but so far I haven't been
successful.  As you can guess from the code above I added
a `redisplay--interactive` variable to the C code to force it code to
use the "interactive" paths at various places and I managed to make it
work well enough that the above code does indeed collect echo messages,
but somehow those messages end up correct regardless of the
offending patch.

So I suspect that the test of `echo_message_buffer` is affected by
other events (which apparently don't occur in batch mode).

I know it's a pretty faint signal, I'm sorry it's all I have so far.

See below the patch I'm using,


        Stefan


diff --git a/src/keyboard.c b/src/keyboard.c
index 9ee4c4f6d6..b2dad0ca2d 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2539,8 +2539,9 @@ read_char (int commandflag, Lisp_Object map,
 
   if (/* There currently is something in the echo area.  */
       !NILP (echo_area_buffer[0])
-      && (/* It's an echo from a different kboard.  */
-	  echo_kboard != current_kboard
+      && (!EQ (echo_area_buffer[0], echo_message_buffer)
+	  /* It's an echo from a different kboard.  */
+	  || echo_kboard != current_kboard
 	  /* Or we explicitly allow overwriting whatever there is.  */
 	  || ok_to_echo_at_next_pause == NULL))
     cancel_echoing ();
@@ -2636,7 +2637,7 @@ read_char (int commandflag, Lisp_Object map,
       && !current_kboard->immediate_echo
       && (this_command_key_count > 0
 	  || !NILP (call0 (Qinternal_echo_keystrokes_prefix)))
-      && ! noninteractive
+      && (redisplay__interactive || ! noninteractive)
       && echo_keystrokes_p ()
       && (/* No message.  */
 	  NILP (echo_area_buffer[0])
diff --git a/src/xdisp.c b/src/xdisp.c
index efca6f641f..3985daa699 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -11150,12 +11150,13 @@ message3_nolog (Lisp_Object m)
 {
   struct frame *sf = SELECTED_FRAME ();
 
-  if (FRAME_INITIAL_P (sf))
+  if (FRAME_INITIAL_P (sf) && !redisplay__interactive)
     message_to_stderr (m);
   /* Error messages get reported properly by cmd_error, so this must be just an
      informative message; if the frame hasn't really been initialized yet, just
      toss it.  */
-  else if (INTERACTIVE && sf->glyphs_initialized_p)
+  else if ((INTERACTIVE || redisplay__interactive)
+           && sf->glyphs_initialized_p)
     {
       /* Get the frame containing the mini-buffer
 	 that the selected frame is using.  */
@@ -35603,6 +35604,10 @@ syms_of_xdisp (void)
 mouse stays within the extent of a single glyph (except for images).  */);
   mouse_fine_grained_tracking = false;
 
+  DEFVAR_BOOL ("redisplay--interactive", redisplay__interactive,
+    doc: /* If non-nil, redisplay will proceed even in batch mode.  */);
+  redisplay__interactive = false;
+
   DEFVAR_BOOL ("redisplay-skip-initial-frame", redisplay_skip_initial_frame,
     doc: /* Non-nil to skip redisplay in initial frame.
 The initial frame is not displayed anywhere, so skipping it is
diff --git a/test/src/keyboard-tests.el b/test/src/keyboard-tests.el
index 607d2eafd4..e9a6c497a9 100644
--- a/test/src/keyboard-tests.el
+++ b/test/src/keyboard-tests.el
@@ -23,14 +23,15 @@
 
 (ert-deftest keyboard-unread-command-events ()
   "Test `unread-command-events'."
-  (should (equal (progn (push ?\C-a unread-command-events)
-                        (read-event nil nil 1))
-                 ?\C-a))
-  (should (equal (progn (run-with-timer
-                         1 nil
-                         (lambda () (push '(t . ?\C-b) unread-command-events)))
-                        (read-event nil nil 2))
-                 ?\C-b)))
+  (let ((unread-command-events nil))
+    (should (equal (progn (push ?\C-a unread-command-events)
+                          (read-event nil nil 1))
+                   ?\C-a))
+    (should (equal (progn (run-with-timer
+                           1 nil
+                           (lambda () (push '(t . ?\C-b) unread-command-events)))
+                          (read-event nil nil 2))
+                   ?\C-b))))
 
 (ert-deftest keyboard-lossage-size ()
   "Test `lossage-size'."
@@ -46,6 +47,27 @@ keyboard-lossage-size
     (should-error (lossage-size (1- min-value)))
     (should (= lossage-orig (lossage-size lossage-orig)))))
 
+(ert-deftest keyboard-tests--echo-keystrokes-bug15332 ()
+  (let ((msgs '())
+        (unread-command-events nil)
+        (redisplay--interactive t)
+        (echo-keystrokes 2))
+    (setq unread-command-events '(?\C-u))
+    (let* ((timer1
+	    (run-with-timer 3 1
+			    (lambda ()
+			      (setq unread-command-events '(?5)))))
+	   (timer2
+	    (run-with-timer 2.5 1
+			    (lambda ()
+			      (push (current-message) msgs)))))
+      (run-with-timer 5 nil
+	              (lambda ()
+	                (cancel-timer timer1)
+	                (cancel-timer timer2)
+	                (throw 'exit msgs)))
+      (recursive-edit)
+      (should (equal msgs '("C-u 55-" "C-u 5-" "C-u-"))))))
 
 (provide 'keyboard-tests)
 ;;; keyboard-tests.el ends here