From bf28b019a85fcc4e16bc7ecad6304c30e48a3223 Mon Sep 17 00:00:00 2001 From: Po Lu Date: Tue, 6 Jun 2023 21:00:44 +0800 Subject: [PATCH] Fix problems resulting from modification of the undo list * doc/lispref/text.texi (Atomic Changes): Describe what not to do inside an atomic change group. * lisp/elec-pair.el (electric-pair-inhibit-if-helps-balance): Don't call `delete-char'; that edits the undo list by removing boundary markers. * lisp/subr.el (atomic-change-group, prepare-change-group): Warn against modifying the undo list inside. --- doc/lispref/text.texi | 11 +++++++++++ lisp/elec-pair.el | 4 +++- lisp/subr.el | 9 +++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi index f15b3c33e0c..dac8d0d8ce9 100644 --- a/doc/lispref/text.texi +++ b/doc/lispref/text.texi @@ -6181,6 +6181,17 @@ would expect. Non-nested use of change groups for the same buffer will get Emacs confused, so don't let it happen; the first change group you start for any given buffer should be the last one finished. + Emacs keeps track of change groups by assuming that by following +each cdr in @code{buffer-undo-list}, it will eventually arrive at the +cons it was set to at the time @code{prepare-change-group} was called. + + If @code{buffer-undo-list} no longer contains that cons, Emacs will +lose track of any change groups, resulting in an error when the change +group is cancelled. To avoid this, do not call any functions which +may edit the undo list in such a manner, when a change group is +active: notably, ``amalgamating'' commands such as @code{delete-char}, +which call @code{undo-auto-amalgamate}. + @node Change Hooks @section Change Hooks @cindex change hooks diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el index b894965eae4..416c95e7a34 100644 --- a/lisp/elec-pair.el +++ b/lisp/elec-pair.el @@ -439,7 +439,9 @@ happened." ;; position some markers. The real fix would be to compute the ;; result without having to modify the buffer at all. (atomic-change-group - (delete-char -1) + ;; Don't use `delete-char'; that may modify the head of the + ;; undo list. + (delete-region (point) (1- (point))) (throw 'done (cond ((eq ?\( syntax) diff --git a/lisp/subr.el b/lisp/subr.el index cef631a69c3..1384215498b 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -3773,6 +3773,9 @@ This means that if BODY exits abnormally, all of its changes to the current buffer are undone. This works regardless of whether undo is enabled in the buffer. +Do not call functions which edit the undo list within BODY; see +`prepare-change-group'. + This mechanism is transparent to ordinary use of undo; if undo is enabled in the buffer and BODY succeeds, the user can undo the change normally." @@ -3839,6 +3842,12 @@ Once you finish the group, don't use the handle again--don't try to finish the same group twice. For a simple example of correct use, see the source code of `atomic-change-group'. +As long as this handle is still in use, do not call functions +which edit the undo list: if it no longer contains its current +value, Emacs will not be able to cancel the change group. This +includes any \"amalgamating\" commands, such as `delete-char', +which call `undo-auto-amalgamate'. + The handle records only the specified buffer. To make a multibuffer change group, call this function once for each buffer you want to cover, then use `nconc' to combine the returned values, like this: -- 2.39.2