bug#60467: 30.0.50; primitive-undo: Changes to be undone by function different from announced

[Gregory Heytings]

> > > My hope is that we can just remove the timestamp special case, in 
> > > which case we can keep the current code mostly unchanged.
> >
> > Then our hopes are different.  What's wrong exactly with replacing a 
> > piece of code that requires a long discussion with question marks 
> > everywhere to be understood by a piece of well-documented code that is 
> > much more readable and "evidently" does what it is supposed to do?
>
> The old code was working for quite a few years, so it isn't all wrong.

It has only a few callers.  In fact, its only callers for a long time have 
been (1) comment-region-default and uncomment-region-default, which, by 
chance, do not insert timestamps between the undo elements, and (2) 
dired-readin in which (as I said upthread) this part of the function is 
simply not executed because buffer-undo-list is let-bound to t around 
body.  In effect, the first caller that actually exercises its logic in 
full is Org.

> Why do you object so much to leaving the timestamps in the undo-list?

I do not object to leaving timestamps in the undo-list, I object to 
leaving the code in the state it is, because I think of the future person 
who will try to make sense of it in 2028.  I'm equally fine with the two 
attached patches, one in which timestamps are kept an the other in which 
they aren't.

And now I believe it's better if I bow out of this thread, and let you and 
Stefan decide what the best course of action is.

Fix-combine-change-call-without-timestamps.patch
Description: Text Data

Fix-combine-change-call-with-timestamps.patch
Description: Text Data