[PATCH] _file_descriptors: initialize `list' array with local -a

[PATCH] _file_descriptors: initialize `list' array with local -a

[Eric Cook]

I noticed the descriptions _file_descriptors offers doesn't match what is currently presented
(only tried linux) _foo() _file_descriptors; compdef _foo foo

It happens when `list' is converted from a scalar to an array using list+=(...)
an empty string is the first element, so the two arrays `list' and `fd' aren't the same length 
when compadd is used.

I also silence stderr for the loops of fds since another error happens when used:
_file_descriptors:zstat:12: /proc/18123/fd/3: no such file or directory


 Completion/Zsh/Type/_file_descriptors | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Completion/Zsh/Type/_file_descriptors b/Completion/Zsh/Type/_file_descriptors
index 3e251b7..438fcc8 100644
--- a/Completion/Zsh/Type/_file_descriptors
+++ b/Completion/Zsh/Type/_file_descriptors
@@ -1,6 +1,7 @@
 #autoload
 
-local i fds expl list link sep
+local i fds expl link sep
+local -a list
 
 fds=( /dev/fd/<0-9>(N:t) )
 
@@ -20,7 +21,7 @@ if zstyle -T ":completion:${curcontext}:" verbose && [[ -h /proc/$$/fd/$fds[1] ]
     for i in "${fds[@]}"; do
       list+=( "$i $sep $(ls -l /proc/$$/fd/$i|sed 's/.*-> //' )" )
     done
-  fi
+  fi 2>/dev/null
 
   if (( $list[(I)* $sep ?*] )); then
     _wanted file-descriptors expl 'file descriptor' compadd "$@" -d list -a - fds
-- 
2.4.2

Re: [PATCH] _file_descriptors: initialize `list' array with local -a

[Oliver Kiddle]

Eric Cook wrote:
> +  fi 2>/dev/null

That redirection causes the description of fd 2 to change to /dev/null
instead of being the tty. I'm not sure in what situation it is needed
but it might be better to put it directly on the readlink/sed or
whichever command actually needs it.

Or perhaps we should hardcode descriptions for fd 0-2 to
stdin/stdout/stderr.

Oliver

Re: [PATCH] _file_descriptors: initialize `list' array with local -a

[Eric Cook]

On 06/16/2015 03:42 AM, Oliver Kiddle wrote:
> Eric Cook wrote:
>> +  fi 2>/dev/null
> 
> That redirection causes the description of fd 2 to change to /dev/null
> instead of being the tty. I'm not sure in what situation it is needed
> but it might be better to put it directly on the readlink/sed or
> whichever command actually needs it.

zstat was the command in question, but placing the redirection there still
changes the description to /dev/null

Question: is there a reason why the upper limit is fd9?
I ask because i have a _zsocket function which is what made me notice all of this,
I normally use an fd above 10 with zsocket.

I also noticed since zstat isn't tested for success and link[1] might be defined still,
the description from the previous loop is used for fd3. Which is closed by the time
compsys is done running. Is there a reason to try to present it?


With the patch below: fd0,1,2 are hardcoded, the upper limit of fd9 is removed and test for the failure of zstat,
readlink and ls.

diff --git a/Completion/Zsh/Type/_file_descriptors b/Completion/Zsh/Type/_file_descriptors
index 3e251b7..0b2cd00 100644
--- a/Completion/Zsh/Type/_file_descriptors
+++ b/Completion/Zsh/Type/_file_descriptors
@@ -1,28 +1,41 @@
 #autoload

-local i fds expl list link sep
+local i fds expl link sep
+local -a list

-fds=( /dev/fd/<0-9>(N:t) )
+fds=( /dev/fd/<3->(N:t) )

 if zstyle -T ":completion:${curcontext}:" verbose && [[ -h /proc/$$/fd/$fds[1] ]]; then
   zstyle -s ":completion:${curcontext}:" list-separator sep || sep=--
-
   if zmodload -F zsh/stat b:zstat; then
     for i in "${fds[@]}"; do
-      zstat +link -A link /proc/$$/fd/$i
-      list+=( "$i $sep ${link[1]}" )
+      if zstat +link -A link /proc/$$/fd/$i; then
+        list+=( "$i $sep ${link[1]}" )
+      else
+        fds[(i)$i]=()
+      fi
     done
   elif (( $+commands[readlink] )); then
     for i in "${fds[@]}"; do
-      list+=( "$i $sep $(readlink /proc/$$/fd/$i)" )
+      if link=$(readlink /proc/$$/fd/$i); then
+        list+=( "$i $sep $link" )
+      else
+        fds[(i)$i]=()
+      fi
     done
   else
     for i in "${fds[@]}"; do
-      list+=( "$i $sep $(ls -l /proc/$$/fd/$i|sed 's/.*-> //' )" )
+      if link=$(ls -l /proc/$$/fd/$i); then
+        list+=( "$i $sep ${link#* -> }" )
+      else
+        fds[(i)$i]=()
+      fi
     done
-  fi
+  fi 2>/dev/null

   if (( $list[(I)* $sep ?*] )); then
+    list=( "0 $sep standard input" "1 $sep standard output" "2 $sep standard error" $list )
+    fds=( 0 1 2 $fds )
     _wanted file-descriptors expl 'file descriptor' compadd "$@" -d list -a - fds
     return
   fi

Re: [PATCH] _file_descriptors: initialize `list' array with local -a

[Oliver Kiddle]

Eric Cook wrote:
> Question: is there a reason why the upper limit is fd9?

Because redirections don't let you use two-digit numbers.
However, the {var} syntax will allocate higher numbered file
descriptors so the limitation is out-of-date and can probably be
ignored.

The updated function looks good. One slight issue is that description
separators don't align now that two digit fds are included:

0 -- standard input
1 -- standard output
10 -- /dev/pts/6

zformat can do that in the general case but you might get away with
  "${(r.$#fds[-1].)i} $sep ...

We have worse examples of that such as git subcommands.

Oliver

Re: [PATCH] _file_descriptors: initialize `list' array with local -a

[Oliver Kiddle]

I wrote:
> The updated function looks good. One slight issue is that description
> separators don't align now that two digit fds are included:

I was going to commit this with that small tweak but noticed that it
also needed to handle sorting manually so that e.g. 10 is placed after
3. Given a few further tweaks and the addition of pfiles use for
Solaris it seems necessary to post a patch.

Oliver

diff --git a/Completion/Zsh/Type/_file_descriptors b/Completion/Zsh/Type/_file_descriptors
index 0b2cd00..52fe4bf 100644
--- a/Completion/Zsh/Type/_file_descriptors
+++ b/Completion/Zsh/Type/_file_descriptors
@@ -1,16 +1,23 @@
 #autoload
 
-local i fds expl link sep
+local i fds expl disp link sep
 local -a list
 
 fds=( /dev/fd/<3->(N:t) )
+fds=( ${(n)fds} )
 
-if zstyle -T ":completion:${curcontext}:" verbose && [[ -h /proc/$$/fd/$fds[1] ]]; then
-  zstyle -s ":completion:${curcontext}:" list-separator sep || sep=--
-  if zmodload -F zsh/stat b:zstat; then
+if zstyle -T ":completion:${curcontext}:file-descriptors" verbose; then
+  zstyle -s ":completion:${curcontext}:file-descriptors" list-separator sep || sep=--
+  if [[ $OSTYPE = solaris* ]]; then
+    fds=( ${${(f)"$(pfiles $$|nawk 'NR==1{next} /^ *[0-9]*:/ {printf "\n%s", $1} / *\// {print $1}')"}:#[012]:*} )
+    zformat -a list " $sep " $fds
+    fds=( ${fds%%:*} )
+  elif [[ ! -h /proc/$$/fd/$fds[1] ]]; then
+    :
+  elif zmodload -F zsh/stat b:zstat; then
     for i in "${fds[@]}"; do
       if zstat +link -A link /proc/$$/fd/$i; then
-        list+=( "$i $sep ${link[1]}" )
+        list+=( "${(r.$#fds[-1].)i} $sep ${link[1]}" )
       else
         fds[(i)$i]=()
       fi
@@ -18,7 +25,7 @@ if zstyle -T ":completion:${curcontext}:" verbose && [[ -h /proc/$$/fd/$fds[1] ]
   elif (( $+commands[readlink] )); then
     for i in "${fds[@]}"; do
       if link=$(readlink /proc/$$/fd/$i); then
-        list+=( "$i $sep $link" )
+        list+=( "${(r.$#fds[-1].)i} $sep $link" )
       else
         fds[(i)$i]=()
       fi
@@ -26,19 +33,23 @@ if zstyle -T ":completion:${curcontext}:" verbose && [[ -h /proc/$$/fd/$fds[1] ]
   else
     for i in "${fds[@]}"; do
       if link=$(ls -l /proc/$$/fd/$i); then
-        list+=( "$i $sep ${link#* -> }" )
+        list+=( "${(r.$#fds[-1].)i} $sep ${link#* -> }" )
       else
         fds[(i)$i]=()
       fi
     done
   fi 2>/dev/null
 
-  if (( $list[(I)* $sep ?*] )); then
-    list=( "0 $sep standard input" "1 $sep standard output" "2 $sep standard error" $list )
-    fds=( 0 1 2 $fds )
-    _wanted file-descriptors expl 'file descriptor' compadd "$@" -d list -a - fds
-    return
+  if (( list[(I)* $sep ?*] )); then
+    list=(
+      "${(r.$#fds[-1].):-0} $sep standard input"
+      "${(r.$#fds[-1].):-1} $sep standard output"
+      "${(r.$#fds[-1].):-2} $sep standard error" $list
+    )
+    disp=( -d list )
   fi
+  fds=( 0 1 2 $fds )
 fi
 
-_wanted file-descriptors expl 'file descriptor' compadd -a "$@" - fds
+_description -V file-descriptors expl 'file descriptor'
+compadd $disp "${@/-J/-V}" "$expl[@]" -a fds

Re: [PATCH] _file_descriptors: initialize `list' array with local -a

[Danek Duvall]

On Thu, Jun 18, 2015 at 11:33:42PM +0200, Oliver Kiddle wrote:

> I wrote:
> > The updated function looks good. One slight issue is that description
> > separators don't align now that two digit fds are included:
> 
> I was going to commit this with that small tweak but noticed that it
> also needed to handle sorting manually so that e.g. 10 is placed after
> 3. Given a few further tweaks and the addition of pfiles use for
> Solaris it seems necessary to post a patch.

Newer versions of Solaris (11 and up?) have /proc/<pid>/path/<->, which
have the same effect as the symlinks in /proc/<pid>/fd on Linux.

Danek

Re: [PATCH] _file_descriptors: initialize `list' array with local -a

[Oliver Kiddle]

On 19 Jun, Danek Duvall wrote:
> Newer versions of Solaris (11 and up?) have /proc/<pid>/path/<->, which
> have the same effect as the symlinks in /proc/<pid>/fd on Linux.

Thanks. That interface is probably more reliable. It is there in Solaris
10 and also for even older versions that are running in a branded zone.

So this is yet another patch to the function. It should also work for
FreeBSD using procstat.

Oliver

diff --git a/Completion/Zsh/Type/_file_descriptors b/Completion/Zsh/Type/_file_descriptors
index 52fe4bf..3e9f496 100644
--- a/Completion/Zsh/Type/_file_descriptors
+++ b/Completion/Zsh/Type/_file_descriptors
@@ -1,44 +1,48 @@
 #autoload
 
 local i fds expl disp link sep
-local -a list
+local -a list proc
 
 fds=( /dev/fd/<3->(N:t) )
 fds=( ${(n)fds} )
 
 if zstyle -T ":completion:${curcontext}:file-descriptors" verbose; then
   zstyle -s ":completion:${curcontext}:file-descriptors" list-separator sep || sep=--
-  if [[ $OSTYPE = solaris* ]]; then
-    fds=( ${${(f)"$(pfiles $$|nawk 'NR==1{next} /^ *[0-9]*:/ {printf "\n%s", $1} / *\// {print $1}')"}:#[012]:*} )
+
+  if [[ $OSTYPE = freebsd* ]]; then
+    fds=( ${(f)"$(procstat -f $$|awk -v OFS=: '$3>2 && $3~/[0-9]/ {print $3,$10}')"} )
     zformat -a list " $sep " $fds
     fds=( ${fds%%:*} )
-  elif [[ ! -h /proc/$$/fd/$fds[1] ]]; then
-    :
-  elif zmodload -F zsh/stat b:zstat; then
-    for i in "${fds[@]}"; do
-      if zstat +link -A link /proc/$$/fd/$i; then
-        list+=( "${(r.$#fds[-1].)i} $sep ${link[1]}" )
-      else
-        fds[(i)$i]=()
-      fi
-    done
-  elif (( $+commands[readlink] )); then
-    for i in "${fds[@]}"; do
-      if link=$(readlink /proc/$$/fd/$i); then
-        list+=( "${(r.$#fds[-1].)i} $sep $link" )
-      else
-        fds[(i)$i]=()
-      fi
-    done
-  else
-    for i in "${fds[@]}"; do
-      if link=$(ls -l /proc/$$/fd/$i); then
-        list+=( "${(r.$#fds[-1].)i} $sep ${link#* -> }" )
-      else
-        fds[(i)$i]=()
-      fi
-    done
-  fi 2>/dev/null
+  elif
+    proc=( /proc/$$/(fd|path)/<->(@N[-1]:h) )
+    [[ -n $proc ]]
+  then
+    if zmodload -F zsh/stat b:zstat; then
+      for i in "${fds[@]}"; do
+	if zstat +link -A link $proc/$i; then
+	  list+=( "${(r.$#fds[-1].)i} $sep ${(D)link[1]}" )
+	else
+	  fds[(i)$i]=()
+	fi
+      done
+    elif (( $+commands[readlink] )); then
+      for i in "${fds[@]}"; do
+	if link=$(readlink $proc/$i); then
+	  list+=( "${(r.$#fds[-1].)i} $sep ${(D)link}" )
+	else
+	  fds[(i)$i]=()
+	fi
+      done
+    else
+      for i in "${fds[@]}"; do
+	if link=$(ls -l $proc/$i); then
+	  list+=( "${(r.$#fds[-1].)i} $sep ${(D)link#* -> }" )
+	else
+	  fds[(i)$i]=()
+	fi
+      done
+    fi 2>/dev/null
+  fi
 
   if (( list[(I)* $sep ?*] )); then
     list=(
@@ -48,8 +52,8 @@ if zstyle -T ":completion:${curcontext}:file-descriptors" verbose; then
     )
     disp=( -d list )
   fi
-  fds=( 0 1 2 $fds )
 fi
+fds=( 0 1 2 $fds )
 
 _description -V file-descriptors expl 'file descriptor'
 compadd $disp "${@/-J/-V}" "$expl[@]" -a fds