[PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Daniel Shahaf]

Found by grepping for patterns that can match needles of various lengths:

    :grep 'compset -[PS].*[\#^*()\|<>?~\]' Completion/
---
I don't have all these programs installed, so in some cases I guessed whether
shortest or longest match was intended.

I'm not sure what to do about the handling of «_path_files -P foo»: that -P
option documents its argument as a string but interprets it as a pattern.
Should we update the documentation to match the implementation, or vice-versa?

Cheers,

Daniel

P.S. For the curious, arguments whose completion is implemented
using 'compset -P' often fell into one of the following patterns:
"comma,separated,list", "user@host", or "option=value".


diff --git a/Completion/Redhat/Command/_rpm b/Completion/Redhat/Command/_rpm
index b2157bd..b24213e 100644
--- a/Completion/Redhat/Command/_rpm
+++ b/Completion/Redhat/Command/_rpm
@@ -298,7 +298,7 @@ _rpm () {
 	  ${(f)"$(_call_program capabilities rpm -qa --queryformat '%\{requirename}\\n' 2>/dev/null)"} && ret=0
       ;;
     relocate)
-      if compset -P '*='; then
+      if compset -P 1 '*='; then
 	_description directories expl 'new path'
       else
 	_description directories expl 'old path'
diff --git a/Completion/Unix/Command/_ant b/Completion/Unix/Command/_ant
index 195a543..19c252a 100644
--- a/Completion/Unix/Command/_ant
+++ b/Completion/Unix/Command/_ant
@@ -83,7 +83,7 @@ case $state in
       "classpath:$state:_path_files -r': ' -/" && ret=0
   ;;
   property)
-    if compset -P '*='; then
+    if compset -P 1 '*='; then
       _default && ret=0
     else
       _message -e properties 'property name'
diff --git a/Completion/Unix/Command/_cpio b/Completion/Unix/Command/_cpio
index 6b07a21..4027cc0 100644
--- a/Completion/Unix/Command/_cpio
+++ b/Completion/Unix/Command/_cpio
@@ -106,10 +106,10 @@ fi
 _arguments -C -s "$args[@]" && ret=0
 
 if [[ $state = afile ]]; then
