Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Campbell Barton]

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

Hi, there have been a handful of times I've needed to treat multiple
actions as a single action,
others too (see stackoverflow question [0]).

While this is supported using change groups, the resulting macro ends
up being nearly identical to the existing `atomic-change-group' macro.
The only difference is a call to undo-amalgamate-change-group.

Attached a patch to add `atomic-change-group-undo-amalgamate' so
packages don't need to use a modified copy of this macro.

[0]: https://emacs.stackexchange.com/questions/7558/how-to-collapse-undo-history

-- 
- Campbell

[-- Attachment #2: emacs-atomic-change-group-undo-amalgamate.diff --]
[-- Type: text/x-patch, Size: 2306 bytes --]

commit 5cf79a19dac5187b6e8f4f6c3798d0f348eff89c
Author: Campbell Barton <ideasman42@gmail.com>
Date:   Fri Oct 29 12:33:29 2021 +1100

    Add a version of atomic-change-group that amalgamates undo steps

diff --git a/lisp/subr.el b/lisp/subr.el
index 39676249cd..4d2ca0b953 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3490,15 +3490,8 @@ y-or-n-p
 \f
 ;;; Atomic change groups.
 
-(defmacro atomic-change-group (&rest body)
-  "Like `progn' but perform BODY as an atomic change group.
-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.
-
-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."
+(defmacro atomic-change-group-1 (undo-amalgamate &rest body)
+  "Implementation of `atomic-change-group' & `atomic-change-group-amalgamate'."
   (declare (indent 0) (debug t))
   (let ((handle (make-symbol "--change-group-handle--"))
 	(success (make-symbol "--change-group-success--")))
@@ -3519,9 +3512,28 @@ atomic-change-group
 	 ;; Either of these functions will disable undo
 	 ;; if it was disabled before.
 	 (if ,success
-	     (accept-change-group ,handle)
+             (progn
+	       (accept-change-group ,handle)
+               (when ,undo-amalgamate
+                 (undo-amalgamate-change-group ,handle)))
 	   (cancel-change-group ,handle))))))
 
