On Fri, 25 Apr 2025 18:06:12 +0200, Ingo Schwarze wrote:
> 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

This one does what I expect with multibyte characters and passes
regress.  OK.

> 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"