[PATCH] Add API wrapper to ${+_comps[...]}

[PATCH] Add API wrapper to ${+_comps[...]}

[Daniel Shahaf]

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

I wonder if this is useful enough to be added?

[-- Attachment #2: 0001-compdef-documentation-Move-an-example.patch --]
[-- Type: text/x-patch, Size: 1515 bytes --]

>From ed7cddd08d7e3bcf7d60349917ecc50461f50dd1 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 30 Sep 2015 17:41:55 +0000
Subject: [PATCH 1/2] compdef documentation: Move an example.

---
 Doc/Zsh/compsys.yo | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index d6b1803..955b300 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -455,6 +455,11 @@ xitem(tt(compdef -k) [ tt(-an) ] var(function style key-sequence) [ var(key-sequ
 item(tt(compdef -K) [ tt(-an) ] var(function name style key-seq) [ var(name style seq) ... ])(
 The first form defines the var(function) to call for completion in the
 given contexts as described for the tt(#compdef) tag above.
+For example,
+
+example(compdef _pids foo)
+
+uses the function tt(_pids) to complete process IDs for the command tt(foo).
 
 Alternatively, all the arguments may have the form
 `var(cmd)tt(=)var(service)'.  Here var(service) should already have been
@@ -513,13 +518,6 @@ autoloadable, equivalent to tt(autoload -U )var(function).
 )
 enditem()
 
-The function tt(compdef) can be used to associate existing completion
-functions with new commands.  For example,
-
-example(compdef _pids foo)
-
-uses the function tt(_pids) to complete process IDs for the command tt(foo).
-
 Note also the tt(_gnu_generic) function described below, which can be
 used to complete options for commands that understand the
 `tt(-)tt(-help)' option.
-- 
2.1.4


[-- Attachment #3: 0002-Add-compexists.patch --]
[-- Type: text/x-patch, Size: 1933 bytes --]

>From a67963b2be0ad72b462996da93e89b734b68efa0 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 30 Sep 2015 17:53:42 +0000
Subject: [PATCH 2/2] Add 'compexists'.

---
 Completion/compinit |  5 +++++
 Doc/Zsh/compsys.yo  | 12 +++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Completion/compinit b/Completion/compinit
index 4b9a778..ac7e672 100644
--- a/Completion/compinit
+++ b/Completion/compinit
@@ -430,6 +430,11 @@ compdef() {
   fi
 }
 
+compexists() {
+  (( $# )) || return 2
+  (( $+_comps[$1] ))
+}
+
 # Now we automatically make the definition files autoloaded.
 
 typeset _i_wdirs _i_wfiles
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 955b300..444b966 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -445,9 +445,9 @@ subsect(Functions)
 The following function is defined by tt(compinit) and may be called
 directly.
 
+startitem()
 findex(compdef)
 cindex(completion system, adding definitions)
-startitem()
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ ))ifnztexi(      )))
 xitem(tt(compdef) [ tt(-ane) ] var(function name) ... [ tt(-){tt(p)|tt(P)} var(pattern) ... [ tt(-N) var(name) ...]])
 xitem(tt(compdef -d) var(name) ...)
@@ -516,6 +516,16 @@ underscore.
 Wherever applicable, the tt(-a) option makes the var(function)
 autoloadable, equivalent to tt(autoload -U )var(function).
 )
+findex(compexists)
+cindex(completion system, checking existence of definitions)
+item(tt(compexists) var(name))(
+This function checks whether the command var(name) has completion.
+The return value is tt(0) if var(name) has a completion and tt(1) if it
+doesn't.  If no var(name) was passed, return tt(2).
+
+Completions for var(name) can be defined with tt(compdef), e.g.,
+with tt(compdef name=)var(foo) or tt(compdef )var(_foo)tt( name).
+)
 enditem()
 
 Note also the tt(_gnu_generic) function described below, which can be
-- 
2.1.4

Re: [PATCH] Add API wrapper to ${+_comps[...]}

[Bart Schaefer]

On Sep 30,  6:29pm, Daniel Shahaf wrote:
} 
} I wonder if this is useful enough to be added?

(Why two separate patches both to compsys.yo?)

I don't see any particular reason not to add it, but I also find
no existence tests of $_comps[...] anywhere in the contributed
functions or my own local startup files or functions, so it may
indeed be only minimally useful.

On the other hand there are tests for whether a particular function
is defined.  If that function is destined to be assigned to _comps[x]
perhaps it would be better to check for _comps[x] already defined
instead.

Re: [PATCH] Add API wrapper to ${+_comps[...]}

[Daniel Shahaf]

Bart Schaefer wrote on Sat, Oct 03, 2015 at 17:49:19 -0700:
> On Sep 30,  6:29pm, Daniel Shahaf wrote:
> } 
> } I wonder if this is useful enough to be added?
> 
> (Why two separate patches both to compsys.yo?)
> 

One logical change per commit.  I might have erred on the side of
splitting too much, but it's easier to unsplit than to split.)

> I don't see any particular reason not to add it, but I also find
> no existence tests of $_comps[...] anywhere in the contributed
> functions or my own local startup files or functions, so it may
> indeed be only minimally useful.
> 

