Re: bug in replace-string: widget loses characters

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On 9 October 2012 12:22, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> I think it's a problem with "undo".  After the function has read the
> original and replacement strings from the area under the command line,
> it's supposed to undo all that, so that you don't see it as part of the
> normal undo history (undo should take you back through the replacement,
> then immediately back through any changes you made before invoking the
> replace widget).  It looks like that undo can go one change too far,
> which means I've got the counting of undo changes wrong somewhere.
>
> I didn't really properly get to grips with the undo system,
> so this isn't that surprising. I suppose it's no use hoping
> anybody else is going to understand it.  I'll take a look
> later.

I think this fix should be robust.  It increments the undo change number
at the point where we pass it as the value of the variable.  This looks
like a cop out, but it does absolutely guarantee the separation of
changes before and after that point regardless of where the wires were
attached to the fence posts.  The zlong range should be big enough to
cope.

The one side effect is that $UNDO_CHANGE_NO is different each time it's
read.  I could fix that if anyone cared, but if it helped you could sort
of look on it as a feature: you are guaranteed that all recorded change
points are unique and monotonically increasing whether or not the line
itself was changed.

Index: Src/Zle/zle_utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_utils.c,v
retrieving revision 1.63
diff -p -u -r1.63 zle_utils.c
--- Src/Zle/zle_utils.c	29 Mar 2012 20:31:33 -0000	1.63
+++ Src/Zle/zle_utils.c	9 Oct 2012 14:24:35 -0000
@@ -1520,23 +1520,25 @@ setlastline(void)
 int
 undo(char **args)
 {
-    zlong last_change = (zlong)0;
+    zlong last_change;

     if (*args)
-    {
 	last_change = zstrtol(*args, NULL, 0);
-    }
+    else
+	last_change = (zlong)-1;

     handleundo();
     do {
-	if(!curchange->prev)
+	struct change *prev = curchange->prev;
+	if(!prev)
 	    return 1;
-	if (unapplychange(curchange->prev))
-	    curchange = curchange->prev;
+	if (prev->changeno < last_change)
+	    break;
+	if (unapplychange(prev))
+	    curchange = prev;
 	else
 	    break;
-    } while (*args ? curchange->changeno != last_change :
-	     (curchange->flags & CH_PREV));
+    } while (last_change >= (zlong)0 || (curchange->flags & CH_PREV));
     setlastline();
     return 0;
 }
@@ -1660,6 +1662,11 @@ zlecallhook(char *name, char *arg)
 zlong
 get_undo_current_change(UNUSED(Param pm))
 {
-    return undo_changeno;
+    /*
+     * Increment the number in case a change is in progress;
+     * we don't want to back off what's already been done when
+     * we return to this change number.  This eliminates any
+     * problem about the point where a change is numbered.
+     */
+    return ++undo_changeno;
 }
-

pws