zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH 1/2] __arguments: New completion function for _arguments.
@ 2021-04-01  0:00 Daniel Shahaf
  2021-04-01  0:00 ` [PATCH 2/2] ___arguments: New completion function for __arguments Daniel Shahaf
  2021-04-01 12:53 ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
  0 siblings, 2 replies; 11+ messages in thread
From: Daniel Shahaf @ 2021-04-01  0:00 UTC (permalink / raw)
  To: zsh-workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] ___arguments: New completion function for __arguments.
  2021-04-01  0:00 [PATCH 1/2] __arguments: New completion function for _arguments Daniel Shahaf
@ 2021-04-01  0:00 ` Daniel Shahaf
  2021-04-01 12:53 ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel Shahaf @ 2021-04-01  0:00 UTC (permalink / raw)
  To: zsh-workers

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"


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] __arguments: New completion function for _arguments.
  2021-04-01  0:00 [PATCH 1/2] __arguments: New completion function for _arguments Daniel Shahaf
  2021-04-01  0:00 ` [PATCH 2/2] ___arguments: New completion function for __arguments Daniel Shahaf
@ 2021-04-01 12:53 ` Oliver Kiddle
  2021-04-01 17:33   ` Daniel Shahaf
  1 sibling, 1 reply; 11+ messages in thread
From: Oliver Kiddle @ 2021-04-01 12:53 UTC (permalink / raw)
  To: Zsh workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] __arguments: New completion function for _arguments.
  2021-04-01 12:53 ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
@ 2021-04-01 17:33   ` Daniel Shahaf
  2021-04-01 22:34     ` _arguments -n / $NORMARG (was: Re: [PATCH 1/2] __arguments: New completion function for _arguments.) Daniel Shahaf
  2021-04-02 23:58     ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
  0 siblings, 2 replies; 11+ messages in thread
From: Daniel Shahaf @ 2021-04-01 17:33 UTC (permalink / raw)
  To: Zsh workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* _arguments -n / $NORMARG (was: Re: [PATCH 1/2] __arguments: New completion function for _arguments.)
  2021-04-01 17:33   ` Daniel Shahaf
@ 2021-04-01 22:34     ` Daniel Shahaf
  2021-04-09 19:24       ` Oliver Kiddle
  2021-04-02 23:58     ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
  1 sibling, 1 reply; 11+ messages in thread
From: Daniel Shahaf @ 2021-04-01 22:34 UTC (permalink / raw)
  To: Zsh workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] __arguments: New completion function for _arguments.
  2021-04-01 17:33   ` Daniel Shahaf
  2021-04-01 22:34     ` _arguments -n / $NORMARG (was: Re: [PATCH 1/2] __arguments: New completion function for _arguments.) Daniel Shahaf
@ 2021-04-02 23:58     ` Oliver Kiddle
  2021-04-10 20:41       ` Lawrence Velázquez
  2021-04-17 12:25       ` Daniel Shahaf
  1 sibling, 2 replies; 11+ messages in thread
From: Oliver Kiddle @ 2021-04-02 23:58 UTC (permalink / raw)
  To: Zsh workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _arguments -n / $NORMARG (was: Re: [PATCH 1/2] __arguments: New completion function for _arguments.)
  2021-04-01 22:34     ` _arguments -n / $NORMARG (was: Re: [PATCH 1/2] __arguments: New completion function for _arguments.) Daniel Shahaf
@ 2021-04-09 19:24       ` Oliver Kiddle
  2021-04-09 22:52         ` Daniel Shahaf
  0 siblings, 1 reply; 11+ messages in thread
From: Oliver Kiddle @ 2021-04-09 19:24 UTC (permalink / raw)
  To: Zsh workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _arguments -n / $NORMARG (was: Re: [PATCH 1/2] __arguments: New completion function for _arguments.)
  2021-04-09 19:24       ` Oliver Kiddle
@ 2021-04-09 22:52         ` Daniel Shahaf
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Shahaf @ 2021-04-09 22:52 UTC (permalink / raw)
  To: zsh-workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] __arguments: New completion function for _arguments.
  2021-04-02 23:58     ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
@ 2021-04-10 20:41       ` Lawrence Velázquez
  2021-04-13 11:28         ` Daniel Shahaf
  2021-04-17 12:25       ` Daniel Shahaf
  1 sibling, 1 reply; 11+ messages in thread
From: Lawrence Velázquez @ 2021-04-10 20:41 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: zsh-workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] __arguments: New completion function for _arguments.
  2021-04-10 20:41       ` Lawrence Velázquez
@ 2021-04-13 11:28         ` Daniel Shahaf
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Shahaf @ 2021-04-13 11:28 UTC (permalink / raw)
  To: Lawrence Velázquez; +Cc: Oliver Kiddle, zsh-workers

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.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] __arguments: New completion function for _arguments.
  2021-04-02 23:58     ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
  2021-04-10 20:41       ` Lawrence Velázquez
@ 2021-04-17 12:25       ` Daniel Shahaf
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel Shahaf @ 2021-04-17 12:25 UTC (permalink / raw)
  To: zsh-workers

> > > > +  _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 ☺


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-04-17 12:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-01  0:00 [PATCH 1/2] __arguments: New completion function for _arguments Daniel Shahaf
2021-04-01  0:00 ` [PATCH 2/2] ___arguments: New completion function for __arguments Daniel Shahaf
2021-04-01 12:53 ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
2021-04-01 17:33   ` Daniel Shahaf
2021-04-01 22:34     ` _arguments -n / $NORMARG (was: Re: [PATCH 1/2] __arguments: New completion function for _arguments.) Daniel Shahaf
2021-04-09 19:24       ` Oliver Kiddle
2021-04-09 22:52         ` Daniel Shahaf
2021-04-02 23:58     ` [PATCH 1/2] __arguments: New completion function for _arguments Oliver Kiddle
2021-04-10 20:41       ` Lawrence Velázquez
2021-04-13 11:28         ` Daniel Shahaf
2021-04-17 12:25       ` Daniel Shahaf

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).