[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Support for undo-amalgamate in a version of the atomic-change-group
From: |
Campbell Barton |
Subject: |
Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch) |
Date: |
Mon, 8 Nov 2021 10:29:45 +1100 |
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,
emacs-with-undo-amalgamate-change-group.diff
Description: Text Data
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Stefan Monnier, 2021/11/01
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Campbell Barton, 2021/11/01
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Stefan Monnier, 2021/11/01
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Campbell Barton, 2021/11/01
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Campbell Barton, 2021/11/07
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Stefan Monnier, 2021/11/07
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Campbell Barton, 2021/11/07
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch),
Campbell Barton <=
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Stefan Monnier, 2021/11/07
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Lars Ingebrigtsen, 2021/11/07
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Campbell Barton, 2021/11/08
- Re: Support for undo-amalgamate in a version of the atomic-change-group macro (with patch), Stefan Monnier, 2021/11/08