device cannot be completed after "ip link show dev"

device cannot be completed after "ip link show dev"

[Han Pingtian]

Hi,

Although in _ip() the "link_show_cmds" is defined to

188 local -a link_show_cmds
189 _regex_words link-show-commands 'link show commands' \
190   'dev:specify device:$subcmd_dev' \
191   'up:limit display to running devices'
192 link_show_cmds=("(" $subcmd_dev "|" ")" "$reply[@]" "#" )
193 

but the device cannot be completed after "ip link show dev":

% ip link show dev <tab>
link show commands
dev  -- specify device
up  -- limit display to running devices

Only "dev" and "up" listed. It looks like something wrong?

Thanks!

Re: device cannot be completed after "ip link show dev"

[Eric Cook]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

On 03/29/2015 11:39 PM, Han Pingtian wrote:
> Hi,
>
> Although in _ip() the "link_show_cmds" is defined to
>
> 188 local -a link_show_cmds
> 189 _regex_words link-show-commands 'link show commands' \
> 190   'dev:specify device:$subcmd_dev' \
> 191   'up:limit display to running devices'
> 192 link_show_cmds=("(" $subcmd_dev "|" ")" "$reply[@]" "#" )
> 193 
>
> but the device cannot be completed after "ip link show dev":
>
> % ip link show dev <tab>
> link show commands
> dev  -- specify device
> up  -- limit display to running devices
>
> Only "dev" and "up" listed. It looks like something wrong?
>
> Thanks!
>
So if you revert my change to the array subcmd_dev, completing after dev
works. But completing after `ip link set lo' or virbr0-nic, "fails to
parse the previous word."

I can't seem to find a pattern that works in both cases, but adding the
previous line back allows completing after: `ip link show dev'.
But after the argument for `dev', the option `up' is no longer show.

Hopefully someone with better knowledge of the _regex_* functions could
help.

[-- Attachment #2: _ip.patch --]
[-- Type: text/x-patch, Size: 655 bytes --]

diff --git a/Completion/Unix/Command/_ip b/Completion/Unix/Command/_ip
index 3b68c35..a6470da 100644
--- a/Completion/Unix/Command/_ip
+++ b/Completion/Unix/Command/_ip
@@ -9,7 +9,10 @@
 # Values encoding simple types
 #
 local -a subcmd_dev
-subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ ':interfaces:network interface:_net_interfaces')
+subcmd_dev=(
+  /$'[[:alnum:][:punct:][:cntrl:]]##\0'/ ':interfaces:network interface:_net_interfaces'
+  /$'[[:alpha:]]##[[:digit:]]##(\.[[:digit:]]##)#\0'/ ':interfaces:network interface:_net_interfaces'
+)
 
 local -a subcmd_onoff
 subcmd_onoff=(/$'(on|off)\0'/ ':onoff:state (on or off):(on off)')

Re: device cannot be completed after "ip link show dev"

[Bart Schaefer]

On Mar 30,  9:27pm, Eric Cook wrote:
}
} I can't seem to find a pattern that works in both cases, but adding the
} previous line back allows completing after: `ip link show dev'.
} But after the argument for `dev', the option `up' is no longer show.
} 
} Hopefully someone with better knowledge of the _regex_* functions could
} help.

I don't really know _regex_* "better" -- in fact I've barely looked at it
before -- but based on the "either aaa or bbb" example in the doc, what
about this?


diff --git a/Completion/Unix/Command/_ip b/Completion/Unix/Command/_ip
index 3b68c35..c0a28d3 100644
--- a/Completion/Unix/Command/_ip
+++ b/Completion/Unix/Command/_ip
@@ -9,7 +9,8 @@
 # Values encoding simple types
 #
 local -a subcmd_dev
-subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ ':interfaces:network interface:_net_interfaces')
+subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ ':interfaces:network interface:_net_interfaces' "|"
+  /$'dev\0'/ ':interfaces:network interface:_net_interfaces')
 
 local -a subcmd_onoff
 subcmd_onoff=(/$'(on|off)\0'/ ':onoff:state (on or off):(on off)')


I suspect there should be some way to use the "-GUARD" syntax to prevent
"dev" from completing again after "dev" ("ip link show dev dev" doesn't
make sense, does it?) but there's no example of using a guard, and as
someone implied, offering too much (as long as it's not *TOO* much) is
better than offering too little.

Re: device cannot be completed after "ip link show dev"

[Peter Stephenson]

On Mon, 30 Mar 2015 11:39:30 +0800
Han Pingtian <hanpt@linux.vnet.ibm.com> wrote:
> Although in _ip() the "link_show_cmds" is defined to
> 
> 188 local -a link_show_cmds
> 189 _regex_words link-show-commands 'link show commands' \
> 190   'dev:specify device:$subcmd_dev' \
> 191   'up:limit display to running devices'
> 192 link_show_cmds=("(" $subcmd_dev "|" ")" "$reply[@]" "#" )
> 193 
> 
> but the device cannot be completed after "ip link show dev":

I see the problem: we're matching way too much where we think there
might be a dev already on the command line.

This is better.  There's still a possible glitch: if a device springs
into existence dynamically we won't accept it as a device already part
of the command line argument, though we will still complete it as that
calls _net_interfaces again.  I think that's fairly minor.

pws

diff --git a/Completion/Unix/Command/_ip b/Completion/Unix/Command/_ip
index 3b68c35..bfa7d99 100644
--- a/Completion/Unix/Command/_ip
+++ b/Completion/Unix/Command/_ip
@@ -8,8 +8,11 @@
 #
 # Values encoding simple types
 #
-local -a subcmd_dev
-subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ ':interfaces:network interface:_net_interfaces')
+local -a subcmd_dev net_intf_disp net_intf_list
+# subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ ':interfaces:network interface:_net_interfaces')
+_find_net_interfaces
+subcmd_dev=(/"(${(j.|.)net_intf_list})"$'\0'/
+	    ':interfaces:network interface:_net_interfaces')
 
 local -a subcmd_onoff
 subcmd_onoff=(/$'(on|off)\0'/ ':onoff:state (on or off):(on off)')
