[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#39680: 27.0.60; electric-pair-mode broken by undo
From: |
Stefan Monnier |
Subject: |
bug#39680: 27.0.60; electric-pair-mode broken by undo |
Date: |
Mon, 09 Mar 2020 14:26:21 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
> The cause of this bug is a bug in cancel-change-group,
Indeed.
Thanks Alan for the nice analysis (and sorry I didn't get to it earlier).
> (unless (eq last-command 'undo) (undo-start)) <=======
> ;; Make sure there's no confusion.
> ============> (when (and (consp elt) (not (eq elt (last
> pending-undo-list))))
> . At the first indicated spot above, last-command is indeed 'undo, so
> undo-start is not invoked.
I think there's a bug right here: the fact that the previous command was
`undo` shouldn't really matter. Worse, we've made changes to the buffer
since that last `undo` so it's plain wrong to pass that old
`pending-undo-list` to `undo-more`.
> Stefan, what is your view on this attempted patch? Is it sound?
I think we need something like the patch below (not really tested yet).
WDYT?
>> Thank you for your time.
> Thank you for a good bug report, conveniently reduced to a minimum test
> case.
Indeed. This is pretty delicate code, so a concise and easy to reproduce
test case is very welcome.
Stefan
diff --git a/lisp/subr.el b/lisp/subr.el
index 13515ca7da..ebc8e320dc 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2972,13 +2972,14 @@ cancel-change-group
;; the body of `atomic-change-group' all changes can be undone.
(widen)
(let ((old-car (car-safe elt))
- (old-cdr (cdr-safe elt)))
+ (old-cdr (cdr-safe elt))
+ (start-pul pending-undo-list))
(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))
+ (setq pending-undo-list buffer-undo-list)
;; Make sure there's no confusion.
(when (and (consp elt) (not (eq elt (last pending-undo-list))))
(error "Undoing to some unrelated state"))
@@ -2991,7 +2992,13 @@ cancel-change-group
;; Reset the modified cons cell ELT to its original content.
(when (consp elt)
(setcar elt old-car)
- (setcdr elt old-cdr))))))))
+ (setcdr elt old-cdr)))
+ ;; Let's not break a sequence of undos just because we
+ ;; tried to make a change and then undid it: preserve
+ ;; the original `pending-undo-list' if it's still valid.
+ (if (eq (undo--last-change-was-undo-p buffer-undo-list)
+ start-pul)
+ (setq pending-undo-list start-pul)))))))
;;;; Display-related functions.
- bug#39680: 27.0.60; electric-pair-mode broken by undo,
Stefan Monnier <=