unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#48042: 26.3; Macros don't work with french-postfix input method
@ 2021-04-26 18:05 harven
  2021-04-26 18:22 ` Eli Zaretskii
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: harven @ 2021-04-26 18:05 UTC (permalink / raw)
  To: 48042

Starting with emacs -Q in the *scratch* buffer, choose 
the french-postfix input method for the buffer, for example by typing

    M-x set-input-method RET french-postfix RET

Then register a simple macro inputing characters in buffer, for example

    C-x ( yes RET C-x )

Finally execute the macro

    C-x e

The word inserted at point is not "yes" but "yeess"
Characters insertion in macros mysteriously prints some characters twice.
This is not specific to the *scratch* buffer and can be demonstrated
in any buffer with the "french-postfix" method enabled. 
Other than that, macros seem to work fine.

In GNU Emacs 26.3 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.14)
 of 2020-03-26, modified by Debian built on lcy01-amd64-020
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
System Description:	Ubuntu 20.04.2 LTS

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Loading quail/latin-post...done
You can run the command ‘set-input-method’ with C-x RET C-\
Defining kbd macro...
Keyboard macro defined
(Type e to repeat macro)

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --enable-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/26.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/26.3/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils --build
 x86_64-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
 --libexecdir=/usr/lib --localstatedir=/var/lib
 --infodir=/usr/share/info --mandir=/usr/share/man --enable-libsystemd
 --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/26.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/26.3/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils --with-x=yes
 --with-x-toolkit=gtk3 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
 -fdebug-prefix-map=/build/emacs-mEZBk7/emacs-26.3+1=. -fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LIBSYSTEMD LCMS2

Important settings:
  value of $LANG: fr_FR.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
mml-sec password-cache epa derived epg epg-config gnus-util rmail
rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils cl-seq cl-extra edmacro kmacro cus-start
cus-load quail help-mode easymenu cl-loaddefs cl-lib elec-pair time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 112963 9527)
 (symbols 48 21805 1)
 (miscs 40 55 154)
 (strings 32 33142 1268)
 (string-bytes 1 820564)
 (vectors 16 16447)
 (vector-slots 8 512262 11228)
 (floats 8 51 104)
 (intervals 56 269 0)
 (buffers 992 12))





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-04-26 18:05 bug#48042: 26.3; Macros don't work with french-postfix input method harven
@ 2021-04-26 18:22 ` Eli Zaretskii
  2021-04-28 18:24 ` harven
  2021-05-14  9:29 ` Gregory Heytings
  2 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2021-04-26 18:22 UTC (permalink / raw)
  To: harven; +Cc: 48042

> From: harven@free.fr
> Date: Mon, 26 Apr 2021 20:05:54 +0200
> 
> Starting with emacs -Q in the *scratch* buffer, choose 
> the french-postfix input method for the buffer, for example by typing
> 
>     M-x set-input-method RET french-postfix RET
> 
> Then register a simple macro inputing characters in buffer, for example
> 
>     C-x ( yes RET C-x )
> 
> Finally execute the macro
> 
>     C-x e
> 
> The word inserted at point is not "yes" but "yeess"

In Emacs 27 and later, I get "yess".  So the situation is better, but
not entirely fixed yet.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-04-26 18:05 bug#48042: 26.3; Macros don't work with french-postfix input method harven
  2021-04-26 18:22 ` Eli Zaretskii
@ 2021-04-28 18:24 ` harven
  2021-05-14  9:29 ` Gregory Heytings
  2 siblings, 0 replies; 26+ messages in thread
From: harven @ 2021-04-28 18:24 UTC (permalink / raw)
  To: 48042

The bug can probably be reproduced with all latin postfix input methods.
I tried the following methods from latin-post.el 
and got the same behavior as the french-postfix method.

french-alt-postfix german-postfix croatian-postfix greek-postfix
latin-postfix latin-1-postfix esperanto-postfix  spanish-postfix 
latin-alt-postfix swedish-postfix scandinavian-postfix
norwegian-postfix italian-postfix icelandic-postfix 

The doubling happens for letters that admit accented versions.

So the bug should probably be renamed 
"Macros don't work with postfix input methods."





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-04-26 18:05 bug#48042: 26.3; Macros don't work with french-postfix input method harven
  2021-04-26 18:22 ` Eli Zaretskii
  2021-04-28 18:24 ` harven
@ 2021-05-14  9:29 ` Gregory Heytings
  2021-05-14  9:55   ` Basil L. Contovounesios
  2021-05-14 11:09   ` Eli Zaretskii
  2 siblings, 2 replies; 26+ messages in thread
From: Gregory Heytings @ 2021-05-14  9:29 UTC (permalink / raw)
  To: harven; +Cc: 48042

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]


This was an interesting bug.  Let's first define a recipe to make it more 
apparent:

C-x C-m C-\ french-postfix RET C-x ( pa^te'_a`_l'Unicode RET C-x ) C-x e

This should insert "pâté_à_l'Unicode".  It worked as expected with Emacs 
21-24.

