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

[Michael Kifer <kifer@cs.stonybrook.edu>]

On 06/01/2016 06:23 PM, Phillip Lord wrote:
> 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

not in  25.0.50
viper worked fine with this for 20 years and I'd say it is Emacs 
breakage, not Viper's.


> 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.

Do you know who made the changes to the emacs undo list? Maybe complain 
to him?



>
>> Have we been able to identify the original problem?
> No, fraid not. Thought about it lots. Failed.

could not  understand what happened either.

--

        --- michael


>
>> 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