PATCH: cursor position after vi yank

PATCH: cursor position after vi yank

[Oliver Kiddle]

When producing the test cases, I noticed that zsh is not quite matching
vi in terms of cursor positioning after a yank. This fixes that.

Oliver

diff --git a/Src/Zle/zle_vi.c b/Src/Zle/zle_vi.c
index 20cece0..53919e3 100644
--- a/Src/Zle/zle_vi.c
+++ b/Src/Zle/zle_vi.c
@@ -457,7 +457,14 @@ viyank(UNUSED(char **args))
 	ret = 0;
     }
     vichgflag = 0;
-    zlecs = oldcs;
+    /* cursor now at the start of the range yanked. For line mode
+     * restore the column position */
+    if (vilinerange) {
+	while (oldcs > 0 && zleline[oldcs - 1] != ZWC('\n') &&
+		zlecs != zlell && zleline[zlecs] != ZWC('\n')) {
+	    ++zlecs; --oldcs;
+	}
+    }
     return ret;
 }
 
diff --git a/Test/X02zlevi.ztst b/Test/X02zlevi.ztst
index b4426a8..d9fa0d5 100644
--- a/Test/X02zlevi.ztst
+++ b/Test/X02zlevi.ztst
@@ -15,6 +15,28 @@
 >BUFFER: good
 >CURSOR: 4
 
+  zletest $'one two\eyb'
+0:yank left moves the cursor
+>BUFFER: one two
+>CURSOR: 4
+
+  zletest $'one two\e0ye'
+0:yank right leaves the cursor
+>BUFFER: one two
+>CURSOR: 0
+
+  zletest $'short\eoand longer\eyk'
+0:yank up line moves cursor up but not to buffer start
+>BUFFER: short
+>and longer
+>CURSOR: 4
+
+  zletest $'one\eotwo\ekyj'
+0:yank down line leaves the cursor
+>BUFFER: one
+>two
+>CURSOR: 2
+
   zletest $'yankee doodle\ebhDyy0"1P'
 0:paste register 1 to get last deletion
 >BUFFER:  doodleyankee

Re: PATCH: cursor position after vi yank

[Oliver Kiddle]

I wrote:
> When producing the test cases, I noticed that zsh is not quite matching
> vi in terms of cursor positioning after a yank. This fixes that.

I took another look at this code because I was concerned that I might
have got it wrong for unicode stuff. Has anyone got any tips on suitable
input for testing things like column positioning? And how to get that
input into a command-line? Trying to put combining characters in with:
  print -z 'a\u0308'
Results in a<0308>. I'm not sure if that was what I wanted or not.

Anyway, in the process I discovered that there is an existing lastcol
variable for preserving column position which I should have used so
this patch does that. The code for resetting the column position is now
copied from elsewhere but I'm not sure that I understand it. Does it
look right?

The difference this makes in behaviour is subtle enough that I'm
noticing differences between nvi, vim and Solaris vi. For the
combinations I've thought of, this is closer to all of them and
especially to vim.

The various widgets that read a movement command shouldn't clear
lastcol. Many movement operators will do that anyway but we should defer
to them.

Oliver

diff --git a/Src/Zle/iwidgets.list b/Src/Zle/iwidgets.list
index ea41a7f..79d62c7 100644
--- a/Src/Zle/iwidgets.list
+++ b/Src/Zle/iwidgets.list
@@ -102,9 +102,9 @@
 "self-insert-unmeta", selfinsertunmeta, ZLE_MENUCMP | ZLE_KEEPSUFFIX
 "send-break", sendbreak, 0
 "set-mark-command", setmarkcommand, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
-"split-undo", splitundo, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_NOTCOMMAND
+"split-undo", splitundo, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL | ZLE_NOTCOMMAND
 "spell-word", spellword, 0
-"set-local-history", setlocalhistory, 0
+"set-local-history", setlocalhistory, ZLE_LASTCOL
 "transpose-chars", transposechars, 0
 "transpose-words", transposewords, 0
 "undefined-key", undefinedkey, 0
@@ -122,12 +122,12 @@
 "vi-backward-kill-word", vibackwardkillword, ZLE_KEEPSUFFIX
 "vi-backward-word", vibackwardword, 0
 "vi-beginning-of-line", vibeginningofline, 0
-"vi-caps-lock-panic", vicapslockpanic, 0
-"vi-change", vichange, 0
+"vi-caps-lock-panic", vicapslockpanic, ZLE_LASTCOL
+"vi-change", vichange, ZLE_LASTCOL
 "vi-change-eol", vichangeeol, 0
 "vi-change-whole-line", vichangewholeline, 0
 "vi-cmd-mode", vicmdmode, 0
-"vi-delete", videlete, ZLE_KEEPSUFFIX
+"vi-delete", videlete, ZLE_KEEPSUFFIX | ZLE_LASTCOL
 "vi-delete-char", videletechar, ZLE_KEEPSUFFIX
 "vi-digit-or-beginning-of-line", vidigitorbeginningofline, 0
 "vi-down-line-or-history", vidownlineorhistory, ZLE_LINEMOVE
