PATCH: make kill ring work in vi mode

PATCH: make kill ring work in vi mode

[Oliver Kiddle]

This makes yank-pop also work together with vi-put-before and
vi-put-after. I find this a lot more usable than trying to work out
which of the vi numbered registers is applicable. In vim, this requires
a plugin (see https://github.com/maxbrunsfeld/vim-yankstack). 

Rather than make a bunch of static variables global, I've moved the vi
put functions to zle_misc.c. So startvichange() loses 'static' instead.
Any objections or alternative suggestions to this? There's a certain
amount of commonality between the four affected functions, should I
perhaps try to factor bits out into some smaller functions?

I'm not too happy with the way this interacts with undo. I'd prefer that
undo would revert everything to before the put instead of stepping back
through each yank-pop. This seems to be consistent with emacs though so
I'm probably stuck with it. I may also look further at what happens with
. (vi-repeat-change).

Oliver

diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo
index 881e56b..06b1659 100644
--- a/Doc/Zsh/zle.yo
+++ b/Doc/Zsh/zle.yo
@@ -1786,7 +1786,7 @@ tindex(yank-pop)
 item(tt(yank-pop) (ESC-y) (unbound) (unbound))(
 Remove the text just yanked, rotate the kill-ring (the history of
 previously killed text) and yank the new top.  Only works following
-tt(yank) or tt(yank-pop).
+tt(yank), tt(vi-put-before), tt(vi-put-after) or tt(yank-pop).
 )
 tindex(vi-yank)
 item(tt(vi-yank) (unbound) (y) (unbound))(
diff --git a/Src/Zle/iwidgets.list b/Src/Zle/iwidgets.list
index 79d62c7..dc7e800 100644
--- a/Src/Zle/iwidgets.list
+++ b/Src/Zle/iwidgets.list
@@ -159,8 +159,8 @@
 "vi-open-line-below", viopenlinebelow, 0
 "vi-oper-swap-case", vioperswapcase, ZLE_LASTCOL
 "vi-pound-insert", vipoundinsert, 0
-"vi-put-after", viputafter, ZLE_YANK | ZLE_KEEPSUFFIX
-"vi-put-before", viputbefore, ZLE_YANK | ZLE_KEEPSUFFIX
+"vi-put-after", viputafter, ZLE_YANKAFTER | ZLE_KEEPSUFFIX
+"vi-put-before", viputbefore, ZLE_YANKBEFORE | ZLE_KEEPSUFFIX
 "vi-quoted-insert", viquotedinsert, ZLE_MENUCMP | ZLE_KEEPSUFFIX
 "vi-repeat-change", virepeatchange, 0
 "vi-repeat-find", virepeatfind, 0
@@ -182,5 +182,5 @@
 "what-cursor-position", whatcursorposition, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
 "where-is", whereis, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
 "which-command", processcmd, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
-"yank", yank, ZLE_YANK | ZLE_KEEPSUFFIX
-"yank-pop", yankpop, ZLE_YANK | ZLE_KEEPSUFFIX
+"yank", yank, ZLE_YANKBEFORE | ZLE_KEEPSUFFIX
+"yank-pop", yankpop, ZLE_KEEPSUFFIX | ZLE_NOTCOMMAND
diff --git a/Src/Zle/zle.h b/Src/Zle/zle.h
index dd6cdcc..8a85ee3 100644
--- a/Src/Zle/zle.h
+++ b/Src/Zle/zle.h
@@ -203,13 +203,15 @@ struct widget {
 #define WIDGET_INT	(1<<0)	/* widget is internally implemented */
 #define WIDGET_NCOMP    (1<<1)	/* new style completion widget */
 #define ZLE_MENUCMP	(1<<2)	/* DON'T invalidate completion list */
-#define ZLE_YANK	(1<<3)
-#define ZLE_LINEMOVE	(1<<4)	/* command is a line-oriented movement */
-#define ZLE_LASTCOL     (1<<5)	/* command maintains lastcol correctly */
-#define ZLE_KILL	(1<<6)
-#define ZLE_KEEPSUFFIX	(1<<7)	/* DON'T remove added suffix */
-#define ZLE_NOTCOMMAND  (1<<8)	/* widget should not alter lastcmd */
-#define ZLE_ISCOMP      (1<<9)	/* usable for new style completion */
+#define ZLE_YANKAFTER	(1<<3)
+#define ZLE_YANKBEFORE	(1<<4)
+#define ZLE_YANK        (ZLE_YANKAFTER | ZLE_YANKBEFORE)
+#define ZLE_LINEMOVE	(1<<5)	/* command is a line-oriented movement */
+#define ZLE_LASTCOL     (1<<6)	/* command maintains lastcol correctly */
+#define ZLE_KILL	(1<<7)
+#define ZLE_KEEPSUFFIX	(1<<8)	/* DON'T remove added suffix */
+#define ZLE_NOTCOMMAND  (1<<9)	/* widget should not alter lastcmd */
+#define ZLE_ISCOMP      (1<<10)	/* usable for new style completion */
 
 /* thingies */
 
diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index 9bc1cf6..10bd71c 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -475,8 +475,10 @@ copyregionaskill(char **args)
 /*
  * kct: index into kill ring, or -1 for original cutbuffer of yank.
  * yankb, yanke: mark the start and end of last yank in editing buffer.
+ * yankcs marks the cursor position preceding the last yank
  */
-static int kct, yankb, yanke;
+static int kct, yankb, yanke, yankcs;
+
 /* The original cutbuffer, either cutbuf or one of the vi buffers. */
 static Cutbuffer kctbuf;
 
@@ -494,8 +496,7 @@ yank(UNUSED(char **args))
 	kctbuf = &cutbuf;
     if (!kctbuf->buf)
 	return 1;
-    mark = zlecs;
-    yankb = zlecs;
+    yankb = yankcs = mark = zlecs;
     while (n--) {
 	kct = -1;
 	spaceinline(kctbuf->len);
@@ -508,6 +509,84 @@ yank(UNUSED(char **args))
 
 /**/
 int
+viputbefore(UNUSED(char **args))
+{
+    int n = zmult;
+
+    startvichange(-1);
+    if (n < 0 || zmod.flags & MOD_NULL)
+	return 1;
+    if (zmod.flags & MOD_VIBUF)
+	kctbuf = &vibuf[zmod.vibuf];
+    else
+	kctbuf = &cutbuf;
+    if (!kctbuf->buf)
+	return 1;
+    kct = -1;
+    yankcs = zlecs;
+    if (kctbuf->flags & CUTBUFFER_LINE) {
+	yankb = zlecs = findbol();
+	spaceinline(kctbuf->len + 1);
+	ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
+	yanke = zlecs + kctbuf->len + 1;
+	zleline[zlecs + kctbuf->len] = ZWC('\n');
+	vifirstnonblank(zlenoargs);
+    } else {
+	yankb = zlecs;
+	while (n--) {
+	    spaceinline(kctbuf->len);
+	    ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
+	    zlecs += kctbuf->len;
+	}
+	yanke = zlecs;
+	if (zlecs)
+	    DECCS();
+    }
+    return 0;
+}
+
+/**/
+int
+viputafter(UNUSED(char **args))
+{
+    int n = zmult;
+
+    startvichange(-1);
+    if (n < 0 || zmod.flags & MOD_NULL)
+	return 1;
+    if (zmod.flags & MOD_VIBUF)
+	kctbuf = &vibuf[zmod.vibuf];
+    else
+	kctbuf = &cutbuf;
+    if (!kctbuf->buf)
+	return 1;
+    kct = -1;
+    yankcs = zlecs;
+    if (kctbuf->flags & CUTBUFFER_LINE) {
+	yankb = zlecs = findeol();
+	spaceinline(kctbuf->len + 1);
+	zleline[zlecs++] = ZWC('\n');
+	yanke = zlecs + kctbuf->len;
+	ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
+	vifirstnonblank(zlenoargs);
+    } else {
+	if (zlecs != findeol())
+	    INCCS();
+	yankb = zlecs;
+	while (n--) {
+	    spaceinline(kctbuf->len);
+	    ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
+	    zlecs += kctbuf->len;
+	}
+	yanke = zlecs;
+	if (zlecs)
+	    DECCS();
+    }
+    return 0;
+}
+
+/**/
+int
 yankpop(UNUSED(char **args))
 {
     int cc, kctstart = kct;
@@ -557,11 +636,33 @@ yankpop(UNUSED(char **args))
 
     zlecs = yankb;
     foredel(yanke - yankb, CUT_RAW);
-    cc = buf->len;
-    spaceinline(cc);
-    ZS_memcpy(zleline + zlecs, buf->buf, cc);
-    zlecs += cc;
-    yanke = zlecs;
+    zlecs = yankcs;
+
+    if (buf->flags & CUTBUFFER_LINE) {
+	if (lastcmd & ZLE_YANKBEFORE) {
+	    yankb = zlecs = findbol();
+	    spaceinline(buf->len + 1);
+	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
+	    yanke = zlecs + buf->len + 1;
+	    zleline[zlecs + buf->len] = ZWC('\n');
+	} else {
+	    yankb = zlecs = findeol();
+	    spaceinline(buf->len + 1);
+	    zleline[zlecs++] = ZWC('\n');
+	    yanke = zlecs + buf->len;
+	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
+	}
+	vifirstnonblank(zlenoargs);
+    } else {
+	if ((lastcmd & ZLE_YANKAFTER) && zlecs != findeol())
+	    INCCS();
+	yankb = zlecs;
+	cc = buf->len;
+	spaceinline(cc);
+	ZS_memcpy(zleline + zlecs, buf->buf, cc);
+	zlecs += cc;
+	yanke = zlecs;
+    }
     return 0;
 }
 
diff --git a/Src/Zle/zle_vi.c b/Src/Zle/zle_vi.c
index 0a8b27d..b0e696b 100644
--- a/Src/Zle/zle_vi.c
+++ b/Src/Zle/zle_vi.c
@@ -75,7 +75,7 @@ static int inrepeat, vichgrepeat;
  */
 
 /**/
-static void
+void
 startvichange(int im)
 {
     if (im != -1) {
@@ -793,72 +793,6 @@ vikillline(UNUSED(char **args))
 
 /**/
 int
-viputbefore(UNUSED(char **args))
-{
-    Cutbuffer buf = &cutbuf;
-    int n = zmult;
-
-    startvichange(-1);
-    if (n < 0 || zmod.flags & MOD_NULL)
-	return 1;
-    if (zmod.flags & MOD_VIBUF)
-	buf = &vibuf[zmod.vibuf];
-    if (!buf->buf)
-	return 1;
-    if(buf->flags & CUTBUFFER_LINE) {
-	zlecs = findbol();
-	spaceinline(buf->len + 1);
-	ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
-	zleline[zlecs + buf->len] = ZWC('\n');
-	vifirstnonblank(zlenoargs);
-    } else {
-	while (n--) {
-	    spaceinline(buf->len);
-	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
-	    zlecs += buf->len;
-	}
-	if (zlecs)
-	    DECCS();
-    }
-    return 0;
-}
-
-/**/
-int
-viputafter(UNUSED(char **args))
-{
-    Cutbuffer buf = &cutbuf;
-    int n = zmult;
-
-    startvichange(-1);
-    if (n < 0 || zmod.flags & MOD_NULL)
-	return 1;
-    if (zmod.flags & MOD_VIBUF)
-	buf = &vibuf[zmod.vibuf];
-    if (!buf->buf)
-	return 1;
-    if(buf->flags & CUTBUFFER_LINE) {
-	zlecs = findeol();
-	spaceinline(buf->len + 1);
-	zleline[zlecs++] = ZWC('\n');
-	ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
-	vifirstnonblank(zlenoargs);
-    } else {
-	if (zlecs != findeol())
-	    INCCS();
-	while (n--) {
-	    spaceinline(buf->len);
-	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
-	    zlecs += buf->len;
-	}
-	if (zlecs)
-	    DECCS();
-    }
-    return 0;
-}
-
-/**/
-int
 vijoin(UNUSED(char **args))
 {
     int x, pos;

Re: PATCH: make kill ring work in vi mode

[Bart Schaefer]

On Oct 30, 12:42am, Oliver Kiddle wrote:
}
} I'm not too happy with the way this interacts with undo. I'd prefer that
} undo would revert everything to before the put instead of stepping back
} through each yank-pop. This seems to be consistent with emacs though so
} I'm probably stuck with it. I may also look further at what happens with
} . (vi-repeat-change).

I think you should be able to store an undo point somewhere specific to
vi-mode and then undo back to that point?  Just internally play around
with UNDO_CHANGE_NO?

-- 
Barton E. Schaefer

Re: PATCH: make kill ring work in vi mode

[Peter Stephenson]

On Thu, 30 Oct 2014 00:42:53 +0100
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Rather than make a bunch of static variables global, I've moved the vi
> put functions to zle_misc.c. So startvichange() loses 'static' instead.
> Any objections or alternative suggestions to this? There's a certain
> amount of commonality between the four affected functions, should I
> perhaps try to factor bits out into some smaller functions?

Nothing here's a real problem, though if it's possible to refactor it
more neatly that's nice to have.
 
> I'm not too happy with the way this interacts with undo. I'd prefer that
> undo would revert everything to before the put instead of stepping back
> through each yank-pop. This seems to be consistent with emacs though so
> I'm probably stuck with it. I may also look further at what happens with
> . (vi-repeat-change).

You might be able to wrap the behaviour of yank-pop with a function that
uses the variable recording the undo state and updates the state
appropriately, but I haven't thought this through.

pws

Re: PATCH: make kill ring work in vi mode

[Oliver Kiddle]

Peter wrote:
> 
> Nothing here's a real problem, though if it's possible to refactor it
> more neatly that's nice to have.

Patch below refactors and adds tests. I left yank() alone which you
might question.

I think I'll ignore the undo issue until I've used it in practice
awhile. At least the current behaviour avoids the need for a
reverse-yank-pop.

Oliver

diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index 10bd71c..3d4a9bb 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -507,6 +507,40 @@ yank(UNUSED(char **args))
     return 0;
 }
 
