bug#23871: 25.1.50; Undo unexpectedly leads to blank buffer

[Phillip Lord]

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

>>> BTW, I notice that in the current code (emacs-25), there's one other
>>> call to record_first_change (in record_property_change), and it could be
>>> replaced with a call to prepare_record, so it begs the question whether
>>> the above hunk should also apply to record_property_change as well.
>
>> Don't understand.
>
> In record_property_change we have (among other things) the exact same
> code as in prepare_record (i.e. there's code duplication).  So we
> could/should replace those 4 lines of code with a call to
> prepare_record.  But if we do, then your (previous) patch changes the
> behavior of record_property_change, whereas if we don't, your (previous)
> patch doesn't change its behavior.
> Anyway, your new patch addresses this.

Good!

>
> More complex but more robust.  I think it'd be worthwhile to put a FIXME
> comment in there, at least.  E.g. the above explanation should be put
> inside the code.

Done.

> I think the comment should explain the intention better.
> It is currently too close to a simple paraphrase of the code.

Yeah, I tell my students off for doing that.

> I suggest "If it's the first change since the last boundary, and the
> upcoming undo record wouldn't restore point correctly, then record where
> it was".

I've updated it (with a something a bit longer).


> Other than that it looks good, thank you for the detailed explanation.

I have pushed this to emacs-25, since Eli was happy with the last
version, and you're happy with this.

Phil