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

Previous Next

Full log

Message #151 received at 22295 <at> debbugs.gnu.org (full text, mbox):

From: phillip.lord <at> russet.org.uk (Phillip Lord)
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 22295 <at> debbugs.gnu.org,
 Jim Meyering <jim <at> meyering.net>, Michael Kifer <kifer <at> cs.stonybrook.edu>
Subject: Re: bug#22295: viper-mode undo bug introduced between Nov 10 and Nov
 14
Date: Fri, 10 Jun 2016 23:18:29 +0100

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

>>> 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.
>
> The problem seems to be the following: when you hit ESC, Viper will remove
> boundaries from the undo-list, without making any changes to the buffer,
> so the top-level loop won't add a boundary before the next command and
> ends up hence "amalgamating" the next command with the previous one.
>
> In the old code, we just always blindly added a boundary to
> current-buffer before running a command, whereas now we only do so if
> the buffer has been modified since the last time we pushed a boundary.
>
> I think the patch below fixes the original problem.
> Another way to fix it would be to change undo-auto--add-boundary so it
> always considers (current-buffer) regardless of
> undo-auto--undoably-changed-buffers.

Stefan

Many thanks for working this out -- I was going crazy trying to track
this down, and I was looking in the wrong place all along. 

I've tried your patch against emacs-25 (after reverting c0139e32f1f3b)
and it appears to work. The current behaviour of undo-auto--add-boundary
seems sensible to me; the difference in behaviour only shows up because
viper is manipulating the undo-list. I can try modifying u-a--a-b though
-- viper is, I am sure, not the only package to fiddle with undo.

I'd propose adding this to emacs-25. Eli, are you happy with this --- my
last fix appears to not work and Stefans approach is much more discrete.


As a separate issue, I'd like to add part of c0139e32f1f3b back
though to master -- adding the variable undo-auto-disable-boundaries.
It's not immediately necessary now, but it seems a useful option to
have.

Phil

Previous Next

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