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

[Phillip Lord]

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

>> different, though. With this fix, viper just disables automatic boundary
>> addition and adds it's own as necessary, which seems cleaner.
>
> I see an annoying side-effect of that change, tho:
>
>     % emacs -Q -f viper-mode
>     y 5 n                   ;; To get past viper's initial questions.
>     i helo C-b k C-/
>
> This last C-/ used to (and should) just undo the insertion of the last
> "k", but instead it now completely wipes out the *scratch* buffer.
>
> So I think that completely disabling undo boundaries is not a good idea:
> while in many use-cases insertion mode is very transient, it is not so
> unusual to spend more time in insertion mode (which is pretty much
> "emacs mode") and to expect undo boundaries to properly inserted during
> this time.

Oh dear. Yes, that is a problem. The difficulty is that viper modifies
the undo-list, removing boundaries only *after* we leave insert mode.
Hence, when you type C-/ above they are still there.

If you do

i helo C-b k escape C-/

before the undo changes, then the whole buffer is deleted also. So, undo
behaves different in insert mode and in <V> command mode.

Viper's solution of introducing a 'viper symbol is a nice one, but has
it's problems. If you do, for example

i helo C-b k C-/ C-/

We get an error from the undo system.

primitive-undo: Unrecognized entry in undo list viper

The deep problem here is that undo boundary == nil -- i.e. there is one
and only one symbol for undo-boundary. If it could carry a value, viper
could do this cleanly.

Only solution that leaps to mind is this:


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".
3) Instead of this....

(if (setq tmp (memq viper-buffer-undo-list-mark buffer-undo-list))

we should now already have the cons cell that represents the tail of the
buffer-undo-list.

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


> Have we been able to identify the original problem?

No, fraid not. Thought about it lots. Failed.

> The old code seemed "simple" enough that the problem was probably
> simple to fix (once identified).

Well, most problems are simple to fix once you actually know what they
are.

I shall look again at the old code, and see if I can figure it out.

Phil