After commit 30a6b1f814, which was the result of the discussion in 
http://lists.gnu.org/archive/html/emacs-devel/2015-08/msg00193.html , and 
before commit 03e3440dbb, it inserts "paâtteé__aà__l'UUnniiccocododee". 
This is what one gets in Emacs 25 and 26.  This is not surprising, as the 
recipe in that discussion and quail.el both use unread-command-events, but 
expect an opposite effect.

After commit 03e3440dbb, which was the result of bug#32108, it inserts 
"pâtté__à__l'Unnicocdode".  This is what one gets in Emacs 27 (and 28 till 
now).

I attach a patch to fix that bug.  I checked that the recipes that led to 
the two above commits still work correctly, and that it does not introduce 
regressions with make check.

Note that the inhibit--record-char variable, which was introduced by 
commit 03e3440dbb, is, after applying that patch, only used once, namely 
in lisp/term/xterm.el, as a result of bug#44908.  It is not used in ELPA 
or MELPA.  I'm not convinced that bug#44908 is a sufficient reason to keep 
that variable.

[-- Attachment #2: Type: text/x-diff, Size: 4616 bytes --]

From 359a644498df85a1b063bfb0aace3df34adac3e0 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 14 May 2021 09:20:03 +0000
Subject: [PATCH] Fix input method bug when recording keyboard macros

* src/keyboard.c (read_char): Do not record character again when
defining a keyboard macro with an input method activated (Bug#48042).
(record_char): Partly revert 03e3440dbb.

* lisp/international/quail.el (quail-start-translation,
quail-start-conversion): Partly revert 03e3440dbb.
---
 lisp/international/quail.el | 20 ++++----------------
 src/keyboard.c              |  7 +++++--
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/lisp/international/quail.el b/lisp/international/quail.el
index fff06deee8..6c2088a95b 100644
--- a/lisp/international/quail.el
+++ b/lisp/international/quail.el
@@ -1385,13 +1385,12 @@ 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))
+	    (setq unread-command-events (cons key unread-command-events)))
 	(while quail-translating
 	  (set-buffer-modified-p modified-p)
 	  (quail-show-guidance)
@@ -1400,13 +1399,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))
@@ -1450,15 +1444,14 @@ 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))
+	    (setq unread-command-events (cons key unread-command-events)))
 	(while quail-converting
 	  (set-buffer-modified-p modified-p)
 	  (or quail-translating
@@ -1474,13 +1467,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)))
diff --git a/src/keyboard.c b/src/keyboard.c
index 47b5e59024..f72c0870fe 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3098,7 +3098,11 @@ read_char (int commandflag, Lisp_Object map,
   /* When we consume events from the various unread-*-events lists, we
      bypass the code that records input, so record these events now if
      they were not recorded already.  */
-  if (!recorded)
+  if (!recorded &&
+      /* However, don't record events when a keyboard macro is being
+         defined and an input method is activated (Bug#48042).  */
+      ! (! NILP (KVAR (current_kboard, defining_kbd_macro)) &&
+         ! NILP (Fsymbol_value (Qcurrent_input_method))))
     {
       record_char (c);
       recorded = true;
@@ -3233,7 +3237,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;
 
-- 
2.30.2


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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14  9:29 ` Gregory Heytings
@ 2021-05-14  9:55   ` Basil L. Contovounesios
  2021-05-14 10:03     ` Gregory Heytings
  2021-05-14 11:09   ` Eli Zaretskii
  1 sibling, 1 reply; 26+ messages in thread
From: Basil L. Contovounesios @ 2021-05-14  9:55 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: 48042, harven

Gregory Heytings <gregory@heytings.org> writes:

> +      ! (! NILP (KVAR (current_kboard, defining_kbd_macro)) &&
> +         ! NILP (Fsymbol_value (Qcurrent_input_method))))

Nit: Can we De Morgan?

  (NILP (KVAR (current_kboard, defining_kbd_macro)) ||
   NILP (Fsymbol_value (Qcurrent_input_method)))

Thanks,

-- 
Basil





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14  9:55   ` Basil L. Contovounesios
@ 2021-05-14 10:03     ` Gregory Heytings
  0 siblings, 0 replies; 26+ messages in thread
From: Gregory Heytings @ 2021-05-14 10:03 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 48042, harven


>> +      ! (! NILP (KVAR (current_kboard, defining_kbd_macro)) &&
>> +         ! NILP (Fsymbol_value (Qcurrent_input_method))))
>
> Nit: Can we De Morgan?
>
>  (NILP (KVAR (current_kboard, defining_kbd_macro)) ||
>   NILP (Fsymbol_value (Qcurrent_input_method)))
>

