Re: NetworkManager completion bug

[Oliver Kiddle <opk@xxxxxxx>]

On 27 Sep, Niret wrote:
> Hello, I found that `nmcli c modify ethernet <complition button>` prints some
> errors:
> (eval):1: parse error near `>'
> _arguments:465: command not found: (
>
> I don't see any reports for the issue and hope it will not be hard to fix
> this... Thank you for this beautiful shell

Thanks for reporting this. The following patch fixes this.

Looking at the history, I suspect this came about due to badly merging
git branches for two separate changes to the function though only
the final squashed commit ever reached the public repository. Making
numbered (next) argument specs optional only works when followed by a
* (rest) argument spec. Perhaps _arguments could do better with that.
Checking for uses, there are some like _uniq where final arguments (the
filenames) are marked optional. That's pretty pointless but harmless.
There's also cases like _jot that would be broken by removing the
limitation from _arguments: both lower and upper bound may be optional
but you can't skip the lower bound while providing an upper one.

The patch also makes use of the saved $words[1] for the nmcli command in
a couple more places and ignores a preceding - or + on the properties.
For the properties state, it should ideally be treating alternate
arguments as values but in the absence of meaningful completion for
values, it didn't seem worth it to add logic that might only make it
more fragile.

Oliver

diff --git a/Completion/Linux/Command/_networkmanager b/Completion/Linux/Command/_networkmanager
index 154f56405..fba517a56 100644
--- a/Completion/Linux/Command/_networkmanager
+++ b/Completion/Linux/Command/_networkmanager
@@ -152,11 +152,12 @@ _nm_connection_modify() {
   _arguments -A "-*" \
     "--temporary" \
     "1:connection:_nm_connection_specs" \
-    '2::remove:(remove):*:setting:->properties' \
-    '3:option:->properties' && ret=0
+    '2::remove:(remove)' \
+    '*::option:->properties' && ret=0
 
   if [[ -n "$state" ]]; then
-    properties=( ${${(f)"$(_call_program properties $nmcli -t connection show $words[CURRENT-1])"}%%:*} )
+    compset -P '[-+]'
+    properties=( ${${(f)"$(_call_program properties $nmcli -t connection show $words[1])"}%%:*} )
     _description properties expl property
     _multi_parts "$expl[@]" . properties && ret=0
   fi
@@ -421,13 +422,13 @@ _nm_device_wifi_show-password() {
 
 _nm_device_wifi_bssids() {
   local -a bssids
-  bssids=(${(f)"$(_call_program nmcli nmcli -t -f bssid device wifi list)"})
+  bssids=(${(f)"$(_call_program nmcli $nmcli -t -f bssid device wifi list)"})
   _describe 'network' bssids
 }
 
 _nm_device_wifi_ssids() {
   local -a ssids
-  ssids=(${(f)"$(_call_program nmcli nmcli -t -f ssid device wifi list)"})
+  ssids=(${(f)"$(_call_program nmcli $nmcli -t -f ssid device wifi list)"})
   _describe 'network' ssids
 }
 
@@ -447,7 +448,7 @@ _nm_device_checkpoint() {
 
 _nm_device_ifnames() {
   local -a ifnames
-  ifnames=(${(f)"$(_call_program nmcli nmcli -t -f device device)"})
+  ifnames=(${(f)"$(_call_program nmcli $nmcli -t -f device device)"})
   _describe 'device' ifnames
 }