GNU bug report logs - #23632
25.1.50; Gratuitous undo boundary in latex-insert-block

Previous Next

Full log

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

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: Chong Yidong <cyd <at> gnu.org>
Cc: 23632 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#23632: 25.1.50; Gratuitous undo boundary in latex-insert-block
Date: Tue, 31 May 2016 22:42:36 +0100

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

phillip.lord <at> russet.org.uk (Phillip Lord) writes:

> Chong Yidong <cyd <at> gnu.org> writes:
>
>>> The attached patch, which gets rid of the undo boundary, seems to fix
>>> this:
>>
>> Actually, the previous patch does not DTRT: if you switch back to the
>> original buffer from the minibuffer, and make further editing changes,
>> those changes would get lost because buffer-undo-list is temporarily
>> rebound.
>>
>> Here is a different patch, which works by removing the undo boundary in
>> buffer-undo-list if there's one.  It also tweaks HTML mode and Texinfo
>> mode, which have similar issues.  It defines a new function
>> `undo-amalgamate', split off from `undo-auto-amalgamate', for
>> convenience.
>
>
> In and off itself, the patch seems fine, but my concern is that that the
> previous heuristic did the right thing, the new heuristic does not. If
> you've found three instances where it's causing a problem, then there
> will be others also.
>
> I'm not 100% sure why the old system didn't insert an undo-boundary.
> But, if we could solve this entirely in the undo system without changes
> to client code that would be nicer.
>
> Not sure how yet -- need a few days to think about it. Perhaps,
> suppressing the auto-boundary functionality when only the mini-buffer
> has changed.

I've debugged this now.

The problem, I think, is that latex-insert-block uses recursive editing
(via `completing-read', then `read-from-minibuffer') -- so the
minibuffer is edited, then the exit-minibuffer command runs, causing an
undo boundary to be added to minibuffer (correctly) but also to the
latex buffer because it has also changed.

The patch below seems to fix -- I need to test it out tomorrow in case
it has any other unexpected effects.

What worries me is that it just deals with the minibuffer. I wonder
whether there are other circumstances where a recursive edit is going to
break things.

Stefan, would welcome your opinion here.

Incidentally, this is a nightmare to debug. Emacs needs to be able to
write to standard out, so I could log without changing any buffers!

[0001-Stop-mini-buffer-causing-undo-boundaries.patch (text/x-diff, inline)]

From 9116e7f00f90fb14857d21698b1e6870fcf98bbd Mon Sep 17 00:00:00 2001
From: Phillip Lord <phillip.lord <at> russet.org.uk>
Date: Mon, 30 May 2016 22:50:36 +0100
Subject: [PATCH] Stop mini-buffer causing undo boundaries

* lisp/simple.el (undo-auto--boundaries): Check whether minibuffer is
  current, and if so limit undo-boundaries to it.

Addresses #23632
---
 lisp/simple.el | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index c5aa292..788cbb2 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2887,12 +2887,21 @@ undo-auto--boundaries
   "Check recently changed buffers and add a boundary if necessary.
 REASON describes the reason that the boundary is being added; see
 `undo-last-boundary' for more information."
-  (dolist (b undo-auto--undoably-changed-buffers)
-          (when (buffer-live-p b)
-            (with-current-buffer b
-              (unless undo-auto-disable-boundaries
-                (undo-auto--ensure-boundary cause)))))
-  (setq undo-auto--undoably-changed-buffers nil))
+  ;; We treat the minibuffer specially, because some commands use the
+  ;; minibuffer after changing the buffer that they are launched
+  ;; from. Changes in the minibuffer force an undo-boundary in the
+  ;; launched buffer without this handling. (see bug #23632)
+  (if (minibufferp)
+      (progn
+        (undo-auto--ensure-boundary cause)
+        (setq undo-auto--undoably-changed-buffers
+              (delq (current-buffer) undo-auto--undoably-changed-buffers)))
+    (dolist (b undo-auto--undoably-changed-buffers)
+      (when (buffer-live-p b)
+        (with-current-buffer b
+          (unless undo-auto-disable-boundaries
+            (undo-auto--ensure-boundary cause)))))
+    (setq undo-auto--undoably-changed-buffers nil)))
 
 (defun undo-auto--boundary-timer ()
   "Timer which will run `undo--auto-boundary-timer'."
-- 
2.8.3

Previous Next

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