I tried this in an earlier version, but find the De Morgan'd version less 
clear.  Perhaps it's just me.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14  9:29 ` Gregory Heytings
  2021-05-14  9:55   ` Basil L. Contovounesios
@ 2021-05-14 11:09   ` Eli Zaretskii
  2021-05-14 13:38     ` Gregory Heytings
  1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-14 11:09 UTC (permalink / raw)
  To: Gregory Heytings, Stefan Monnier; +Cc: 48042, harven

> Date: Fri, 14 May 2021 09:29:08 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: 48042@debbugs.gnu.org
> 
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -3098,7 +3098,11 @@ read_char (int commandflag, Lisp_Object map,
>    /* When we consume events from the various unread-*-events lists, we
>       bypass the code that records input, so record these events now if
>       they were not recorded already.  */
> -  if (!recorded)
> +  if (!recorded &&
> +      /* However, don't record events when a keyboard macro is being
> +         defined and an input method is activated (Bug#48042).  */
> +      ! (! NILP (KVAR (current_kboard, defining_kbd_macro)) &&
> +         ! NILP (Fsymbol_value (Qcurrent_input_method))))

Bother: AFAIK, current-input-method non-nil means the user activated
an input method, it doesn't mean we are in the middle of typing a key
sequence that will yield a character via the input method.  That is, a
user could activate an input method, but still keep typing just ASCII
characters.  So why is this condition correct here to avoid recording
input more than once?

This is why I tried to have a variable that quail.el binds while
actually processing keys.  I'd appreciate some explanation for why
that didn't work 100% in the case in point (it still avoided recording
twice some of the keys, so it isn't entirely wrong).

Stefan, any comments?

Also, please avoid quoting a bug number in comments as a replacement
for explaining the code, unless the explanation would need a very long
and convoluted text.  We should strive to make our comments
self-explanatory.  (That some code was added/modified to fix a certain
bug is very easy to find out using "git log -L" and similar commands,
so a bug number is almost always redundant in comments.)

Thanks.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 11:09   ` Eli Zaretskii
@ 2021-05-14 13:38     ` Gregory Heytings
  2021-05-14 13:54       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-05-14 14:04       ` Eli Zaretskii
  0 siblings, 2 replies; 26+ messages in thread
From: Gregory Heytings @ 2021-05-14 13:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Stefan Monnier, harven, 48042