+static void pastebuf(Cutbuffer buf, int mult, int after)
+{
+    int cc;
+    if (buf->flags & CUTBUFFER_LINE) {
+	if (after) {
+	    yankb = zlecs = findeol();
+	    spaceinline(buf->len + 1);
+	    zleline[zlecs++] = ZWC('\n');
+	    yanke = zlecs + buf->len;
+	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
+	} else {
+	    yankb = zlecs = findbol();
+	    spaceinline(buf->len + 1);
+	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
+	    yanke = zlecs + buf->len + 1;
+	    zleline[zlecs + buf->len] = ZWC('\n');
+	}
+	vifirstnonblank(zlenoargs);
+    } else {
+	if (after && zlecs != findeol())
+	    INCCS();
+	yankb = zlecs;
+	cc = buf->len;
+	while (mult--) {
+	    spaceinline(cc);
+	    ZS_memcpy(zleline + zlecs, buf->buf, cc);
+	    zlecs += cc;
+	}
+	yanke = zlecs;
+	if (zlecs)
+	    DECCS();
+    }
+}
+
 /**/
 int
 viputbefore(UNUSED(char **args))
@@ -524,24 +558,7 @@ viputbefore(UNUSED(char **args))
 	return 1;
     kct = -1;
     yankcs = zlecs;
