[FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Marlon Richert]

[-- Attachment #1: Type: text/plain, Size: 92 bytes --]

This allows local executables to be completed more easily, without the
need for . in $path.

[-- Attachment #2: 0001-Complete-local-executables-with-.-prefix-if-prefix-nee.txt --]
[-- Type: text/plain, Size: 3651 bytes --]

From 6edea832ff20aa31852042286cdba3602f8cd527 Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlonrichert@users.noreply.github.com>
Date: Tue, 7 Dec 2021 21:07:53 +0200
Subject: [PATCH] Complete local executables with ./ prefix, if prefix-needed
 is false

This allows local executables to be completed more easily, without the need for . in $path.
---
 Completion/Zsh/Type/_command_names | 15 +++++++++------
 Doc/Zsh/compsys.yo                 |  5 ++++-
 Test/Y01completion.ztst            | 11 +++++++++++
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/Completion/Zsh/Type/_command_names b/Completion/Zsh/Type/_command_names
index b1c35f013..68df9522b 100644
--- a/Completion/Zsh/Type/_command_names
+++ b/Completion/Zsh/Type/_command_names
@@ -8,16 +8,15 @@ local args defs ffilt
 
 zstyle -t ":completion:${curcontext}:commands" rehash && rehash
 
-zstyle -t ":completion:${curcontext}:functions" prefix-needed && \
- [[ $PREFIX != [_.]* ]] && \
- ffilt='[(I)[^_.]*]'
-
 defs=(
   'commands:external command:_path_commands'
 )
 
-[[ -n "$path[(r).]" || $PREFIX = */* ]] &&
-    defs+=( 'executables:executable file:_files -g \*\(-\*\)' )
+if [[ -n "$path[(r).]" || $PREFIX = */* ]]; then
+  defs+=( 'executables:executable file:_files -g \*\(-\*\)' )
+elif ! zstyle -T ":completion:${curcontext}:executables" prefix-needed; then
+  defs+=( 'executables:executable file:_files -g ./\*\(-\*\)' )
+fi
 
 if [[ "$1" = -e ]]; then
   shift
@@ -26,6 +25,10 @@ elif (( ${#precommands:|builtin_precommands} )); then
 else
   [[ "$1" = - ]] && shift
 
+zstyle -t ":completion:${curcontext}:functions" prefix-needed && \
+    [[ $PREFIX != [_.]* ]] && \
+        ffilt='[(I)[^_.]*]'
+
   defs=( "$defs[@]"
     'builtins:builtin command:compadd -Qk builtins'
     "functions:shell function:compadd -k 'functions$ffilt'"
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index f85293ac7..6f04b8ea8 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -2414,7 +2414,7 @@ set to `true' this common prefix must be typed by the user to generate
 the matches.
 
 The style is applicable to the tt(options), tt(signals), tt(jobs),
-tt(functions), and tt(parameters) completion tags.
+tt(executables), tt(functions) and tt(parameters) completion tags.
 
 For command options, this means that the initial `tt(-)', `tt(+)', or
 `tt(-)tt(-)' must be typed explicitly before option names will be
@@ -2426,6 +2426,9 @@ be completed.
 For jobs, an initial `tt(%)' is required before job names will be
 completed.
 
+For executables, an initial `tt(./)' is required before executables that are in
+the current dir (but not in var($path)) will be completed.
+
 For function and parameter names, an initial `tt(_)' or `tt(.)' is
 required before function or parameter names starting with those
 characters will be completed.
diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst
index 6af0efc6d..8ff00af6f 100644
--- a/Test/Y01completion.ztst
+++ b/Test/Y01completion.ztst
@@ -266,6 +266,17 @@ F:regression test workers/31611
 >FI:{file1}
 >FI:{file2}
 
+  chmod u+x file1 file2
+  comptesteval 'zstyle ":completion:*:executables" prefix-needed no'
+  comptesteval 'zstyle ":completion:*" tag-order executables'
+  comptesteval 'zstyle ":completion:*" matcher-list "l:|=*"'
+  comptest $'f\C-D'
+0:list local executables with ./ prefix, if prefix-needed is false
+>DESCRIPTION:{executable file}
+>EX:{./file1}
+>EX:{./file2}
+
+  comptesteval 'zstyle -d ":completion:*" matcher-list'
   comptesteval "typeset -a bar=({$'\\0'..$'\\C-?'})"
   comptesteval 'typeset -A bat=( "$bar[@]" )'
   comptesteval 'typeset bay="$bar"'
-- 
2.33.0

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Aaron Schrab]

At 21:15 +0200 07 Dec 2021, Marlon Richert <marlon.richert@gmail.com> wrote:
>This allows local executables to be completed more easily, without the
>need for . in $path.

The already existing uses of `prefix-needed` are ones that I don't think 
setting it to false would cause any serious problems. But for this I 
think it's fairly dangerous, although certainly not as dangerous as just 
adding `.` to $path. So I don't know if it's a good idea to use that 
same name here.

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On Tue, Dec 7, 2021 at 4:23 PM Aaron Schrab <aaron@schrab.com> wrote:

>
> The already existing uses of `prefix-needed` are ones that I don't think
> setting it to false would cause any serious problems. But for this I
> think it's fairly dangerous
>

Hm.  I haven't applied the patch to see what effect it has, but it appears
it would cause the "./" to be prefixed onto the command word?  If so,
there's visible feedback; if not, the danger would be of invoking a command
that's in $path instead of the intended local executable.

So I'm not strongly concerned about danger; what is the behavior you'd find
troubling?

My question would be whether this needs any further changes to interoperate
with prefix-hidden.

[-- Attachment #2: Type: text/html, Size: 1080 bytes --]

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Aaron Schrab]

At 16:50 -0800 07 Dec 2021, Bart Schaefer <schaefer@brasslantern.com> wrote:
>On Tue, Dec 7, 2021 at 4:23 PM Aaron Schrab <aaron@schrab.com> wrote:
>
>>
>> The already existing uses of `prefix-needed` are ones that I don't think
>> setting it to false would cause any serious problems. But for this I
>> think it's fairly dangerous
>>
>
>Hm.  I haven't applied the patch to see what effect it has, but it appears
>it would cause the "./" to be prefixed onto the command word?  If so,
>there's visible feedback; if not, the danger would be of invoking a command
>that's in $path instead of the intended local executable.
>
>So I'm not strongly concerned about danger; what is the behavior you'd find
>troubling?

My main concern is if someone would be in the habit of doing command 
completion and then hitting enter without looking at the results. If 
done when there's a typo in the prefix that was used it could complete 
to something from an untrusted directory instead.

Yes, this requires a few things to go wrong in just the right way for it 
to happen. But enabling this option would generally be a one-time thing 
for a user, and they may not consider this type of case at the time, and 
after awhile even forget about it.

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Daniel Shahaf]

Aaron Schrab wrote on Wed, 08 Dec 2021 01:16 +00:00:
> [...] enabling this option would generally be a one-time thing for
> a user, and they may not consider this type of case at the time, and
> after awhile even forget about it.

Furthermore, anyone who has already added «zstyle ':…:*' prefix-needed
false» to their zshrc will silently get the new behaviour when they upgrade.

>

I'm not sure how intuitive the new behaviour is.  First, other uses of
prefix-needed generally hide a tag from the matches unless the common
prefix of all matches in that tag is typed; that is, those uses of prefix-needed
could be simulated with a «zstyle -e … tag-order …» setting that
conditionally excludes "signals" or "options" depending on $PREFIX et
al..  This patch's use does not follow that pattern: if the style is set
to true, setting it to false would not cause files in $PWD to be offered
as matches immediately.

That makes me wonder whether a better approach here would be to complete
both executables in $PATH and executables in $PWD, but under separate
tags.  If we did that, prefix-needed would have its usual semantics in
this case.

Second, if Alice's $PATH does not include "." and her zstyle settings
are such that «zstyle -T :…:executables prefix-needed» is false, why
should the implied prefix be "./"?

Cheers,

Daniel

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Bart Schaefer]

On Wed, Dec 8, 2021 at 8:31 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Furthermore, anyone who has already added «zstyle ':…:*' prefix-needed
> false» to their zshrc will silently get the new behaviour when they upgrade.

Yeah, that's probably the biggest objection to re-using this style name.

> ...[Perhaps] a better approach here would be to complete
> both executables in $PATH and executables in $PWD, but under separate
> tags.  If we did that, prefix-needed would have its usual semantics in
> this case.
>
> Second, if Alice's $PATH does not include "." and her zstyle settings
> are such that «zstyle -T :…:executables prefix-needed» is false, why
> should the implied prefix be "./"?

The intent of the original patch appears to be to automatically offer
local executables when no matching name is found in the non-local
$path.  Does your question imply that it should instead be possible to
complete executables from arbitrary location(s) outside $path, rather
than only locally?

If so, then instead of (or as well as?) prefix-needed, an array-valued
style 'executable-prefixes' (or whatever) could provide a list of
places that should be searched and automatically inserted ahead of the
command word.  If combined with the patch's prefix-needed semantic,
one would need to both set prefix-needed to false and set
executable-prefixes to include "./" in order to get Marlon's effect.
That would solve the "silently get new behavior" issue and (I think)
also allow one to e.g. add "/etc/" when completing commands after
"sudo"?

One might then also be able to use the existing mechanisms to create
additional tag names, rather than wiring a new tag name into
_command_names.  (This is vaguely similar to Zach's question about
adding a custom tag to "cd" completion.)

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Daniel Shahaf]

Bart Schaefer wrote on Wed, 08 Dec 2021 17:07 +00:00:
> On Wed, Dec 8, 2021 at 8:31 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>> ...[Perhaps] a better approach here would be to complete
>> both executables in $PATH and executables in $PWD, but under separate
>> tags.  If we did that, prefix-needed would have its usual semantics in
>> this case.
>>
>> Second, if Alice's $PATH does not include "." and her zstyle settings
>> are such that «zstyle -T :…:executables prefix-needed» is false, why
>> should the implied prefix be "./"?
>
> […] Does your question imply that it should instead be possible to
> complete executables from arbitrary location(s) outside $path, rather
> than only locally?

I was not trying to imply that we should complete executables from
arbitrary locations, but to ask whether we should do that.

What use-cases are there for wanting a prefix other than "./" to be
implied? (not as the default behaviour, but as an opt-innable behaviour)

Brainstorming:

    ./bin/
    # Could be useful at the root of a project tree whose local scripts
    # (such as our Util/preconfig) are under an FHS-esque ./bin/
    # directory.

    .//
    # to complete in subdirectories of $PWD.  E.g., «mk<TAB>» would
    # offer «Util/mkdisttree.sh» in zsh.git and «src/mkhelp.pl» in
    # curl.git, as «.//mk<TAB>» does.
    #
    # Today, to complete either of those, one would type «s/mk<TAB>» or
    # «U/mk<TAB>».  Supporting this would save typing "s/" or "U/", just
    # like the OP saves typing "./".

    ../
    # to complete in $PWD's parent.  E.g., when one has version control
    # worktrees of several branches of a single repository in sibling
    # directories (e.g., ~/src/zsh/{master,5.8,5.7}/), the common parent
    # directory is an obvious place for scripts that might need to run
    # in any branch's worktree.

    "$(git rev-parse --show-toplevel)/../"
    # Ditto, but from anywhere in the worktree.

    /absolute/path/to/the/src/tree/
    # when $PWD is the build tree of an out-of-tree build.  Useful for
    # running «configure» a second time, or for running just one piece
    # of a test suite (calling the scripts directly rather than via
    # «make test»).

    /sbin/
    # This would allow completing «ifcon<TAB>» to «/sbin/ifconfig»
    # without affecting child processes, and would cause such command
    # names to always appear in history as absolute paths.  Alternatives
    # including adding /sbin to $PATH or «command-path».

I also considered a prefix of «=» and semantics involving the EQUALS
option, but haven't come up with anything.

> If so, then instead of (or as well as?) prefix-needed, an array-valued
> style 'executable-prefixes' (or whatever) could provide a list of
> places that should be searched and automatically inserted ahead of the
> command word.  If combined with the patch's prefix-needed semantic,
> one would need to both set prefix-needed to false and set
> executable-prefixes to include "./" in order to get Marlon's effect.
> That would solve the "silently get new behavior" issue

*nod*

> and (I think) also allow one to e.g. add "/etc/" when completing
> commands after "sudo"?

I guess that would involve inspecting ${+funcstack[(r)_sudo]} or
${_comp_priv_prefix}.

> One might then also be able to use the existing mechanisms to create
> additional tag names, rather than wiring a new tag name into
> _command_names.  (This is vaguely similar to Zach's question about
> adding a custom tag to "cd" completion.)

Thanks, Bart.

Daniel

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Oliver Kiddle]

Marlon Richert wrote:
> This allows local executables to be completed more easily, without the
> need for . in $path.

This is the type of thing which often can already be achieved with
styles and, where not, a few tweaks may enable it. The nearest example
from my own config involves using the fake style and a matcher to ignore
the prefix. But that's for a more specific context[1].

In this case for the style context, the tag 'commands' is used for a lot
of commands that aren't external Unix commands. The 'executables' tag is
better but guarded by a condition that will evaluate to false:

[[ -n "$path[(r).]" || $PREFIX = */* ]] &&
    defs+=( 'executables:executable file:_files -g \*\(-\*\)' 

The fake style is looked up in _description so a dummy call to it is one
option. The patch below adds that but I'd be interested in any thoughts
on that. With caveats[2], that allows:

  zstyle -e ':completion:*:executables' fake 'reply=( ./*(-*) )'
  zstyle ':completion:*:executables' matcher 'b:=./'

There is a command-path style looked up by _command_names to override
$path. But that doesn't entirely help for a couple of reasons. External
commands are completed based on keys of the commands association but
that will not contain commands based on relative directories listed in
$path. Even if it did, they can't be identified to add back a ./ prefix
so that they actually work. In the patch below, I do change it to use
path=( $cmdpath:A ) when the command-path style is set. That resolves
only the first of these issues and allows for relative paths in the
command-path style to work as intended, even if that isn't especially
useful.

Oliver

[1] If anyone's interested, that example is for ssh public keys. Just
the fake and matcher style are needed but it looks like this:
zstyle -e ':completion:*:((ssh|scp|sftp):*:option-i-1|ssh-add:*:argument-rest)' fake '[[ -prefix - ]] || reply=( ~/.ssh/id^*.pub(-.) )'
zstyle -e ':completion:*:((ssh|scp|sftp):*:option-i-1|ssh-add:*:argument-rest)' matcher '[[ -prefix - ]] || reply=( "l:|=*" )'
zstyle ':completion:*:((ssh|scp|sftp):*:option-i-1|ssh-add:*:argument-rest)' menu yes=0 search
zstyle ':completion:*:((ssh|scp|sftp):*:option-i-1|ssh-add:*:argument-rest)' list-colors "=(#b)(*/)(*)==38;5;208=0"

[2] With the fake style in general, the return status of the calling
completion function doesn't account for fake matches which may mean
unwanted later completers are invoked (e.g. _approximate).

diff --git a/Completion/Zsh/Type/_command_names b/Completion/Zsh/Type/_command_names
index b1c35f013..12cbd69c1 100644
--- a/Completion/Zsh/Type/_command_names
+++ b/Completion/Zsh/Type/_command_names
@@ -4,7 +4,7 @@
 # complete only external commands and executable files. This and a
 # `-' as the first argument is then removed from the arguments.
 
-local args defs ffilt
+local args defs expl ffilt
 
 zstyle -t ":completion:${curcontext}:commands" rehash && rehash
 
@@ -16,8 +16,12 @@ defs=(
   'commands:external command:_path_commands'
 )
 
-[[ -n "$path[(r).]" || $PREFIX = */* ]] &&
-    defs+=( 'executables:executable file:_files -g \*\(-\*\)' )
+if [[ -n "$path[(r).]" || $PREFIX = */* ]]; then
+  defs+=( 'executables:executable file:_files -g \*\(-\*\)' )
+else
+  # this is ignored but exists to facilitate the use of the fake style
+  _description executables expl 'executable file'
+fi
 
 if [[ "$1" = -e ]]; then
   shift
@@ -58,7 +62,7 @@ fi
 if (( $#cmdpath )); then
   local -a +h path
   local -A +h commands
-  path=( $cmdpath )
+  path=( $cmdpath:A )
 fi
 
 _alternative -O args "$defs[@]"

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Bart Schaefer]

On Wed, Dec 8, 2021 at 3:18 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> This is the type of thing which often can already be achieved with
> styles and, where not, a few tweaks may enable it. The nearest example
> from my own config involves using the fake style

Yeah, I was pretty sure there was a way to get here using the "fake"
style but couldn't put my finger on the argument Daniel found to argue
against changes to prefix-needed.

> The fake style is looked up in _description so a dummy call to it is one
> option. The patch below adds that but I'd be interested in any thoughts
> on that.

Is it going to matter that this happens before command-path is
consulted?  And before the defs array is passed to _alternative?

> path=( $cmdpath:A ) when the command-path style is set. That resolves
> only the first of these issues and allows for relative paths in the
> command-path style to work as intended, even if that isn't especially
> useful.

Does that have any unexpected interaction with PATH_DIRS ?

Re: [FEATURE][PATCH] Complete local executables with ./ prefix, if prefix-needed is false

[Oliver Kiddle]

Bart Schaefer wrote:
> > The fake style is looked up in _description so a dummy call to it is one
> > option. The patch below adds that but I'd be interested in any thoughts
> > on that.
>
> Is it going to matter that this happens before command-path is
> consulted?  And before the defs array is passed to _alternative?

I don't think so, other than the point I already mentioned about the
return status from _command_names not accounting for the fake matches.
fake is usually handled early because _description needs to be called
before compadd.

> > path=( $cmdpath:A ) when the command-path style is set. That resolves

> Does that have any unexpected interaction with PATH_DIRS ?

Not with the actual path_dirs functionality because of path being
local in the function. In terms of completion down in _path_commands
that still works. It ought to filter out "." from $path anyway to
avoid duplicated completions. It also doesn't add a / suffix for
the directories and didn't account for symlinks to executables. So
I've included a patch against the completion handling for path_dirs.
:A will subvert the filtering of "." but the effect is very minor.
I'm unsure whether command-path should perhaps be looked up before
the $path[(r).] check in _command_names.

Oliver

diff --git a/Completion/Unix/Type/_path_commands b/Completion/Unix/Type/_path_commands
index 66795ae0f..4d5a6c5af 100644
--- a/Completion/Unix/Type/_path_commands
+++ b/Completion/Unix/Type/_path_commands
@@ -87,18 +87,19 @@ fi
 # 'if' block move up to the "_command_names -" branch of _command_names?
 if [[ -o path_dirs ]]; then
   local -a path_dirs
-  path_dirs=(${^path}/*(/N:t))
-  (( ${#path_dirs} )) &&
-  _wanted path-dirs expl 'directory in path' compadd "$@" -a path_dirs && ret=0
 
   if [[ $PREFIX$SUFFIX = */* ]]; then
+    path_dirs=( ${path:#.} )
     # Find command from path, not hashed
-    _wanted commands expl 'external command' _path_files -W path -g '*(*)' &&
-    ret=0
+    _wanted commands expl 'external command' _path_files -W path_dirs -g '*(-*)' && ret=0
+  else
+    path_dirs=(${^path}/*(/N:t))
+    (( ${#path_dirs} )) &&
+        _wanted path-dirs expl 'directory in path' compadd "$@" -S / -a path_dirs && ret=0
   fi
 fi
 
-return $ret
+return ret
 }
 
 _path_commands "$@"