[-- Attachment #1: Type: text/plain, Size: 1849 bytes --]


>
> Bother: AFAIK, current-input-method non-nil means the user activated an 
> input method, it doesn't mean we are in the middle of typing a key 
> sequence that will yield a character via the input method.  That is, a 
> user could activate an input method, but still keep typing just ASCII 
> characters.  So why is this condition correct here to avoid recording 
> input more than once?
>

I'm not sure I understand your question.  With an input method is 
activated and characters that are not "reprocessed" by the input method 
are typed, they are not added to the unread-command-events list, so this 
part of the code (which is entered with the goto reread_for_input_method 
above) is simply not executed.  When they are "reprocessed", they are 
added to unread-command-events, but the keys typed in by the user have 
already been recorded.

Until commit 30a6b1f814, that part of the code (if (!recorded)...) did not 
exist, and my patch simply restores the behavior that quail.el expects by 
skipping it.  Note that in 2015 you yourself described that commit as a 
"naïve attempt at fixing" the problem.

I attach an even better patch, which removes the condition that a keyboard 
macro is being defined.  Now the keys displayed in C-h l are also correct.

>
> This is why I tried to have a variable that quail.el binds while 
> actually processing keys.  I'd appreciate some explanation for why that 
> didn't work 100% in the case in point (it still avoided recording twice 
> some of the keys, so it isn't entirely wrong).
>

I don't claim to understand everything, but what I see is that 
unread-command-events is also used (set) in quail-update-translation, 
quail-next-translation, quail-prev-translation, 
quail-next-translation-block, quail-prev-translation-block and 
quail-minibuffer-message.

[-- Attachment #2: Type: text/x-diff, Size: 4657 bytes --]

From aabb9d7a2fc8bd0f24429d0b632ef11950a65fba Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 14 May 2021 13:23:12 +0000
Subject: [PATCH] Fix key recording bug when an input method is activated

* src/keyboard.c (read_char): Do not record character again when an
input method is activated (Bug#48042).
(record_char): Partly revert 03e3440dbb.

* lisp/international/quail.el (quail-start-translation,
quail-start-conversion): Partly revert 03e3440dbb.
---
 lisp/international/quail.el | 20 ++++----------------
 src/keyboard.c              |  9 ++++++---
 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/lisp/international/quail.el b/lisp/international/quail.el
index fff06deee8..6c2088a95b 100644
--- a/lisp/international/quail.el
+++ b/lisp/international/quail.el
@@ -1385,13 +1385,12 @@ 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))
+	    (setq unread-command-events (cons key unread-command-events)))
 	(while quail-translating
 	  (set-buffer-modified-p modified-p)
 	  (quail-show-guidance)
@@ -1400,13 +1399,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))
@@ -1450,15 +1444,14 @@ 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))
+	    (setq unread-command-events (cons key unread-command-events)))
 	(while quail-converting
 	  (set-buffer-modified-p modified-p)
 	  (or quail-translating
@@ -1474,13 +1467,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)))
diff --git a/src/keyboard.c b/src/keyboard.c
index 47b5e59024..6849229bb2 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3097,8 +3097,12 @@ read_char (int commandflag, Lisp_Object map,
     }
   /* When we consume events from the various unread-*-events lists, we
      bypass the code that records input, so record these events now if
-     they were not recorded already.  */
-  if (!recorded)
+     they were not recorded already.
+     Don't record events when an input method is activated however,
+     quail.el puts keys back in unread-command-events to handle them
+     again, but the keys actually typed by the user have already been
+     recorded.  */
+  if (!recorded && NILP (Fsymbol_value (Qcurrent_input_method)))
     {
       record_char (c);
       recorded = true;
@@ -3233,7 +3237,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;
 
-- 
2.30.2


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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 13:38     ` Gregory Heytings
@ 2021-05-14 13:54       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-05-14 14:08         ` Gregory Heytings
  2021-05-14 14:04       ` Eli Zaretskii
  1 sibling, 1 reply; 26+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-05-14 13:54 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Eli Zaretskii, 48042, harven

> I'm not sure I understand your question.  With an input method is activated
> and characters that are not "reprocessed" by the input method are typed,
> they are not added to the unread-command-events list, so this part of the

Not by quail, indeed, but `unread-command-events` may be set for other
reasons and some of those should be recorded.


        Stefan






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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 13:38     ` Gregory Heytings
  2021-05-14 13:54       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-05-14 14:04       ` Eli Zaretskii
  2021-05-14 14:16         ` Gregory Heytings
  1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-14 14:04 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: monnier, harven, 48042

> Date: Fri, 14 May 2021 13:38:03 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Stefan Monnier <monnier@iro.umontreal.ca>, harven@free.fr, 
>     48042@debbugs.gnu.org
> 
> > Bother: AFAIK, current-input-method non-nil means the user activated an 
> > input method, it doesn't mean we are in the middle of typing a key 
> > sequence that will yield a character via the input method.  That is, a 
> > user could activate an input method, but still keep typing just ASCII 
> > characters.  So why is this condition correct here to avoid recording 
> > input more than once?
> 
> I'm not sure I understand your question.  With an input method is 
> activated and characters that are not "reprocessed" by the input method 
> are typed, they are not added to the unread-command-events list, so this 
> part of the code (which is entered with the goto reread_for_input_method 
> above) is simply not executed.

If you search the Lisp files for unread-command-events, you will see
how many features unrelated to input methods do add stuff to
unread-command-events.  What if an input method were active while one
of those unrelated features does that?

> Until commit 30a6b1f814, that part of the code (if (!recorded)...) did not 
> exist, and my patch simply restores the behavior that quail.el expects by 
> skipping it.

That part didn't exist because once upon a time we recorded everything
immediately as it happened.  When that changed (for a good reason, as
described in the discussion cited by the commit log message), we
needed to keep track of what was and what wasn't recorded.  So
restoring the behavior back to what it was then doesn't help me to
gain confidence in the correctness of the patch, sorry.  Going back to
what we had there is certainly not something we should do.

> Note that in 2015 you yourself described that commit as a "naïve
> attempt at fixing" the problem.

Every change in keyboard.c is "naïve" nowadays, since no one among us
really understands all of its intricacies.  Of course, if you do have
a clear understanding of what's going on there, it'd be splendid, but
I hope you will then be able to convince in the correctness of what
you propose.

> I attach an even better patch, which removes the condition that a keyboard 
> macro is being defined.  Now the keys displayed in C-h l are also correct.

Hmm... but the from_macro label seems to indicate that this code runs
for macros as well, even when input method is not active?

> > This is why I tried to have a variable that quail.el binds while 
> > actually processing keys.  I'd appreciate some explanation for why that 
> > didn't work 100% in the case in point (it still avoided recording twice 
> > some of the keys, so it isn't entirely wrong).
> 
> I don't claim to understand everything, but what I see is that 
> unread-command-events is also used (set) in quail-update-translation, 
> quail-next-translation, quail-prev-translation, 
> quail-next-translation-block, quail-prev-translation-block and 
> quail-minibuffer-message.

So you are saying we should bind the variable there as well?  If that
works, I'd be much happier, since in that case we will be sure we only
bypass recording exactly where that's needed.  However, ISTR that at
the time I tried adding the binding at least in some of those places,
and it didn't work well, or maybe didn't have any effect.  I think I
then convinced myself that only the first event needs this protection
(but I might be misremembering).  So it would be good to have a clear
understanding why this particular use case evades the current
protection against duplicate recording.

Thanks.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 13:54       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-05-14 14:08         ` Gregory Heytings
  2021-05-14 14:12           ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Gregory Heytings @ 2021-05-14 14:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 48042, harven


>> I'm not sure I understand your question.  With an input method is 
>> activated and characters that are not "reprocessed" by the input method 
>> are typed, they are not added to the unread-command-events list, so 
>> this part of the
>
> Not by quail, indeed, but `unread-command-events` may be set for other 
> reasons and some of those should be recorded.
>

They will be recorded, unless an input method is activated.  What could 
happen now is that when an input method is activated and 
unread-command-events is set, these events will not be recorded.  Let's 
perhaps wait for another bug report to see whether this happens and how it 
can be fixed?





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 14:08         ` Gregory Heytings
@ 2021-05-14 14:12           ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-14 14:12 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: monnier, harven, 48042

> Date: Fri, 14 May 2021 14:08:06 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, 48042@debbugs.gnu.org, harven@free.fr
> 
> > Not by quail, indeed, but `unread-command-events` may be set for other 
> > reasons and some of those should be recorded.
> 
> They will be recorded, unless an input method is activated.  What could 
> happen now is that when an input method is activated and 
> unread-command-events is set, these events will not be recorded.  Let's 
> perhaps wait for another bug report to see whether this happens and how it 
> can be fixed?

I don't think this is wise.  We may be breaking features much more
important than postfix input methods used in macros.

I suggest that we first try to fully understand why binding
inhibit--record-char doesn't work in this case, and take it from
there.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 14:04       ` Eli Zaretskii
@ 2021-05-14 14:16         ` Gregory Heytings
  2021-05-14 14:36           ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Gregory Heytings @ 2021-05-14 14:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, harven, 48042


>
> If you search the Lisp files for unread-command-events,
>

Of course I did this.

>
> you will see how many features unrelated to input methods do add stuff 
> to unread-command-events.  What if an input method were active while one 
> of those unrelated features does that?
>

As I just said to Stefan, we could perhaps wait for an actual bug report 
to see whether this happens?  I tried to fix this bug, it still seems to 
me that what I proposed is TRT because it fixes the problem at its "root", 
but I cannot fix yet unknown bugs.

>
> That part didn't exist because once upon a time we recorded everything 
> immediately as it happened.  When that changed (for a good reason, as 
> described in the discussion cited by the commit log message), we needed 
> to keep track of what was and what wasn't recorded.  So restoring the 
> behavior back to what it was then doesn't help me to gain confidence in 
> the correctness of the patch, sorry.  Going back to what we had there is 
> certainly not something we should do.
>

We're not going back to what we had earlier, the patch goes back to what 
we had only when an input method is activated.

>
> Hmm... but the from_macro label seems to indicate that this code runs 
> for macros as well, even when input method is not active?
>

And in that case, given that the condition checks whether an input method 
is active, the event will be recorded.

>
> So you are saying we should bind the variable there as well?
>

I don't know, and didn't test that.  As I said above, it seems to me that 
fixing a bug at its "root" is much better than trying to add code 
elsewhere to circumvent the bug.

>
> Thanks.
>

What do you expect me to do now?





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 14:16         ` Gregory Heytings
@ 2021-05-14 14:36           ` Eli Zaretskii
  2021-05-14 15:00             ` Gregory Heytings
  2021-05-14 15:51             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-14 14:36 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: monnier, harven, 48042

> Date: Fri, 14 May 2021 14:16:26 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: monnier@iro.umontreal.ca, harven@free.fr, 48042@debbugs.gnu.org
> 
> What do you expect me to do now?

If you are asking how I'd like to proceed with this bug, then, as I
wrote in another message, I'd like us to have a full understanding of
why binding inhibit--record-char didn't work well enough in the case
in point.  It did solve some of the problem we had before that change,
but didn't solve all of it -- why?

Once we understand that, I think we will have a much better basis for
discussing the possible solutions.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 14:36           ` Eli Zaretskii
@ 2021-05-14 15:00             ` Gregory Heytings
  2021-05-14 15:11               ` Eli Zaretskii
  2021-05-14 15:51             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 26+ messages in thread
From: Gregory Heytings @ 2021-05-14 15:00 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, harven, 48042


>> What do you expect me to do now?
>
> If you are asking how I'd like to proceed with this bug, then, as I 
> wrote in another message, I'd like us to have a full understanding of 
> why binding inhibit--record-char didn't work well enough in the case in 
> point.  It did solve some of the problem we had before that change, but 
> didn't solve all of it -- why?
>
> Once we understand that, I think we will have a much better basis for 
> discussing the possible solutions.
>

Okay, I'll have a look.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 15:00             ` Gregory Heytings
@ 2021-05-14 15:11               ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-14 15:11 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: monnier, harven, 48042

> Date: Fri, 14 May 2021 15:00:28 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: monnier@iro.umontreal.ca, harven@free.fr, 48042@debbugs.gnu.org
> 
> > If you are asking how I'd like to proceed with this bug, then, as I 
> > wrote in another message, I'd like us to have a full understanding of 
> > why binding inhibit--record-char didn't work well enough in the case in 
> > point.  It did solve some of the problem we had before that change, but 
> > didn't solve all of it -- why?
> >
> > Once we understand that, I think we will have a much better basis for 
> > discussing the possible solutions.
> 
> Okay, I'll have a look.

Thank you.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 14:36           ` Eli Zaretskii
  2021-05-14 15:00             ` Gregory Heytings
@ 2021-05-14 15:51             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-05-14 15:59               ` Eli Zaretskii
  2021-05-14 17:07               ` Gregory Heytings
  1 sibling, 2 replies; 26+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-05-14 15:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Gregory Heytings, 48042, harven

> If you are asking how I'd like to proceed with this bug, then, as I
> wrote in another message, I'd like us to have a full understanding of
> why binding inhibit--record-char didn't work well enough in the case
> in point.  It did solve some of the problem we had before that change,
> but didn't solve all of it -- why?

BTW, another possible way to attack the problem is to arrange for the
code that pushes to `unread-command-events` to use events of the form
`(t . EVENT)` so as to tell explicitly that we don't want them recorded.

I haven't re-read enough of the thread and corresponding code to know
which approach might be more applicable here; I'm just mentioning it in
case someone had forgotten about that option (been there, done that ;-)


        Stefan






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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 15:51             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-05-14 15:59               ` Eli Zaretskii
  2021-05-14 17:07               ` Gregory Heytings
  1 sibling, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-14 15:59 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: gregory, 48042, harven

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Gregory Heytings <gregory@heytings.org>,  harven@free.fr,
>   48042@debbugs.gnu.org
> Date: Fri, 14 May 2021 11:51:41 -0400
> 
> BTW, another possible way to attack the problem is to arrange for the
> code that pushes to `unread-command-events` to use events of the form
> `(t . EVENT)` so as to tell explicitly that we don't want them recorded.

Maybe we will do something like that, or maybe we'll decide something
else.  But before we do that, I'd like to have a good understanding
how the current code malfunctions and why.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 15:51             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-05-14 15:59               ` Eli Zaretskii
@ 2021-05-14 17:07               ` Gregory Heytings
  2021-05-14 17:13                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 26+ messages in thread
From: Gregory Heytings @ 2021-05-14 17:07 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 48042, harven


>> If you are asking how I'd like to proceed with this bug, then, as I 
>> wrote in another message, I'd like us to have a full understanding of 
>> why binding inhibit--record-char didn't work well enough in the case in 
>> point.  It did solve some of the problem we had before that change, but 
>> didn't solve all of it -- why?
>
> BTW, another possible way to attack the problem is to arrange for the 
> code that pushes to `unread-command-events` to use events of the form 
> `(t . EVENT)` so as to tell explicitly that we don't want them recorded.
>

I guess you mean '(no-record . EVENT)'?  The docstring indicates that '(t 
. EVENT)' means that EVENT is added to 'this-command-keys'.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 17:07               ` Gregory Heytings
@ 2021-05-14 17:13                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-05-15  9:46                   ` Gregory Heytings
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-05-14 17:13 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Eli Zaretskii, 48042, harven

>>> If you are asking how I'd like to proceed with this bug, then, as I wrote
>>> in another message, I'd like us to have a full understanding of why
>>> binding inhibit--record-char didn't work well enough in the case in
>>> point.  It did solve some of the problem we had before that change, but
>>> didn't solve all of it -- why?
>>
>> BTW, another possible way to attack the problem is to arrange for the code
>> that pushes to `unread-command-events` to use events of the form `(t
>> . EVENT)` so as to tell explicitly that we don't want them recorded.
>>
>
> I guess you mean '(no-record . EVENT)'?  The docstring indicates that '(t
> . EVENT)' means that EVENT is added to 'this-command-keys'.

Oops, indeed,


        Stefan






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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-14 17:13                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-05-15  9:46                   ` Gregory Heytings
  2021-05-15 10:21                     ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Gregory Heytings @ 2021-05-15  9:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 48042, harven