-  if compset -P '*:'; then
-    # TODO: doesn't need to be rsh.
-    _wanted files expl 'remote files' \
-       compadd $(rsh ${words[CURRENT]%:*} echo ${words[CURRENT]#*:}\*) && ret=0
+  if [[ $ig != gnu ]]; then
+    _files && ret=0
+  elif compset -P 1 '*:'; then
+    _remote_files -- ssh && ret=0
   elif compset -P '*@'; then
     _wanted hosts expl 'remote host name' _hosts && ret=0
   else
diff --git a/Completion/Unix/Command/_cvs b/Completion/Unix/Command/_cvs
index 18383c3..0552d21 100644
--- a/Completion/Unix/Command/_cvs
+++ b/Completion/Unix/Command/_cvs
@@ -595,7 +595,7 @@ _cvs_tempdir() {
 
 (( $+functions[_cvs_user_variable] )) ||
 _cvs_user_variable() {
-  if compset -P '*='; then
+  if compset -P 1 '*='; then
     _default
   else
     _message -e variables "variable"
diff --git a/Completion/Unix/Command/_dbus b/Completion/Unix/Command/_dbus
index b24a6e9..fd03574 100644
--- a/Completion/Unix/Command/_dbus
+++ b/Completion/Unix/Command/_dbus
@@ -32,7 +32,7 @@ esac
 case $state in
   addresses)
     compset -P '*;'
-    if compset -P '*='; then
+    if compset -P 1 '*='; then
       _files && ret=0
     else
       _message -e addresses address
diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index a00e1d7..877c932 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -1335,7 +1335,7 @@ _git-push () {
     '--follow-tags[also push missing annotated tags reachable from the pushed refs]' \
     '(--receive-pack --exec)'{--receive-pack=-,--exec=-}'[path to git-receive-pack on remote]:remote git-receive-pack:_files' \
     '(--force-with-lease --no-force-with-lease)*--force-with-lease=-[allow refs that are not ancestors to be updated if current ref matches expected value]::ref and expectation:->lease' \
-    '(--force-with-lease --no-force-with-lease)--no-force-with-lease=-[cancel all previous force-with-lease specifications]' \
+    '(--force-with-lease --no-force-with-lease)--no-force-with-lease[cancel all previous force-with-lease specifications]' \
     '(-f --force)'{-f,--force}'[allow refs that are not ancestors to be updated]' \
     '(:)--repo=[default repository to use]:repository:__git_any_repositories' \
     '(-u --set-upstream)'{-u,--set-upstream}'[add upstream reference for each branch that is up to date or pushed]' \
diff --git a/Completion/Unix/Command/_graphicsmagick b/Completion/Unix/Command/_graphicsmagick
index 150f5ae..cc541d8 100644
--- a/Completion/Unix/Command/_graphicsmagick
+++ b/Completion/Unix/Command/_graphicsmagick
@@ -360,7 +360,7 @@ case "$words[2]" in
       '*:picture file:_imagemagick' && return
 
     if [[ "$state" = profile ]]; then
-      if compset -P '*:'; then
+      if compset -P 1 '*:'; then
 	_files
       else
 	_wanted prefixes expl 'profile type' compadd icc: iptc:
diff --git a/Completion/Unix/Command/_growisofs b/Completion/Unix/Command/_growisofs
index 36b45d5..f67961f 100644
--- a/Completion/Unix/Command/_growisofs
+++ b/Completion/Unix/Command/_growisofs
@@ -116,7 +116,7 @@ _mkisofs_sparc_boot_images () {
 _mkisofs_pathspec () {
   local sep
   if (( $words[(I)-graft-points] )); then
-    if ! compset -P '*[^\\]\='; then
+    if ! compset -P 1 '*[^\\]\='; then
       sep='-qS='
     fi
   fi
@@ -335,7 +335,7 @@ else
 
     case "$state" in
       (devimg)
-      if compset -P \*=; then
+      if compset -P 1 '*='; then
         _files
       else
         _files -g "*(%,@)"
diff --git a/Completion/Unix/Command/_gs b/Completion/Unix/Command/_gs
index 22f3c78..98ba149 100644
--- a/Completion/Unix/Command/_gs
+++ b/Completion/Unix/Command/_gs
@@ -32,7 +32,7 @@ else
     fi
     ;;
   sname)
-    if compset -P '*='; then
+    if compset -P 1 '*='; then
       case "$IPREFIX" in
       *DEVICE\=)
         _wanted devices expl 'ghostscript device' \
diff --git a/Completion/Unix/Command/_gsettings b/Completion/Unix/Command/_gsettings
index f47bbc6..72f0157 100644
--- a/Completion/Unix/Command/_gsettings
+++ b/Completion/Unix/Command/_gsettings
@@ -31,7 +31,7 @@ case $state in
     state=''
   ;;
   schemata)
-    if compset -P '*:'; then
+    if compset -P 1 '*:'; then
       _directories && ret=0
     else
       _wanted schemata expl 'schema' compadd -M 'r:|.=* r:|=*' \
diff --git a/Completion/Unix/Command/_head b/Completion/Unix/Command/_head
index 4f956ac..c76cce3 100644
--- a/Completion/Unix/Command/_head
+++ b/Completion/Unix/Command/_head
@@ -32,7 +32,7 @@ case $state in
     sign='sign:sign:((-\:"print all but the last specified bytes/lines"'
     sign+=' +\:"print the first specified bytes/lines (default)"))'
     digit='digits:digit:(0 1 2 3 4 5 6 7 8 9)'
-    if compset -P '*[0-9]'; then
+    if compset -P '[0-9]##'; then
       _alternative $mlt $digit && ret=0
     elif [[ -z $PREFIX ]]; then
       _alternative $sign $digit && ret=0
diff --git a/Completion/Unix/Command/_imagemagick b/Completion/Unix/Command/_imagemagick
index 4a9b62a..c2c9dc4 100644
--- a/Completion/Unix/Command/_imagemagick
+++ b/Completion/Unix/Command/_imagemagick
@@ -364,7 +364,7 @@ case "$service" in
       '*:picture file:_imagemagick' && return
 
     if [[ "$state" = profile ]]; then
-      if compset -P '*:'; then
+      if compset -P 1 '*:'; then
 	_files
       else
 	_wanted prefixes expl 'profile type' compadd icc: iptc:
diff --git a/Completion/Unix/Command/_java b/Completion/Unix/Command/_java
index 2aef15a..7d5bd42 100644
--- a/Completion/Unix/Command/_java
+++ b/Completion/Unix/Command/_java
@@ -578,7 +578,7 @@ encoding)
   ;;
 
 property)
-  if compset -P '*='; then
+  if compset -P 1 '*='; then
     _default && return
   else
     _message -e property-names 'property name'
diff --git a/Completion/Unix/Command/_lp b/Completion/Unix/Command/_lp
index 8da84a1..63fbab7 100644
--- a/Completion/Unix/Command/_lp
+++ b/Completion/Unix/Command/_lp
@@ -44,7 +44,7 @@ _lp_job_options()
 
   # The program specified by the style list-printer-options should list jobs in
   # the same style as lpoptions -l.
-  if compset -P '*='; then
+  if compset -P 1 '*='; then
     # List values for the option
     case ${IPREFIX%=} in
       (media)
diff --git a/Completion/Unix/Command/_lzop b/Completion/Unix/Command/_lzop
index 6f09cf4..7661d11 100644
--- a/Completion/Unix/Command/_lzop
+++ b/Completion/Unix/Command/_lzop
@@ -69,8 +69,9 @@ case "$state" in
       "F $sep Append a \`*' for executable files"
       "G $sep Inhibit display of group information"
       "Q $sep Enclose file names in double quotes" )