+(defmacro atomic-change-group (&rest body)
+  "Like `progn' but perform BODY as an atomic change group.
+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.
+
+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."
+  `(atomic-change-group-1 nil ,@body))
+
+(defmacro atomic-change-group-undo-amalgamate (&rest body)
+  "A version of  `atomic-change-group' that amalgamates
+the change groups undo data."
+  `(atomic-change-group-1 t ,@body))
+
 (defun prepare-change-group (&optional buffer)
   "Return a handle for the current buffer's state, for a change group.
 If you specify BUFFER, make a handle for BUFFER's state instead.

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Stefan Monnier]

> Hi, there have been a handful of times I've needed to treat multiple
> actions as a single action,
> others too (see stackoverflow question [0]).
>
> While this is supported using change groups, the resulting macro ends
> up being nearly identical to the existing `atomic-change-group' macro.
> The only difference is a call to undo-amalgamate-change-group.

The other difference would be the call to `cancel-change-group` (you
may want to not undo the changes in case of errors).

> Attached a patch to add `atomic-change-group-undo-amalgamate' so
> packages don't need to use a modified copy of this macro.

I suspect that we'd be better off with a separate macro which only does
the amalgamation without the cancel-change-group-on-error.

But I wonder how often such a macro would be useful: undo boundaries are
normally added only by the command-loop (i.e. the loop which repeatedly
reads a key and calls the corresponding command), so unless your macro's
body calls `recursive-edit` it's very unlikely that you'll get any undo
entries that you would then want to remove.

(Re)reading your stackexchange question, it seems you want to amalgamate
undo entries from different commands.  So do you run those commands
within a `recursive-edit`?  If not, how do you manage to run those
commands from within your (new) macro?


        Stefan

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Campbell Barton]

On 11/2/21 04:43, Stefan Monnier wrote:
>> Hi, there have been a handful of times I've needed to treat multiple
>> actions as a single action,
>> others too (see stackoverflow question [0]).
>>
>> While this is supported using change groups, the resulting macro ends
>> up being nearly identical to the existing `atomic-change-group' macro.
>> The only difference is a call to undo-amalgamate-change-group.
> 
> The other difference would be the call to `cancel-change-group` (you
> may want to not undo the changes in case of errors).

Good point (updated my answer)

>> Attached a patch to add `atomic-change-group-undo-amalgamate' so
>> packages don't need to use a modified copy of this macro.
> 
> I suspect that we'd be better off with a separate macro which only does
> the amalgamation without the cancel-change-group-on-error.

Agree, if the cancel group is removed it's no longer atomic and can be a 
separate macro.

> But I wonder how often such a macro would be useful: undo boundaries are
> normally added only by the command-loop (i.e. the loop which repeatedly
> reads a key and calls the corresponding command), so unless your macro's
> body calls `recursive-edit` it's very unlikely that you'll get any undo
> entries that you would then want to remove.

I'm not sure why most other people are running into this (the original 
question relates specifically voice commands), nevertheless the stats on 
the stackexchange question lead me to think it's not so unusual to spawn 
multiple commands that happen to be treated as separate undo steps.

> (Re)reading your stackexchange question, it seems you want to amalgamate
> undo entries from different commands.  So do you run those commands
> within a `recursive-edit`?  If not, how do you manage to run those
> commands from within your (new) macro?

As far as I know I'm not using recursive edit (unless indirectly).

In these situations collapsing undo data was necessary.

- Calling kmacro-exec-ring-item (based on this answer [0])
- Calling some evil functions for e.g.

   (defun evil-paste-and-indent-after ()
     (interactive)
     (with-undo-collapse
       (evil-paste-after 1)
       (evil-indent (evil-get-marker ?\[)
                    (evil-get-marker ?\]))))

[0]: https://emacs.stackexchange.com/questions/70

> 
> 
>          Stefan
> 

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Stefan Monnier]

> In these situations collapsing undo data was necessary.
> - Calling kmacro-exec-ring-item (based on this answer [0])

Aha, yes, this one makes a lot of sense.

> - Calling some evil functions for e.g.
>
>   (defun evil-paste-and-indent-after ()
>     (interactive)
>     (with-undo-collapse
>       (evil-paste-after 1)
>       (evil-indent (evil-get-marker ?\[)
>                    (evil-get-marker ?\]))))

Yes, there are indeed a few commands that explicitly call
`undo-boundary`.  These are rather rare (and you can often argue that
they're bugs, where the undo-boundary should only be inserted if the
command is used interactively).


        Stefan

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Campbell Barton]

On Tue, Nov 2, 2021 at 12:04 PM Campbell Barton <ideasman42@gmail.com> wrote:
>
> On 11/2/21 04:43, Stefan Monnier wrote:
> >> Hi, there have been a handful of times I've needed to treat multiple
> >> actions as a single action,
> >> others too (see stackoverflow question [0]).
> >>
> >> While this is supported using change groups, the resulting macro ends
> >> up being nearly identical to the existing `atomic-change-group' macro.
> >> The only difference is a call to undo-amalgamate-change-group.
> >
> > The other difference would be the call to `cancel-change-group` (you
> > may want to not undo the changes in case of errors).
>
> Good point (updated my answer)
>
> >> Attached a patch to add `atomic-change-group-undo-amalgamate' so
> >> packages don't need to use a modified copy of this macro.
> >
> > I suspect that we'd be better off with a separate macro which only does
> > the amalgamation without the cancel-change-group-on-error.
>
> Agree, if the cancel group is removed it's no longer atomic and can be a
> separate macro.
>
> > But I wonder how often such a macro would be useful: undo boundaries are
> > normally added only by the command-loop (i.e. the loop which repeatedly
> > reads a key and calls the corresponding command), so unless your macro's
> > body calls `recursive-edit` it's very unlikely that you'll get any undo
> > entries that you would then want to remove.
>
> I'm not sure why most other people are running into this (the original
> question relates specifically voice commands), nevertheless the stats on
> the stackexchange question lead me to think it's not so unusual to spawn
> multiple commands that happen to be treated as separate undo steps.
>
> > (Re)reading your stackexchange question, it seems you want to amalgamate
> > undo entries from different commands.  So do you run those commands
> > within a `recursive-edit`?  If not, how do you manage to run those
> > commands from within your (new) macro?
>
> As far as I know I'm not using recursive edit (unless indirectly).
>
> In these situations collapsing undo data was necessary.
>
> - Calling kmacro-exec-ring-item (based on this answer [0])
> - Calling some evil functions for e.g.
>
>    (defun evil-paste-and-indent-after ()
>      (interactive)
>      (with-undo-collapse
>        (evil-paste-after 1)
>        (evil-indent (evil-get-marker ?\[)
>                     (evil-get-marker ?\]))))
>
> [0]: https://emacs.stackexchange.com/questions/70

To follow up on my previous post, from checking packages on melpa, the
following use an undo-collapse macro:

- recomplete (recomplete--with-undo-collapse)
- jupyter (jupyter-repl-with-single-undo)
- meow (meow--wrap-collapse-undo)
- symex (symex--with-undo-collapse)

Candidates which could potentially use an undo collapse macro but mix
undo-amalgamate with other logic:

- eglot (eglot--apply-text-edits)
- lsp-mode (lsp--apply-text-edits)
- parinfer-rust-mode (parinfer-rust--execute)

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Campbell Barton]

[-- Attachment #1: Type: text/plain, Size: 3264 bytes --]

On Tue, Nov 2, 2021 at 1:14 PM Campbell Barton <ideasman42@gmail.com> wrote:
>
> On Tue, Nov 2, 2021 at 12:04 PM Campbell Barton <ideasman42@gmail.com> wrote:
> >
> > On 11/2/21 04:43, Stefan Monnier wrote:
> > >> Hi, there have been a handful of times I've needed to treat multiple
> > >> actions as a single action,
> > >> others too (see stackoverflow question [0]).
> > >>
> > >> While this is supported using change groups, the resulting macro ends
> > >> up being nearly identical to the existing `atomic-change-group' macro.
> > >> The only difference is a call to undo-amalgamate-change-group.
> > >
> > > The other difference would be the call to `cancel-change-group` (you
> > > may want to not undo the changes in case of errors).
> >
> > Good point (updated my answer)
> >
> > >> Attached a patch to add `atomic-change-group-undo-amalgamate' so
> > >> packages don't need to use a modified copy of this macro.
> > >
> > > I suspect that we'd be better off with a separate macro which only does
> > > the amalgamation without the cancel-change-group-on-error.
> >
> > Agree, if the cancel group is removed it's no longer atomic and can be a
> > separate macro.
> >
> > > But I wonder how often such a macro would be useful: undo boundaries are
> > > normally added only by the command-loop (i.e. the loop which repeatedly
> > > reads a key and calls the corresponding command), so unless your macro's
> > > body calls `recursive-edit` it's very unlikely that you'll get any undo
> > > entries that you would then want to remove.
> >
> > I'm not sure why most other people are running into this (the original
> > question relates specifically voice commands), nevertheless the stats on
> > the stackexchange question lead me to think it's not so unusual to spawn
> > multiple commands that happen to be treated as separate undo steps.
> >
> > > (Re)reading your stackexchange question, it seems you want to amalgamate
> > > undo entries from different commands.  So do you run those commands
> > > within a `recursive-edit`?  If not, how do you manage to run those
> > > commands from within your (new) macro?
> >
> > As far as I know I'm not using recursive edit (unless indirectly).
> >
> > In these situations collapsing undo data was necessary.
> >
> > - Calling kmacro-exec-ring-item (based on this answer [0])
> > - Calling some evil functions for e.g.
> >
> >    (defun evil-paste-and-indent-after ()
> >      (interactive)
> >      (with-undo-collapse
> >        (evil-paste-after 1)
> >        (evil-indent (evil-get-marker ?\[)
> >                     (evil-get-marker ?\]))))
> >
> > [0]: https://emacs.stackexchange.com/questions/70
>
> To follow up on my previous post, from checking packages on melpa, the
> following use an undo-collapse macro:
>
> - recomplete (recomplete--with-undo-collapse)
> - jupyter (jupyter-repl-with-single-undo)
> - meow (meow--wrap-collapse-undo)
> - symex (symex--with-undo-collapse)
>
> Candidates which could potentially use an undo collapse macro but mix
> undo-amalgamate with other logic:
>
> - eglot (eglot--apply-text-edits)
> - lsp-mode (lsp--apply-text-edits)
> - parinfer-rust-mode (parinfer-rust--execute)

Here is an updated patch with a separate macro to amalgamate undo barriers.

-- 
- Campbell

[-- Attachment #2: emacs-with-undo-amalgamate-change-group.diff --]
[-- Type: text/x-patch, Size: 1470 bytes --]

commit 7caaa017aa4cfc5243178618a88633525a8e18b3
Author: Campbell Barton <ideasman42@gmail.com>
Date:   Sun Nov 7 19:39:29 2021 +1100

    Add 'with-undo-amalgamate-change-group' macro
    
    This allows commands to be made without adding undo-barriers, e.g.
    kmacro-exec-ring-item.

diff --git a/lisp/subr.el b/lisp/subr.el
index f6dbd00532..945aaa8fef 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3542,6 +3542,25 @@ atomic-change-group
 	     (accept-change-group ,handle)
 	   (cancel-change-group ,handle))))))
 
+(defmacro with-undo-amalgamate-change-group (&rest body)
+  "Like `progn' but perform BODY with amalgamated undo barriers.
+
+This allows multiple operations to be undone in a single step."
+  (declare (indent 0) (debug t))
+  (let ((handle (make-symbol "--change-group-handle--")))
+    `(let ((,handle (prepare-change-group))
+           ;; Don't truncate any undo data in the middle of this.
+           (undo-outer-limit nil)
+           (undo-limit most-positive-fixnum)
+           (undo-strong-limit most-positive-fixnum))
+       (unwind-protect
+           (progn
+             (activate-change-group ,handle)
+             ,@body)
+         (progn
+           (accept-change-group ,handle)
+           (undo-amalgamate-change-group ,handle))))))
+
 (defun prepare-change-group (&optional buffer)
   "Return a handle for the current buffer's state, for a change group.
 If you specify BUFFER, make a handle for BUFFER's state instead.

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Stefan Monnier]

> Here is an updated patch with a separate macro to amalgamate undo barriers.

Looks pretty good, thanks.
I was about to install it into `master` but noticed the following:

- You seem not to have signed copyright paperwork yet.  If you're OK
  with it, please fill the form below and send it to the FSF so they can
  send you the relevant paperwork to sign.
  [ The change is sufficiently small that we can accept it right away,
    but since the paperwork process takes some time, it's good to do it
    "in advance" so it's out of the way for your next contributions.  ]

- I see the macro binds undo limits, but AFAICT this is an "accident"
  resulting from copy&pasting code from the other macro: for the
  atomic-change macro it's very important that undo info is not thrown
  away since the macro uses the undo info internally to cancel changes
  on error, but for this macro I can't see any harm in having the undo
  info truncated, so I think we shouldn't change the undo limit
  vars.  WDYT?


        Stefan

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Campbell Barton]

On Mon, Nov 8, 2021 at 12:21 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > Here is an updated patch with a separate macro to amalgamate undo barriers.
>
> Looks pretty good, thanks.
> I was about to install it into `master` but noticed the following:
>
> - You seem not to have signed copyright paperwork yet.  If you're OK
>   with it, please fill the form below and send it to the FSF so they can
>   send you the relevant paperwork to sign.
>   [ The change is sufficiently small that we can accept it right away,
>     but since the paperwork process takes some time, it's good to do it
>     "in advance" so it's out of the way for your next contributions.  ]

Thanks for checking the patch.
I'll look into signing the paperwork, I may do other contributions in
the future, so best get that handled sooner than later.

> - I see the macro binds undo limits, but AFAICT this is an "accident"
>   resulting from copy&pasting code from the other macro: for the
>   atomic-change macro it's very important that undo info is not thrown
>   away since the macro uses the undo info internally to cancel changes
>   on error, but for this macro I can't see any harm in having the undo
>   info truncated, so I think we shouldn't change the undo limit
>   vars.  WDYT?

This was intentional, the reasoning is that it should not be possible
for the amalgamated undo information to form a partial undo step.

In other words, I believe there should be a guarantee that a single
undo restores the state before `with-undo-amalgamate-change-group' is
used. Or, in the unlikely case a single undo-step exceeds memory
limits, that undo fails entirely instead of undoing into a state
part-way through the body within the macro.
This is in keeping with how you would expect undo to work for any
other command AFAICS.

In my use case this is an important guarantee since undo/redoing an
action needs to ensure the action was fully undone before performing
the new action. Failure to do so will cause bugs that could be
difficult to track down.

This should be noted in the code-comments, if the rationale above
seems reasonable I'll update the patch.

Another note on naming, this could be called `with-undo-amalgamate',
not sure if the "-change-group" suffix is worth including (I did this
to fit in with existing names, although it seems a bit verbose).

>
>         Stefan
>


-- 
- Campbell

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Campbell Barton]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]

On Mon, Nov 8, 2021 at 10:09 AM Campbell Barton <ideasman42@gmail.com> wrote:
>
> On Mon, Nov 8, 2021 at 12:21 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >
> > > Here is an updated patch with a separate macro to amalgamate undo barriers.
> >
> > Looks pretty good, thanks.
> > I was about to install it into `master` but noticed the following:
> >
> > - You seem not to have signed copyright paperwork yet.  If you're OK
> >   with it, please fill the form below and send it to the FSF so they can
> >   send you the relevant paperwork to sign.
> >   [ The change is sufficiently small that we can accept it right away,
> >     but since the paperwork process takes some time, it's good to do it
> >     "in advance" so it's out of the way for your next contributions.  ]
>
> Thanks for checking the patch.
> I'll look into signing the paperwork, I may do other contributions in
> the future, so best get that handled sooner than later.
>
> > - I see the macro binds undo limits, but AFAICT this is an "accident"
> >   resulting from copy&pasting code from the other macro: for the
> >   atomic-change macro it's very important that undo info is not thrown
> >   away since the macro uses the undo info internally to cancel changes
> >   on error, but for this macro I can't see any harm in having the undo
> >   info truncated, so I think we shouldn't change the undo limit
> >   vars.  WDYT?
>
> This was intentional, the reasoning is that it should not be possible
> for the amalgamated undo information to form a partial undo step.
>
> In other words, I believe there should be a guarantee that a single
> undo restores the state before `with-undo-amalgamate-change-group' is
> used. Or, in the unlikely case a single undo-step exceeds memory
> limits, that undo fails entirely instead of undoing into a state
> part-way through the body within the macro.
> This is in keeping with how you would expect undo to work for any
> other command AFAICS.
>
> In my use case this is an important guarantee since undo/redoing an
> action needs to ensure the action was fully undone before performing
> the new action. Failure to do so will cause bugs that could be
> difficult to track down.
>
> This should be noted in the code-comments, if the rationale above
> seems reasonable I'll update the patch.
>
> Another note on naming, this could be called `with-undo-amalgamate',
> not sure if the "-change-group" suffix is worth including (I did this
> to fit in with existing names, although it seems a bit verbose).

Send the copyright assignment email to `assign(_at_)gnu.org'
updated the patch with added comment,

[-- Attachment #2: emacs-with-undo-amalgamate-change-group.diff --]
[-- Type: text/x-patch, Size: 1658 bytes --]

commit 204c5d03adf6407d7676ad241b139187f5a84ea1
Author: Campbell Barton <ideasman42@gmail.com>
Date:   Sun Nov 7 19:39:29 2021 +1100

    Add 'with-undo-amalgamate-change-group' macro
    
    This allows commands to be made without adding undo-barriers, e.g.
    kmacro-exec-ring-item.

diff --git a/lisp/subr.el b/lisp/subr.el
index f6dbd00532..c68b334e42 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3542,6 +3542,28 @@ atomic-change-group
 	     (accept-change-group ,handle)
 	   (cancel-change-group ,handle))))))
 