@@ -148,7 +148,7 @@
 "vi-goto-mark-line", vigotomarkline, 0
 "vi-history-search-backward", vihistorysearchbackward, 0
 "vi-history-search-forward", vihistorysearchforward, 0
-"vi-indent", viindent, 0
+"vi-indent", viindent, ZLE_LASTCOL
 "vi-insert", viinsert, 0
 "vi-insert-bol", viinsertbol, 0
 "vi-join", vijoin, 0
@@ -157,7 +157,7 @@
 "vi-match-bracket", vimatchbracket, 0
 "vi-open-line-above", viopenlineabove, 0
 "vi-open-line-below", viopenlinebelow, 0
-"vi-oper-swap-case", vioperswapcase, 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
@@ -172,11 +172,11 @@
 "vi-set-buffer", visetbuffer, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
 "vi-set-mark", visetmark, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
 "vi-substitute", visubstitute, 0
-"vi-swap-case", viswapcase, 0
+"vi-swap-case", viswapcase, ZLE_LASTCOL
 "vi-undo-change", viundochange, ZLE_KEEPSUFFIX
-"vi-unindent", viunindent, 0
+"vi-unindent", viunindent, ZLE_LASTCOL
 "vi-up-line-or-history", viuplineorhistory, ZLE_LINEMOVE
-"vi-yank", viyank, 0
+"vi-yank", viyank, ZLE_LASTCOL
 "vi-yank-eol", viyankeol, 0
 "vi-yank-whole-line", viyankwholeline, 0
 "what-cursor-position", whatcursorposition, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL
diff --git a/Src/Zle/zle_vi.c b/Src/Zle/zle_vi.c
index 53919e3..0a8b27d 100644
--- a/Src/Zle/zle_vi.c
+++ b/Src/Zle/zle_vi.c
@@ -248,6 +248,7 @@ getvirange(int wf)
      * point we just need to make the range encompass entire lines.   */
     if(vilinerange) {
 	int newcs = findbol();
+	lastcol = zlecs - newcs;
 	zlecs = pos;
 	pos = findeol();
 	zlecs = newcs;
@@ -348,6 +349,7 @@ videlete(UNUSED(char **args))
 	forekill(c2 - zlecs, CUT_RAW);
 	ret = 0;
 	if (vilinerange && zlell) {
+	    lastcol = -1;
 	    if (zlecs == zlell)
 		DECCS();
 	    foredel(1, 0);
@@ -449,7 +451,7 @@ vichangewholeline(char **args)
 int
 viyank(UNUSED(char **args))
 {
-    int oldcs = zlecs, c2, ret = 1;
+    int c2, ret = 1;
 
     startvichange(1);
     if ((c2 = getvirange(0)) != -1) {
@@ -459,11 +461,19 @@ viyank(UNUSED(char **args))
     vichgflag = 0;
     /* cursor now at the start of the range yanked. For line mode
      * restore the column position */
-    if (vilinerange) {
-	while (oldcs > 0 && zleline[oldcs - 1] != ZWC('\n') &&
-		zlecs != zlell && zleline[zlecs] != ZWC('\n')) {
-	    ++zlecs; --oldcs;
+    if (vilinerange && lastcol != -1) {
+	int x = findeol();
+
+	if ((zlecs += lastcol) >= x) {
+	    zlecs = x;
+	    if (zlecs > findbol() && invicmdmode())
+		DECCS();
 	}
+#ifdef MULTIBYTE_SUPPORT
+	else
+	    CCRIGHT();
+#endif
+	lastcol = -1;
     }
     return ret;
 }
diff --git a/Test/X02zlevi.ztst b/Test/X02zlevi.ztst
index d9fa0d5..2af6f06 100644
--- a/Test/X02zlevi.ztst
+++ b/Test/X02zlevi.ztst
@@ -37,6 +37,25 @@
 >two
 >CURSOR: 2
 
+  zletest $'long\eo  s\eolong\ekjy-k'
+0:yank up clears lastcol
+>BUFFER: long
+>  s
+>long
+>CURSOR: 2
+
+  zletest $'long\eos\eklljyk'
+0:yank up honours lastcol
+>BUFFER: long
+>s
+>CURSOR: 2
+
+  zletest $'long\eolong\eo  s\eolong\ekjd-k'
+0:delete up clears lastcol
+>BUFFER: long
+>long
+>CURSOR: 0
+
   zletest $'yankee doodle\ebhDyy0"1P'
 0:paste register 1 to get last deletion
 >BUFFER:  doodleyankee

Re: PATCH: cursor position after vi yank

[Mikael Magnusson]

On Sat, Oct 25, 2014 at 12:44 AM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> I wrote:
>> When producing the test cases, I noticed that zsh is not quite matching
>> vi in terms of cursor positioning after a yank. This fixes that.
>
> I took another look at this code because I was concerned that I might
> have got it wrong for unicode stuff. Has anyone got any tips on suitable
> input for testing things like column positioning? And how to get that
> input into a command-line? Trying to put combining characters in with:
>   print -z 'a\u0308'
> Results in a<0308>. I'm not sure if that was what I wanted or not.

You need to setopt combiningchars to enable the a and " to combine in
zle. I have no clue about the rest :). Trying some double width
characters would probably be a good idea too though, 306e is の for
example.

-- 
Mikael Magnusson