[BUG] Any type of file in command position gets misleadingly completed as 'executable file'

[BUG] Any type of file in command position gets misleadingly completed as 'executable file'

[Marlon Richert]

% zsh -f
% autoload compinit; compinit
% zstyle '*' format '%d'
% zstyle '*' group-name ''
% mkdir foo
% foo^D
executable file
foo/
% touch bar
% bar^D
executable file
bar

The problem is in _files and it is two-fold:
* _files always adds '*:all-files', which ignores the pattern passed
with the -g flag.
* Even though _files passes its file pattern tags to _next_label, if
_files was passed a tag and/or description, it _always_ prefers these
over the ones returned by _next_label's call to _description.

I propose fixing this as follows:
* _files should not add '*:all-files'. Why would anyone want
_non-matching_ files to be listed?
* The 'globbed-files' tag should be renamed to just 'files' and have a
default description of 'file'. (As an aside, the docs speak of an
'other-files' tag used when zstyle list-directories-first is set, but
this is never actually offered. Let's remove that, too.)
* If _files was passed a tag and/or a description, then these should
replace the 'files' tag (formerly 'globbed-files') and its
description, respectively -- but not the 'directories' tag and/or its
description -- and these should be passed to _next_label.
* For each tag and description that _files passes to _next_label, it
should always prefer the resulting "$expl[@]" over "$opts[@]" when
calling _path_files.

This way:
* Directories will always be listed as directories, unless overridden
through the zstyles supported by _description.
* Any tag and/or description passed to _files will be used to override
only the 'files' tag and its description, and these will be passed to
_description.
* Non-matching files will not be listed.

Re: [BUG] Any type of file in command position gets misleadingly completed as 'executable file'

[Bart Schaefer]

On Wed, Nov 10, 2021 at 2:01 PM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> The problem is in _files and it is two-fold:
> * _files always adds '*:all-files', which ignores the pattern passed
> with the -g flag.

There's a comment:
  # People prefer to have directories shown on first try as default.
  # Even if the calling function didn't use -/.

In fact the actual assumption is that most people prefer to have
SOMETHING completed rather than nothing, unless they've specifically
requested otherwise.

You can change this behavior by

zstyle ":completion:*" file-patterns ...

(although I'm not sure why that overrides list-dirs-first).  There's
even an explicit paragraph and example in the documentation explaining
how to do this.

> * Even though _files passes its file pattern tags to _next_label, if
> _files was passed a tag and/or description, it _always_ prefers these
> over the ones returned by _next_label's call to _description.

Someone else will have to comment on that.

> * The 'globbed-files' tag should be renamed to just 'files' and have a
> default description of 'file'.

That would actually break my zstyle setting for file-patterns.

> (As an aside, the docs speak of an
> 'other-files' tag used when zstyle list-directories-first is set, but
> this is never actually offered. Let's remove that, too.)

It appears "other-files" was (inadvertently?) removed by workers/36165
(Oliver, back in 2015).

Re: [BUG] Any type of file in command position gets misleadingly completed as 'executable file'

[Oliver Kiddle]

Marlon Richert wrote:
> % foo^D
> executable file
> foo/
> % touch bar
> % bar^D
> executable file
> bar

I was about to say I cannot reproduce this and you must have something
in a system rc file. However, I can reproduce it by putting . in $PATH.
Is that perhaps the case? If so, I wouldn't think that's a particularly
clever idea.

Bart Schaefer wrote:
> On Wed, Nov 10, 2021 at 2:01 PM Marlon Richert <marlon.richert@gmail.com> wrote:
> >
> > The problem is in _files and it is two-fold:
> > * _files always adds '*:all-files', which ignores the pattern passed
> > with the -g flag.
>
> There's a comment:
>   # People prefer to have directories shown on first try as default.
>   # Even if the calling function didn't use -/.
>
> In fact the actual assumption is that most people prefer to have
> SOMETHING completed rather than nothing, unless they've specifically
> requested otherwise.

I'm fairly sure that's where the consensus was at the time. If you press
tab you expect something. Globs specified in completions may not always
be perfect. With some other setup like a plugin that does continuous
completion after each key then I can see it being less desirable.

> You can change this behavior by
>
> zstyle ":completion:*" file-patterns ...

Where the all-files fallback is less ideal is where other things are
completed alongside files, e.g for:
  _alternative 'files:file:_files -g "*.png(-.)"' 'others:other:(one two three)'

You can avoid that by using _complete twice. I've never used the feature
to call _complete twice because it used to be too slow but I should
perhaps reconsider:
  zstyle ':completion:*::::' completer ... _complete _complete:-all ...
  zstyle ':completion:*' file-patterns '%p:globbed-files *(-/):directories'
  zstyle ':completion:*:complete-all:*' file-patterns '*:all-files'

> > * Even though _files passes its file pattern tags to _next_label, if
> > _files was passed a tag and/or description, it _always_ prefers these
> > over the ones returned by _next_label's call to _description.

It was never intended that file-patterns would divide matches into
separate groups by default.

It doesn't "always" prefer them. Only when the [[ -n "$end" ]] condition
is set. That corresponds to the value in file-patterns including a
description, e.g. *(-/):directories:directory

Without this, it'd be using empty descriptions.

However, it is surely a bug that $end is not reinitialised with each
iteration of the for loop. That makes it difficult to use the
description we were passed for the globbed-files but to put the
globbed-files first and directories second.

It also looks like a bug that the recursive-files style is only
implemented where $end is set.

I'll put together a patch for those two points tomorrow. And add an
example to the documentation.

> > (As an aside, the docs speak of an
> > 'other-files' tag used when zstyle list-directories-first is set, but
> > this is never actually offered. Let's remove that, too.)
>
> It appears "other-files" was (inadvertently?) removed by workers/36165
> (Oliver, back in 2015).

That was intentional. Leaving it in the documentation wasn't.

Oliver

diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 89b918d60..a8beece2d 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -979,11 +979,6 @@ kindex(other-accounts, completion tag)
 item(tt(other-accounts))(
 used to look up the tt(users-hosts) style
 )
-kindex(other-files, completion tag)
-item(tt(other-files))(
-for the names of any non-directory files.  This is used instead
-of tt(all-files) when the tt(list-dirs-first) style is in effect.
-)
 kindex(packages, completion tag)
 item(tt(packages))(
 for packages (e.g. tt(rpm) or installed tt(Debian) packages)
@@ -1978,9 +1973,7 @@ kindex(list-dirs-first, completion style)
 item(tt(list-dirs-first))(
 This is used by file completion.  If set, directories to be completed
 are listed separately from and before completion for other files,
-regardless of tag ordering.  In addition, the tag tt(other-files)
-is used in place of tt(all-files) for the remaining files, to indicate
-that no directories are presented with that tag.
+regardless of tag ordering.
 )
 kindex(list-grouped, completion style)
 item(tt(list-grouped))(

Re: [BUG] Any type of file in command position gets misleadingly completed as 'executable file'

[Oliver Kiddle]

I wrote:
> However, it is surely a bug that $end is not reinitialised with each
> iteration of the for loop. That makes it difficult to use the
> description we were passed for the globbed-files but to put the
> globbed-files first and directories second.
>
> It also looks like a bug that the recursive-files style is only
> implemented where $end is set.

And here is a patch to address those two points. The recursive-files
code should really consider directories passed with -W instead of
$PWD but all the handling of that is down in _path_files so that's
not trivial. I've not changed the workings of recursive-files, it
appears in full in the patch due to reindentation.

The documentation claims that you can use %d in the description to get
at the description supplied to the function. This isn't working, at
least as far back as 4.2.1 and probably well beyond. It isn't even clear
how this might ever have been implemented. The zformat in _next_label,
comptags -A and $curtag perhaps. I'm not going to go further down that
rabbit hole and have removed the line in the documentation.

Oliver

diff --git a/Completion/Unix/Type/_files b/Completion/Unix/Type/_files
index 4ddec1e12..f03b4a148 100644
--- a/Completion/Unix/Type/_files
+++ b/Completion/Unix/Type/_files
@@ -92,7 +92,10 @@ for def in "$pats[@]"; do
     pat="${${sdef%%:${tag}*}//\\:/:}"
 
     if [[ "$sdef" = *:${tag}:* ]]; then
+      # If the file-patterns spec includes a description, use it and give the
+      # group/description options from it precedence over passed in parameters.
       descr="${(Q)sdef#*:${tag}:}"
+      end=
     else
       if (( $opts[(I)-X] )); then
         descr=
@@ -108,26 +111,30 @@ for def in "$pats[@]"; do
       while _next_label "$tag" expl "$descr"; do
         _comp_ignore=( $_comp_ignore $ign )
         if [[ -n "$end" ]]; then
-          if _path_files -g "$pat" "$opts[@]" "$expl[@]"; then
-	    ret=0
-	  elif [[ $PREFIX$SUFFIX != */* ]] && zstyle -a ":completion:${curcontext}:$tag" recursive-files rfiles; then
-	    for rfile in $rfiles; do
-	      if [[ $PWD/ = ${~rfile} ]]; then
-		if [[ -z $subtree ]]; then
-		  subtree=( **/*(/) )
-		fi
-		for prepath in $subtree; do
-		  oprefix=$PREFIX
-		  PREFIX=$prepath/$PREFIX
-		  _path_files -g "$pat" "$opts[@]" "$expl[@]" && ret=0
-		  PREFIX=$oprefix
-		done
-		break
-	      fi
-	    done
-	  fi
+          expl=( "$opts[@]" "$expl[@]" )
         else
-          _path_files "$expl[@]" -g "$pat" "$opts[@]" && ret=0
+          expl+=( "$opts[@]" )
+        fi
+
+        if _path_files -g "$pat" "$expl[@]"; then
+          ret=0
+        elif [[ $PREFIX$SUFFIX != */* ]] && \
+            zstyle -a ":completion:${curcontext}:$tag" recursive-files rfiles
+        then
+          for rfile in $rfiles; do
+            if [[ $PWD/ = ${~rfile} ]]; then
+              if [[ -z $subtree ]]; then
+                subtree=( **/*(/) )
+              fi
+              for prepath in $subtree; do
+                oprefix=$PREFIX
+                PREFIX=$prepath/$PREFIX
+                _path_files -g "$pat" "$expl[@]" && ret=0
+                PREFIX=$oprefix
+              done
+              break
+            fi
+          done
         fi
       done
       (( ret )) || break
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index a8beece2d..1adceb536 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -1524,9 +1524,10 @@ If no `tt(:)var(tag)' is given the `tt(files)' tag will be used.
 The var(tag) may also be followed by an optional second colon and a
 description, which will be used for the `tt(%d)' in the value of
 the tt(format) style (if that is set) instead of the default
-description supplied by the completion function.  If the description
-given here contains itself a `tt(%d)', that is replaced with the
-description supplied by the completion function.
+description supplied by the completion function.  The inclusion
+of a description also gives precedence to associated options such as
+for completion grouping so it can be used where files should be
+separated.
 
 For example, to make the tt(rm) command first complete only names of
 object files and then the names of all files if there is no matching
@@ -1548,6 +1549,15 @@ all files using the pattern `tt(*)' at the first step and stops when it
 sees this pattern.  Note also it will never try a pattern more than once
 for a single completion attempt.
 
+To separate directories into a separate group from the files but still
+complete them at the first attempt, a description needs to be given.
+Note that directories need to be explicitly excluded from the
+globbed-files because `tt(*)' will match directories. For grouping, it
+is also necessary to set the tt(group-name) style.
+
+example(zstyle ':completion:*' file-patterns \ 
+    '%p+LPAR()^-/RPAR():globbed-files *(-/):directories:location')
+
 During the execution of completion functions, the tt(EXTENDED_GLOB)
 option is in effect, so the characters `tt(#)', `tt(~)' and `tt(^)' have
 special meanings in the patterns.
@@ -1971,9 +1981,10 @@ obtained by setting the style to an empty string (i.e. tt('')).
 )
 kindex(list-dirs-first, completion style)
 item(tt(list-dirs-first))(
-This is used by file completion.  If set, directories to be completed
-are listed separately from and before completion for other files,
-regardless of tag ordering.
+This is used by file completion and corresponds to a particular
+setting of the tt(file-patterns) style.
+If set, the default directories to be completed
+are listed separately from and before completion for other files.
 )
 kindex(list-grouped, completion style)
 item(tt(list-grouped))(