bug#23906: 25.0.95; Undo boundary after process output is not consistent

[Stefan Monnier <monnier@iro.umontreal.ca>]

> As far as I understand the code, this keeps track, via a special element
> that normally does not occur in the undo list, of where the boundary
> should be.  Later, it removes all undo boundaries up to and including
> that marker.  I have a quite uneasy feeling about this: Can there be a
> situation where the marker element unexpectedly remains in the undo
> list?

Yes, Viper's implementation is not 100% satisfactory.
It could be improved in 2 ways:
- Don't add anything to the undo list; instead keep a reference to the
  original top of the list.
- Do add something, but make sure it's a valid element (e.g. a dummy
  "empty insertion" record).

There are also some issues that deserve care:
- The user may decide to undo past this special record, so it won't be
  found in buffer-undo-list any more.
- The buffer-undo-list may grow large enough that when Emacs's GC comes
  around to cut off its tail, it ends up throwing away the special record.

So the record may not be present in the end any more, and there can be
2 very different reasons for that (in one case what should be done is
to remove all undo-boundaries, and in the other to remove none).

> From a quick glance, maybe unwind-protect is what would be needed
> in ediprolog to reliably remove such a marker also if C-g is pressed.

Yes, if you have proper nesting, then unwind-protect should work well
and reliably.

> So, how do undo boundaries that are inserted by timers help to
> improve efficiency?

The undo logs may grow arbitrarily large, so Emacs's GC always
automatically trims the end of those logs (according to undo-*limit) to
make sure we don't end up using too much memory.  This trimming can only
be done at undo-boundaries, so in the absence of undo-boundaries, the
auto-trimming can't do it job.

>> Maybe we should add some support in some "core" file (subr.el or
>> something) for that operation.
> That would be awesome! Like "undo-begin-transaction", which could
> prepare an atomic undo operation over many subsequent operations, and
> "undo-end-transaction", which would remove all undo boundaries in it?

Yup.  It would seem to fit nicely next to atomic-change-group.


        Stefan