From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Alan Mackenzie <acm@muc.de>
Cc: 39680@debbugs.gnu.org, "Kévin Le Gouguec" <kevin.legouguec@gmail.com>
Subject: bug#39680: 27.0.60; electric-pair-mode broken by undo
Date: Mon, 09 Mar 2020 14:26:21 -0400 [thread overview]
Message-ID: <jwvimjd4dfr.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <20200225193451.GA5896@ACM> (Alan Mackenzie's message of "Tue, 25 Feb 2020 19:34:51 +0000")
> 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)))))))
\f
;;;; Display-related functions.
next prev parent reply other threads:[~2020-03-09 18:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-19 18:34 bug#39680: 27.0.60; electric-pair-mode broken by undo Kévin Le Gouguec
2020-02-25 19:34 ` Alan Mackenzie
2020-03-09 18:26 ` Stefan Monnier [this message]
2020-03-10 6:45 ` Kévin Le Gouguec
2020-03-11 15:31 ` Stefan Monnier
2020-03-11 16:20 ` Eli Zaretskii
2020-03-12 14:04 ` Stefan Monnier
[not found] ` <87d078ajzs.fsf@gmail.com>
[not found] ` <87blmstg0v.fsf@gmail.com>
[not found] ` <87blms6ww9.fsf@gmail.com>
[not found] ` <877dxgtae9.fsf@gmail.com>
2020-05-19 22:05 ` bug#39680: Test case Kévin Le Gouguec
2020-05-19 22:26 ` Kévin Le Gouguec
2020-05-19 23:17 ` João Távora
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvimjd4dfr.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=39680@debbugs.gnu.org \
--cc=acm@muc.de \
--cc=kevin.legouguec@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.