Subject: Re: bug#48042: 26.3; Macros don't work with french-postfix input method

From: Gregory Heytings <[email-masked]>
Date: Sat, 15 May 2021 20:17:31 +0000

>> IOW, if the user presses something at that moment, the terminal 
>> initialization process does not work as expected.
>
> So maybe we shouldn't discard the input if the initialization fails? 
> Otherwise we leave no traces that could allow understanding what 
> happened.
>

Okay, updated patch attached.

From 601ec3c2d6472426f3763f30ee2d7710924dfa1f Mon Sep 17 00:00:00 2001
From: Gregory Heytings <[email-masked]>
Date: Sat, 15 May 2021 20:15:59 +0000
Subject: [PATCH] Fix key recording bug when an input method is activated

* lisp/international/quail.el (quail-add-unread-command-events):
New function.
(quail-start-translation, quail-start-conversion,
quail-update-translation, quail-next-translation,
quail-prev-translation, quail-next-translation-block,
quail-prev-translation-block, quail-minibuffer-message): Use the
new function (and partly revert 03e3440dbb).

* lisp/subr.el (inhibit--record-char): New obsolete variable.

* lisp/term/xterm.el (xterm--init): New function, with most of the
code of former terminal-init-xterm.
(terminal-init-xterm): Clear the lossage after terminal
initialization (see Bug#44908).
(xterm--read-event-for-query): Do not use inhibit--record-char
anymore (revert 3e6525d69f).

* src/keyboard.c (syms_of_keyboard): Remove inhibit--record-char
(partly revert 03e3440dbb).
(record_char, syms_of_keyboard_for_pdumper): Do not use
inhibit_record_char anymore.
---
 lisp/international/quail.el | 72 ++++++++++++++++++-------------------
 lisp/subr.el                |  6 ++++
 lisp/term/xterm.el          | 20 +++++++----
 src/keyboard.c              | 13 -------
 4 files changed, 55 insertions(+), 56 deletions(-)

diff --git a/lisp/international/quail.el b/lisp/international/quail.el
index fff06deee8..33851f09a1 100644
--- a/lisp/international/quail.el
+++ b/lisp/international/quail.el
@@ -1368,6 +1368,27 @@ quail-delete-region
       (delete-region (overlay-start quail-overlay)
 		     (overlay-end quail-overlay))))
 
+(defun quail-add-unread-command-events (key &optional reset)
+  "Add KEY to `unread-command-events', ensuring that it is not recorded.
+If KEY is a character, it is prepended to `unread-command-events' as
+a cons cell of the form (no-record . KEY).
+If KEY is a vector of events, the events in the vector are prepended
+to `unread-command-events', after converting each event to a cons cell
+of the form (no-record . EVENT).
+Quail puts keys back in `unread-command-events' to be handled again,
+and when it does this these keys have already been recorded in the
+recent keys and in the keyboard macro being defined, which means that
+recording them again creates duplicates.
+When RESET is non-nil, the events in `unread-command-events' are first
+discarded."
+  (if reset (setq unread-command-events nil))
+  (setq unread-command-events
+        (if (characterp key)
+            (cons (cons 'no-record key) unread-command-events)
+          (append (mapcan (lambda (e) (list (cons 'no-record e)))
+                          (append key nil))
+                  unread-command-events))))
+
 (defun quail-start-translation (key)
   "Start translation of the typed character KEY by the current Quail package.
 Return the input string."
@@ -1385,13 +1406,11 @@ quail-start-translation
 	     ;; (generated-events nil)     ;FIXME: What is this?
 	     (input-method-function nil)
 	     (modified-p (buffer-modified-p))
-	     last-command-event last-command this-command inhibit-record)
+	     last-command-event last-command this-command)
 	(setq quail-current-key ""
 	      quail-current-str ""
 	      quail-translating t)
-	(if key
-	    (setq unread-command-events (cons key unread-command-events)
-                  inhibit-record t))
+	(if key (quail-add-unread-command-events key))
 	(while quail-translating
 	  (set-buffer-modified-p modified-p)
 	  (quail-show-guidance)
@@ -1400,13 +1419,8 @@ quail-start-translation
 				     (or input-method-previous-message "")
 				     quail-current-str
 				     quail-guidance-str)))
-                 ;; We inhibit record_char only for the first key,
-                 ;; because it was already recorded before read_char
-                 ;; called quail-input-method.
-                 (inhibit--record-char inhibit-record)
 		 (keyseq (read-key-sequence prompt nil nil t))
 		 (cmd (lookup-key (quail-translation-keymap) keyseq)))
-            (setq inhibit-record nil)
 	    (if (if key
 		    (and (commandp cmd) (not (eq cmd 'quail-other-command)))
 		  (eq cmd 'quail-self-insert-command))
@@ -1420,9 +1434,7 @@ quail-start-translation
 		    (quail-error (message "%s" (cdr err)) (beep))))
 	      ;; KEYSEQ is not defined in the translation keymap.
 	      ;; Let's return the event(s) to the caller.
-	      (setq unread-command-events
-		    (append (this-single-command-raw-keys)
-                            unread-command-events))
+	      (quail-add-unread-command-events (this-single-command-raw-keys))
 	      (setq quail-translating nil))))
 	(quail-delete-region)
 	quail-current-str)
@@ -1450,15 +1462,13 @@ quail-start-conversion
 	     ;; (generated-events nil)     ;FIXME: What is this?
 	     (input-method-function nil)
 	     (modified-p (buffer-modified-p))
-	     last-command-event last-command this-command inhibit-record)
+	     last-command-event last-command this-command)
 	(setq quail-current-key ""
 	      quail-current-str ""
 	      quail-translating t
 	      quail-converting t
 	      quail-conversion-str "")
