GNU bug report logs - #45029
27.1.50; Regression: Yanking into externally modified file with delete-selection-mode

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45029 in the body.
You can then email your comments to 45029 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lars Ljung <lars <at> matholka.se>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.1.50; Regression: Yanking into externally modified file with
 delete-selection-mode
Date: Thu, 3 Dec 2020 21:10:05 +0100

Hi,

I get an error when I do this, and the content of the buffer is
completely replaced by a single "y".

1. Start with "emacs -Q"
2. M-x delete-selection-mode
3. Open some text file (fundamental mode)
4. Modify the file externally
5. While a selection is active, yank some text
6. Answer yes to the question "<file> changed on disk..."

The content of the buffer is now completely replaced by a single "y".
The message "No catch for tag: exit, nil" is shown in the minibuffer.

I can't reproduce this on Emacs 26.3.

Kind regards,
Lars Ljung

Message #8 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Lars Ljung <lars <at> matholka.se>
Cc: 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Fri, 04 Dec 2020 11:18:29 +0100

Lars Ljung <lars <at> matholka.se> writes:

> I get an error when I do this, and the content of the buffer is
> completely replaced by a single "y".
>
> 1. Start with "emacs -Q"
> 2. M-x delete-selection-mode
> 3. Open some text file (fundamental mode)
> 4. Modify the file externally
> 5. While a selection is active, yank some text
> 6. Answer yes to the question "<file> changed on disk..."
>
> The content of the buffer is now completely replaced by a single "y".
> The message "No catch for tag: exit, nil" is shown in the minibuffer.

I can confirm that this bug is still present in Emacs 28.

The backtrace is included below, if that helps anybody.

Debugger entered--Lisp error: (no-catch exit nil)
  throw(exit nil)
  exit-minibuffer()
  read-char-from-minibuffer-insert-char()
  funcall-interactively(read-char-from-minibuffer-insert-char)
  call-interactively(read-char-from-minibuffer-insert-char nil nil)
  command-execute(read-char-from-minibuffer-insert-char)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #13 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Juri Linkov <juri <at> linkov.net>
Cc: lars <at> matholka.se, 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50;
 Regression: Yanking into externally modified file with
 delete-selection-mode
Date: Sat, 05 Dec 2020 13:31:45 +0200

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Fri, 04 Dec 2020 11:18:29 +0100
> Cc: 45029 <at> debbugs.gnu.org
> 
> Lars Ljung <lars <at> matholka.se> writes:
> 
> > I get an error when I do this, and the content of the buffer is
> > completely replaced by a single "y".
> >
> > 1. Start with "emacs -Q"
> > 2. M-x delete-selection-mode
> > 3. Open some text file (fundamental mode)
> > 4. Modify the file externally
> > 5. While a selection is active, yank some text
> > 6. Answer yes to the question "<file> changed on disk..."
> >
> > The content of the buffer is now completely replaced by a single "y".
> > The message "No catch for tag: exit, nil" is shown in the minibuffer.
> 
> I can confirm that this bug is still present in Emacs 28.
> 
> The backtrace is included below, if that helps anybody.
> 
> Debugger entered--Lisp error: (no-catch exit nil)
>   throw(exit nil)
>   exit-minibuffer()
>   read-char-from-minibuffer-insert-char()
>   funcall-interactively(read-char-from-minibuffer-insert-char)
>   call-interactively(read-char-from-minibuffer-insert-char nil nil)
>   command-execute(read-char-from-minibuffer-insert-char)

I'm guessing this is some unintended consequence of replacing
read-char-choice in userlock.el with read-char-from-minibuffer.

Juri, could you please look into this regression in Emacs 27?  I'd
like to try to fix this for 27.2.

Thanks.

Message #16 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, lars <at> matholka.se, 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Sat, 05 Dec 2020 21:42:10 +0200

[Message part 1 (text/plain, inline)]

> I'm guessing this is some unintended consequence of replacing
> read-char-choice in userlock.el with read-char-from-minibuffer.
>
> Juri, could you please look into this regression in Emacs 27?  I'd
> like to try to fix this for 27.2.

Here is a brief trace that explains the cause of the problem:

