From: Ingo Schwarze <schwarze@usta.de>
Subject: Re: ksh vi mode: stop 'P' command from moving the cursor
To: Walter Alejandro Iglesias <wai@roquesor.com>
Cc: Anton Lindqvist <anton@basename.se>, millert@openbsd.org, tech@openbsd.org
Date: Tue, 22 Apr 2025 18:07:02 +0200

Hello Walter,

Walter Alejandro Iglesias wrote on Tue, Apr 22, 2025 at 10:08:19AM +0200:
> On Mon, Apr 21, 2025 at 11:38:01PM +0200, Ingo Schwarze wrote:

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

> Contary to vim or traditional vi (https://ex-vi.sourceforge.net/), our
> version of vi(1) (as well as nvi2 from ports) doesn't move the cursor
> when pasting from the buffer.

To be more precise:
                  put the cursor
 [n]p our vi(1):  on the first byte inserted
      our ksh(1): on the last byte inserted
 P    our vi(1):  on the first byte inserted
      our ksh(1): after the last byte inserted
 2P   our vi(1):  on the first byte inserted
      our ksh(1): on the last byte inserted

So behaviour of vi(1) and ksh(1) VI mode is totally different with respect
to these commands.  While that does seem unfortunate - i mean, what is
the point of having a VI mode that differs a lot from vi(1) - changing
ksh(1) VI mode to better match vi(1) would likely cause a more significant
amount of work than i'm willing to invest at this point (you know, while
i do occasionally use VI mode in ksh(1), i use emacs mode much more often,
so investing that much work seems a dubious proposition).
On top of that, such major changes to how our ksh(1) works might put
people off who are used to ksh(1) on other platforms - i'm not sure
that would indeed happen, but it might need to be considered.

My goal is
 1. to stop UTF-8 input from corrupting the ksh(1) VI command line
 2. to fix internal inconsistencies *inside* single commands when that
    is the most logical way to achieve 1.
That is much more limited in scope than what you propose.

> You could imitate this behavior by adding
> one more modification to your diff:

Whoa, that third chunk you added feels *much* too aggressive.

You need to be more careful.  When you change a function, you need
to check all call sites of that function and you need to make sure
that you indeed want the behaviour changed at all the call sites
(and convince others that they want all that behaviour changed, too).

In this case, your hunk would change behavour for at least these commands:

 * P (as you apparently intend)
 * p (not sure you intended that, but maybe it would match vi(1))
 * r
 * _
 * bottom line handling when initiating a search command 
 * redo_insert()
 * x_vi_putbuf()
 * expand_word()
 * complete_word()

The impact of this change is so massive and so whitespread that i'm
not even willing to try to understand the consequences.

Did you try running the test suite

  cd /usr/src/regress/bin/ksh/edit/
  make vi

before posting this patch?  I guess this would cause at least a dozen
of the tests to fail.

Yours,
  Ingo


> Index: vi.c
> ===================================================================
> RCS file: /cvs/src/bin/ksh/vi.c,v
> diff -u -p -r1.61 vi.c
> --- vi.c	21 Apr 2025 20:06:15 -0000	1.61
> +++ vi.c	22 Apr 2025 07:47:56 -0000
> @@ -695,7 +695,6 @@ vi_cmd(int argcnt, const char *cmd)
>  {
>  	int		ncursor;
>  	int		cur, c1, c2, c3 = 0;
> -	int		any;
>  	struct edstate	*t;
>  
>  	if (argcnt == 0 && !is_zerocount(*cmd))
> @@ -848,11 +847,8 @@ vi_cmd(int argcnt, const char *cmd)
>  
>  		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;
> @@ -1486,7 +1482,6 @@ putbuf(const char *buf, int len, int rep
>  		es->linelen += len;
>  	}
>  	memmove(&es->cbuf[es->cursor], buf, len);
> -	es->cursor += len;
>  	return 0;
>  }
>