[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#23632: 25.1.50; Gratuitous undo boundary in latex-insert-block
From: |
Phillip Lord |
Subject: |
bug#23632: 25.1.50; Gratuitous undo boundary in latex-insert-block |
Date: |
Tue, 31 May 2016 22:42:36 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.0.94 (gnu/linux) |
phillip.lord@russet.org.uk (Phillip Lord) writes:
> Chong Yidong <cyd@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!
>From 9116e7f00f90fb14857d21698b1e6870fcf98bbd Mon Sep 17 00:00:00 2001
From: Phillip Lord <phillip.lord@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