Completion of prefix redirections is -command- instead of -redirect-

Completion of prefix redirections is -command- instead of -redirect-

[Daniel Shahaf]

Typing "<" and <TAB> at the prompt tries these completions [wrapped by me]:

tags in context :completion::complete:-command-::
    commands builtins functions aliases suffix-aliases \
      reserved-words jobs parameters  (_alternative _command_names _autocd) 
    commands                          (_path_commands _path_commands _alternative _command_names _autocd) 
    jobs                              (_jobs _alternative _command_names _autocd) 
    parameters                        (_parameters _alternative _command_names _autocd)

it shouldn't be completing commands but files.

Re: Completion of prefix redirections is -command- instead of -redirect-

[Bart Schaefer]

On Aug 26,  5:39am, Daniel Shahaf wrote:
}
} Typing "<" and <TAB> at the prompt tries these completions [wrapped by me]:
} 
} tags in context :completion::complete:-command-::

Confirm this is new since 5.0.8.

Looking at _complete_debug output, it seems to be ignoring the "<" and
completing as if the buffer were empty; there is nothing in the $words
array when entering compsys, so of course it tries commands.

Interestingly, compctl doesn't have this problem (though maybe not for
the right reason).

Re: Completion of prefix redirections is -command- instead of -redirect-

[Mikael Magnusson]

On Wed, Aug 26, 2015 at 8:18 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 26,  5:39am, Daniel Shahaf wrote:
> }
> } Typing "<" and <TAB> at the prompt tries these completions [wrapped by me]:
> }
> } tags in context :completion::complete:-command-::
>
> Confirm this is new since 5.0.8.
>
> Looking at _complete_debug output, it seems to be ignoring the "<" and
> completing as if the buffer were empty; there is nothing in the $words
> array when entering compsys, so of course it tries commands.
>
> Interestingly, compctl doesn't have this problem (though maybe not for
> the right reason).

249999cfb485914fe6f8630b1f4079e1227a5097 is the first bad commit
commit 249999cfb485914fe6f8630b1f4079e1227a5097
Author: Peter Stephenson <p.w.stephenson@ntlworld.com>
Date:   Sun Aug 9 18:40:54 2015 +0100

    36025: Fix completion aftoer "foo=bar; setopt ".

    Done by catching a case that doesn't seem to be handled otherwise,
    so there's some hope it doesn't screw up too much.


With the above commit reverted, < <tab> completes files again, but so
does ; <tab>.

-- 
Mikael Magnusson

Re: Completion of prefix redirections is -command- instead of -redirect-

[Bart Schaefer]

On Aug 27,  2:16am, Mikael Magnusson wrote:
}
} commit 249999cfb485914fe6f8630b1f4079e1227a5097
} Author: Peter Stephenson <p.w.stephenson@ntlworld.com>
} Date:   Sun Aug 9 18:40:54 2015 +0100
} 
}     36025: Fix completion aftoer "foo=bar; setopt ".
} 
}     Done by catching a case that doesn't seem to be handled otherwise,
}     so there's some hope it doesn't screw up too much.
} 
} With the above commit reverted, < <tab> completes files again, but so
} does ; <tab>.

Does this do it?


diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index b87b99b..cb17bde 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -1184,11 +1184,12 @@ get_comp_string(void)
 	 * considering a new command.  Consequently, although this looks
 	 * relatively harmless by itself, it's probably incomplete.
 	 */
-	lincmd = (incmdpos && !ins && !incond) ||
-	    (oins == 2 && wordpos == 2) ||
-	    (ins == 3 && wordpos == 1) ||
-	    (cmdtok == NULLTOK && !incond);
 	linredir = (inredir && !ins);
+	lincmd = !linredir &&
+	    ((incmdpos && !ins && !incond) ||
+	     (oins == 2 && wordpos == 2) ||
+	     (ins == 3 && wordpos == 1) ||
+	     (cmdtok == NULLTOK && !incond));
 	oins = ins;
 	/* Get the next token. */
 	if (linarr)

Re: Completion of prefix redirections is -command- instead of -redirect-

[Bart Schaefer]

On Aug 26,  9:35pm, Bart Schaefer wrote:
}
} Does this do it?

To answer my own question, no, it doesn't ... or at least not entirely.

Both WITH and WITHOUT the change in 36285, completing after this:

% ls < ;

completes files rather than commands.  I suppose if you've botched the
syntax you have no right to expect sanity, and the foregoing hasn't
changed since 5.0.7, so we're no worse off.

Re: Completion of prefix redirections is -command- instead of -redirect-

[Mikael Magnusson]

On Thu, Aug 27, 2015 at 6:47 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 26,  9:35pm, Bart Schaefer wrote:
> }
> } Does this do it?
>
> To answer my own question, no, it doesn't ... or at least not entirely.
>
> Both WITH and WITHOUT the change in 36285, completing after this:
>
> % ls < ;
>
> completes files rather than commands.  I suppose if you've botched the
> syntax you have no right to expect sanity, and the foregoing hasn't
> changed since 5.0.7, so we're no worse off.

