I've made it clear on more than one occasion that I'm not an experienced
C programmer.  And I'm not part of the acredited team of developers
here, I'm not allowed to modify anything, hence, I don't see how it's
dangerous or aggressive for me to relax given that nothing I say or code
will have any impact.  On these bases, I prefer to be eloquent in
explaining the issue I encounter and let those who are accredited to fix
them do so in the way they see fit.

In this case it seemed to me helpful to point out this difference in the
behavior of vi(1) and ksh(1), that was the scope of my whole post.  It
never occurred to me that the modification I made to your patch was
functional, and I don't even need to refer to the test suite to verify
this, it just seemed to me a practical way to point out the problem.

What I explained also leaves open analyzing which of the two behaviors
is the most coherent or practical.  Perhaps it's worth considering
modifying vi(1).  I didn't stop to analyze it, but broadly speaking, I
dare say I'd be happy with either.


On Tue, Apr 22, 2025 at 06:07:02PM +0200, Ingo Schwarze wrote:
> 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;
> >  }
> > 
> 

-- 
Walter