zsh-workers
 help / Atom feed
* [PATCH] Completion: Fix use of -A and -S options to _arguments
@ 2019-04-24 17:59 dana
  2019-04-25  8:00 ` Oliver Kiddle
  0 siblings, 1 reply; 2+ messages in thread
From: dana @ 2019-04-24 17:59 UTC (permalink / raw)
  To: Zsh hackers list

Following again on workers/44242.

Patch #2: Make use of the -A and -S options to _arguments consistent in these
functions.

I can't think of any reason you would ever *not* want to use -S for these, but
someone did go out of their way to make it conditional in _ln — if anyone
knows why that might be, please tell me.

Aside: The use of *:: in _ln and _rm prevents those functions from offering
options after an operand has been given. I can't recall if this behaviour has
ever been discussed here before — is there a way to deal with it?

dana


diff --git a/Completion/Unix/Command/_chmod b/Completion/Unix/Command/_chmod
index e39cfd8ee..f72055b17 100644
--- a/Completion/Unix/Command/_chmod
+++ b/Completion/Unix/Command/_chmod
@@ -1,7 +1,7 @@
 #compdef chmod gchmod zf_chmod
 
 local curcontext="$curcontext" state line expl ret=1 variant
-local -a args privs
+local -a args privs aopts=( -A '-*' )
 
 args=( '*: :->files' '1: :_file_modes' )
 
@@ -17,6 +17,7 @@ case "$variant" in
     )
     ;;
   gnu)
+    aopts=()
     args+=(
       '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
       '(-v --verbose -c --changes)'{-v,--verbose}'[output a diagnostic for every file processed]'
@@ -63,7 +64,7 @@ case "$variant" in
     ;;
 esac
 
-_arguments -C -s "$args[@]" && ret=0
+_arguments -C -s -S $aopts "$args[@]" && ret=0
 
 case "$state" in
   files)
diff --git a/Completion/Unix/Command/_chown b/Completion/Unix/Command/_chown
index 8fde02096..849607448 100644
--- a/Completion/Unix/Command/_chown
+++ b/Completion/Unix/Command/_chown
@@ -1,11 +1,12 @@
 #compdef chown chgrp gchown=chown gchgrp=chgrp zf_chown=chown zf_chgrp=chgrp
 
 local curcontext="$curcontext" state line expl ret=1 variant
-local suf usr grp req deref pattern arg args
+local suf usr grp req deref pattern arg args aopts=( -A '-*' )
 
 _pick_variant -r variant -b zsh gnu=Free\ Soft $OSTYPE --version
 case "$variant" in
   gnu)
+    aopts=()
     args=(
       '(-c --changes -v --verbose)'{-c,--changes}'[report each change made]'
       '(-c --changes -v --verbose)'{-v,--verbose}'[output info for every file processed]'
@@ -65,7 +66,7 @@ case "$variant" in
 esac
 
 (( $+words[(r)--reference*] )) || args+=( '(--reference)1: :->owner' )
-_arguments -C -s "$args[@]" '*: :->files' && ret=0
+_arguments -C -s -S $aopts "$args[@]" '*: :->files' && ret=0
 
 case $state in
   owner)
diff --git a/Completion/Unix/Command/_ln b/Completion/Unix/Command/_ln
index 339d327fe..d67e54264 100644
--- a/Completion/Unix/Command/_ln
+++ b/Completion/Unix/Command/_ln
@@ -3,7 +3,7 @@
 local curcontext="$curcontext" state line ret=1 variant
 local -A opt_args
 
-local -a args opts
+local -a args opts=( -A '-*' )
 args=(
   '(-i)-f[remove existing destination files]'
   '-s[create symbolic links instead of hard links]'
@@ -12,7 +12,7 @@ args=(
 _pick_variant -r variant -b zsh gnu=gnu $OSTYPE --help
 case $variant; in
   gnu)
-    opts=(-S)
+    opts=()
     args=(
       '(-b --backup)-b[create a backup of each existing destination file]' \
       '(-b --backup)--backup=[create a backup of each existing destination file]::method:((
@@ -70,7 +70,7 @@ case $variant; in
     ;;
 esac
 
-_arguments -C -s $opts : \
+_arguments -C -s -S $opts : \
   $args \
   ':link target:_files' \
   '*:: :->files' && ret=0
diff --git a/Completion/Unix/Command/_mkdir b/Completion/Unix/Command/_mkdir
index 58d1b8f48..e5c99a1e1 100644
--- a/Completion/Unix/Command/_mkdir
+++ b/Completion/Unix/Command/_mkdir
@@ -1,12 +1,13 @@
 #compdef mkdir gmkdir zf_mkdir
 
-local curcontext="$curcontext" state line expl args variant ret=1
+local curcontext="$curcontext" variant ret=1
+local -a state line expl args aopts=( -A '-*' )
 typeset -A opt_args
 
 args=(
   '(-m --mode)'{-m,--mode=}'[set permission mode]: :_file_modes'
   '(-p --parents)'{-p,--parents}'[make parent directories as needed]'
-  '(-)*: :->directories'
+  '*: :->directories'
 )
 
 _pick_variant -r variant -b zsh gnu=gnu $OSTYPE --help
@@ -18,6 +19,7 @@ case $variant in
     )
   ;|
   gnu)
+    aopts=()
     if [[ $OSTYPE == linux* ]]; then
       args+=(
         '(-Z --context)'{-Z,--context=}'[set SELinux context]:SELinux context'
@@ -33,7 +35,7 @@ case $variant in
   ;;
 esac
 
-_arguments -C -s $args && ret=0
+_arguments -C -s -S $aopts $args && ret=0
 
 case "$state" in
   directories)
diff --git a/Completion/Unix/Command/_mv b/Completion/Unix/Command/_mv
index 2b8ac3273..027b2e68c 100644
--- a/Completion/Unix/Command/_mv
+++ b/Completion/Unix/Command/_mv
@@ -1,10 +1,11 @@
 #compdef mv gmv zf_mv
 
-local args variant
+local args variant aopts=( -A '-*' )
 
 _pick_variant -r variant -b zsh gnu=GNU $OSTYPE --version
 case $variant; in
   gnu)
+    aopts=()
     args=(
       '(-b --backup -n --no-clobber)--backup=[make a backup of each existing destination file]: : _values "backup type"
         {none,off}"[never make backups]"
@@ -50,5 +51,5 @@ case $variant; in
     ;;
 esac
 
-_arguments -s -S $args \
+_arguments -s -S $aopts $args \
   '*:file:_files'
diff --git a/Completion/Unix/Command/_rm b/Completion/Unix/Command/_rm
index dfd3a394a..ea9190de2 100644
--- a/Completion/Unix/Command/_rm
+++ b/Completion/Unix/Command/_rm
@@ -1,7 +1,7 @@
 #compdef rm grm zf_rm
 
 local variant
-declare -a opts args
+declare -a args opts=( -A '-*' )
 args=(
   '(-f --force)'{-f,--force}'[ignore nonexistent files, never prompt]'
   '(-I --interactive)-i[prompt before every removal]'
@@ -11,7 +11,7 @@ args=(
 _pick_variant -r variant -b zsh gnu=gnu $OSTYPE --help
 case $variant; in
   gnu)
-    opts+=(-S)
+    opts=()
     args+=(
       '(-i --interactive)-I[prompt when removing many files]'
       '(-i -I)--interactive=-[prompt under given condition (defaulting to always)]::when:((once\:"prompt when removing many files"
@@ -64,7 +64,7 @@ esac
 local curcontext=$curcontext state line ret=1
 declare -A opt_args
 
-_arguments -C -s $opts \
+_arguments -C -s -S $opts \
   $args && ret=0
 
 case $state in
diff --git a/Completion/Unix/Command/_rmdir b/Completion/Unix/Command/_rmdir
index 004511558..d330e0aef 100644
--- a/Completion/Unix/Command/_rmdir
+++ b/Completion/Unix/Command/_rmdir
@@ -1,7 +1,7 @@
 #compdef rmdir grmdir zf_rmdir
 
 local variant
-local -a args
+local -a args aopts=( -A '-*' )
 
 args=(
   '(-p --parents)'{-p,--parents}'[remove each component of the specified paths]'
@@ -10,6 +10,7 @@ args=(
 _pick_variant -r variant -b zsh gnu=GNU $OSTYPE --version
 case $variant; in
   gnu)
+    aopts=()
     args+=(
       '--ignore-fail-on-non-empty[ignore failure if directory is non-empty]'
       '(-v --verbose)'{-v,--verbose}'[be verbose]'
@@ -26,6 +27,6 @@ case $variant; in
     ;;
 esac
 
-_arguments -s -S -A '-*' \
+_arguments -s -S $aopts \
   $args \
   '*:directories:_directories'



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

* Re: [PATCH] Completion: Fix use of -A and -S options to _arguments
  2019-04-24 17:59 [PATCH] Completion: Fix use of -A and -S options to _arguments dana
@ 2019-04-25  8:00 ` Oliver Kiddle
  0 siblings, 0 replies; 2+ messages in thread