diff --git a/Completion/Unix/Type/_find_net_interfaces b/Completion/Unix/Type/_find_net_interfaces
index e69de29..1f5ca9e 100644
--- a/Completion/Unix/Type/_find_net_interfaces
+++ b/Completion/Unix/Type/_find_net_interfaces
@@ -0,0 +1,42 @@
+#autoload
+
+# This can be called independently of completion.  It returns
+# arrays net_intf_disp and net_intf_list which the caller should
+# make local.
+
+local sep list
+
+# Make sure needed tools are in the path.
+local PATH=$PATH
+PATH=/sbin:$PATH
+
+case $OSTYPE in
+  aix*)
+    net_intf_list=( ${(f)"$(lsdev -C -c if -F 'name:description')"} )
+    if zstyle -T ":completion:${curcontext}" verbose; then
+      zstyle -s ":completion:${curcontext}:" list-separator sep || sep=--
+      zformat -a list " $sep " "$net_intf_list[@]"
+      net_intf_disp=(-ld list)
+    fi
+  ;;
+  darwin*|freebsd*|dragonfly*) net_intf_list=( $(ifconfig -l) ) ;;
+  irix*) net_intf_list=( ${${${(f)"$(/usr/etc/netstat -i)"}%% *}[2,-1]} ) ;;
+  *linux*)
+    if (( $+commands[ip] )); then
+      net_intf_list=( ${${(m)${(f)"$(ip -o link)"}#*: }%%: *} )
+    fi
+  ;&
+
+  *)
+  if [[ ${#net_intf_list} -eq 0 ]]; then
+    # linux's deprecated ifconfig may truncate long interface names
+    net_intf_list=( $(ifconfig -a 2>/dev/null | sed -n 's/^\([^ 	:]*\).*/\1/p') )
+    if [[ -d /proc/sys/net/ipv4/conf ]]; then
+      # On linux we used to use the following as the default.
+      # However, we now use ip or ifconfig since it finds additional devices such
+      # as tunnels.  So only do this if that didn't work.
+      net_intf_list=( /proc/sys/net/ipv4/conf/*~*(all|default)(N:t) )
+    fi
+  fi
+  ;;
+esac
diff --git a/Completion/Unix/Type/_net_interfaces b/Completion/Unix/Type/_net_interfaces
index 2cac3e3..5be66d7 100644
--- a/Completion/Unix/Type/_net_interfaces
+++ b/Completion/Unix/Type/_net_interfaces
@@ -1,42 +1,9 @@
 #compdef ifup ifdown
 
-local expl list intf sep
-local -a disp
+local expl
+local -a net_intf_disp net_intf_list
 
-# Make sure needed tools are in the path.
-local PATH=$PATH
-PATH=/sbin:$PATH
-
-case $OSTYPE in
-  aix*)
-    intf=( ${(f)"$(lsdev -C -c if -F 'name:description')"} )
-    if zstyle -T ":completion:${curcontext}" verbose; then
-      zstyle -s ":completion:${curcontext}:" list-separator sep || sep=--
-      zformat -a list " $sep " "$intf[@]"
-      disp=(-ld list)
-    fi
-  ;;
-  darwin*|freebsd*|dragonfly*) intf=( $(ifconfig -l) ) ;;
-  irix*) intf=( ${${${(f)"$(/usr/etc/netstat -i)"}%% *}[2,-1]} ) ;;
-  *linux*)
-    if (( $+commands[ip] )); then
-      intf=( ${${(m)${(f)"$(ip -o link)"}#*: }%%: *} )
-    fi
-  ;&
-
-  *)
-  if [[ ${#intf} -eq 0 ]]; then
-    # linux's deprecated ifconfig may truncate long interface names
-    intf=( $(ifconfig -a 2>/dev/null | sed -n 's/^\([^ 	:]*\).*/\1/p') )
-    if [[ -d /proc/sys/net/ipv4/conf ]]; then
-      # On linux we used to use the following as the default.
-      # However, we now use ip or ifconfig since it finds additional devices such
-      # as tunnels.  So only do this if that didn't work.
-      intf=( /proc/sys/net/ipv4/conf/*~*(all|default)(N:t) )
-    fi
-  fi
-  ;;
-esac
+_find_net_interfaces
 
 _wanted interfaces expl 'network interface' \
-    compadd "$@" "$disp[@]" - "${(@)intf%%:*}"
+    compadd "$@" "$net_intf_disp[@]" - "${(@)net_intf_list%%:*}"

Re: device cannot be completed after "ip link show dev"

[Han Pingtian]

On Mon, Mar 30, 2015 at 09:52:20PM -0700, Bart Schaefer wrote:
> diff --git a/Completion/Unix/Command/_ip b/Completion/Unix/Command/_ip
> index 3b68c35..c0a28d3 100644
> --- a/Completion/Unix/Command/_ip
> +++ b/Completion/Unix/Command/_ip
> @@ -9,7 +9,8 @@
>  # Values encoding simple types
>  #
>  local -a subcmd_dev
> -subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ ':interfaces:network interface:_net_interfaces')
> +subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ ':interfaces:network interface:_net_interfaces' "|"
> +  /$'dev\0'/ ':interfaces:network interface:_net_interfaces')
>  
>  local -a subcmd_onoff
>  subcmd_onoff=(/$'(on|off)\0'/ ':onoff:state (on or off):(on off)')
> 
This works pretty good, besides "dev" will be completed after "dev".
> 
> I suspect there should be some way to use the "-GUARD" syntax to prevent
> "dev" from completing again after "dev" ("ip link show dev dev" doesn't
> make sense, does it?) but there's no example of using a guard, and as
> someone implied, offering too much (as long as it's not *TOO* much) is
> better than offering too little.

I found that we can use "-'false'" as the the guard here:

subcmd_dev=(/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ -'false'
':interfaces:network interface:_net_interfaces')

then only network device will be completed after dev:

% ip link show dev<tab>
network interface
em1     lo      tun0    wlp3s0

or even using "[]" as the pattern has the same effect:

subcmd_dev=(/$'[]'/ ':interfaces:network interface:_net_interfaces')

But both of them will report parsing error when trying to complete after
the interface:

% ip link show dev lo<tab>
parse failed before current word

Re: device cannot be completed after "ip link show dev"

[Han Pingtian]

On Tue, Mar 31, 2015 at 07:57:01PM +0100, Peter Stephenson wrote:
> On Mon, 30 Mar 2015 11:39:30 +0800
> Han Pingtian <hanpt@linux.vnet.ibm.com> wrote:
> > Although in _ip() the "link_show_cmds" is defined to
> > 
> > 188 local -a link_show_cmds
> > 189 _regex_words link-show-commands 'link show commands' \
> > 190   'dev:specify device:$subcmd_dev' \
> > 191   'up:limit display to running devices'
> > 192 link_show_cmds=("(" $subcmd_dev "|" ")" "$reply[@]" "#" )
> > 193 
> > 
> > but the device cannot be completed after "ip link show dev":
> 
> I see the problem: we're matching way too much where we think there
> might be a dev already on the command line.
> 
What's going on when

% ip link show dev <tab>

, please? It looks like the old pattern
/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ will match here, so the
action ':interfaces:network interface:_net_interfaces' will be bypassed?
But I think there is nothing at the position of <tab>, why the old
pattern would match a empty string?

> This is better.  There's still a possible glitch: if a device springs
> into existence dynamically we won't accept it as a device already part
> of the command line argument, though we will still complete it as that
> calls _net_interfaces again.  I think that's fairly minor.
> 
I have tested (by set a fake net_intf_list) that if there is a interface 
which named "dev", the completion will fail with this patch.

And I have figured out this works:

subcmd_dev=(
/$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ 
-'if [[ $words[CURRENT-1] = dev ]];then false;else true;fi'
':interfaces:network interface:_net_interfaces'
)

Re: device cannot be completed after "ip link show dev"

[Peter Stephenson]

On Wed, 1 Apr 2015 18:07:19 +0800
Han Pingtian <hanpt@linux.vnet.ibm.com> wrote:
> What's going on when
> 
> % ip link show dev <tab>
> 
> , please? It looks like the old pattern
> /$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ will match here, so the
> action ':interfaces:network interface:_net_interfaces' will be bypassed?
> But I think there is nothing at the position of <tab>, why the old
> pattern would match a empty string?

At that point it's not using the pattern, it's constructing a
completion.  It only uses the pattern to determine if a *previous* entry
on the line is of a particular type.

> > This is better.  There's still a possible glitch: if a device springs
> > into existence dynamically we won't accept it as a device already part
> > of the command line argument, though we will still complete it as that
> > calls _net_interfaces again.  I think that's fairly minor.
> > 
> I have tested (by set a fake net_intf_list) that if there is a interface 
> which named "dev", the completion will fail with this patch.

I certain wouldn't expect to support a device called "dev" in the
general case, that will screw up parsing horribly.

> And I have figured out this works:
> 
> subcmd_dev=(
> /$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ 
> -'if [[ $words[CURRENT-1] = dev ]];then false;else true;fi'
> ':interfaces:network interface:_net_interfaces'
> )

I didn't even know what format guards could take in general... that
seems a reasonable optimisation since after a "dev" you don't care what
the pattern is, it's automatically a device.  Except I don't really
understand what guards do, so this may need a bit more testing...

pws

Re: device cannot be completed after "ip link show dev"

[Bart Schaefer]

On Apr 1,  1:15pm, Peter Stephenson wrote:
}
} > And I have figured out this works:
} > 
} > subcmd_dev=(
} > /$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ 
} > -'if [[ $words[CURRENT-1] = dev ]];then false;else true;fi'
} > ':interfaces:network interface:_net_interfaces'
} > )
} 
} I didn't even know what format guards could take in general...

They're just passed to "eval" and the exit status checked.  If false,
the pattern is presumed not to match.  The pattern is evaluated with
backreferences enabled, so normally you'd refer to $match in the guard,
but in this case maybe we want to examine previous words rather than
the one to which the pattern is being applied.

subcmd_dev=(
 /$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/
 -'[[ $words[CURRENT-1] != dev ]]'
 ':interfaces:network interface:_net_interfaces'
)

should be equivalent, but I suspect we might really want to examine the
whole array slice $words[2,CURRENT-1] to see if "dev" appears anywhere.

Re: device cannot be completed after "ip link show dev"

[Han Pingtian]

On Wed, Apr 01, 2015 at 08:47:44AM -0700, Bart Schaefer wrote:
> On Apr 1,  1:15pm, Peter Stephenson wrote:
> }
> } > And I have figured out this works:
> } > 
> } > subcmd_dev=(
> } > /$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/ 
> } > -'if [[ $words[CURRENT-1] = dev ]];then false;else true;fi'
> } > ':interfaces:network interface:_net_interfaces'
> } > )
> } 
> } I didn't even know what format guards could take in general...
> 
> They're just passed to "eval" and the exit status checked.  If false,
> the pattern is presumed not to match.  The pattern is evaluated with
> backreferences enabled, so normally you'd refer to $match in the guard,
> but in this case maybe we want to examine previous words rather than
> the one to which the pattern is being applied.
> 
> subcmd_dev=(
>  /$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/
>  -'[[ $words[CURRENT-1] != dev ]]'
>  ':interfaces:network interface:_net_interfaces'
> )
> 
> should be equivalent, but I suspect we might really want to examine the
> whole array slice $words[2,CURRENT-1] to see if "dev" appears anywhere.

If using -'[[ $words[2,CURRENT-1] != *dev* ]]', then "dev" and "up"
won't be completed after dev (this is right, dev after dev is wrong
syntax), but there is a prase failed warning :

% ip link show dev em1 <tab>
parse failed before current word

Re: device cannot be completed after "ip link show dev"

[Bart Schaefer]

On Apr 2,  4:03pm, Han Pingtian wrote:
}
} > subcmd_dev=(
} >  /$'[[:alnum:][:punct:][:cntrl:][:digit:]]##\0'/
} >  -'[[ $words[CURRENT-1] != dev ]]'
} >  ':interfaces:network interface:_net_interfaces'
} > )
} 
} If using -'[[ $words[2,CURRENT-1] != *dev* ]]', then "dev" and "up"
} won't be completed after dev (this is right, dev after dev is wrong
} syntax), but there is a prase failed warning :
} 
} % ip link show dev em1 <tab>
} parse failed before current word

Indeed, what we actually want for the guard would appear to be that the
string "dev" does not appear *or* that an actual device name already
appears somewhere *after* "dev".

We're wandering into "is the result worth the effort?" territory, so it
may be that $words[CURRENT-1] is the best compromise.

Re: device cannot be completed after "ip link show dev"

[Peter Stephenson]

On Thu, 2 Apr 2015 09:55:47 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> We're wandering into "is the result worth the effort?" territory, so it
> may be that $words[CURRENT-1] is the best compromise.

That's my current feeling, barring the infinite number of developer
monkeys thing.

pws