-    if (kctbuf->flags & CUTBUFFER_LINE) {
-	yankb = zlecs = findbol();
-	spaceinline(kctbuf->len + 1);
-	ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
-	yanke = zlecs + kctbuf->len + 1;
-	zleline[zlecs + kctbuf->len] = ZWC('\n');
-	vifirstnonblank(zlenoargs);
-    } else {
-	yankb = zlecs;
-	while (n--) {
-	    spaceinline(kctbuf->len);
-	    ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
-	    zlecs += kctbuf->len;
-	}
-	yanke = zlecs;
-	if (zlecs)
-	    DECCS();
-    }
+    pastebuf(kctbuf, n, 0);
     return 0;
 }
 
@@ -562,26 +579,7 @@ viputafter(UNUSED(char **args))
 	return 1;
     kct = -1;
     yankcs = zlecs;
-    if (kctbuf->flags & CUTBUFFER_LINE) {
-	yankb = zlecs = findeol();
-	spaceinline(kctbuf->len + 1);
-	zleline[zlecs++] = ZWC('\n');
-	yanke = zlecs + kctbuf->len;
-	ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
-	vifirstnonblank(zlenoargs);
-    } else {
-	if (zlecs != findeol())
-	    INCCS();
-	yankb = zlecs;
-	while (n--) {
-	    spaceinline(kctbuf->len);
-	    ZS_memcpy(zleline + zlecs, kctbuf->buf, kctbuf->len);
-	    zlecs += kctbuf->len;
-	}
-	yanke = zlecs;
-	if (zlecs)
-	    DECCS();
-    }
+    pastebuf(kctbuf, n, 1);
     return 0;
 }
 