+    # The "Z" on the next line is sentinel to prevent the character class from being empty.
     disp=( ${disp[@]:#[Z$PREFIX]*} )
-    compset -P '[FGQ]*'
+    compset -P '[FGQ]#'
     compadd -d disp - ${disp[@]%% *} && ret=0
   ;;
 esac
diff --git a/Completion/Unix/Command/_mount b/Completion/Unix/Command/_mount
index a43085a..e2c3cfd 100644
--- a/Completion/Unix/Command/_mount
+++ b/Completion/Unix/Command/_mount
@@ -856,7 +856,7 @@ fsopt)
 devordir)
   local dev_tmp mp_tmp mline
 
-  if compset -P '*:'; then
+  if compset -P 1 '*:'; then
     _wanted exports expl 'exported path' compadd \
 	${${(f)"$(path+=( {/usr,}/sbin(N) ) _call_program exports \
 	showmount -e ${IPREFIX%:} 2>/dev/null)"}[2,-1]%% *} && ret=0
diff --git a/Completion/Unix/Command/_perl b/Completion/Unix/Command/_perl
index 52559b8..0914264 100644
--- a/Completion/Unix/Command/_perl
+++ b/Completion/Unix/Command/_perl
@@ -56,7 +56,7 @@ _perl_normal() {
 _perl_m_opt () {
   compset -P '-'
 
-  if compset -P '*='; then
+  if compset -P 1 '*='; then
     _message -e module-arguments 'module arguments, comma separated'
   else
     _perl_modules -S= -q
diff --git a/Completion/Unix/Command/_php b/Completion/Unix/Command/_php
index 0e84003..d03f339 100644
--- a/Completion/Unix/Command/_php
+++ b/Completion/Unix/Command/_php
@@ -50,7 +50,7 @@ case $state in
     local -a directives suf
     local code='foreach (ini_get_all() as $k => $v) { echo "$k\n"; }'
     directives=( $(_call_program directives $words[1] -r ${(q)code} 2>/dev/null) )
-    if compset -P '*='; then
+    if compset -P 1 '*='; then
       _default && return 0
     else
       compset -S '=*' || suf=( -qS '=' )
diff --git a/Completion/Unix/Command/_rlogin b/Completion/Unix/Command/_rlogin
index 8f74939..685e4e5 100644
--- a/Completion/Unix/Command/_rlogin
+++ b/Completion/Unix/Command/_rlogin
@@ -38,9 +38,9 @@ _rlogin () {
       '*:files:->files' && ret=0
 
     if [[ -n "$state" ]]; then
-      if compset -P '*:'; then
+      if compset -P 1 '*:'; then
 	_remote_files -- rsh && ret=0
-      elif compset -P '*@'; then
+      elif compset -P 1 '*@'; then
         _wanted hosts expl host _rlogin_hosts -S: && ret=0
       else
         _alternative \
diff --git a/Completion/Unix/Command/_ssh b/Completion/Unix/Command/_ssh
index 5ee4fd2..8432819 100644
--- a/Completion/Unix/Command/_ssh
+++ b/Completion/Unix/Command/_ssh
@@ -200,7 +200,7 @@ _ssh () {
 
     case "$lstate" in
     option)
-      if compset -P '*='; then
+      if compset -P 1 '*='; then
         case "${IPREFIX#-o}" in
           (#i)(ciphers|macs|kexalgorithms|hostkeyalgorithms|pubkeyacceptedkeytypes|hostbasedkeytypes)=)
           if ! compset -P +; then
@@ -628,9 +628,9 @@ _ssh () {
       fi
       ;;
     file)
-      if compset -P '[^./][^/]#:'; then
+      if compset -P 1 '[^./][^/]#:'; then
         _remote_files -- ssh ${(kv)~opt_args[(I)-[FP1246]]/-P/-p} && ret=0
-      elif compset -P '*@'; then
+      elif compset -P 1 '*@'; then
         suf=( -S '' )
         compset -S ':*' || suf=( -r: -S: )
         _wanted hosts expl 'remote host name' _ssh_hosts $suf && ret=0
@@ -642,9 +642,9 @@ _ssh () {
       fi
       ;;
     rfile)
-      if compset -P '*:'; then
+      if compset -P 1 '*:'; then
         _remote_files -- ssh && ret=0
-      elif compset -P '*@'; then
+      elif compset -P 1 '*@'; then
         _wanted hosts expl host _ssh_hosts -r: -S: && ret=0
       else
         _alternative \
diff --git a/Completion/Unix/Command/_tail b/Completion/Unix/Command/_tail
index fbe30f1..89e9cfb 100644
--- a/Completion/Unix/Command/_tail
+++ b/Completion/Unix/Command/_tail
@@ -56,7 +56,7 @@ case $state in
     sign='signs:sign:((+\:"start at the specified byte/line"'
     sign+=' -\:"output the last specified bytes/lines (default)"))'
     digit='digits:digit:(0 1 2 3 4 5 6 7 8 9)'
-    if compset -P '*[0-9]'; then
+    if compset -P '[0-9]##'; then
       _alternative $mlt $digit && ret=0
     elif [[ -z $PREFIX ]]; then
       _alternative $sign $digit && ret=0
diff --git a/Completion/Unix/Command/_w3m b/Completion/Unix/Command/_w3m
index 9569368..6e83a67 100644
--- a/Completion/Unix/Command/_w3m
+++ b/Completion/Unix/Command/_w3m
@@ -90,7 +90,7 @@ case "$state" in
   option)
     local -a options
     options=( ${${(M)${(f)"$(_call_program options $words[1] -show-option 2>/dev/null)"}:#    -o *}/(#b)    -o (*)=[^ ]#[[:blank:]]##(*)/$match[1]:${match[2]:l}} )
-    if compset -P '*='; then
+    if compset -P 1 '*='; then
       _message -e values 'value'
     else
       compset -S '=*' || suf=( -S '=' )
@@ -98,7 +98,7 @@ case "$state" in
     fi
   ;;
   pauth)
-    if compset -P '*:'; then
+    if compset -P 1 '*:'; then
       _message -e passwords 'password'
     else
       compset -S ':*' || suf=( -S ':' )
diff --git a/Completion/Unix/Type/_path_files b/Completion/Unix/Type/_path_files
index 6a1e89f..a29f348 100644
--- a/Completion/Unix/Type/_path_files
+++ b/Completion/Unix/Type/_path_files
@@ -69,6 +69,7 @@ fi
 pats=( "${(@)pats:# #}" )
 
 if (( $#pfx )); then
+  # ### Is it correct to interpret -P as a (greedy) pattern here?
   compset -P "$pfx[2]" || pfxsfx=( "$pfx[@]" "$pfxsfx[@]" )
 fi
 
diff --git a/Completion/X/Command/_rdesktop b/Completion/X/Command/_rdesktop
index 5e480b3..55a6ea7 100644
--- a/Completion/X/Command/_rdesktop
+++ b/Completion/X/Command/_rdesktop
@@ -89,9 +89,8 @@ case $state in
   ;;
   redirection)
     redir="${PREFIX%%:*}"
-    if compset -P '*='; then
+    if compset -P 1 '*='; then
       curcontext="${curcontext%:*}:$redir"
-      compset -P '*='
       case $redir in
 	comport|lptport) _wanted devices expl device _files -g '*(-%)' && ret=0 ;;
 	disk) _directories && ret=0 ;;
diff --git a/Completion/X/Command/_x_utils b/Completion/X/Command/_x_utils
index ebc6aac..13c5572 100644
--- a/Completion/X/Command/_x_utils
+++ b/Completion/X/Command/_x_utils
@@ -82,12 +82,16 @@ xev)
 xhost)
   local type tmp match
 
+  if [[ -z $PREFIX ]]; then
+    _describe prefixes '(-:disallow +:allow)' -S '' -r ''
+  fi
+
   if compset -P '-'; then
     tmp=(${(f)"$(xhost)"})
     shift tmp
     tmp=(${tmp:#LOCAL:|<*>})
     if [[ "$tmp" = *:* ]]; then
-      if compset -P '(#b)(*):'; then
+      if compset -P 1 '(#b)(*):'; then
 	type="$match[1]"
 	_tags displays
 	while _tags; do
@@ -118,7 +122,7 @@ xhost)
 
     if [[ "$PREFIX" = *:* ]]; then
       type="${(L)PREFIX%%:*}"
-      compset -P '*:'
+      compset -P 1 '*:'
 
       case "$type" in
       inet) _hosts && ret=0;;

Re: [PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Daniel Shahaf]

Daniel Shahaf wrote on Wed, Sep 28, 2016 at 17:46:37 +0000:
> Found by grepping for patterns that can match needles of various lengths:
> 
>     :grep 'compset -[PS].*[\#^*()\|<>?~\]' Completion/

A few more hunks of the same patch.

diff --git a/Completion/Debian/Command/_debfoster b/Completion/Debian/Command/_debfoster
index 154d0e9..08a1078 100644
--- a/Completion/Debian/Command/_debfoster
+++ b/Completion/Debian/Command/_debfoster
@@ -26,10 +26,10 @@ _arguments -C \
   && ret=0
 
 if [[ -n "$state" ]]; then
-  if compset -P '*='; then
+  if compset -P 1 '*='; then
     case "$IPREFIX" in
     *(#i)(install|remove|info)cmd*)
-      _wanted values expl 'command string' _command && ret=0
+      _wanted values expl 'command string' _cmdstring && ret=0
       ;;
     *(#i)(keeperfile|dpkg(status|available))*)
       _wanted values expl 'metadata file' _files && ret=0
diff --git a/Completion/Linux/Command/_modutils b/Completion/Linux/Command/_modutils
index 0732aa1..7de97f6 100644
--- a/Completion/Linux/Command/_modutils
+++ b/Completion/Linux/Command/_modutils
@@ -123,7 +123,7 @@ _modutils() {
 	    ;;
 
 	params)
-	    if compset -P '*='; then
+	    if compset -P 1 '*='; then
 		_message -e value 'parameter value'
 	    else
 		local params
diff --git a/Completion/Zsh/Command/_fc b/Completion/Zsh/Command/_fc
index 68456cc..b90436a 100644
--- a/Completion/Zsh/Command/_fc
+++ b/Completion/Zsh/Command/_fc
@@ -68,7 +68,7 @@ esac
 
 if [[ -n $state ]]; then
   zstyle -s ":completion:${curcontext}:" list-separator sep || sep=--
-  if [[ -z ${line:#*=*} ]] && compset -P '*='; then
+  if [[ -z ${line:#*=*} ]] && compset -P 1 '*='; then
     _message -e replacements 'replacement'
   elif [[ -prefix [0-9] ]]; then
     events=( ${(0)"$(printf "%-${#HISTNO}.${#HISTNO}s $sep %s\0" "${(kv)history[@]}")"} )
diff --git a/Completion/Zsh/Type/_arrays b/Completion/Zsh/Type/_arrays
index 24c8957..c28fb17 100644
--- a/Completion/Zsh/Type/_arrays
+++ b/Completion/Zsh/Type/_arrays
@@ -2,4 +2,4 @@
 
 local expl
 
-_wanted arrays expl array _parameters "$@" - -g '*array*'
+_wanted arrays expl array _parameters "$@" -g '*array*'
diff --git a/Completion/Zsh/Type/_globflags b/Completion/Zsh/Type/_globflags
index 5833dc8..13ef14c 100644
--- a/Completion/Zsh/Type/_globflags
+++ b/Completion/Zsh/Type/_globflags
@@ -1,5 +1,8 @@
 #autoload
 
+# Complete 'globbing flags', i.e., '(#x)'; everything up to the '#' will
+# have been "compset -P"'d by the caller.
+
 local ret=1
 local -a flags
 
diff --git a/Completion/Zsh/Type/_ps1234 b/Completion/Zsh/Type/_ps1234
index 8edf0d0..0671ceb 100644
--- a/Completion/Zsh/Type/_ps1234
+++ b/Completion/Zsh/Type/_ps1234
@@ -58,11 +58,14 @@ if compset -P '%[FK]'; then
 fi
 
 if compset -P '%[0-9-\\]#(\\|)\([0-9-]#[^0-9]'; then
+  # ternary conditional: first delimiter
   compset -S '*'
   _delimiters && ret=0
 elif compset -P '%[0-9-\\]#[<>\]]'; then
+  # truncation
   _message -e replacements 'replacement string'
 elif compset -P '%[0-9-\\]#(\\|)\([0-9-]#'; then
+  # ternary conditional: condition character
   compset -S '[.:+/-%]*' || suf=( -S . )
   compset -S '*'
   specs=(
diff --git a/Completion/Zsh/Type/_vars b/Completion/Zsh/Type/_vars
index 0f97d6c..ec59c03 100644
--- a/Completion/Zsh/Type/_vars
+++ b/Completion/Zsh/Type/_vars
@@ -1,7 +1,7 @@
 #compdef getopts unset
 
 # This will handle completion of keys of associative arrays, e.g. at
-# `vared foo[<TAB>'.
+# `vared foo[<TAB>' could complete to `vared foo[key]'.
 
 local ret=1
 

Re: [PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Bart Schaefer]

I'm going to leave it to others to say whether -P 1 is really correct
in all of these cases.  I have concerns about _lzop and _tail because
the pattern semantics have actually been changed but I haven't looked
closely to see if that matters.

On Sep 28,  5:46pm, Daniel Shahaf wrote:
} Subject: [PATCH] Completion: audit 'compset -P' calls to use shortest matc
}
} I'm not sure what to do about the handling of "_path_files -P foo": that -P
} option documents its argument as a string but interprets it as a pattern.
} Should we update the documentation to match the implementation, or vice-versa?

That's this hunk:

}  if (( $#pfx )); then
} +  # ### Is it correct to interpret -P as a (greedy) pattern here?
}    compset -P "$pfx[2]" || pfxsfx=( "$pfx[@]" "$pfxsfx[@]" )
}  fi

I think it would be OK / correct to use ${(b)pfx[2]} there.  Presumably if
$pfx[2] doesn't literally match the filename, we're falling back to the
pfxsfx assignment to resolve it later.

Here in _cpio:

} -  if compset -P '*:'; then
} -    # TODO: doesn't need to be rsh.

The TODO comment may have been meant to indicate there should be a zstyle
to choose what remote shell program to use?  Dumping the comment and
forcing ssh might not be what was intended, but maybe it's OK, as it's
consistent with what we do for remote shell pretty much everywhere else.

In _rdesktop:

}      redir="${PREFIX%%:*}"
} -    if compset -P '*='; then
} +    if compset -P 1 '*='; then
}        curcontext="${curcontext%:*}:$redir"
} -      compset -P '*='
}        case $redir in