+(defmacro with-undo-amalgamate-change-group (&rest body)
+  "Like `progn' but perform BODY with amalgamated undo barriers.
+
+This allows multiple operations to be undone in a single step."
+  (declare (indent 0) (debug t))
+  (let ((handle (make-symbol "--change-group-handle--")))
+    `(let ((,handle (prepare-change-group))
+           ;; Don't truncate any undo data in the middle of this.
+           ;; This guarantees the resulting undo step is complete,
+           ;; so the user can't undo into an intermediate step
+           ;; performed within the body of this macro.
+           (undo-outer-limit nil)
+           (undo-limit most-positive-fixnum)
+           (undo-strong-limit most-positive-fixnum))
+       (unwind-protect
+           (progn
+             (activate-change-group ,handle)
+             ,@body)
+         (progn
+           (accept-change-group ,handle)
+           (undo-amalgamate-change-group ,handle))))))
+
 (defun prepare-change-group (&optional buffer)
   "Return a handle for the current buffer's state, for a change group.
 If you specify BUFFER, make a handle for BUFFER's state instead.

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Stefan Monnier]

> In my use case this is an important guarantee since undo/redoing an
> action needs to ensure the action was fully undone before performing
> the new action. Failure to do so will cause bugs that could be
> difficult to track down.

Fair enough.  I think the better way to document it is that we want to
mimic the behavior we'd get if the undo-boundaries were never added in
the first place.

> Another note on naming, this could be called `with-undo-amalgamate',
> not sure if the "-change-group" suffix is worth including (I did this
> to fit in with existing names, although it seems a bit verbose).

The less verbose name is better, indeed.
Could even be `with-single-undo`.


        Stefan

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The less verbose name is better, indeed.
> Could even be `with-single-undo`.

I like it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Campbell Barton]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

On Mon, Nov 8, 2021 at 3:46 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
> > The less verbose name is better, indeed.
> > Could even be `with-single-undo`.
>
> I like it.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

Updated the patch, for what it's worth - I'm not so keen on the name
`with-single-undo'.

- `with-undo-amagamate' hints this is a wrapper for
undo-amalgamate-change-group.
- The term "amagamate" emphasizes that any undo barriers are merged,
whereas "single" reads as if a single undo step might always be added
(which isn't the case).

... another alternative is `with-undo-collapse` which is used in some
of the packages that include this functionality on MELPA.
Although I'd err on the side of not introducing new terminology to
describe concepts already used elsewhere hence with-`undo-amagamate'.

-- 
- Campbell

[-- Attachment #2: emacs-with-undo-amalgamate.diff --]
[-- Type: text/x-patch, Size: 1677 bytes --]

commit 2964fa126a059bb6f610c43fd8bcf1e700a3f107
Author: Campbell Barton <ideasman42@gmail.com>
Date:   Mon Nov 8 16:33:39 2021 +1100

    Add 'with-undo-amalgamate' macro

    This allows commands to be made without adding undo-barriers, e.g.
    kmacro-exec-ring-item.

diff --git a/lisp/subr.el b/lisp/subr.el
index f6dbd00532..73b799c65b 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3542,6 +3542,29 @@ atomic-change-group
 	     (accept-change-group ,handle)
 	   (cancel-change-group ,handle))))))

+(defmacro with-undo-amalgamate (&rest body)
+  "Like `progn' but perform BODY with amalgamated undo barriers.
+
+This allows multiple operations to be undone in a single step.
+When undo is disabled this behaves like `progn'."
+  (declare (indent 0) (debug t))
+  (let ((handle (make-symbol "--change-group-handle--")))
+    `(let ((,handle (prepare-change-group))
+           ;; Don't truncate any undo data in the middle of this.
+           ;; This guarantees the resulting undo step is complete,
+           ;; so the user can't undo into an intermediate step
+           ;; performed within the body of this macro.
+           (undo-outer-limit nil)
+           (undo-limit most-positive-fixnum)
+           (undo-strong-limit most-positive-fixnum))
+       (unwind-protect
+           (progn
+             (activate-change-group ,handle)
+             ,@body)
+         (progn
+           (accept-change-group ,handle)
+           (undo-amalgamate-change-group ,handle))))))
+
 (defun prepare-change-group (&optional buffer)
   "Return a handle for the current buffer's state, for a change group.
 If you specify BUFFER, make a handle for BUFFER's state instead.

Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch)