-	(if key
-	    (setq unread-command-events (cons key unread-command-events)
-                  inhibit-record t))
+	(if key (quail-add-unread-command-events key))
 	(while quail-converting
 	  (set-buffer-modified-p modified-p)
 	  (or quail-translating
@@ -1474,13 +1484,8 @@ quail-start-conversion
 				     quail-conversion-str
 				     quail-current-str
 				     quail-guidance-str)))
-                 ;; We inhibit record_char only for the first key,
-                 ;; because it was already recorded before read_char
-                 ;; called quail-input-method.
-                 (inhibit--record-char inhibit-record)
 		 (keyseq (read-key-sequence prompt nil nil t))
 		 (cmd (lookup-key (quail-conversion-keymap) keyseq)))
-            (setq inhibit-record nil)
 	    (if (if key (commandp cmd) (eq cmd 'quail-self-insert-command))
 		(progn
 		  (setq last-command-event (aref keyseq (1- (length keyseq)))
@@ -1503,9 +1508,7 @@ quail-start-conversion
 			    (setq quail-converting nil)))))
 	      ;; KEYSEQ is not defined in the conversion keymap.
 	      ;; Let's return the event(s) to the caller.
-	      (setq unread-command-events
-		    (append (this-single-command-raw-keys)
-                            unread-command-events))
+	      (quail-add-unread-command-events (this-single-command-raw-keys))
 	      (setq quail-converting nil))))
 	(setq quail-translating nil)
 	(if (overlay-start quail-conv-overlay)
@@ -1551,9 +1554,8 @@ quail-update-translation
 	       (or input-method-exit-on-first-char
 		   (while (> len control-flag)
 		     (setq len (1- len))
-		     (setq unread-command-events
-			   (cons (aref quail-current-key len)
-				 unread-command-events))))))
+		     (quail-add-unread-command-events
+		      (aref quail-current-key len))))))
 	    ((null control-flag)
 	     (unless quail-current-str
 	       (setq quail-current-str
@@ -1799,8 +1801,7 @@ quail-next-translation
 	  (setcar indices (1+ (car indices)))
 	  (quail-update-current-translations)
 	  (quail-update-translation nil)))
-    (setq unread-command-events
-	  (cons last-command-event unread-command-events))
+    (quail-add-unread-command-events last-command-event)
     (quail-terminate-translation)))
 
 (defun quail-prev-translation ()
@@ -1814,8 +1815,7 @@ quail-prev-translation
 	  (setcar indices (1- (car indices)))
 	  (quail-update-current-translations)
 	  (quail-update-translation nil)))
-    (setq unread-command-events
-	  (cons last-command-event unread-command-events))
+    (quail-add-unread-command-events last-command-event)
     (quail-terminate-translation)))
 
 (defun quail-next-translation-block ()
@@ -1830,8 +1830,7 @@ quail-next-translation-block
 	  (setcar indices (+ (nth 2 indices) offset))
 	  (quail-update-current-translations)
 	  (quail-update-translation nil)))
-    (setq unread-command-events
-	  (cons last-command-event unread-command-events))
+    (quail-add-unread-command-events last-command-event)
     (quail-terminate-translation)))
 
 (defun quail-prev-translation-block ()
@@ -1850,8 +1849,7 @@ quail-prev-translation-block
 		(setcar indices (+ (nth 1 indices) offset))
 		(quail-update-current-translations)))
 	  (quail-update-translation nil)))
