Re: [PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Bart Schaefer wrote on Wed, Sep 28, 2016 at 12:28:17 -0700:
> On Sep 28,  5:46pm, Daniel Shahaf wrote:
> } I'm not sure what to do about the handling of "_path_files -P foo": that -P
> } option documents its argument as a string but interprets it as a pattern.
> } Should we update the documentation to match the implementation, or vice-versa?
> 
> }  if (( $#pfx )); then
> } +  # ### Is it correct to interpret -P as a (greedy) pattern here?
> }    compset -P "$pfx[2]" || pfxsfx=( "$pfx[@]" "$pfxsfx[@]" )
> }  fi
> 
> I think it would be OK / correct to use ${(b)pfx[2]} there.  Presumably if
> $pfx[2] doesn't literally match the filename, we're falling back to the
> pfxsfx assignment to resolve it later.
> 

I'll make the change then.

> Here in _cpio:
> 
> } -  if compset -P '*:'; then
> } -    # TODO: doesn't need to be rsh.
> 
> The TODO comment may have been meant to indicate there should be a zstyle
> to choose what remote shell program to use?  Dumping the comment and
> forcing ssh might not be what was intended, but maybe it's OK, as it's
> consistent with what we do for remote shell pretty much everywhere else.
> 

I note that this block is for GNU cpio which has a --rsh-command flag.
Perhaps the correctest thing to do would be to use that.  I don't intend
to pursue this, though.

> In _rdesktop:
> 
> }      redir="${PREFIX%%:*}"
> } -    if compset -P '*='; then
> } +    if compset -P 1 '*='; then
> }        curcontext="${curcontext%:*}:$redir"
> } -      compset -P '*='
> }        case $redir in
> 
> The old code does look suspicious, but does anyone know if it was doing
> two "compset"s intentionally?

The second compset would have been a no-op on all inputs, wouldn't it?
Even its exit code was discarded.

Thanks for the review.

Daniel