[PATCH] _compdef: Use zsh/param instead of a glob.

[PATCH] _compdef: Use zsh/param instead of a glob.

[Daniel Shahaf]

In particular, this allows functions defined inside another file to be
offered, such as various __git_foo() helpers defined in ${^fpath}/_git.
---
 Completion/Zsh/Command/_compdef | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Completion/Zsh/Command/_compdef b/Completion/Zsh/Command/_compdef
index 7a64da835..003c37da3 100644
--- a/Completion/Zsh/Command/_compdef
+++ b/Completion/Zsh/Command/_compdef
@@ -1,6 +1,6 @@
 #compdef compdef
 
-local state line expl list disp curcontext="$curcontext" pat normal ret=1
+local state line expl disp curcontext="$curcontext" pat normal ret=1
 local args1 args2
 typeset -A opt_args
 
@@ -57,7 +57,8 @@ case $state in
     _wanted commands expl 'completed command' compadd -k _comps && ret=0
   ;;
   cfun)
-    list=( ${^fpath:/.}/_(|*[^~])(:t) )
+    typeset -aU list=( ${(k)functions[(I)_*]} )
+    (( ${+list[1]} )) || list+=( ${^fpath:/.}/_(|*[^~])(:t) )
     if zstyle -T ":completion:${curcontext}:functions" prefix-hidden; then
       disp=( ${list[@]#_} )
       _wanted functions expl 'completion function' \

Re: [PATCH] _compdef: Use zsh/param instead of a glob.

[Matthew Martin]

On Sun, Aug 26, 2018 at 10:54:04AM +0000, Daniel Shahaf wrote:
> In particular, this allows functions defined inside another file to be
> offered, such as various __git_foo() helpers defined in ${^fpath}/_git.

I don't think this is a good idea at present. While the completion
system does use the _ prefix namespace, I don't see anything in the
documentation reserving the namespace to the completion system. There
are already a number of projects[0] that use the _* namespace. Perhaps
in 5.6 document that only completion functions should start with an
underscore and then apply this patch for 5.7 so that users have time to
adapt.

- Matthew Martin


0: From the first page of searching zsh on github there's oh-my-zsh,
pretzo, zsh-autosuggestions, and zsh-syntax-highlighting that use
_ prefixed functions.
> ---
>  Completion/Zsh/Command/_compdef | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Completion/Zsh/Command/_compdef b/Completion/Zsh/Command/_compdef
> index 7a64da835..003c37da3 100644
> --- a/Completion/Zsh/Command/_compdef
> +++ b/Completion/Zsh/Command/_compdef
> @@ -1,6 +1,6 @@
>  #compdef compdef
>  
> -local state line expl list disp curcontext="$curcontext" pat normal ret=1
> +local state line expl disp curcontext="$curcontext" pat normal ret=1
>  local args1 args2
>  typeset -A opt_args
>  
> @@ -57,7 +57,8 @@ case $state in
>      _wanted commands expl 'completed command' compadd -k _comps && ret=0
>    ;;
>    cfun)
> -    list=( ${^fpath:/.}/_(|*[^~])(:t) )
> +    typeset -aU list=( ${(k)functions[(I)_*]} )
> +    (( ${+list[1]} )) || list+=( ${^fpath:/.}/_(|*[^~])(:t) )
>      if zstyle -T ":completion:${curcontext}:functions" prefix-hidden; then
>        disp=( ${list[@]#_} )
>        _wanted functions expl 'completion function' \

Re: [PATCH] _compdef: Use zsh/param instead of a glob.

[Daniel Shahaf]

Matthew Martin wrote on Sun, 26 Aug 2018 20:09 -0500:
> On Sun, Aug 26, 2018 at 10:54:04AM +0000, Daniel Shahaf wrote:
> > In particular, this allows functions defined inside another file to be
> > offered, such as various __git_foo() helpers defined in ${^fpath}/_git.
> 
> I don't think this is a good idea at present. While the completion
> system does use the _ prefix namespace, I don't see anything in the
> documentation reserving the namespace to the completion system. There
> are already a number of projects[0] that use the _* namespace.

In general, it is better to offer too many completions than too few.
(In other words, better to have false positives than false negatives.)
For example, the other day I needed __git_branches; I didn't
particularly mind to have the entire timestamp-named z-sy-h cast there
since it didn't get in the way, due to namespacing.

That said, if there is some way to generate a set of names that is less
complete but has fewer false positives, we could offer that set under
one tag and the ${(k)functions[(I)_*]} set under another tag, to allow
users to get their preferred way by setting the tag-order style.

> Perhaps in 5.6 document that only completion functions should start
> with an underscore and then apply this patch for 5.7 so that users
> have time to adapt.

I don't know if we should deprecate user functions being named `_*`.  We
could point out in the docs that the completion system uses this
namespace so the world would be a _slightly_ better place if user code
used some other namespace, but I would stop short of making this a hard
requirement.

(Incidentally, I never understood why completion functions didn't use a
proper namespace, zshfoo_* or some such, like virtually everyone else —
but that ship has sailed.)

Cheers,

Daniel
(except gettext)

Re: [PATCH] _compdef: Use zsh/param instead of a glob.

[Bart Schaefer]

On Mon, Aug 27, 2018 at 3:07 AM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> That said, if there is some way to generate a set of names that is less
> complete but has fewer false positives, we could offer that set under
> one tag and the ${(k)functions[(I)_*]} set under another tag, to allow
> users to get their preferred way by setting the tag-order style.

The ${(k)functions[(I)_*]} set will only differ from the
${^fpath:/.}/_(|*[^~])(:t) set in two ways:
(1) The fpath search will find functions that have not yet been marked autoload.
(2) After autoload functions have been invoked for the first time
${(k)functions[(I)_*]} will have the additional functions from the
base files.

Skipping (1) for only (2) might be considered just as much a loss as
not including (2).  Also, ${(V)_comps} might be a more reasonable
source of the extra functions.

> (Incidentally, I never understood why completion functions didn't use a
> proper namespace, zshfoo_* or some such, like virtually everyone else —
> but that ship has sailed.)

Can't say anybody was really thinking about needing function
namespaces back then.  Miinimize length of identifiers and maximize
similarity to the command for which the completion was being defined
were pretty much the only things in mind.  Remember, this was pretty
close to the era when commands like "ls", "rm", "mv", "ld", etc. were
named.

Aside:  For going horribly overboard the other direction IMO, see:
https://github.com/aecolley/client_bash/blob/master/prometheus.bash

Re: [PATCH] _compdef: Use zsh/param instead of a glob.

[Daniel Shahaf]

Bart Schaefer wrote on Mon, 27 Aug 2018 11:22 -0700:
> On Mon, Aug 27, 2018 at 3:07 AM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > That said, if there is some way to generate a set of names that is less
> > complete but has fewer false positives, we could offer that set under
> > one tag and the ${(k)functions[(I)_*]} set under another tag, to allow
> > users to get their preferred way by setting the tag-order style.
> 
> The ${(k)functions[(I)_*]} set will only differ from the
> ${^fpath:/.}/_(|*[^~])(:t) set in two ways:
> (1) The fpath search will find functions that have not yet been marked autoload.
> (2) After autoload functions have been invoked for the first time
> ${(k)functions[(I)_*]} will have the additional functions from the
> base files.
> 
> Skipping (1) for only (2) might be considered just as much a loss as
> not including (2).

I suppose we could do both.

An fpath search could be expensive (it does I/O) but no one has
complained so let's assume the cost is acceptable.

> Also, ${(V)_comps} might be a more reasonable
> source of the extra functions.
> 

I like this.  That will filter out things in fpath that aren't completion
functions, and will filter in things like «compdef 'compadd -a foo' bar».

> > (Incidentally, I never understood why completion functions didn't use a
> > proper namespace, zshfoo_* or some such, like virtually everyone else —
> > but that ship has sailed.)
> 
> Can't say anybody was really thinking about needing function
> namespaces back then.  Miinimize length of identifiers and maximize
> similarity to the command for which the completion was being defined
> were pretty much the only things in mind.  Remember, this was pretty
> close to the era when commands like "ls", "rm", "mv", "ld", etc. were
> named.

Would that also be why all the variables in the C code are named as the
acronyms of whatever their names _should_ be? :-)

Re: [PATCH] _compdef: Use zsh/param instead of a glob.

[Bart Schaefer]

On Mon, Aug 27, 2018 at 11:47 AM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Would that also be why all the variables in the C code are named as the
> acronyms of whatever their names _should_ be? :-)

Oh, definitely.

[PATCH v2] _compdef: Change and add sources for completed completion function names.

[Daniel Shahaf]

Use ${(v)_comps} instead of going through $fpath again.  Use
${functions} as well to find more legitimate matches, such as various
__git_foo() helpers defined in ${^fpath}/_git.
---
 Completion/Zsh/Command/_compdef | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Completion/Zsh/Command/_compdef b/Completion/Zsh/Command/_compdef
index 7a64da835..22af7f561 100644
--- a/Completion/Zsh/Command/_compdef
+++ b/Completion/Zsh/Command/_compdef
@@ -1,6 +1,6 @@
 #compdef compdef
 
-local state line expl list disp curcontext="$curcontext" pat normal ret=1
+local state line expl disp curcontext="$curcontext" pat normal ret=1
 local args1 args2
 typeset -A opt_args
 
@@ -57,7 +57,8 @@ case $state in
     _wanted commands expl 'completed command' compadd -k _comps && ret=0
   ;;
   cfun)
-    list=( ${^fpath:/.}/_(|*[^~])(:t) )
+    typeset -aU list=( ${(v)_comps} ${(k)functions[(I)_*]} )
+    (( ${+list[1]} )) || list+=( ${^fpath:/.}/_(|*[^~])(:t) )
     if zstyle -T ":completion:${curcontext}:functions" prefix-hidden; then
       disp=( ${list[@]#_} )
       _wanted functions expl 'completion function' \