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

[Phillip Lord]

Jim Meyering <jim@meyering.net> writes:

> On Mon, May 16, 2016 at 5:41 AM, Phillip Lord
> <phillip.lord@russet.org.uk> wrote:
> ...
>> I have pushed a first attempt at a fix to branch fix/viper-undo.
>> Clearly, it's not finished yet, but I'd like to get your feedback as to
>> whether it works; it should fix the example given, but I haven't used
>> viper at all, so if it breaks other things I won't know.
>>
>> I haven't managed to work out exactly why the error is happening --
>> probably though because the timing of the undo-boundary is somewhat
>> different, though. With this fix, viper just disables automatic boundary
>> addition and adds it's own as necessary, which seems cleaner.
>
> Thank you.
> That does indeed fix the case I mentioned.
> Here are some cases where it does not work as expected:
>
>   start with an empty buffer in viper-mode
>   type 'i 1 2 3 4 5ESC'
>   type 'F2dw' to delete the 2 and a space.
>   type 'wdw' to delete the 4 and a space.
>
> Now, if I were to hit "u" to undo, I would expect that most recent
> deletion to be undone and the 4 would reappear.
> Then I would hit '.' to undo the deletion of the '2'. Finally one more
> '.' would undo the creation of that first line.
> However, with the current patches, that first 'u' undoes everything
> and leaves me with the empty initial file.
>
> Another example starting with an empty file:
> Create some content via ':r!seq 999|fmt RETURN'
> Then remove e.g., "222 " and "444 " via '/222' RET 'dw',
> then '/444' RET 'dw'. Now, we expect a single 'u' to restore the '444 ',
> yet it undoes everything, leaving an empty buffer.
> Hmm... that's probably no different from the first example.
>
> One more, then. Starting with this input:
>
>   1 2 3 4 5 6
>
> advance to the '2' with 'w', 'dw' to delete the 2, then three '.'s to
> delete the 3, then 4 and 5.
> Then begin to undo with 'u', then '.' to repeat it. Those first two
> work, restoring the 5 and 4.
> However, one more '.' restores both the 3 and the 2.

How do you ever learn all of these keypresses?

I've pushed to fix/viper-undo, and AFAICT, it handles all of these
situations.

I *thought* viper was essentially switching off Emacs undo-boundary
behaviour, but, actually, it only does this very specific circumstances
(which it does by adding 'viper, then deleting all the other boundaries
upto 'viper). So, the fix I have now just disables the addition of
undo-boundaries when 'viper would have been added, and switches them
back on again when the boundaries and 'viper would have been removed.
This *should* behave the same. As an added bonus, the code is quite a
bit simpler.

Can you test a bit further for me? Any key sequences you want to send
would be gratefully recieved; I'm converting them into kbd-macros and
will use them as test cases. So:

  (execute-kbd-macro
   [
    ?i
    ?1 ?  ?2 ?  ?3 ?  ?4 ?  ?5
    escape
    ?F ?2 ?d ?w
    ?w ?d ?w
    ?u
    ])

in a clean buffer should leave "1 2 4 5".

Phil