[Stefan Monnier]

> Updated the patch, for what it's worth - I'm not so keen on the name
> `with-single-undo'.

Fair enough.  Besides, I believe it would be slightly misleading because
it doesn't guarantee that this makes exactly one undo step (it might be
include changes performed before/after).

I pushed your change as below.

Please note the changes in the commit message format to follow our
conventions, and the documentation that should ideally come with
such patches.


        Stefan


commit 5861b8d027382ecbd4c0d3dffc283b8ac95b5692
Author: Campbell Barton <ideasman42@gmail.com>
Date:   Mon Nov 8 16:33:39 2021 +1100

    * lisp/subr.el (with-undo-amalgamate): New macro
    
    This allows commands to be made without adding undo-barriers, e.g.
    kmacro-exec-ring-item.

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index fa1135b8026..937680c200d 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -1506,6 +1506,11 @@ Undo
 This function does not bind @code{undo-in-progress}.
 @end defun
 
+@defmac with-undo-amalgamate body@dots{}
+This macro removes all the undo boundaries inserted during the
+execution of @var{body} so that it can be undone as a single step.
+@end defmac
+
 Some commands leave the region active after execution in such a way that
 it interferes with selective undo of that command.  To make @code{undo}
 ignore the active region when invoked immediately after such a command,
diff --git a/etc/NEWS b/etc/NEWS
index a297948a11a..874af33c752 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -598,6 +598,10 @@ Use 'exif-parse-file' and 'exif-field' instead.
 \f
 * Lisp Changes in Emacs 29.1
 
++++
+*** New macro 'with-undo-amalgamate'
+It records a particular sequence of operations as a single undo step
+
 +++
 *** New command 'yank-media'.
 This command supports yanking non-plain-text media like images and
diff --git a/lisp/subr.el b/lisp/subr.el
index f6dbd00532e..5a5842d4287 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3542,6 +3542,29 @@ atomic-change-group
 	     (accept-change-group ,handle)
 	   (cancel-change-group ,handle))))))
 
+(defmacro with-undo-amalgamate (&rest body)
+  "Like `progn' but perform BODY with amalgamated undo barriers.
+
+This allows multiple operations to be undone in a single step.
+When undo is disabled this behaves like `progn'."
+  (declare (indent 0) (debug t))
+  (let ((handle (make-symbol "--change-group-handle--")))
+    `(let ((,handle (prepare-change-group))
+           ;; Don't truncate any undo data in the middle of this,
+           ;; otherwise Emacs might truncate part of the resulting
+           ;; undo step: we want to mimic the behavior we'd get if the
+           ;; undo-boundaries were never added in the first place.
+           (undo-outer-limit nil)
+           (undo-limit most-positive-fixnum)
+           (undo-strong-limit most-positive-fixnum))
+       (unwind-protect
+           (progn
+             (activate-change-group ,handle)
+             ,@body)
+         (progn
+           (accept-change-group ,handle)
+           (undo-amalgamate-change-group ,handle))))))
+
 (defun prepare-change-group (&optional buffer)
   "Return a handle for the current buffer's state, for a change group.
 If you specify BUFFER, make a handle for BUFFER's state instead.