The old code does look suspicious, but does anyone know if it was doing
two "compset"s intentionally?

Re: [PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Daniel Shahaf]

Bart Schaefer wrote on Wed, Sep 28, 2016 at 12:28:17 -0700:
> On Sep 28,  5:46pm, Daniel Shahaf wrote:
> } I'm not sure what to do about the handling of "_path_files -P foo": that -P
> } option documents its argument as a string but interprets it as a pattern.
> } Should we update the documentation to match the implementation, or vice-versa?
> 
> }  if (( $#pfx )); then
> } +  # ### Is it correct to interpret -P as a (greedy) pattern here?
> }    compset -P "$pfx[2]" || pfxsfx=( "$pfx[@]" "$pfxsfx[@]" )
> }  fi
> 
> I think it would be OK / correct to use ${(b)pfx[2]} there.  Presumably if
> $pfx[2] doesn't literally match the filename, we're falling back to the
> pfxsfx assignment to resolve it later.
> 

I'll make the change then.

> Here in _cpio:
> 
> } -  if compset -P '*:'; then
> } -    # TODO: doesn't need to be rsh.
> 
> The TODO comment may have been meant to indicate there should be a zstyle
> to choose what remote shell program to use?  Dumping the comment and
> forcing ssh might not be what was intended, but maybe it's OK, as it's
> consistent with what we do for remote shell pretty much everywhere else.
> 