1. C-y (yank) calls delete-selection-pre-hook from pre-command-hook
2. delete-selection-helper calls delete-active-region
3. this activates ask-user-about-supersession-threat on a modified file
4. it calls read-char-from-minibuffer ('this-command' is still 'yank')
5. read-char-from-minibuffer waits when user types 'y'
   (bound to the command 'read-char-from-minibuffer-insert-char')
6. after typing 'y', read-char-from-minibuffer returns 'y', but
   now 'this-command' is 'read-char-from-minibuffer-insert-char'
7. when delete-selection-pre-hook finishes, due to 'this-command'
   'read-char-from-minibuffer-insert-char' is called again,
   and this time operates on the buffer as if it's the minibuffer

It seems there is a fundamental problem when read-from-minibuffer
is used from pre-command-hook it modifies the value of this-command.
But I wonder why this problem doesn't occur in other places.
Maybe other places doesn't try to operate on the buffer as on
the minibuffer?

Anyway, here is a patch with two fixes:
1. Guards read-char-from-minibuffer-insert-char against inadvertent
   operating on the non-minibuffer buffer;
2. Prevents read-char-from-minibuffer from changing the value of
   'this-command' by read-from-minibuffer:

[protect-this-command.patch (text/x-diff, inline)]

diff --git a/lisp/subr.el b/lisp/subr.el
index 4b75268c04..7f1450d4a2 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2745,20 +2746,22 @@ read-char-from-minibuffer-insert-char
   "Insert the character you type in the minibuffer and exit.
 Discard all previous input before inserting and exiting the minibuffer."
   (interactive)
-  (delete-minibuffer-contents)
-  (insert last-command-event)
-  (exit-minibuffer))
+  (when (minibufferp)
+    (delete-minibuffer-contents)
+    (insert last-command-event)
+    (exit-minibuffer)))
 
 (defun read-char-from-minibuffer-insert-other ()
   "Handle inserting of a character other than allowed.
 Display an error on trying to insert a disallowed character.
 Also discard all previous input in the minibuffer."
   (interactive)
-  (delete-minibuffer-contents)
-  (ding)
-  (discard-input)
-  (minibuffer-message "Wrong answer")
-  (sit-for 2))
+  (when (minibufferp)
+    (delete-minibuffer-contents)
+    (ding)
+    (discard-input)
+    (minibuffer-message "Wrong answer")
+    (sit-for 2)))
 
 (defvar empty-history)
 
@@ -2802,6 +2805,8 @@ read-char-from-minibuffer
                                  map read-char-from-minibuffer-map-hash)
                         map))
                 read-char-from-minibuffer-map))
+         ;; Protect this-command when called from pre-command-hook (bug#45029)
+         (this-command this-command)
          (result
           (read-from-minibuffer prompt nil map nil
                                 (or history 'empty-history)))
@@ -2856,28 +2861,31 @@ y-or-n-p-insert-y
   "Insert the answer \"y\" and exit the minibuffer of `y-or-n-p'.
 Discard all previous input before inserting and exiting the minibuffer."
   (interactive)
-  (delete-minibuffer-contents)
-  (insert "y")
-  (exit-minibuffer))
+  (when (minibufferp)
+    (delete-minibuffer-contents)
+    (insert "y")
+    (exit-minibuffer)))
 
 (defun y-or-n-p-insert-n ()
   "Insert the answer \"n\" and exit the minibuffer of `y-or-n-p'.
 Discard all previous input before inserting and exiting the minibuffer."
   (interactive)
-  (delete-minibuffer-contents)
-  (insert "n")
-  (exit-minibuffer))
+  (when (minibufferp)
+    (delete-minibuffer-contents)
+    (insert "n")
+    (exit-minibuffer)))
 
 (defun y-or-n-p-insert-other ()
   "Handle inserting of other answers in the minibuffer of `y-or-n-p'.
 Display an error on trying to insert a disallowed character.
 Also discard all previous input in the minibuffer."
   (interactive)
-  (delete-minibuffer-contents)
-  (ding)
-  (discard-input)
-  (minibuffer-message "Please answer y or n")
-  (sit-for 2))
+  (when (minibufferp)
+    (delete-minibuffer-contents)
+    (ding)
+    (discard-input)
+    (minibuffer-message "Please answer y or n")
+    (sit-for 2)))
 
 (defvar empty-history)
 
