Index | Thread | Search

From:
Ingo Schwarze <schwarze@usta.de>
Subject:
Re: ksh vi mode: stop 'P' command from moving the cursor
To:
Pascal Stumpf <pascal@stumpf.co>
Cc:
Anton Lindqvist <anton@basename.se>, millert@openbsd.org, tech@openbsd.org
Date:
Fri, 25 Apr 2025 18:06:12 +0200

Download raw body.

Thread
Hello Pascal,

Pascal Stumpf wrote on Thu, Apr 24, 2025 at 10:46:53PM +0200:
> Ingo Schwarze wrote:

>> So, which behaviour do people want?
>> 
>>  0) minimally invasive, 1P unchanged:
>>     p  unchanged -> to last character inserted
>>     1P unchanged -> after last character inserted
>>     2P changed to match 1P -> after last character inserted
>> 
>>  1) meximally invasive -> always to first charcter inserted
>>     like in our vi(1)
>> 
>>  2) minimally invasive, 2P unchanged -> always to last character inserted
>>     p  unchanged
>>     1P changed to match 2P
>>     2P unchanged

> As a user of vi mode, I must say that I prefer option 2).  It is
> consistent among both the p and P commands, and also the behaviour of
> the ksh93 package, as well as all historical ksh versions I tested on
> sdf.org.  That is probably also the reason bash behaves this way.
> pdksh is the one that has an inaccuracy here in its implementation.

Hmmm, likely you have a point.  To further support your reasoning.
one can observe that the variable naming "any" suggests that the
authors of this code (mistakenly) thought that the flag would express
whether *anything* was inserted.  They possibly thought that the
putbuf() while loop might end up not inserting anything (which
actually cannot happen) and they would have to prevent backing up
the cursor in that (impossible) case, so they ended up unwittingly
preventing cusor backup in the 1P case, a classic case of off-by-one.

Had they understood their own code and had they intended what they coded,
they probably would have called that variable "insert_more_than_once"
or "multiple" or something like that rather than "any".

So, here is the patch to adjust the behaviour of 1P to become
more similar to 2P (even though, as Nils Ola noted, this doesn't
make PP match 2P for multi-character insertions).
It also makes 'P' work with UTF-8.

OK for this one?
  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	25 Apr 2025 15:40:46 -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,11 @@
 
 		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;
+			while (es->cursor > 0)
+				if (!isu8cont(es->cbuf[--es->cursor]))
+					break;
 			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	25 Apr 2025 15:40:46 -0000
@@ -117,8 +117,14 @@
 testseq "abc\00330 rx" " # abc\b\b\bax\b"
 
 # P: Paste at current position.
-testseq "abcde\0033hDhP" " # abcde\b\b  \b\b\b\bdebc\b\b"
+testseq "abcde\0033hDhP" " # abcde\b\b  \b\b\b\bdebc\b\b\b"
 testseq "abcde\0033hDh2P" " # abcde\b\b  \b\b\b\bdedebc\b\b\b"
+testseq "A\0033xa\0303\0200\00332Px" \
+	" # A\b \b\0303\0200\bAA\0303\0200\b\b\0303\0200 \b\b"
+testseq "\0302\0251\0033xaA\0033Px" \
+	" # \0302\0251\b  \b\bA\b\0302\0251A\b\bA  \b\b\b"
+testseq "\0302\0251\0033xa\0303\0200\0033Px" \
+ " # \0302\0251\b  \b\b\0303\0200\b\0302\0251\0303\0200\b\b\0303\0200  \b\b\b"
 
 # p: Paste after current position.
 testseq "abcd\0033hDhp" " # abcd\b\b  \b\b\b\bacdb\b\b"