I note that this block is for GNU cpio which has a --rsh-command flag.
Perhaps the correctest thing to do would be to use that.  I don't intend
to pursue this, though.

> In _rdesktop:
> 
> }      redir="${PREFIX%%:*}"
> } -    if compset -P '*='; then
> } +    if compset -P 1 '*='; then
> }        curcontext="${curcontext%:*}:$redir"
> } -      compset -P '*='
> }        case $redir in
> 
> The old code does look suspicious, but does anyone know if it was doing
> two "compset"s intentionally?

The second compset would have been a no-op on all inputs, wouldn't it?
Even its exit code was discarded.

Thanks for the review.

Daniel

Re: [PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Jun T.]

Sorry for not responding quickly.

On 2016/09/29, at 2:46, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:

> --- a/Completion/Unix/Command/_head
> +++ b/Completion/Unix/Command/_head
> @@ -32,7 +32,7 @@ case $state in
(snip)
> -    if compset -P '*[0-9]'; then
> +    if compset -P '[0-9]##'; then

What is intended by this patch?
With this, the following completes nothing:

$ head -n -5<TAB>

Same for _tail.

Re: [PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Daniel Shahaf]

Jun T. wrote on Thu, Oct 27, 2016 at 14:58:04 +0900:
> With this, the following completes nothing:
> 
> $ head -n -5<TAB>
> 
> Same for _tail.