From: Oliver Kiddle @ 2019-04-25  8:00 UTC (permalink / raw)
  To: Zsh workers

dana wrote:
> Patch #2: Make use of the -A and -S options to _arguments consistent in these
> functions.
>
> I can't think of any reason you would ever *not* want to use -S for these, but

Generally, it's purely a question of does the command-line parser for
the command in question allow options after ???--???.

> someone did go out of their way to make it conditional in _ln ??? if anyone
> knows why that might be, please tell me.

I went to check on the assumption that it was likely to have been me
but it wasn't. Given that it is done conditionally for GNU, I'd guess that
???-S??? got confused with ???-A "-*"???. In general, GNU tools tend to allow
options after other arguments while it is less common elsewhere. -- is
more widely recognised.

> Aside: The use of *:: in _ln and _rm prevents those functions from offering
> options after an operand has been given. I can't recall if this behaviour has
> ever been discussed here before ??? is there a way to deal with it?

It is behaviour I was aware of and know that I thought about it when I
was making changes to the comparguments. There were at least a couple of
such oddities that I was reluctant to change for backward compatibility
reasons and there may have been reasons for it that are apparent when
you dig into the code. In combination with a state, it isn't really
fixable. I don't remember if it has been explicitly discussed as such.

Oliver

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

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 17:59 [PATCH] Completion: Fix use of -A and -S options to _arguments dana
2019-04-25  8:00 ` Oliver Kiddle

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/ public-inbox