[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#39680: 27.0.60; electric-pair-mode broken by undo
From: |
Alan Mackenzie |
Subject: |
bug#39680: 27.0.60; electric-pair-mode broken by undo |
Date: |
Tue, 25 Feb 2020 19:34:51 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Hello, Kévin and Stefan.
On Wed, Feb 19, 2020 at 19:34:37 +0100, Kévin Le Gouguec wrote:
> Hello,
> Commit e66d5a1c45 (2019-02-19T00:00:44Z!monnier@iro.umontreal.ca) might
> have introduced a bug. From emacs -Q:
> 1. C-x b foo RET
> 2. M-x electric-pair-mode RET
> 3. (
> - A closing parenthesis has been inserted.
> 4. C-b C-f
> - This is to break undo grouping.
> 5. a
> 6. C-_
> 7. (
> In Emacs 26.3, buffer foo contains "(())" and point is after the
> innermost opening bracket.
> In Emacs 27, buffer foo contains "()" and point is after the closing
> bracket. The *Messages* buffer shows:
> > cancel-change-group: Undoing to some unrelated state
The cause of this bug is a bug in cancel-change-group, part of the
atomic-change-group group of functions. The commit you (Kévin) refer to
above substitutes atomic-change-group for a simple insertion and
deletion of a character (for a good reason).
cancel-change-group looks like this:
(defun cancel-change-group (handle)
"Finish a change group made with `prepare-change-group' (which see).
This finishes the change group by reverting all of its changes."
(dolist (elt handle)
(with-current-buffer (car elt)
(setq elt (cdr elt))
(save-restriction
;; Widen buffer temporarily so if the buffer was narrowed within
;; the body of `atomic-change-group' all changes can be undone.
(widen)
(let ((old-car (car-safe elt))
(old-cdr (cdr-safe elt)))
(unwind-protect
(progn
;; Temporarily truncate the undo log at ELT.
(when (consp elt)
(setcar elt nil) (setcdr elt nil))
(unless (eq last-command 'undo) (undo-start)) <=======
;; Make sure there's no confusion.
============> (when (and (consp elt) (not (eq elt (last pending-undo-list))))
(error "Undoing to some unrelated state"))
;; Undo it all.
(save-excursion
(while (listp pending-undo-list) (undo-more 1)))
;; Revert the undo info to what it was when we grabbed
;; the state.
(setq buffer-undo-list elt))
;; Reset the modified cons cell ELT to its original content.
(when (consp elt)
(setcar elt old-car)
(setcdr elt old-cdr))))))))
On entry to this function, HANDLE has the value:
((#<buffer asdf> (2 . 3) nil ("a" . 2) (#<marker at 2 in asdf> . -1)
nil (2 . 3)))
. At the first indicated spot above, last-command is indeed 'undo, so
undo-start is not invoked.
Since the undo which undid "a" emptied pending-undo-list,
pending-undo-list has been set to t.
So when the eq is done in the second indicated spot, pending-undo-list
is not ELT (the first cons cell from (cdr handle)). The function thus
spuriously signals the error "Undoing to some unrelated state".
#########################################################################
I admit I don't fully understand the mechanism of atomic-change-group,
but I see the problem as the EQ comparison on the <======= line. If
pending-undo-list has been replaced by t (after being exhausted by the
previous 'undo operation), there is no point EQing it with the cons from
HANDLE.
So, as a first approximation to a fix, I added a (consp
pending-undo-list) into this test, as follow:
diff --git a/lisp/subr.el b/lisp/subr.el
index b5ec0de156..8b7d9b5451 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2975,7 +2975,9 @@ cancel-change-group
;; Temporarily truncate the undo log at ELT.
(when (consp elt)
(setcar elt nil) (setcdr elt nil))
- (unless (eq last-command 'undo) (undo-start))
+ (unless (and (eq last-command 'undo)
+ (consp pending-undo-list))
+ (undo-start))
;; Make sure there's no confusion.
(when (and (consp elt) (not (eq elt (last pending-undo-list))))
(error "Undoing to some unrelated state"))
Stefan, what is your view on this attempted patch? Is it sound?
[ .... ]
> Thank you for your time.
Thank you for a good bug report, conveniently reduced to a minimum test
case.
> In GNU Emacs 28.0.50 (build 5, x86_64-pc-linux-gnu, GTK+ Version 3.24.13,
> cairo version 1.16.0)
> of 2020-02-19 built on my-little-tumbleweed
> Repository revision: e1e1bd8f85c53fea9f61b6ec99b461ddd93461b9
> Repository branch: master
> Windowing system distributor 'The X.Org Foundation', version 11.0.12007000
> System Description: openSUSE Tumbleweed
> Configured using:
> 'configure --with-xwidgets --with-cairo'
--
Alan Mackenzie (Nuremberg, Germany).