-    (setq unread-command-events
-	  (cons last-command-event unread-command-events))
+    (quail-add-unread-command-events last-command-event)
     (quail-terminate-translation)))
 
 (defun quail-abort-translation ()
@@ -2006,8 +2004,8 @@ quail-minibuffer-message
     (sit-for 1000000)
     (delete-region point-max (point-max))
     (when quit-flag
-      (setq quit-flag nil
-	    unread-command-events '(7)))))
+      (setq quit-flag nil)
+      (quail-add-unread-command-events 7 t))))
 
 (defun quail-show-guidance ()
   "Display a guidance for Quail input method in some window.
diff --git a/lisp/subr.el b/lisp/subr.el
index 7a055f2ba1..5572e64468 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1757,6 +1757,12 @@ 'inhibit-nul-byte-detection
 (make-obsolete-variable 'load-dangerous-libraries
                         "no longer used." "27.1")
 
+(defvar inhibit--record-char nil
+  "Obsolete variable.
+This was used internally by quail.el and keyboard.c in Emacs 27.
+It does nothing in Emacs 28.")
+(make-obsolete-variable 'inhibit--record-char nil "28.1")
+
 ;; We can't actually make `values' obsolete, because that will result
 ;; in warnings when using `values' in let-bindings.
 ;;(make-obsolete-variable 'values "no longer used" "28.1")
diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
index eeaf805930..8bcae37afe 100644
--- a/lisp/term/xterm.el
+++ b/lisp/term/xterm.el
@@ -770,8 +770,7 @@ xterm--read-event-for-query
 By not redisplaying right away for xterm queries, we can avoid
 unsightly flashing during initialization. Give up and redisplay
 anyway if we've been waiting a little while."
-  (let ((start-time (current-time))
-        (inhibit--record-char t))
+  (let ((start-time (current-time)))
     (or (let ((inhibit-redisplay t))
           (read-event nil nil xterm-query-redisplay-timeout))
         (read-event nil nil
@@ -839,8 +838,8 @@ xterm--push-map
    basemap
    (make-composed-keymap map (keymap-parent basemap))))
 
-(defun terminal-init-xterm ()
-  "Terminal initialization function for xterm."
+(defun xterm--init ()
+  "Initialize the terminal for xterm."
   ;; rxvt terminals sometimes set the TERM variable to "xterm", but
   ;; rxvt's keybindings are incompatible with xterm's. It is
   ;; better in that case to use rxvt's initialization function.
@@ -882,9 +881,18 @@ terminal-init-xterm
   ;; support it just ignore the sequence.
   (xterm--init-bracketed-paste-mode)
   ;; We likewise unconditionally enable support for focus tracking.
-  (xterm--init-focus-tracking)
+  (xterm--init-focus-tracking))
 
-  (run-hooks 'terminal-init-xterm-hook))
+(defun terminal-init-xterm ()
+  "Terminal initialization function for xterm."
+  (unwind-protect
+      (progn
+        (xterm--init)
+        ;; If the terminal initialization completed without errors, clear
+        ;; the lossage to discard the responses of the terminal emulator
+        ;; during initialization; otherwise they appear in the recent keys.
+        (clear-this-command-keys))
+    (run-hooks 'terminal-init-xterm-hook)))
 
 (defun xterm--init-modify-other-keys ()
   "Terminal initialization for xterm's modifyOtherKeys support."
diff --git a/src/keyboard.c b/src/keyboard.c
index 47b5e59024..c855d45afa 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3233,10 +3233,6 @@ help_char_p (Lisp_Object c)
 static void
 record_char (Lisp_Object c)
 {
-  /* quail.el binds this to avoid recording keys twice.  */
-  if (inhibit_record_char)
-    return;
-
   int recorded = 0;
 
   if (CONSP (c) && (EQ (XCAR (c), Qhelp_echo) || EQ (XCAR (c), Qmouse_movement)))
@@ -12343,13 +12339,6 @@ syms_of_keyboard (void)
                Vwhile_no_input_ignore_events,
                doc: /* Ignored events from while-no-input.  */);
 
-  DEFVAR_BOOL ("inhibit--record-char",
-	       inhibit_record_char,
-	       doc: /* If non-nil, don't record input events.
-This inhibits recording input events for the purposes of keyboard
-macros, dribble file, and `recent-keys'.
-Internal use only.  */);
-
   pdumper_do_now_and_after_load (syms_of_keyboard_for_pdumper);
 }
 
@@ -12383,8 +12372,6 @@ syms_of_keyboard_for_pdumper (void)
   /* Create the initial keyboard.  Qt means 'unset'.  */
   eassert (initial_kboard == NULL);
   initial_kboard = allocate_kboard (Qt);
-
-  inhibit_record_char = false;
 }
 
 void
-- 
2.30.2