In the penultimate paragraph of the comment, the two possibilities for the relative path cover three cases: - The comment is read in the source tree - The comment is read in an installed tree with --enable-function-subdirs - The comment is read in an installed tree with --disable-function-subdirs Review-by: Matthew Martin --- Completion/Zsh/Function/__arguments | 44 +++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 Completion/Zsh/Function/__arguments diff --git a/Completion/Zsh/Function/__arguments b/Completion/Zsh/Function/__arguments new file mode 100644 index 000000000..40a4e4b60 --- /dev/null +++ b/Completion/Zsh/Function/__arguments @@ -0,0 +1,44 @@ +#compdef _arguments + +# Dear reader: This function is called "__arguments" and is the completion +# function for the completion function _arguments. This function, however, is +# not the completion function _arguments. If you're looking for the function +# _arguments, then you've come to the wrong place. +# +# _arguments is a completion utility function. It is called by completion +# functions for command-line tools. +# +# __arguments is a completion function for _arguments. It runs when one does +# `_arguments -<TAB>' at the shell prompt to jog one's memory about _arguments' +# option flags. +# +# _arguments is in documented in the manual. If you were looking for its +# source code, that'd be either in ../../../Completion/Base/Utility/_arguments +# or in ./_arguments, depending on where you're reading this file. +# +# __arguments takes no arguments. + +if (( ${words[(i)--]} < CURRENT )); then + # "Deriving spec forms from the help output" + _arguments : \ + '*-i[specify option name exclude patterns]:option name exclude pattern' \ + '*-s[specify option aliases]:pattern and replacement as "(this that)"' \ + '*:helpspec (pattern\:message\:action)' +else + _arguments -A '-*' : \ + '-n[set $NORMARG]' \ + '-s[enable single-letter option stacking (-x -y == -xy)]' \ + '-w[(rarely needed) enable single-letter option stacking with arguments (-x X -y == -xy X)]' \ + '-W[(rarely needed) enable single-letter option stacking with arguments in the same word (-x X -y == -xXy)]' \ + "-C[modify \$curcontext for \`->action' (requires \`local curcontext')]" \ + "-R[when \`->action' matches, return 300]" \ + "-S[honour \`--' as end-of-options guard]" \ + "-A[do not complete options after non-options]:pattern matching unknown options (e.g., '-*')" \ + '-O[pass elements of array variable to function calls in actions]:array variable name:_parameters -g array' \ + "-M[specify matchspec for completing option names and values]:matchspec for completing option names and values (default\\: 'r\\:|[_-]=* r\\:|=*')" \ + '-0[have ${(v)opt_args} be NUL-joined rather than colon-escaped and colon-joined]' \ + "--[derive optspecs from \`\${command} --help' output]" \ + '1::optional delimiter:(\:)' \ + '*:spec (e.g., "*(-t --to)"{-t+,--to=}"[specify recipient]\:recipient'\''s address\:_email_addresses)' + # TODO: doesn't support "Specifying Multiple Sets of Arguments" +fi
The new function, ___arguments, would not be called by _default by default, nor by _nothing, nor by nothing, but by _normal — at least, if the normal _normal _setup setup is used. ___arguments could also have completed __arguments by calling _arguments without arguments, but then nothing would have printed the "no argument or option" message that _nothing prints. I assume the decision to have ___arguments as a separate file, rather than simply add something to _nothing or have __arguments complete its own (__arguments') arguments by changing its first line from "#compdef _arguments" to "#compdef _arguments __arguments", may be somewhat controversial. However, while there are good arguments both for having __arguments completed by __arguments and for having __arguments completed by ___arguments, let us please have no long arguments about whether __arguments should be completed by __arguments or by ___arguments. My argument is that a complex argument about whether __arguments or ___arguments should complete __arguments' arguments would take time and tuits away from reviewing the arguments to the _arguments calls in __arguments' completion of the arguments to _arguments; that is: from reviewing the arguments to __arguments' _arguments _arguments' arguments completion calls. Review-by: Matthew Martin Thanks-to: comm -12 /usr/share/dict/words =(() { print -o -raC1 -- ${${@:t}#_} } Completion/{Base,Zsh}/**/_*(N) | uniq) --- Completion/Zsh/Function/___arguments | 11 +++++++++++ Completion/Zsh/Function/__arguments | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 Completion/Zsh/Function/___arguments diff --git a/Completion/Zsh/Function/___arguments b/Completion/Zsh/Function/___arguments new file mode 100644 index 000000000..187ae644c --- /dev/null +++ b/Completion/Zsh/Function/___arguments @@ -0,0 +1,11 @@ +#compdef __arguments ___arguments + +# Dear reader: This is the function ___arguments. This function is the +# completion function of the completion function of the completion utility +# function _arguments. If you're looking for the completion utility function +# _arguments or for its completion function __arguments, whose completion +# function is this function, then you've come to the wrong place. See +# ./__arguments for __arguments and for pointers to _arguments. + +# Unlike _arguments, __arguments and ___arguments take no arguments. +_nothing diff --git a/Completion/Zsh/Function/__arguments b/Completion/Zsh/Function/__arguments index 40a4e4b60..444a1c809 100644 --- a/Completion/Zsh/Function/__arguments +++ b/Completion/Zsh/Function/__arguments @@ -16,7 +16,7 @@ # source code, that'd be either in ../../../Completion/Base/Utility/_arguments # or in ./_arguments, depending on where you're reading this file. # -# __arguments takes no arguments. +# __arguments takes no arguments, as ___arguments would tell you. if (( ${words[(i)--]} < CURRENT )); then # "Deriving spec forms from the help output"
You wrote: > +++ b/Completion/Zsh/Function/__arguments It occurs to me that if completion for completion functions is a useful thing, _compadd could define a pattern to complete compadd options for all commands named _*. And with your __ convention, _nothing could specify a pattern for completion of completions for completions. I'm not fond of _git's use of an initial __ on it's private functions rather than just _git_... (or _git-... for subcommands). We should perhaps decide and then document the convention. I don't know whether you were planning to push this or if it only served as setup for your follow-up but I'll just make a few, um, arguments: > + _arguments -A '-*' : \ This is a rare case where the parameter to -A needs to be more complicated than just '-*'. I'd actually be inclined to use _regex_arguments for this function. Would allow specs to be broken up. > + '-n[set $NORMARG]' \ -n should not be used, it is fundamentally broken and will never be fixed because the right answer to $NORMARG as it is defined can be a list of possibilities. It grabs an internal variable from the first element of a linked-list and dumps it out. The problem it was added to solve would have had better solutions. Could hide it with ! or add a comment. > + "-C[modify \$curcontext for \`->action' (requires \`local curcontext')]" \ This is instead of the $context array. Should only be used where multiple valid states is not a possibility. I fear that the note in parentheses implies just `local curcontext' and not local curcontext="$curcontext" is needed. > + "-R[when \`->action' matches, return 300]" \ I don't think that really gets used much [[ -n $state ]] is more readable. > + "-M[specify matchspec for completing option names and values]:matchspec for completing option names and values (default\\: 'r\\:|[_-]=* r\\:|=*')" \ The convention widely used elsewhere is to represent defaults in square brackets without any `default: ' tag. _description does have mostly unused features for more custom formatting. We perhaps ought to consider whether this could be better used for units, defaults, examples, maximums and minimums so that this is under the control of the user. > + '*:spec (e.g., "*(-t --to)"{-t+,--to=}"[specify recipient]\:recipient'\''s address\:_email_addresses)' In your example, the * needs to come after the exclusion list. ! for ignored options comes before. Oliver
Oliver Kiddle wrote on Thu, Apr 01, 2021 at 14:53:02 +0200: > You wrote: > > +++ b/Completion/Zsh/Function/__arguments > > It occurs to me that if completion for completion functions is a useful > thing, Well, I do look up compadd and _arguments' options in the manual somewhat frequently. That's what gave me the idea of the 1/2 patch in the first place. > _compadd could define a pattern to complete compadd options for > all commands named _*. That might be useful for writing completion functions at the prompt. Of course, it's not needed for the "jog one's memory" use-case, since in that case one can just do «compadd -<TAB>». > And with your __ convention, _nothing could specify a pattern for > completion of completions for completions. To make this work, presumably compadd would want to register itself not for «_*» but for «_[^_]*» (glob, not regexp), so the patterns don't overlap. Whether it's a good idea to default __* functions to be completed by _nothing I don't have an intuition for, yet. > I'm not fond of _git's use of an initial __ on it's private functions > rather than just _git_... (or _git-... for subcommands). We should > perhaps decide and then document the convention. _git didn't exactly invent this. Using single or double leading underscores for private symbols is standard this way is standard or idiomatic in other languages too. > I don't know whether you were planning to push this I was/am. > or if it only served as setup for your follow-up but I'll just make a > few, um, arguments: > > > + _arguments -A '-*' : \ > > This is a rare case where the parameter to -A needs to be more > complicated than just '-*'. Hmm. How about «-A '-*|:|(!|)\(*'»? > I'd actually be inclined to use _regex_arguments for this function. > Would allow specs to be broken up. Could we leave this for future work? Not to let the perfect be the enemy of the good, etc.. > > + '-n[set $NORMARG]' \ > > -n should not be used, it is fundamentally broken and will never be > fixed because the right answer to $NORMARG as it is defined can be a > list of possibilities. It grabs an internal variable from the first > element of a linked-list and dumps it out. The problem it was added to > solve would have had better solutions. > > Could hide it with ! or add a comment. > I'll add !, and we should document that in the manual too if it isn't already. > > + "-C[modify \$curcontext for \`->action' (requires \`local curcontext')]" \ > > This is instead of the $context array. > Should only be used where multiple valid states is not a possibility. There isn't room to say all this inside the brackets. > I fear that the note in parentheses implies just `local curcontext' and > not local curcontext="$curcontext" is needed. Good point. How about deleting the parenthetical? Or replacing it by "(see manual)"? > > + "-R[when \`->action' matches, return 300]" \ > > I don't think that really gets used much [[ -n $state ]] is more > readable. How does this affect the patch? Are you suggesting to hide (with «!») this too? > > + "-M[specify matchspec for completing option names and values]:matchspec for completing option names and values (default\\: 'r\\:|[_-]=* r\\:|=*')" \ > > The convention widely used elsewhere is to represent defaults in square > brackets without any `default: ' tag. > I'll make it so. > _description does have mostly unused features for more custom > formatting. We perhaps ought to consider whether this could be better > used for units, defaults, examples, maximums and minimums so that this > is under the control of the user. Sounds like a question for a new thread. > > + '*:spec (e.g., "*(-t --to)"{-t+,--to=}"[specify recipient]\:recipient'\''s address\:_email_addresses)' > > In your example, the * needs to come after the exclusion list. ! for > ignored options comes before. Good catch; will fix. Thanks for the review, Daniel
Daniel Shahaf wrote on Thu, Apr 01, 2021 at 17:33:05 +0000:
> Oliver Kiddle wrote on Thu, Apr 01, 2021 at 14:53:02 +0200:
> > You wrote:
> > > + '-n[set $NORMARG]' \
> >
> > -n should not be used, it is fundamentally broken and will never be
> > fixed because the right answer to $NORMARG as it is defined can be a
> > list of possibilities. It grabs an internal variable from the first
> > element of a linked-list and dumps it out. The problem it was added to
> > solve would have had better solutions.
> >
> > Could hide it with ! or add a comment.
> >
>
> I'll add !, and we should document that in the manual too if it isn't
> already.
The manual doesn't already include such a warning, but I don't
understand how exactly $NORMARG is broken, so I can't document that.
Could you elaborate on what cases «_arguments -n», as implemented, would
cause breakage in?
The comment above the definition of the linked list's type (struct
castate) says the list comprises one node per set. Does that refer to
_arguments option sets? If so, perhaps -n is fine to use so long as
option sets aren't also used?
Cheers,
Daniel
Daniel Shahaf wrote: > > _compadd could define a pattern to complete compadd options for > > all commands named _*. I've attached a patch to do this. It is a post pattern and I don't have any real commands named with an initial _ on my system. Perhaps it should also call _default for non-option arguments? It also checks $service and handles a few functions specially. I couldn't be bothered to make this more extensive because, well, it is all a bit meta and of questionable value. And, no I don't think these need separating into __ functions. > > > + _arguments -A '-*' : \ > Hmm. How about «-A '-*|:|(!|)\(*'»? I'd have expected it to be the same as the pattern used at the top of _arguments - '-([AMO]*|[0CRSWnsw])' but that doesn't seem to work. I wonder if some bug means it only works with -* > > > + "-C[modify \$curcontext for \`->action' (requires \`local curcontext')]" \ > > > > This is instead of the $context array. > > Should only be used where multiple valid states is not a possibility. > > There isn't room to say all this inside the brackets. > > > I fear that the note in parentheses implies just `local curcontext' and > > not local curcontext="$curcontext" is needed. > > Good point. How about deleting the parenthetical? Or replacing it by > "(see manual)"? I'd remove it. There's room in the square brackets for one of the points. - "instead of $context". Descriptions are truncated if needed so less important information coming at the end doesn't do much harm. > > > > + "-R[when \`->action' matches, return 300]" \ > > > > I don't think that really gets used much [[ -n $state ]] is more > > readable. > > How does this affect the patch? Are you suggesting to hide (with «!») > this too? Not really, it was just an aside. Oliver diff --git a/Completion/Zsh/Command/_compadd b/Completion/Zsh/Command/_compadd index 781fa2af8..9c92cda76 100644 --- a/Completion/Zsh/Command/_compadd +++ b/Completion/Zsh/Command/_compadd @@ -1,47 +1,94 @@ -#compdef compadd +#compdef compadd -P _* -local curcontext="$curcontext" state line ret=1 +local curcontext="$curcontext" ret=1 +local -a state line args typeset -A opt_args -_arguments -C -s -S -A "-*" \ - '-P+[specify prefix]:prefix' \ - '-S+[specify suffix]:suffix' \ - '-p+[specify hidden prefix]:hidden prefix' \ - '-s+[specify hidden suffix]:hidden suffix' \ - '-i+[specify ignored prefix]:ignored prefix' \ - '-I+[specify ignored suffix]:ignored suffix' \ - '(-k)-a[matches are elements of specified arrays]' \ - '(-a)-k[matches are keys of specified associative arrays]' \ - '-d+[specify display strings]:array:_parameters -g "*array*"' \ - '-l[list display strings one per line, not in columns]' \ +args=( + '-P+[specify prefix]:prefix' + '-S+[specify suffix]:suffix' + '-p+[specify hidden prefix]:hidden prefix' + '-s+[specify hidden suffix]:hidden suffix' + '-i+[specify ignored prefix]:ignored prefix' + '-I+[specify ignored suffix]:ignored suffix' '-o[specify order for matches by match string not by display string]:: : _values -s , order "match[order by match not by display string]" "nosort[matches are pre-ordered]" "numeric[order numerically]" - "reverse[order backwards]"' \ - '(-1 -E)-J+[specify match group]:group' \ - '!-V+:group' \ - '(-J -E)-1[remove only consecutive duplicates from group]' \ - '-2[preserve all duplicates]' \ - '(-x)-X[specify explanation]:explanation' \ - '(-X)-x[specify unconditional explanation]:explanation' \ - '-q[make suffix autoremovable]' \ - '-r+[specify character class for suffix autoremoval]:character class' \ - '-R+[specify function for suffix autoremoval]:function:_functions' \ - '-f[mark matches as being files]' \ - '-e[mark matches as being parameters]' \ - '-W[specify location for matches marked as files]' \ - '-F+[specify array of ignore patterns]:array:_parameters -g "*array*"' \ - '-Q[disable quoting of possible completions]' \ - '*-M[specify matching specifications]' \ - '-n[hide matches in completion listing]' \ - '-U[disable internal matching of completion candidates]' \ - '-O+[populate array with matches instead of adding them]:array:_parameters -g "*array*"' \ - '-A+[populate array with expanded matches instead of adding them]:array:_parameters -g "*array*"' \ - '-D+[delete elements from array corresponding to non-matching candidates]:array:_parameters -g "*array*"' \ - '-C[add special match that expands to all other matches]' \ - '(-1 -J)-E+[add specified number of display only matches]:number' \ - '*:candidate:->candidates' && ret=0 + "reverse[order backwards]"' + '(-1 -E)-J+[specify match group]:group' + '!-V+:group' + '(-J -E)-1[remove only consecutive duplicates from group]' + '-2[preserve all duplicates]' + '(-x)-X[specify explanation]:explanation' + '(-X)-x[specify unconditional explanation]:explanation' + '-q[make suffix autoremovable]' + '-r+[specify character class for suffix autoremoval]:character class' + '-R+[specify function for suffix autoremoval]:function:_functions' + '-F+[specify array of ignore patterns]:array:_parameters -g "*array*"' + '-Q[disable quoting of possible completions]' + '*-M[specify matching specifications]' + '-n[hide matches in completion listing]' + '-O+[populate array with matches instead of adding them]:array:_parameters -g "*array*"' + '-A+[populate array with expanded matches instead of adding them]:array:_parameters -g "*array*"' + '-D+[delete elements from array corresponding to non-matching candidates]:array:_parameters -g "*array*"' +) + +case $service in + compadd|_(path_|)files) + args+=( + '-W[specify location for matches marked as files]' + ) + ;| + compadd) + args+=( + '(-k)-a[matches are elements of specified arrays]' + '(-a)-k[matches are keys of specified associative arrays]' + '-d+[specify display strings]:array:_parameters -g "*array*"' + '-l[list display strings one per line, not in columns]' + '-f[mark matches as being files]' + '-e[mark matches as being parameters]' + '-C[add special match that expands to all other matches]' + '(-1 -J)-E+[add specified number of display only matches]:number' + '-U[disable internal matching of completion candidates]' + '*:candidate:->candidates' + ) + ;; + _dates) + args=( ${args:#([(][^)]##\)|)-[12noOAD]*} + '-f[specify format for matches]:format:_date_formats' + '-F[select a future rather than past date]' + ) + ;; + _(path_|)files) + args=( ${args:#([(][^)]##\)|)-[OAD]*} + '-g+[specify file glob pattern]:glob pattern' + '-/[complete only directories]' + ) + ;; + _parameters) + args+=( + '-g+[specify pattern to filter parameter type by]:pattern' + ) + ;; + _pids) + args+=( '-m+[pattern to filter process command line by]:pattern' ) + ;; + _process_names) + args+=( + '-a[include all processes]' + '-t[use truncated process names]' + ) + ;; + _sys_calls) + args+=( + '-a[add "all" as an additional match]' + '-n[add "none" as an additional match]' + ) + ;; +esac + +_arguments -C -s -S -A "-*" $args && ret=0 if [[ -n $state ]]; then if (( $+opt_args[-a] )); then
On 1 Apr, Daniel Shahaf wrote: > The manual doesn't already include such a warning, but I don't > understand how exactly $NORMARG is broken, so I can't document that. It'd be less effort to simply drop it from the documentation or mark it strictly deprecated. It isn't particularly needed. > Could you elaborate on what cases «_arguments -n», as implemented, would > cause breakage in? I can't remember the exact details, it was a while ago that I went over the _arguments source code. But I think you can cause issues by having optional arguments to options and not just sets - those also produce multiple possible states. The field in castate was being misunderstood in general. Sorry I realize I'm being a bit vague but I don't remember the details sufficiently. I think I intended to fixup each of the uses of the option first before raising the issue but I never got around to that. Oliver
Oliver Kiddle wrote on Fri, Apr 09, 2021 at 21:24:58 +0200:
> On 1 Apr, Daniel Shahaf wrote:
> > Could you elaborate on what cases «_arguments -n», as implemented, would
> > cause breakage in?
>
> I can't remember the exact details, it was a while ago that I went over
> the _arguments source code. But I think you can cause issues by having
> optional arguments to options and not just sets - those also produce
> multiple possible states. The field in castate was being misunderstood
> in general. Sorry I realize I'm being a bit vague but I don't remember
> the details sufficiently.
>
> I think I intended to fixup each of the uses of the option first before
> raising the issue but I never got around to that.
>
> > The manual doesn't already include such a warning, but I don't
> > understand how exactly $NORMARG is broken, so I can't document that.
>
> It'd be less effort to simply drop it from the documentation or mark it
> strictly deprecated. It isn't particularly needed.
Well, I'm not fond of documenting problems from memory, but
a deprecation notice would probably be useful to readers, so no
objections from me to doing that.
Thanks,
Daniel
On Fri, Apr 2, 2021, at 7:58 PM, Oliver Kiddle wrote:
> Daniel Shahaf wrote:
> > > _compadd could define a pattern to complete compadd options for
> > > all commands named _*.
>
> I've attached a patch to do this.
bump
vq
Lawrence Velázquez wrote on Sat, Apr 10, 2021 at 16:41:18 -0400:
> On Fri, Apr 2, 2021, at 7:58 PM, Oliver Kiddle wrote:
> > Daniel Shahaf wrote:
> > > > _compadd could define a pattern to complete compadd options for
> > > > all commands named _*.
> >
> > I've attached a patch to do this.
>
> bump
Already committed in zsh-5.8-360-geaff11c74.
> > > > + _arguments -A '-*' : \
>
> > Hmm. How about «-A '-*|:|(!|)\(*'»?
>
> I'd have expected it to be the same as the pattern used at the top of
> _arguments - '-([AMO]*|[0CRSWnsw])' but that doesn't seem to work.
I tried that pattern and it seemed to DTRT, so I committed it this way.
If I overlooked something, we can change it back to «-*».
Thanks for the review.
Daniel
P.S. Already have an idea for next April; anyone interested in helping,
ping me offlist ☺