[-- Attachment #1: Type: text/plain, Size: 127 bytes --]


Here is an updated patch, which uses the '(no-record . EVENT)' approach, 
and whose changes are therefore limited to quail.el.

[-- Attachment #2: Type: text/x-diff, Size: 8606 bytes --]

From 55679336ddf8242b05e7397c472c0eb0dac26637 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sat, 15 May 2021 08:52:57 +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).

* src/keyboard.c (read_char): Remove the reference to quail.el.
---
 lisp/international/quail.el | 67 +++++++++++++++++--------------------
 src/keyboard.c              |  1 -
 2 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/lisp/international/quail.el b/lisp/international/quail.el
index fff06deee8..30b0353fa5 100644
--- a/lisp/international/quail.el
+++ b/lisp/international/quail.el
@@ -1368,6 +1368,22 @@ 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'.
+When KEY is a character, it is prepended to `unread-command-events' as a
+cons with a no-record car.
+When KEY is a vector, its elements are prepended to `unread-command-events'
+as conses with a no-record car.
+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 +1401,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 +1414,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 +1429,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 +1457,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 +1479,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 +1503,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 +1549,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 +1796,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 +1810,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 +1825,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 +1844,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 +1999,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/src/keyboard.c b/src/keyboard.c
index 47b5e59024..135d6a6f50 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3233,7 +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;
 
-- 
2.30.2


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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-15  9:46                   ` Gregory Heytings
@ 2021-05-15 10:21                     ` Eli Zaretskii
  2021-05-15 18:47                       ` Gregory Heytings
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-15 10:21 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: monnier, harven, 48042

> Date: Sat, 15 May 2021 09:46:02 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, harven@free.fr, 48042@debbugs.gnu.org
> 
> Here is an updated patch, which uses the '(no-record . EVENT)' approach, 
> and whose changes are therefore limited to quail.el.

Thanks.

To tell you the truth, I'm a bit worried that you inhibit recording
everywhere in quail.el, which seems to contradict my analysis from
back when I made the inhibit--record-char.  How many different input
methods did you try with macros and "C-h l" to make sure this change
indeed causes Emacs to record each produced character just once, and
nothing is either omitted or recorded more than once?  Did you try
some CJK input methods, for example, which offer several alternatives
for each key sequence the user typed?

Other than this main issue, this LGTM, modulo some minor comments
below.

> +(defun quail-add-unread-command-events (key &optional reset)
> +  "Add KEY to `unread-command-events'.

This summary line should mention that the function arranges for the
events not to be recorded, and perhaps also explain in the rest of the
doc string (or in a comment) why is that needed here.

> +When KEY is a character, it is prepended to `unread-command-events' as a
> +cons with a no-record car.
> +When KEY is a vector, its elements are prepended to `unread-command-events'
> +as conses with a no-record car.

The last sentence above is not clear enough, IMO; I originally
interpreted it incorrectly.  I would suggest to reword:

  If KEY is a vector of events, the events are prepended
  to `unread-command-events', after converting each event
  to a cons cell of the form (no-record . EVENT).

> +When RESET is non-nil, the events in `unread-command-events' are first
> +discarded."

I'm not sure we need this as part of the function: it makes the
function more complicated for no good reason.  Why not reset
unread-command-events "by hand" in the one place where that is needed?
Or maybe even explicitly use (no-record . 7) in that one place, and
then you can avoid calling this function, since that one place does
something very different from the rest.

Please also make a single changeset out of this one and the one where
you remove inhibit--record-char; see there for more comments.  I see
no need to separate them into two commits.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-15 10:21                     ` Eli Zaretskii
@ 2021-05-15 18:47                       ` Gregory Heytings
  2021-05-15 18:52                         ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Gregory Heytings @ 2021-05-15 18:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, harven, 48042

[-- Attachment #1: Type: text/plain, Size: 3109 bytes --]


I attach the updated patch.

Some comments below:

>
> To tell you the truth, I'm a bit worried that you inhibit recording 
> everywhere in quail.el, which seems to contradict my analysis from back 
> when I made the inhibit--record-char.  How many different input methods 
> did you try with macros and "C-h l" to make sure this change indeed 
> causes Emacs to record each produced character just once, and nothing is 
> either omitted or recorded more than once?  Did you try some CJK input 
> methods, for example, which offer several alternatives for each key 
> sequence the user typed?
>

I tried french-postfix, russian-computer, ucs, TeX, arabic, japanese and 
chinese-py.  From what I can see, everything works as expected, but I 
don't speak or read these languages, and there are more than 200 input 
methods, I cannot test them all.

>> +When RESET is non-nil, the events in `unread-command-events' are first
>> +discarded."
>
> I'm not sure we need this as part of the function: it makes the function 
> more complicated for no good reason.  Why not reset 
> unread-command-events "by hand" in the one place where that is needed? 
> Or maybe even explicitly use (no-record . 7) in that one place, and then 
> you can avoid calling this function, since that one place does something 
> very different from the rest.
>

I did this to simplify future debugging sessions.  Having Quail interact 
with unread-command-events in a single place will make that easier.  And 
the added complexity is really small.

>> @@ -883,6 +882,9 @@ terminal-init-xterm
>>    (xterm--init-bracketed-paste-mode)
>>    ;; We likewise unconditionally enable support for focus tracking.
>>    (xterm--init-focus-tracking)
>> +  ;; Clear the lossage to discard the responses of the terminal emulator
>> +  ;; during initialization (Bug#44908)
>> +  (clear-this-command-keys)
>
> Btw, what happens if while this code runs, the user types something? We 
> don't want those events to be cleared.
>

It's quite hard to type something at that moment AFAICS, the whole 
terminal initialization typically takes at most 50 ms.  I tried to type 
something as fast as possible during the terminal initialization, and what 
happens is this (visible in *Messages*):

q is undefined
M-] is undefined
; is undefined
r is undefined
g is undefined
b is undefined
: is undefined
f is undefined [4 times]
/ is undefined
f is undefined [4 times]
/ is undefined
f is undefined [4 times]

or this:

q is undefined
M-[ > is undefined
; is undefined [2 times]
c is undefined

The first key is the pressed key ("q"), the other keys are some of the 
keys of the terminal initialization process (the digits are bound to 
digit-argument in the *GNU Emacs* buffer and are therefore not undefined):

ESC [ > ESC [ > 4 1 ; 3 6 6 ; 0 c ESC ] 1 1 ; r g b : f f f f / f f f f / 
f f f f ESC \

IOW, if the user presses something at that moment, the terminal 
initialization process does not work as expected.

(FWIW, I would have closed bug#44908 as a wontfix, seeing a few unwanted 
keys in the lossage after starting Emacs is a really minor annoyance.)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Fix-key-recording-bug-when-an-input-method-is-activa.patch, Size: 12798 bytes --]

From 6ad3f13bfcf2f2a41a26d3ae561836462133f235 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sat, 15 May 2021 18:32:31 +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          | 17 ++++++---
 src/keyboard.c              | 13 -------
 4 files changed, 53 insertions(+), 55 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 would create 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..77e95894e4 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,7 +881,15 @@ 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))
+
+(defun terminal-init-xterm ()
+  "Terminal initialization function for xterm."
+  (unwind-protect
+      (xterm--init)
+    ;; 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))
 
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


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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-15 18:47                       ` Gregory Heytings
@ 2021-05-15 18:52                         ` Eli Zaretskii
  2021-05-15 20:17                           ` Gregory Heytings
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-15 18:52 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: monnier, harven, 48042

> Date: Sat, 15 May 2021 18:47:00 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: monnier@iro.umontreal.ca, harven@free.fr, 48042@debbugs.gnu.org
> 
> > To tell you the truth, I'm a bit worried that you inhibit recording 
> > everywhere in quail.el, which seems to contradict my analysis from back 
> > when I made the inhibit--record-char.  How many different input methods 
> > did you try with macros and "C-h l" to make sure this change indeed 
> > causes Emacs to record each produced character just once, and nothing is 
> > either omitted or recorded more than once?  Did you try some CJK input 
> > methods, for example, which offer several alternatives for each key 
> > sequence the user typed?
> 
> I tried french-postfix, russian-computer, ucs, TeX, arabic, japanese and 
> chinese-py.  From what I can see, everything works as expected, but I 
> don't speak or read these languages, and there are more than 200 input 
> methods, I cannot test them all.

No one can test all of them, that's not what I meant.

> > Btw, what happens if while this code runs, the user types something? We 
> > don't want those events to be cleared.
> >
> 
> It's quite hard to type something at that moment AFAICS, the whole 
> terminal initialization typically takes at most 50 ms.  I tried to type 
> something as fast as possible during the terminal initialization, and what 
> happens is this (visible in *Messages*):
> 
> q is undefined
> M-] is undefined
> ; is undefined
> r is undefined
> g is undefined
> b is undefined
> : is undefined
> f is undefined [4 times]
> / is undefined
> f is undefined [4 times]
> / is undefined
> f is undefined [4 times]
> 
> or this:
> 
> q is undefined
> M-[ > is undefined
> ; is undefined [2 times]
> c is undefined
> 
> The first key is the pressed key ("q"), the other keys are some of the 
> keys of the terminal initialization process (the digits are bound to 
> digit-argument in the *GNU Emacs* buffer and are therefore not undefined):
> 
> ESC [ > ESC [ > 4 1 ; 3 6 6 ; 0 c ESC ] 1 1 ; r g b : f f f f / f f f f / 
> f f f f ESC \
> 
> 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.

Thanks.





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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-15 18:52                         ` Eli Zaretskii
@ 2021-05-15 20:17                           ` Gregory Heytings
  2021-05-29  8:20                             ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Gregory Heytings @ 2021-05-15 20:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, harven, 48042

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]


>> 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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Fix-key-recording-bug-when-an-input-method-is-activa.patch, Size: 13046 bytes --]

From 601ec3c2d6472426f3763f30ee2d7710924dfa1f Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
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


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

* bug#48042: 26.3; Macros don't work with french-postfix input method
  2021-05-15 20:17                           ` Gregory Heytings
@ 2021-05-29  8:20                             ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2021-05-29  8:20 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: monnier, harven, 48042-done

> Date: Sat, 15 May 2021 20:17:31 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: monnier@iro.umontreal.ca, harven@free.fr, 48042@debbugs.gnu.org
> 
> Okay, updated patch attached.

Thanks, installed.

Please note that I needed to make a couple of minor fixes in the
commit log message, both in form and in contents.  Please in the
future try following our conventions there.





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

end of thread, other threads:[~2021-05-29  8:20 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26 18:05 bug#48042: 26.3; Macros don't work with french-postfix input method harven
2021-04-26 18:22 ` Eli Zaretskii
2021-04-28 18:24 ` harven
2021-05-14  9:29 ` Gregory Heytings
2021-05-14  9:55   ` Basil L. Contovounesios
2021-05-14 10:03     ` Gregory Heytings
2021-05-14 11:09   ` Eli Zaretskii
2021-05-14 13:38     ` Gregory Heytings
2021-05-14 13:54       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-14 14:08         ` Gregory Heytings
2021-05-14 14:12           ` Eli Zaretskii
2021-05-14 14:04       ` Eli Zaretskii
2021-05-14 14:16         ` Gregory Heytings
2021-05-14 14:36           ` Eli Zaretskii
2021-05-14 15:00             ` Gregory Heytings
2021-05-14 15:11               ` Eli Zaretskii
2021-05-14 15:51             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-14 15:59               ` Eli Zaretskii
2021-05-14 17:07               ` Gregory Heytings
2021-05-14 17:13                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-15  9:46                   ` Gregory Heytings
2021-05-15 10:21                     ` Eli Zaretskii
2021-05-15 18:47                       ` Gregory Heytings
2021-05-15 18:52                         ` Eli Zaretskii
2021-05-15 20:17                           ` Gregory Heytings
2021-05-29  8:20                             ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).