Yeah, it's better than either with or without the 36285 patch, but
still doesn't handle all cases,

< <tab> gives files (correct)
; <tab> gives commands (correct)
< ; <tab> gives commands (correct)
foo < ; <tab> gives files (wrong-ish)
; < <tab> gives commands (wrong)
foo=bar; < <tab> gives commands (wrong, presumably same case as the above)
; < : <tab> gives commands (correct)
; < : : <tab> gives files (correct)

Is there any reason we can't just reset state to 'this is a new
command' when we see an unquoted solo semicolon? Or is determining
that this is the case in fact the trouble?

-- 
Mikael Magnusson

Re: Completion of prefix redirections is -command- instead of -redirect-

[Bart Schaefer]

On Aug 27,  6:54am, Mikael Magnusson wrote:
}
} < ; <tab> gives commands (correct)

This is better than 5.0.7, which gives files here.

} foo < ; <tab> gives files (wrong-ish)

Hasn't changed from 5.0.7.

} ; < <tab> gives commands (wrong)

This has changed, 5.0.7 gets it right.

} foo=bar; < <tab> gives commands (wrong, presumably same case as the above)

Hasn't changed since 5.0.7.

} Is there any reason we can't just reset state to 'this is a new
} command' when we see an unquoted solo semicolon? Or is determining
} that this is the case in fact the trouble?

My recollection of all of this is that it has to do with completing
in the middle of for/while/repeat type constructs, where the ";" may
be followed by "do" after which you have to stay in command position,
or you may be in shortloops syntax, etc.  But I don't recall (and
likely never really understood Sven's explanation of) all the details.

The below seems to make it better, the only case that is still wrong
after this is "foo < ; <tab>".  But I haven't more than minimally
checked that completing in e.g. "for" loops still works properly.

diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index cb17bde..97bdcc0 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -1185,7 +1185,7 @@ get_comp_string(void)
 	 * relatively harmless by itself, it's probably incomplete.
 	 */
 	linredir = (inredir && !ins);
-	lincmd = !linredir &&
+	lincmd = !inredir &&
 	    ((incmdpos && !ins && !incond) ||
 	     (oins == 2 && wordpos == 2) ||
 	     (ins == 3 && wordpos == 1) ||

Re: Completion of prefix redirections is -command- instead of -redirect-

[Mikael Magnusson]

On Thu, Aug 27, 2015 at 7:14 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 27,  6:54am, Mikael Magnusson wrote:
> }
> } < ; <tab> gives commands (correct)
>
> This is better than 5.0.7, which gives files here.
>
> } foo < ; <tab> gives files (wrong-ish)
>
> Hasn't changed from 5.0.7.
>
> } ; < <tab> gives commands (wrong)
>
> This has changed, 5.0.7 gets it right.
>
> } foo=bar; < <tab> gives commands (wrong, presumably same case as the above)
>
> Hasn't changed since 5.0.7.
>
> } Is there any reason we can't just reset state to 'this is a new
> } command' when we see an unquoted solo semicolon? Or is determining
> } that this is the case in fact the trouble?
>
> My recollection of all of this is that it has to do with completing
> in the middle of for/while/repeat type constructs, where the ";" may
> be followed by "do" after which you have to stay in command position,
> or you may be in shortloops syntax, etc.  But I don't recall (and
> likely never really understood Sven's explanation of) all the details.
>
> The below seems to make it better, the only case that is still wrong
> after this is "foo < ; <tab>".  But I haven't more than minimally
> checked that completing in e.g. "for" loops still works properly.

When I try this patch, I get all my previous cases right except the
one you mentioned, but also
; < : <tab>
completes files instead of commands now (before, it completed commands
both with and without the : there, so not more correct in total).
However,
foo=bar; < : <tab>
does work, so I can't imagine anyone complaining about this particular
case soon.

I'll run with both of the patches applied for a while and see if
anything more pops out.

PS
If anyone can get
for (a b) {<tab>
to complete commands, that would be nice too :). (this never worked)

-- 
Mikael Magnusson

Re: Completion of prefix redirections is -command- instead of -redirect-

[Bart Schaefer]

On Aug 27,  8:22am, Mikael Magnusson wrote:
} Subject: Re: Completion of prefix redirections is -command- instead of -re
}
} When I try this patch, I get all my previous cases right except the
} one you mentioned, but also
} ; < : <tab>
} completes files instead of commands now (before, it completed commands
} both with and without the : there, so not more correct in total).

It thinks ":" IS the command.

torch% ; < ls
tags in context :completion::complete:ls::
    argument-rest options  (_arguments _ls)
tags in context :completion::complete:ls:argument-rest:
    globbed-files  (_files _arguments _ls)