@@ -589,7 +587,7 @@ viputafter(UNUSED(char **args))
 int
 yankpop(UNUSED(char **args))
 {
-    int cc, kctstart = kct;
+    int kctstart = kct;
     Cutbuffer buf;
 
     if (!(lastcmd & ZLE_YANK) || !kring || !kctbuf) {
@@ -637,32 +635,7 @@ yankpop(UNUSED(char **args))
     zlecs = yankb;
     foredel(yanke - yankb, CUT_RAW);
     zlecs = yankcs;
-
-    if (buf->flags & CUTBUFFER_LINE) {
-	if (lastcmd & ZLE_YANKBEFORE) {
-	    yankb = zlecs = findbol();
-	    spaceinline(buf->len + 1);
-	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
-	    yanke = zlecs + buf->len + 1;
-	    zleline[zlecs + buf->len] = ZWC('\n');
-	} else {
-	    yankb = zlecs = findeol();
-	    spaceinline(buf->len + 1);
-	    zleline[zlecs++] = ZWC('\n');
-	    yanke = zlecs + buf->len;
-	    ZS_memcpy(zleline + zlecs, buf->buf, buf->len);
-	}
-	vifirstnonblank(zlenoargs);
-    } else {
-	if ((lastcmd & ZLE_YANKAFTER) && zlecs != findeol())
-	    INCCS();
-	yankb = zlecs;
-	cc = buf->len;
-	spaceinline(cc);
-	ZS_memcpy(zleline + zlecs, buf->buf, cc);
-	zlecs += cc;
-	yanke = zlecs;
-    }
+    pastebuf(buf, 1, lastcmd & ZLE_YANKAFTER);
     return 0;
 }
 
diff --git a/Test/X02zlevi.ztst b/Test/X02zlevi.ztst
index 2af6f06..185980b 100644
--- a/Test/X02zlevi.ztst
+++ b/Test/X02zlevi.ztst
@@ -116,6 +116,34 @@
 >BUFFER: stnwararart
 >CURSOR: 9
 
+  zpty_run 'bindkey -a "^P" yank-pop'
+  zletest $'word\C-wline\eddiSE\eP\C-P'
+0:line based put before followed by character based yank-pop
+>BUFFER: SwordE
+>CURSOR: 4
+
+  zletest $'line\eddiword\C-w\eiSE\eP\C-P'
+0:character based put before followed by line based yank-pop
+>BUFFER: line
+>SE
+>CURSOR: 0
+
+  zletest $'one two three\C-w\C-w\C-wSE\e0p\C-P\C-P'
+0:put after cycled twice with yank-pop
+>BUFFER: SthreeE
+>CURSOR: 5
+
+  zletest $'word\C-wline\eddiSE\ehp\C-P'
+0:line based put after followed by character based yank-pop
+>BUFFER: SwordE
+>CURSOR: 5
+
+  zletest $'line\eddiword\C-w\eiSE\ehp\C-P'
+0:character based after before followed by line based yank-pop
+>BUFFER: SE
+>line
+>CURSOR: 3
+
   zletest $'word\euaend'
 0:undo initial change
 >BUFFER: end

Re: PATCH: make kill ring work in vi mode

[Vin Shelton]

Oliver -

I see a test failure from the latest sources; it looks related to your
recent yank/pop changes:

ZTST_VERBOSE=3 make TESTNUM=X02 check
cd Test ; make check
make[1]: Entering directory `/raid-3tb/opt/build/zsh-2014-10-31/Test'
if test -n "gcc"; then \
 cd .. && DESTDIR= \
 make MODDIR=`pwd`/Test/Modules install.modules > /dev/null; \
fi
if ZTST_testlist="`for f in /opt/src/zsh-2014-10-31/Test/X02*.ztst; \
           do echo $f; done`" \
ZTST_srcdir="/opt/src/zsh-2014-10-31/Test" \
ZTST_exe=../Src/zsh \
../Src/zsh +Z -f /opt/src/zsh-2014-10-31/Test/runtests.zsh; then \
stat=0; \
else \
stat=1; \
fi; \
sleep 1; \
rm -rf Modules .zcompdump; \
exit $stat
/opt/src/zsh-2014-10-31/Test/X02zlevi.ztst: starting.
*** /tmp/zsh.ztst.out.10011 Fri Oct 31 07:31:52 2014
--- /tmp/zsh.ztst.tout.10011 Fri Oct 31 07:31:52 2014
***************
*** 1,2 ****
  BUFFER: SwordE
! CURSOR: 5
--- 1,2 ----
  BUFFER: SwordE
! CURSOR: 4
Test /opt/src/zsh-2014-10-31/Test/X02zlevi.ztst failed: output differs
from expected as shown above for:
  zletest $'word\C-wline\eddiSE\ehp\C-P'
Was testing: line based put after followed by character based yank-pop
/opt/src/zsh-2014-10-31/Test/X02zlevi.ztst: test failed.
**************************************
0 successful test scripts, 1 failure, 0 skipped
**************************************
make[1]: *** [check] Error 1
make[1]: Leaving directory `/raid-3tb/opt/build/zsh-2014-10-31/Test'
make: *** [check] Error 2

This is on linux mint 17, in case that matters.

Regards,
  Vin