[PATCH] compstate[to_end] and suffixes

[PATCH] compstate[to_end] and suffixes

[Daniel Shahaf]

I wrote in workers/39930:

> Known issue: completing «git -c user.em<CURSOR>=foo» inserts "ail."
> (properly) but puts cursor to the right of the dot, rather than to its
> left.

An offlist discussion resulted in a patch for that; bringing that
discussion back on-list (quoting with permission).

Oliver Kiddle wrote:
> Daniel Shahaf wrote:
> > Oliver Kiddle wrote on Sun, Nov 13, 2016 at 00:05:06 +0100:
> > > I'm inclined to think that the default behaviour of compstate[to_end]
> > > should move the cursor after the suffix if it added one. It looks
> > > like one of those cases where completions early bias towards shell
> > > words is still apparent.
> > > 
> > > diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
> > > index b7ec18f..9934fd8 100644
> > > --- a/Src/Zle/compresult.c
> > > +++ b/Src/Zle/compresult.c
> > > @@ -1174,6 +1174,7 @@ do_single(Cmatch m)
> > >  	zlemetacs = minfo.end;
> > >  	if (zlemetacs + m->qisl == lastend)
> > >  	    zlemetacs += minfo.insc;
> > > +	zlemetacs += m->suf ? strlen(m->suf) : 0;
> > >      }
> > >      {
> > >  	Cmatch *om = minfo.cur;
> >
> > The following patch achieves that.  However, I don't know whether it is
> > correct; perhaps a more correct fix would be to monkey with
> > add_match_data() such that this if() in do_single() would be taken?
> 
> Your patch seems to work with tests I've thrown at it, e.g suffix
> not added due to ambiguous completion. I've also checked prefixes.
> 
> I think it should also allow for a hidden suffix, however (compadd -s).
> But not for an ignored suffix.
> 

Here's an updated patch to handle 'compadd -s' as well:

diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
index b7ec18f..0579939 100644
--- a/Src/Zle/compresult.c
+++ b/Src/Zle/compresult.c
@@ -1174,6 +1174,10 @@ do_single(Cmatch m)
 	zlemetacs = minfo.end;
 	if (zlemetacs + m->qisl == lastend)
 	    zlemetacs += minfo.insc;
+
+	/* Advance CURSOR past compadd -s/-S suffixes. */
+	zlemetacs += strlen(psuf);
+	zlemetacs += m->suf ? strlen(m->suf) : 0;
     }
     {
 	Cmatch *om = minfo.cur;

What's a minimal example that triggers this code?  (So I can add a unit
test)

> There aren't too many cases where we set compstate[to_end] – just
> _urls and _path_files. I've checked both and the change doesn't
> affect either - no suffix is being used.  Took me a while to work
> out how to use the one in _path_files. It was added back in
> workers/6686.
> 
> On further reflection, I'm convinced it really is just a bug and
> placing the cursor after the suffix is the right thing to do. If
> it really does turn out that the current beahviour has a point, it
> probably wouldn't be hard to add compstate[to_end]=suffix for the
> new behaviour.

Re: [PATCH] compstate[to_end] and suffixes

[Oliver Kiddle]

Daniel Shahaf wrote:
> What's a minimal example that triggers this code?  (So I can add a unit
> test)

How about the following?

Oliver

diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst
index b3f8be4..7d8d70d 100644
--- a/Test/Y01completion.ztst
+++ b/Test/Y01completion.ztst
@@ -9,6 +9,7 @@
     cd comp.tmp
     comptestinit -z $ZTST_testdir/../Src/zsh &&
     {
+      comptesteval 'compdef _tst tst'
       mkdir dir1 &&
       mkdir dir2 &&
       touch file1 &&
@@ -87,6 +88,16 @@ F:regression test workers/31611
 0:directory name is not a glob qualifier
 >line: {cd ../C/}{}
 
+  comptesteval "_tst() { compadd -U -s : -S / -I . word; compstate[to_end]= }"
+  comptest $'tst .\C-b\t'
+0:allow for suffixes when moving cursor to end of match (with ignored suffix)
+>line: {tst word:/}{.}
+
+  comptesteval "_tst() { compadd -s : -S / word; compstate[to_end]= }"
+  comptest $'tst \t'
+0:allow for suffixes when moving cursor to end of match (without ignored suffix)
+>line: {tst word:/}{}
+
 %clean
 
   zmodload -ui zsh/zpty