Subject: bug#22295: viper-mode undo bug introduced between Nov 10 and Nov 14

From: [email-masked] (Phillip Lord)
Date: Thu, 02 Jun 2016 09:45:27 +0100

Stefan Monnier <[email-masked]> writes:

>>> Oh dear. Yes, that is a problem. The difficulty is that viper modifies
>>> the undo-list, removing boundaries only *after* we leave insert mode.
>
> I don't see any difficulty in there.

Sorry, "difficulty" in the sense that this is the cause of the different
behaviour.

>
>>> Viper's solution of introducing a 'viper symbol is a nice one, but has
>>> it's problems.
>
> Agreed.
>
>>> primitive-undo: Unrecognized entry in undo list viper
>> not in  25.0.50 viper worked fine with this for 20 years and I'd say
>> it is Emacs breakage, not Viper's.
>
> FWIW, Viper adds an entry to buffer-undo-list which is incompatible with
> the documented format of the entries, so while it worked, Viper was
> doing something unkosher.
>
> In any case it shouldn't be difficult to fix that problem.
> E.g. you could use
>
>     (defconst viper--undo-marker '(1 . 1))
>
> instead of a symbol.

I think that has the advantage of fulfilling the spec by putting what is
a nop in the undo.

>>> The deep problem here is that undo boundary == nil
>
> I know you like to think of it as a problem ;-)

It is. In my poking through Emacs, I have found several places where the
code say "remove the last undo-boundary which should be..." or "don't
remove undo-boundary which are explicit". But we have to guess which is
which.

Instead we could do:

(nil . :explicit)
(nil . :auto)
(nil . :timer)

>>> 1) Restore all the old viper code
>>> 2) Instead of adding a 'viper mark, copy the buffer-undo-list to
>>> "viper-old-buffer-undo-list".
>
> Don't copy, just (setq viper--original-undo-list buffer-undo-list).

Agreed; meant "copy the reference". Poor choice of words.

>
>>> This is also quite a big change, and I worry about buffer compaction --
>>> viper-old-buffer-undo-list would not be open for GC.
>
> Why not?

Oh, yes, you are correct. I was thinking of this as some sort of weak
reference, but actually, buffer-undo-list is specifically truncated by
GC. It wouldn't matter if there were a copy elsewhere.

> This said, AFAIK the OP's problem may not be directly linked to the addition
> of a special symbol in the undo-list.  We should first figure out what's
> actually going wrong there.

I will try and investigate again, but I am running out of ideas to test.

Phil