From: Pascal Stumpf Subject: Re: ksh vi mode: stop 'P' command from moving the cursor To: Ingo Schwarze Cc: Anton Lindqvist , millert@openbsd.org, tech@openbsd.org Date: Fri, 25 Apr 2025 19:24:16 +0200 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"