From: Ingo Schwarze <schwarze@usta.de>
Subject: ksh vi mode: stop 'P' command from moving the cursor
To: Anton Lindqvist <anton@basename.se>, millert@openbsd.org
Cc: tech@openbsd.org
Date: Mon, 21 Apr 2025 23:38:01 +0200

Hello,

here is a patch to change the behaviour of the "paste before" (P) command
in the VI command line editing mode of ksh(1).

The current behaviour is very weird.  When you insert just once (1P),
it leaves the cursor on the character where it was before the insertion.
But if you insert more than once (for example, 2P or 3P), it moves
the cursor left by one byte after the insertion, such that it gets
to sit on the last byte inserted.  That is inconsistent behaviour
in the sense that "PP" does *not* do the same as "2P".  The same
new text is inserted by both commands, but "2P" then moves the cursor
left by one byte, whereas "PP" does not.

I consider this weirdness a user-visible implementation detail
because the manual page does not describe at all what is supposed
to happen to the cursor position:

     [n]p    Paste the contents of the yank buffer just after the
             current position, n times.
     [n]P    Same as p, except the buffer is pasted at the current
             position.

If i read CVS history correctly, the inconsistent behaviour appears
to be present since vi.c rev. 1.1 (1996/08/14), so other versions
of pdksh may or may not share the same quirk.

I suggest regularizing the behaviour of P by always leaving the cursor
position untouched, i.e. by leaving the behaviour of 1P unchanged,
but changing the behaviour of 2P to exactly match PP.

A side benefit of this change is that right now, using the 2P command
to insert a string ending with a UTF-8 character corrupts the cursor
position, leaving the cursor in the middle of the last inserted
character rather than at its beginning.  The patch fixes that bug
because it no longer moves the cursor at all.

I do not fear issues with compatibility because this does not affect
the execution of shell scripts but merely interactive behaviour in
the command line editor.  On top of that, VI mode is not the default
mode, P is probably relatively rarely used as an interactive command,
and 2P even more so, which is likely why nobody ever complained about
the inconsistent behaviour.

I'm not convinced anything like communicating with upstream needs
to be done.  I do not see pdksh in our ports tree, and the old
links to

  http://www.cs.mun.ca/~michael/pdksh
  https://staffwiki.cs.mun.ca/~michael/pdksh/

all appear to be dead.

Regarding the (addmittedly tricky) regression tests:

 * One pure ASCII test changes because it tests "2P" at the end,
   and the cursor now backs up one less column.
 * The first new test tests inserting ASCII before UTF-8.
 * The second new test tests inserting UTF-8 before ASCII.
 * The third new test tests inserting UTF-8 before UTF-8.

OK?
  Ingo


Index: bin/ksh/vi.c
===================================================================
RCS file: /cvs/src/bin/ksh/vi.c,v
diff -u -r1.61 vi.c
--- bin/ksh/vi.c	21 Apr 2025 20:06:15 -0000	1.61
+++ bin/ksh/vi.c	21 Apr 2025 21:24:59 -0000
@@ -695,7 +695,6 @@
 {
 	int		ncursor;
 	int		cur, c1, c2, c3 = 0;
-	int		any;
 	struct edstate	*t;
 
 	if (argcnt == 0 && !is_zerocount(*cmd))
@@ -848,11 +847,8 @@
 
 		case 'P':
 			modified = 1; hnum = hlast;
-			any = 0;
 			while (putbuf(ybuf, yanklen, 0) == 0 && --argcnt > 0)
-				any = 1;
-			if (any && es->cursor != 0)
-				es->cursor--;
+				continue;
 			if (argcnt != 0)
 				return -1;
 			break;
Index: regress/bin/ksh/edit/vi.sh
===================================================================
RCS file: /cvs/src/regress/bin/ksh/edit/vi.sh,v
diff -u -r1.10 vi.sh
--- regress/bin/ksh/edit/vi.sh	21 Apr 2025 20:06:15 -0000	1.10
+++ regress/bin/ksh/edit/vi.sh	21 Apr 2025 21:24:59 -0000
@@ -118,7 +118,13 @@
 
 # P: Paste at current position.
 testseq "abcde\0033hDhP" " # abcde\b\b  \b\b\b\bdebc\b\b"
-testseq "abcde\0033hDh2P" " # abcde\b\b  \b\b\b\bdedebc\b\b\b"
+testseq "abcde\0033hDh2P" " # abcde\b\b  \b\b\b\bdedebc\b\b"
+testseq "A\0033xa\0303\0200\00332Px" \
+	" # A\b \b\0303\0200\bAA\0303\0200\b  \b\b\b"
+testseq "\0302\0251\0033xaA\0033Px" \
+	" # \0302\0251\b  \b\bA\b\0302\0251A\b \b\b"
+testseq "\0302\0251\0033xa\0303\0200\0033Px" \
+	" # \0302\0251\b  \b\b\0303\0200\b\0302\0251\0303\0200\b  \b\b\b"
 
 # p: Paste after current position.
 testseq "abcd\0033hDhp" " # abcd\b\b  \b\b\b\bacdb\b\b"