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

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 14 May 2016 12:25:13 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: 22295@debbugs.gnu.org, Jim Meyering <jim@meyering.net>
>> 
>> > From: Jim Meyering <jim@meyering.net>
>> > Date: Sat, 2 Jan 2016 20:01:36 -0800
>> > 
>> > Hello,
>> > I noticed that viper-mode's "undo" ('u') command began to undo too much
>> > and was able quickly to determine that it worked fine with my snapshot
>> > built from git master some time on Nov 10, yet that it began to undo
>> > too much four days later.
>> > 
>> > To demonstrate the problem (without risking changing anything in your
>> > home directory), run this:
>> > 
>> >   mkdir /tmp/x && HOME=/tmp/x emacs -Q -f viper-mode -nw
>> > ~/previously-nonexistent-file
>> > 
>> > then respond "y", "y", "5" to get past the "viperize" setup questions.
>> > To reproduce the error, insert two lines, terminating each "insertion" 
>> > with ESC,
>> > so that each is recorded as a separate undo'able operation. I.e., type this
>> > 
>> >    a 1 ESC
>> > 
>> > to create the first line, then
>> > 
>> >   o 2 ESC
>> > 
>> > to create the second.
>> > Finally, hit "u" to undo creation of the second and you'll see that it 
>> > undoes
>> > both operations, erasing both lines.  This is rather disruptive when that 
>> > first
>> > bit of text was a long paragraph or two -- the novice may think that it's 
>> > lost,
>> > because redo does not restore it -- however, it is available in emacs's
>> > yank buffer.
>> 
>> Phillip, could you please look into this?  This sounds like a annoying
>> problem for users of viper-mode, and AFAIU it happens on the release
>> branch as well.
>
> (Adding Michael to the addressees.)
>
> I took a short look, and it sounds like we need more experts here.
> Undo in viper has its own implementation, which tries to do something
> that is not immediately clear to me, and is not really documented
> anywhere.  I guess vi users will know that, but I'm not one of them.
>
> The viper-undo command and related functions manipulate the Emacs undo
> data structures directly, see viper-adjust-undo.  I guess the recent
> changes in low-level undo implementation run afoul of what viper-mode
> tries to do.
>
> I hope the above provides enough hints to find the reason for this
> problem and solve it.
>


Sorry for slow response -- was travelling.

Yep, viper is doing strange things to undo -- it adds a symbol ('viper)
to the undo list, then removes it later, amalgamating everything upto
'viper.

I've got a complete test case (below in case anyone is interested --
I'll make a proper unit test of it on master eventually).


On e0f64e7b4f9c3bbc12c4909ca8c8aa751f1fca4a (a random commit around
the time of the error). This produced a undo list like so:

(nil 
 (2 . 4) 
 nil 
 (2 . 3) 
 (1 . 2) 
 (t . -1))

While on emacs-25 it gives (my comments):


((3 . 4) ;; insertion from 3 to 4
 (2 . 3) ;; insertion from 2 to 3
 nil     ;; boundary
 (2 . 3) ;; insertion from 2 to 3
 (1 . 2) ;; insertion from 1 to 2
 (t . -1) ;; buffer created with file that does not exist
)


So it looks like the amalgamation that Emacs is supposed to be doing is
failing. No idea at all where the head "nil" has gone.

Will work on this more.



(setq viper-inhibit-startup-message 't)
(setq viper-expert-level '5)

(find-file "/tmp/file.txt")
(setq viper-mode t)
(require 'viper)

;; leave emacs lisp in Emacs mode or edebug becomes impossible
(setq viper-vi-state-mode-list (delq
                                'emacs-lisp-mode
                                viper-vi-state-mode-list))

(setq viper-emacs-state-mode-list (cons
                                'emacs-lisp-mode
                                viper-vi-state-mode-list))
;; alas edebug "print last eval" is still broken


(require 'edebug)
(edebug-instrument-function 'viper-adjust-undo)

;; a 1
(execute-kbd-macro
   [?a ?1 escape ?o ?2 escape])