(The context for the ":" command is not ideal either since ":" is the
field separator ...)

} PS
} If anyone can get
} for (a b) {<tab>
} to complete commands, that would be nice too :). (this never worked)

I presume you mean without the parens around "a b"?  Otherwise it's not
valid syntax.

As PWS said, this is really a job for a state machine ... and not for a
hack this close to a release if it's something that hasn't ever worked.

Re: Completion of prefix redirections is -command- instead of -redirect-

[Mikael Magnusson]

On Thu, Aug 27, 2015 at 6:47 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 27,  8:22am, Mikael Magnusson wrote:
> } Subject: Re: Completion of prefix redirections is -command- instead of -re
> }
> } When I try this patch, I get all my previous cases right except the
> } one you mentioned, but also
> } ; < : <tab>
> } completes files instead of commands now (before, it completed commands
> } both with and without the : there, so not more correct in total).
>
> It thinks ":" IS the command.

Hmm, indeed, I'm not sure what I was thinking there, my context is
long gone. It is possible that I meant to say "commands instead of
files", which is what happens when I test it now.

> } PS
> } If anyone can get
> } for (a b) {<tab>
> } to complete commands, that would be nice too :). (this never worked)
>
> I presume you mean without the parens around "a b"?  Otherwise it's not
> valid syntax.

I meant for a (b) {<tab>, yeah.

> As PWS said, this is really a job for a state machine ... and not for a
> hack this close to a release if it's something that hasn't ever worked.

Yeah, it was just a PS.

-- 
Mikael Magnusson

Re: Completion of prefix redirections is -command- instead of -redirect-

[Mikael Magnusson]

On Thu, Aug 27, 2015 at 8:54 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On Thu, Aug 27, 2015 at 6:47 PM, Bart Schaefer
> <schaefer@brasslantern.com> wrote:
>> On Aug 27,  8:22am, Mikael Magnusson wrote:
>> } Subject: Re: Completion of prefix redirections is -command- instead of -re
>> }
>> } When I try this patch, I get all my previous cases right except the
>> } one you mentioned, but also
>> } ; < : <tab>
>> } completes files instead of commands now (before, it completed commands
>> } both with and without the : there, so not more correct in total).
>>
>> It thinks ":" IS the command.
>
> Hmm, indeed, I'm not sure what I was thinking there, my context is
> long gone. It is possible that I meant to say "commands instead of
> files", which is what happens when I test it now.

Ignore the last part of this, I had the final patch reverted for
testing earlier and apparently upgraded libcap after that and the
subsequent reinstall failed, or something. It works correctly now.

-- 
Mikael Magnusson

Re: Completion of prefix redirections is -command- instead of -redirect-

[Bart Schaefer]

On Aug 27,  8:54pm, Mikael Magnusson wrote:
} Subject: Re: Completion of prefix redirections is -command- instead of -re
}
} On Thu, Aug 27, 2015 at 6:47 PM, Bart Schaefer
} <schaefer@brasslantern.com> wrote:
} > On Aug 27,  8:22am, Mikael Magnusson wrote:
} > } Subject: Re: Completion of prefix redirections is -command- instead of -re
} > }
} > } ; < : <tab>
} > } completes files instead of commands now (before, it completed commands
} > } both with and without the : there, so not more correct in total).
} >
} > It thinks ":" IS the command.
} 
} Hmm, indeed, I'm not sure what I was thinking there, my context is
} long gone. It is possible that I meant to say "commands instead of
} files", which is what happens when I test it now.

I get files instead of commands, as you originally wrote.  So something
is still not quite right there, but this is such a corner case that I
don't think we need to continue to wrestle with it before 5.1.

Re: Completion of prefix redirections is -command- instead of -redirect-

[Peter Stephenson]

On Wed, 26 Aug 2015 22:14:13 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> The below seems to make it better, the only case that is still wrong
> after this is "foo < ; <tab>".  But I haven't more than minimally
> checked that completing in e.g. "for" loops still works properly.
> 
> diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
> index cb17bde..97bdcc0 100644
> --- a/Src/Zle/zle_tricky.c
> +++ b/Src/Zle/zle_tricky.c
> @@ -1185,7 +1185,7 @@ get_comp_string(void)
>  	 * relatively harmless by itself, it's probably incomplete.
>  	 */
>  	linredir = (inredir && !ins);
> -	lincmd = !linredir &&
> +	lincmd = !inredir &&
>  	    ((incmdpos && !ins && !incond) ||
>  	     (oins == 2 && wordpos == 2) ||
>  	     (ins == 3 && wordpos == 1) ||

That certainly ought to be on the right lines.  I would vaguely imagine
the incond and inredir ought to be doing roughly the same job, so maybe
if this works !incond should be pulled out.  The oins / ins / wordpos are
doing the cleverness with complex commands that no one quite
understands, which probably don't apply to either redirections or
conditions.

(You'd think there was a better way to do this using a state machine of
some sort.  There's a project for someone.)

pws