GNU bug report logs - #22295
viper-mode undo bug introduced between Nov 10 and Nov 14

Previous Next

Full log

View this message in rfc822 format

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Kifer <kifer <at> cs.stonybrook.edu>, 22295 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Jim Meyering <jim <at> meyering.net>
Subject: bug#22295: viper-mode undo bug introduced between Nov 10 and Nov 14
Date: Thu, 02 Jun 2016 09:45:27 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.