Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally modified file with delete-selection-mode

From: Stefan Monnier <[email-masked]>
Date: Fri, 15 Sep 2023 09:17:14 -0400

>> The let-binding of `this-command` fixes the problem, doesn't it?
> But might other uses of a recursive edit break it again?

What do you mean by `it`?

There's only one recursive edit associated with
`read-char-from-minibuffer-insert-char`, so no definitely not that
(unless the user binds (or somehow uses) that command in keymaps used in
other recursive edits, of course).

And AFAICT, the bug see is that `delete-selection-mode` ends up running
a command (originally run inside a recursive edit) a second time.
IOW it's not "recursive edit broke `read-char-from-minibuffer-insert-char`"
but "recursive edit broke `delete-selection-mode`".
This can/will cause problems with loads of commands, not just
`read-char-from-minibuffer-insert-char`.

So yes, other uses of recursive edits can break `delete-selection-mode`
as well, but no amount of changing
`read-char-from-minibuffer-insert-char` will hide the problem because
those other uses will use other commands.

OTOH, the let-binding of `this-command` should fix the problem once and
for all uses of recursive edit *if* it's placed deep enough that it
affects all recursive edits.

> I guess `when (minibufferp)` can't be removed from
> `read-char-from-minibuffer-insert-other` until all cases below
> are fixed.

I don't see the "cases below" you're referring to.

> I see, so this problem is more general and not specific to
> `read-char-from-minibuffer`?

Exactly.  Any recursive edit happening from the `pre-command-hook`.

> It would be nice to find a test case for other commands to confirm
> that a change in `recursive_edit_1` fixes them.

Even without trying to reproduce the specific problem that caused this
bug report, think of this scenario:

   - User causes COMMAND to be run.
   - `this-command` is thus set to COMMAND.
   - We run the `pre-command-hook`.
   - One of the functions on `pre-command-hook` use a recursive edit.
   - `this-command` is now set to whichever command was run last in the
     recursive edit, which is just plain wrong.

I remember seeing places in the code where we try and circumvent this
problem, but this bug report makes it clear what is the real source of
the problem.

        Stefan