I use them like this in my .zshrc:

    # TODO neither of this actually works, since _gnu_generic wants
    # --foo=[VALUE] and these have a space instead, but they get 90%
    # right and are better than nothing...
    _has_completion() { (( $# == 1 )) || return 2; (( $+_comps[$1] )) }
    _has_completion howdoi || compdef _gnu_generic howdoi
    _has_completion ag || compdef _gnu_generic ag

The idea is to be forward compatible — to only install the _gnu_generic
definition if there isn't an _ag already defined.

> On the other hand there are tests for whether a particular function
> is defined.  If that function is destined to be assigned to _comps[x]
> perhaps it would be better to check for _comps[x] already defined
> instead.

_comps[x] being already defined in what sense?  I can think of three
meanings: (a) hash key exists; (b) hash key exists and the value has
exists as a key in $functions; (c) same, plus the function is not
a "marked-for-autoload" stub.

The patch implements (a).  I think we could leave the patch doing (a)
— that's meaningful, it checks whether 'x <TAB>' has been hooked — and
let callers that care about (b) or (c) implement the extra check (b)
or (c) do it themselves?

Re: [PATCH] Add API wrapper to ${+_comps[...]}

[Bart Schaefer]

On Oct 5,  9:51pm, Daniel Shahaf wrote:
} Subject: Re: [PATCH] Add API wrapper to ${+_comps[...]}
}
} Bart Schaefer wrote on Sat, Oct 03, 2015 at 17:49:19 -0700:
} > 
} > (Why two separate patches both to compsys.yo?)
} 
} One logical change per commit.

But you don't care about one logical commit per zsh-workers sequence
number?  I mean, I'm not unreasonably sticky about it, but I try to
keep it that way.

} > On the other hand there are tests for whether a particular function
} > is defined.  If that function is destined to be assigned to _comps[x]
} > perhaps it would be better to check for _comps[x] already defined
} > instead.
} 
} _comps[x] being already defined in what sense?  I can think of three
} meanings: (a) hash key exists

That sense.

The point being that if we're going to bother defining "compexists"
then maybe we ought to use it instead of (( ${+functions[_name]} )).
It couldn't replace all such tests, since in most cases those are
"helper" functions rather than values in $_comps, but perhaps some.

I'm trying to find a metric for usefulness.

Re: [PATCH] Add API wrapper to ${+_comps[...]}

[Daniel Shahaf]

Bart Schaefer wrote on Mon, Oct 05, 2015 at 15:11:48 -0700:
> On Oct 5,  9:51pm, Daniel Shahaf wrote:
> } Subject: Re: [PATCH] Add API wrapper to ${+_comps[...]}
> }
> } Bart Schaefer wrote on Sat, Oct 03, 2015 at 17:49:19 -0700:
> } > 
> } > (Why two separate patches both to compsys.yo?)
> } 
> } One logical change per commit.
> 
> But you don't care about one logical commit per zsh-workers sequence
> number?  I mean, I'm not unreasonably sticky about it, but I try to
> keep it that way.
> 

How would I handle a case such as 36725 (three patches to a single
module)?

I know some people handle this with git-send-email, but I don't have
that set up.

Would it help to write the commit messages and changelog entries as
36725/0001, 36725/0002, etc, where the running counter is the first part
of the filename?  (which is generated by 'git format-patch')

> } > On the other hand there are tests for whether a particular function
> } > is defined.  If that function is destined to be assigned to _comps[x]
> } > perhaps it would be better to check for _comps[x] already defined
> } > instead.
> } 
> } _comps[x] being already defined in what sense?  I can think of three
> } meanings: (a) hash key exists
> 
> That sense.
> 
> The point being that if we're going to bother defining "compexists"
> then maybe we ought to use it instead of (( ${+functions[_name]} )).
> It couldn't replace all such tests, since in most cases those are
> "helper" functions rather than values in $_comps, but perhaps some.
> 

It seems most cases are of the "allow an helper function to be
overridden" variety.  The usage in _calendar:3 is an exception, but
couldn't benefit from compexists.

> I'm trying to find a metric for usefulness.

The difference between

   f() {
     [[ $# == 1 ]] || return 2
     (( $+_comps[$1] ))
   }
and
   g() {
     [[ $# == 1 ]] || return 2
     (( $+_comps[$1] )) && (( $+functions[$_comps[$1]] ))
   }

is how they handle the case 'compdef _foo foo' with _foo() not having
being defined yet (nor marked for autoload).  Is this a common case?
When does it happen?

Re: [PATCH] Add API wrapper to ${+_comps[...]}

[Bart Schaefer]

On Oct 5, 10:33pm, Daniel Shahaf wrote:
}
} How would I handle a case such as 36725 (three patches to a single
} module)?

I'd have sent three separate messages, if they couldn't all be treated
as part of the same logical change.

} The difference between
} 
}    f() {
}      [[ $# == 1 ]] || return 2
}      (( $+_comps[$1] ))
}    }
} and
}    g() {
}      [[ $# == 1 ]] || return 2
}      (( $+_comps[$1] )) && (( $+functions[$_comps[$1]] ))
}    }
} 
} is how they handle the case 'compdef _foo foo' with _foo() not having
} being defined yet (nor marked for autoload).  Is this a common case?
} When does it happen?

Presently it doesn't EVER happen (except maybe in your startup files).
Which is why a function for (( $+_comps[$1] )) may not be very useful.

OTOH we've probably now batted this back and forth way too much, too.