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

From: Eli Zaretskii <[email-masked]>
Date: Fri, 14 May 2021 17:04:59 +0300

> Date: Fri, 14 May 2021 13:38:03 +0000
> From: Gregory Heytings <[email-masked]>
> cc: Stefan Monnier <[email-masked]>, [email-masked], 
>     [email-masked]
> 
> > 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.