Re: An idea: combine-change-calls

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan.

On Wed, Mar 28, 2018 at 17:26:30 -0400, Stefan Monnier wrote:
> > It would need amendment, of course, but that wouldn't be difficult.

> I'd rather try and avoid that.  And if we really do want to extent the
> format because we consider that (apply ...) is not good enough here,
> ....

I don't think "consider" is the right word here.  I don't think it will
work at all.  In primitive-undo, some undo list is an argument, and it
has elements removed from it and it is then the return value.  If we try
to call primitive-undo recursively through an (apply ...) form, there is
no interface to return the depleted list to the calling p-u.  We could,
of course, use some global variables instead of arguments, but this
would involve changes just as incompatible as making additions to the
undo list elements.

> ... than I'd want this new extension to be generic rather than
> specific for this particular use-case.

It is generic, in the sense it handles any case where
before/after-change-functions are to be condensed into one call of each.

What do you mean by generic, here?

[ .... ]

> > What we've been discussing goes beyond hiding information, it is the
> > destruction of information.  Users, maybe just a few, won't like
> > that at all.

> Not at all.  It's just optimizing the representation of the undo-log.
> If there's an undo-boundary in there, then indeed, we'd be throwing away
> a bit of information, but I assumed we wouldn't care about that case.

It does a good deal more than "optimizing the representation" - it makes
an irreversible change which loses information.  Somebody, sometime, is
going to need that info.

> Whatever you decide to do with the undo-log, handling undo-boundary
> pushed during the execution of `body` will be tricky I suspect (except
> if we just don't touch the undo-list, of course).

In my current code, the only undo-boundary pushed (in the handling of
combine-change-begin) is immediately acted upon to terminate the
recursive invocation of primitive-undo.  This is pushed onto the LIST
variable in the nested p-u, and doesn't affect buffer-undo-list or
pending-undo-list.

> > Incidentally, position elements in the undo list don't work: `undo'
> > removes them from buffer-undo-list.

> Are you sure they "don't work" (they seemed to work in my test)?

> IIUC The code you cite only strips them from the undo elements added
> while performing an undo (i.e. from "redo" elements), so they should
> still work for a plain "edit .... undo".

Ah, is that it?  I had some difficulty understanding it properly.

> > I think you amended that bit of code some years ago.  Can you say why
> > this is done?  The comment in the code:

> >     ;; Don't specify a position in the undo record for the undo command.
> >     ;; Instead, undoing this should move point to where the change is.

> > doesn't give any reason, and the various pertinent commit messages
> > aren't any help either.

> Hmm... good question.  I see this code basically dates back to

>   commit 2512c9f0f0e6cc71c601ffdb0690b9cf5642734b
>   Author: Richard M. Stallman <rms@gnu.org>
>   Date:   Wed Mar 16 23:41:32 1994 +0000

>     (undo): Don't let the undo entries for the undo
>     contain a specific buffer position.  Delete it if there is one.

Yes.

> and no, I don't know why we do this.

Thinking about it, that comment above ("Don't specify a position ....")
reads as if it was originally in place on some code which added elements
to buffer-undo-list, and then got hurredly moved to `undo' when the
strategy was changed to delete such elements.  Again, it isn't clear why
position elements get deleted.  Any code which adds them (such as my new
code) will have a reason for doing so.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).