@@ -2955,6 +2963,8 @@ y-or-n-p
                              (let ((help-form msg)) ; lexically bound msg
                                (help-form-show)))))
                        map))
+             ;; Protect this-command when called from pre-command-hook (bug#45029)
+             (this-command this-command)
              (str (read-from-minibuffer
                    prompt nil keymap nil
                    (or y-or-n-p-history-variable 'empty-history))))

Message #19 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: larsi <at> gnus.org, lars <at> matholka.se, 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Sat, 05 Dec 2020 22:04:23 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,  lars <at> matholka.se,
>   45029 <at> debbugs.gnu.org
> Date: Sat, 05 Dec 2020 21:42:10 +0200
> 
> Anyway, here is a patch with two fixes:
> 1. Guards read-char-from-minibuffer-insert-char against inadvertent
>    operating on the non-minibuffer buffer;
> 2. Prevents read-char-from-minibuffer from changing the value of
>    'this-command' by read-from-minibuffer:

Thanks!

Message #22 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, lars <at> matholka.se, 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Sun, 06 Dec 2020 23:12:36 +0200

tags 45029 fixed
close 45029 27.2
quit

>> I'd like to try to fix this for 27.2.

So I pushed the patch to master, and backported it to the emacs-27 branch
(separate commits were needed to avoid merge conflicts).

Message #29 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: larsi <at> gnus.org, lars <at> matholka.se, 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Mon, 07 Dec 2020 05:27:28 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,  lars <at> matholka.se,
>   45029 <at> debbugs.gnu.org
> Date: Sun, 06 Dec 2020 23:12:36 +0200
> 
> >> I'd like to try to fix this for 27.2.
> 
> So I pushed the patch to master, and backported it to the emacs-27 branch
> (separate commits were needed to avoid merge conflicts).

Thanks.

Message #36 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Wed, 13 Sep 2023 13:59:55 -0400

Stefan Monnier [2023-09-13 13:36:54] wrote:

> Here is a brief trace that explains the cause of the problem:
>
> 1. C-y (yank) calls delete-selection-pre-hook from pre-command-hook
> 2. delete-selection-helper calls delete-active-region
> 3. this activates ask-user-about-supersession-threat on a modified file
> 4. it calls read-char-from-minibuffer ('this-command' is still 'yank')
> 5. read-char-from-minibuffer waits when user types 'y'
>    (bound to the command 'read-char-from-minibuffer-insert-char')
> 6. after typing 'y', read-char-from-minibuffer returns 'y', but
>    now 'this-command' is 'read-char-from-minibuffer-insert-char'
> 7. when delete-selection-pre-hook finishes, due to 'this-command'
>    'read-char-from-minibuffer-insert-char' is called again,
>    and this time operates on the buffer as if it's the minibuffer

Yuck!
And thanks for the investigation.

> Anyway, here is a patch with two fixes:
> 1. Guards read-char-from-minibuffer-insert-char against inadvertent
>    operating on the non-minibuffer buffer;

I suspect this will/would just hide a problem (such as the current one)
under the rug.

> 2. Prevents read-char-from-minibuffer from changing the value of
>    'this-command' by read-from-minibuffer:

Right, but I think that this needs to apply to all recursive edits
rather than only `read-char-from-minibuffer`.


         Stefan

Message #39 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Thu, 14 Sep 2023 09:39:48 +0300

>> Here is a brief trace that explains the cause of the problem:
>>
>> 1. C-y (yank) calls delete-selection-pre-hook from pre-command-hook
>> 2. delete-selection-helper calls delete-active-region
>> 3. this activates ask-user-about-supersession-threat on a modified file
>> 4. it calls read-char-from-minibuffer ('this-command' is still 'yank')
>> 5. read-char-from-minibuffer waits when user types 'y'
>>    (bound to the command 'read-char-from-minibuffer-insert-char')
>> 6. after typing 'y', read-char-from-minibuffer returns 'y', but
>>    now 'this-command' is 'read-char-from-minibuffer-insert-char'
>> 7. when delete-selection-pre-hook finishes, due to 'this-command'
>>    'read-char-from-minibuffer-insert-char' is called again,
>>    and this time operates on the buffer as if it's the minibuffer
>
> Yuck!
> And thanks for the investigation.
>
>> Anyway, here is a patch with two fixes:
>> 1. Guards read-char-from-minibuffer-insert-char against inadvertent
>>    operating on the non-minibuffer buffer;
>
> I suspect this will/would just hide a problem (such as the current one)
> under the rug.

I admit this is a workaround, but I have no idea what would be a proper fix.

>> 2. Prevents read-char-from-minibuffer from changing the value of
>>    'this-command' by read-from-minibuffer:
>
> Right, but I think that this needs to apply to all recursive edits
> rather than only `read-char-from-minibuffer`.

I tried to test recursive commands at the read-char-from-minibuffer's
prompt, but can't reproduce the same problem.

Message #42 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Thu, 14 Sep 2023 09:44:00 -0400

>>> Anyway, here is a patch with two fixes:
>>> 1. Guards read-char-from-minibuffer-insert-char against inadvertent
>>>    operating on the non-minibuffer buffer;
>> I suspect this will/would just hide a problem (such as the current one)
>> under the rug.
> I admit this is a workaround, but I have no idea what would be a proper fix.

The let-binding of `this-command` fixes the problem, doesn't it?

>>> 2. Prevents read-char-from-minibuffer from changing the value of
>>>    'this-command' by read-from-minibuffer:
>> Right, but I think that this needs to apply to all recursive edits
>> rather than only `read-char-from-minibuffer`.
> I tried to test recursive commands at the read-char-from-minibuffer's
> prompt, but can't reproduce the same problem.

No, I mean other uses of a recursive edit from `pre-command-hook` would
cause the same problem, so the let-binding should be placed deeper than
in `read-char-from-minibuffer`, e.g. inside `recursive_edit_1`.


        Stefan

Message #45 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Fri, 15 Sep 2023 09:36:12 +0300

>>>> Anyway, here is a patch with two fixes:
>>>> 1. Guards read-char-from-minibuffer-insert-char against inadvertent
>>>>    operating on the non-minibuffer buffer;
>>> I suspect this will/would just hide a problem (such as the current one)
>>> under the rug.
>> I admit this is a workaround, but I have no idea what would be a proper fix.
>
> The let-binding of `this-command` fixes the problem, doesn't it?

But might other uses of a recursive edit break it again?
I guess `when (minibufferp)` can't be removed
from `read-char-from-minibuffer-insert-other`
until all cases below are fixed.

>>>> 2. Prevents read-char-from-minibuffer from changing the value of
>>>>    'this-command' by read-from-minibuffer:
>>> Right, but I think that this needs to apply to all recursive edits
>>> rather than only `read-char-from-minibuffer`.
>> I tried to test recursive commands at the read-char-from-minibuffer's
>> prompt, but can't reproduce the same problem.
>
> No, I mean other uses of a recursive edit from `pre-command-hook` would
> cause the same problem, so the let-binding should be placed deeper than
> in `read-char-from-minibuffer`, e.g. inside `recursive_edit_1`.

I see, so this problem is more general and not specific to
`read-char-from-minibuffer`?  It would be nice to find
a test case for other commands to confirm that a change in
`recursive_edit_1` fixes them.

Message #48 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
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

Message #51 received at 45029 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 45029 <at> debbugs.gnu.org
Subject: Re: bug#45029: 27.1.50; Regression: Yanking into externally
 modified file with delete-selection-mode
Date: Fri, 15 Sep 2023 19:06:20 +0300

>>> 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`?

Thanks for explanations.  I thought it is specific to
`delete-selection-mode` and `read-char-from-minibuffer`.
But since a combination of `pre-command-hook` and a recursive command
is a general problem, and OTOH `read-char-from-minibuffer` is already
safeguarded by the let-binding of `this-command`, so probably
`when (minibufferp)` could be removed from
`read-char-from-minibuffer-insert-other`.

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

Frankly speaking, I'm afraid of making such change at great depths ;-)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.