Thanks for the report; is this better?

diff --git a/Completion/Unix/Command/_head b/Completion/Unix/Command/_head
index c76cce3..75fc1f0 100644
--- a/Completion/Unix/Command/_head
+++ b/Completion/Unix/Command/_head
@@ -32,7 +32,7 @@ case $state in
     sign='sign:sign:((-\:"print all but the last specified bytes/lines"'
     sign+=' +\:"print the first specified bytes/lines (default)"))'
     digit='digits:digit:(0 1 2 3 4 5 6 7 8 9)'
-    if compset -P '[0-9]##'; then
+    if compset -P '(-|+|)[0-9]##'; then
       _alternative $mlt $digit && ret=0
     elif [[ -z $PREFIX ]]; then
       _alternative $sign $digit && ret=0
diff --git a/Completion/Unix/Command/_tail b/Completion/Unix/Command/_tail
index 89e9cfb..4e64226 100644
--- a/Completion/Unix/Command/_tail
+++ b/Completion/Unix/Command/_tail
@@ -56,7 +56,7 @@ case $state in
     sign='signs:sign:((+\:"start at the specified byte/line"'
     sign+=' -\:"output the last specified bytes/lines (default)"))'
     digit='digits:digit:(0 1 2 3 4 5 6 7 8 9)'
-    if compset -P '[0-9]##'; then
+    if compset -P '(-|+|)[0-9]##'; then
       _alternative $mlt $digit && ret=0
     elif [[ -z $PREFIX ]]; then
       _alternative $sign $digit && ret=0

Re: [PATCH] Completion: audit 'compset -P' calls to use shortest match where applicable, plus random drive-by tweaks.

[Jun T.]

2016/10/27 21:48, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:

> Thanks for the report; is this better?
> 
> -    if compset -P '[0-9]##'; then
> +    if compset -P '(-|+|)[0-9]##'; then

OK, so you wanted to exclude illegal sequences like a12.

Yes, the new path works fine for me.