Re: zle: vi mode: wrong undo handling on fresh lines

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

Peter Stephenson wrote:
> Or startvitext() and startvichange() may be buried too far in to want
> any changing.
> 
> I think this is starting to get somewhere, but I suspect it needs
> tweaking.  For example, should that synthesised 'i' really be an 'a'?

If someone has the overstrike option set, it should probably be 'R'.
Actually that option seems to be fully non-working for vi mode.

With a blank starting line, it really is much of a muchness. If pushed
for an answer, I'd say 'a'.

A non-blank starting line is possible with at least
accept-line-and-down-history and vi-push-input. In such cases, it
depends on the cursor position. If the cursor starts at the
beginning of the line then 'i' makes more sense.

> Is it OK to assume we're not in insert mode when vi-repeat is executed?

Yes. Of course someone can bind a key to the widget but that currently
just beeps which is fine. To do that in vi, you'd have to bind your key
to escape, followed by a dot.

> Some vi user will need to take over at this point.  All the action I'm
> aware of is in the functions I've changed and if you've looked at the
> code you understand it as well as I do.
> 
> I haven't thought further about the completion thing.  I've no idea
> how that ever did anything useful.

How about the following approach to the undo problem:
The variable undoing was serving as a flag to indicate whether each
change should be added as an undo event. In vi-mode this would be
skipped so that the whole vi change became a single undo change. What
this does is remove that handling and instead merge all the undo events
corresponding to the vi change in the vi-cmd-mode widget.

This means, undo done the vi way works as in vi. undo invoked in insert
mode works exactly as in emacs mode and exactly as it used to do (up
till the first time vi command mode is invoked after which there are
differences). It also avoids the need for any special extra
vi-complete-word etc widgets that call handleundo(). The main
disadvantage is that undo will appear to be doing different things in
insert verses command mode which would probably need documenting under
the undo widget even though the change is in vi-cmd-mode. Any thoughts?

Are there other ways to get into vi command mode besides vi-cmd-mode
that might get around this? In vim, cursor movement in insert mode works
as if you had switched to command mode so we have some differences
there.

Oliver

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index a2b20df..f5aec84 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -157,10 +157,10 @@ mod_export char *statusline;
 /**/
 int stackhist, stackcs;
 
-/* != 0 if we are making undo records */
+/* position in undo stack from when the current vi change started */
 
 /**/
-int undoing;
+zlong vistartchange;
 
 /* current modifier status */
 
@@ -1080,8 +1080,7 @@ zlecore(void)
 	    if (invicmdmode() && zlecs > findbol() &&
 		(zlecs == zlell || zleline[zlecs] == ZWC('\n')))
 		DECCS();
-	    if (undoing)
-		handleundo();
+	    handleundo();
 	} else {
 	    errflag = 1;
 	    break;
@@ -1190,7 +1189,7 @@ zleread(char **lp, char **rp, int flags, int context, char *init, char *finish)
     zlereadflags = flags;
     zlecontext = context;
     histline = curhist;
-    undoing = 1;
+    vistartchange = 0;
     zleline = (ZLE_STRING_T)zalloc(((linesz = 256) + 2) * ZLE_CHAR_SIZE);
     *zleline = ZWC('\0');
     virangeflag = lastcmd = done = zlecs = zlell = mark = 0;
@@ -1198,6 +1197,7 @@ zleread(char **lp, char **rp, int flags, int context, char *init, char *finish)
     viinsbegin = 0;
     statusline = NULL;
     selectkeymap("main", 1);
+    initundo();
     /*
      * If main is linked to the viins keymap, we need to register
      * explicitly that we're now in vi insert mode as there's
@@ -1222,7 +1222,6 @@ zleread(char **lp, char **rp, int flags, int context, char *init, char *finish)
 	    stackhist = -1;
 	}
     }
-    initundo();
     if (isset(PROMPTCR))
 	putc('\r', shout);
     if (tmout)
diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index 9d163ad..2689d0f 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -611,8 +611,7 @@ docomplete(int lst)
     active = 1;
     comprecursive = 0;
     makecommaspecial(0);
-    if (undoing)
-	setlastline();
+    setlastline();
 
     /* From the C-code's point of view, we can only use compctl as a default
      * type of completion. Load it if it hasn't been loaded already and
diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index b82e54c..61ae85c 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -1354,7 +1354,10 @@ handlesuffix(UNUSED(char **args))
 
 /* head of the undo list, and the current position */
 
-static struct change *changes, *curchange;
+/**/
+struct change *curchange;
+
+static struct change *changes;
 
 /* list of pending changes, not yet in the undo system */
 
diff --git a/Src/Zle/zle_vi.c b/Src/Zle/zle_vi.c
index 31f2933..9e9cc2f 100644
--- a/Src/Zle/zle_vi.c
+++ b/Src/Zle/zle_vi.c
@@ -107,7 +107,7 @@ startvitext(int im)
 {
     startvichange(im);
     selectkeymap("main", 1);
-    undoing = 0;
+    vistartchange = (curchange && curchange->prev) ? curchange->prev->changeno : 0;
     viinsbegin = zlecs;
 }
 
@@ -399,7 +399,7 @@ vichange(UNUSED(char **args))
 	forekill(c2 - zlecs, CUT_RAW);
 	selectkeymap("main", 1);
 	viinsbegin = zlecs;
-	undoing = 0;
+	vistartchange = curchange->prev->changeno;
     }
     return ret;
 }
@@ -584,7 +584,13 @@ vicmdmode(UNUSED(char **args))
 {
     if (invicmdmode() || selectkeymap("vicmd", 0))
 	return 1;
-    undoing = 1;
+    struct change *current = curchange->prev;
+    while (current && current->changeno > vistartchange+1) {
+	current->flags |= CH_PREV;
+	current = current->prev;
+	if (!current) break;
+	current->flags |= CH_NEXT;
+    }
     vichgflag = 0;
     if (zlecs != findbol())
 	DECCS();