perf(_comp_count_args): skip reinitializing cword and words

akinomyoga · akinomyoga · commit ee4247432bd1 · 2023-08-13T10:19:46.000+09:00
In most of the contexts calling `_comp_count_args` in the current
codebase, `cword` and `words` are already initialized with the same
set of exclude chars (-n chars) by _comp_initialize, so there seems to
be no need to again reassemble `cword` and `words`.  This patch
removes the redundant initialization of `cword` and `words`.

* When `_comp_initialize` is called without `-s` or `-n chars` and
  `_comp_count_args` is called with an empty $1, they are compatible
  so `cword` and `words` outside can be directly used.

* `7z` and `nslookup` specify the same set of exclude chars as
  `_comp_initialize`, so existing `cword` and `words` are compatible.

Nevertheless, we need to reinitialize `cword` and `words` when the
specified exclude chars are different from those specified to
_comp_initialize.

* The `ssh` completion calls `_comp_count_args` with excluding `=`
  while it calls `_comp_initialize` with `-n :`.

* The `chown` completion calls `_comp_count_args` with excluding `:`
  while it calls `_comp_initialize` with `-sn :`.

* The `nc` completions calls `_comp_count_args` without exclude chars
  while it calls `_comp_initialize` with `-n :`.

* The completions for `chmod`, `cryptsetup`, `hcitool`, `mkinitrd`,
  `patch`, `quota`, and `zopflipng` does not specify exclude chars to
  `_comp_count_args` while they specify `-s` to `_comp_initialize`.

It is not clear whether these discrepancies are intended ones are not,
but this patch tries to keep the current behavior by explicitly
specifying the original exclude chars.
diff --git a/bash_completion b/bash_completion
@@ -2216,8 +2216,10 @@ _comp_count_args()
     done
     shift "$((OPTIND - 1))"
 
-    local cword words
-    _comp__reassemble_words "$exclude<>&" words cword
+    if [[ $has_exclude ]]; then
+        local cword words
+        _comp__reassemble_words "$exclude<>&" words cword
+    fi
 
     local i
     ret=1
diff --git a/completions/7z b/completions/7z
@@ -85,7 +85,7 @@ _comp_cmd_7z()
     fi
 
     local ret
-    _comp_count_args -n "="
+    _comp_count_args
     if ((ret == 2)); then
         _filedir_xspec unzip "${@:2}"
         # TODO: parsing 7z i output?
diff --git a/completions/chmod b/completions/chmod
@@ -28,7 +28,7 @@ _comp_cmd_chmod()
     fi
 
     local ret
-    _comp_count_args -i "$modearg"
+    _comp_count_args -n "" -i "$modearg"
 
     case $ret in
         1) ;; # mode
diff --git a/completions/cryptsetup b/completions/cryptsetup
@@ -37,7 +37,7 @@ _comp_cmd_cryptsetup()
     local ret
     if _comp_get_first_arg; then
         local arg=$ret
-        _comp_count_args -a "-${noargopts}[chslSbopitTdM]"
+        _comp_count_args -n "" -a "-${noargopts}[chslSbopitTdM]"
         local args=$ret
         case $arg in
             open | create | luksOpen | loopaesOpen | tcryptOpen)
diff --git a/completions/hcitool b/completions/hcitool
@@ -49,7 +49,7 @@ _comp_cmd_hcitool()
     if _comp_get_first_arg; then
         case $ret in
             name | info | dc | rssi | lq | afh | auth | key | clkoff | lst)
-                _comp_count_args
+                _comp_count_args -n ""
                 if ((ret == 2)); then
                     _comp_cmd_hcitool__bluetooth_addresses
                 fi
@@ -58,30 +58,30 @@ _comp_cmd_hcitool()
                 if [[ $cur == -* ]]; then
                     _comp_compgen -- -W '--role --pkt-type'
                 else
-                    _comp_count_args
+                    _comp_count_args -n ""
                     if ((ret == 2)); then
                         _comp_cmd_hcitool__bluetooth_addresses
                     fi
                 fi
                 ;;
             sr)
-                _comp_count_args
+                _comp_count_args -n ""
                 if ((ret == 2)); then
                     _comp_cmd_hcitool__bluetooth_addresses
                 else
                     _comp_compgen -- -W 'master slave'
                 fi
                 ;;
             cpt)
-                _comp_count_args
+                _comp_count_args -n ""
                 if ((ret == 2)); then
                     _comp_cmd_hcitool__bluetooth_addresses
                 else
                     _comp_cmd_hcitool__bluetooth_packet_types
                 fi
                 ;;
             tpl | enc | clock)
-                _comp_count_args
+                _comp_count_args -n ""
                 if ((ret == 2)); then
                     _comp_cmd_hcitool__bluetooth_addresses
                 else
diff --git a/completions/mkinitrd b/completions/mkinitrd
@@ -31,7 +31,7 @@ _comp_cmd_mkinitrd()
         [[ ${COMPREPLY-} == *= ]] && compopt -o nospace
     else
         local ret
-        _comp_count_args
+        _comp_count_args -n ""
 
         case $ret in
             1)
diff --git a/completions/nc b/completions/nc
@@ -39,7 +39,7 @@ _comp_cmd_nc()
 
     # Complete 1st non-option arg only
     local ret
-    _comp_count_args -a "-*[IiMmOPpqsTVWwXx]"
+    _comp_count_args -n "" -a "-*[IiMmOPpqsTVWwXx]"
     ((ret == 1)) || return
 
     _known_hosts_real -- "$cur"
diff --git a/completions/nslookup b/completions/nslookup
@@ -53,7 +53,7 @@ _comp_cmd_nslookup()
     fi
 
     local ret
-    _comp_count_args -n "="
+    _comp_count_args
     if ((ret <= 2)); then
         _known_hosts_real -- "$cur"
         [[ $ret -eq 1 && $cur == @(|-) ]] && COMPREPLY+=(-)
diff --git a/completions/patch b/completions/patch
@@ -56,7 +56,7 @@ _comp_cmd_patch()
     fi
 
     local ret
-    _comp_count_args
+    _comp_count_args -n ""
     case $ret in
         1)
             _comp_compgen_filedir
diff --git a/completions/quota b/completions/quota
@@ -84,7 +84,7 @@ _comp_cmd_setquota()
         _comp_cmd_quota__parse_help "$1"
     else
         local ret
-        _comp_count_args
+        _comp_count_args -n ""
 
         case $ret in
             1)
diff --git a/completions/zopflipng b/completions/zopflipng
@@ -28,7 +28,7 @@ _comp_cmd_zopflipng()
     if [[ ${words[*]} != *\ --prefix=* ]]; then
         # 2 png args only if --prefix not given
         local ret
-        _comp_count_args
+        _comp_count_args -n ""
         ((ret < 3)) && _comp_compgen_filedir png
     else
         # otherwise arbitrary number of png args
diff --git a/test/t/unit/test_unit_count_args.py b/test/t/unit/test_unit_count_args.py
@@ -4,50 +4,59 @@
 
 
 @pytest.mark.bashcomp(
-    cmd=None, ignore_env=r"^[+-](ret|COMP_(WORDS|CWORD|LINE|POINT))="
+    cmd=None,
+    ignore_env=r"^[+-](ret|cword|words|COMP_(WORDS|CWORD|LINE|POINT))=",
 )
 class TestUnitCountArgs(TestUnitBase):
-    def _test(self, *args, **kwargs):
-        return self._test_unit(
-            '_comp_count_args %s; echo "$ret"', *args, **kwargs
+    @pytest.fixture
+    def functions(self, bash):
+        assert_bash_exec(
+            bash,
+            '_comp__test_unit() { local -a words=(); local cword ret=""; _comp__reassemble_words "<>&" words cword; _comp_count_args "$@"; echo "$ret"; }',
         )
 
+    def _test(self, *args, **kwargs):
+        return self._test_unit("_comp__test_unit %s", *args, **kwargs)
+
     def test_1(self, bash):
-        assert_bash_exec(bash, "COMP_LINE= COMP_POINT=0 COMP_WORDS=() COMP_CWORD=; _comp_count_args")
+        assert_bash_exec(
+            bash,
+            'COMP_LINE= COMP_POINT=0 COMP_WORDS=() COMP_CWORD=; _comp_count_args -n ""',
+        )
 
-    def test_2(self, bash):
+    def test_2(self, bash, functions):
         """a b| should set args to 1"""
         output = self._test(bash, "(a b)", 1, "a b", 3)
         assert output == "1"
 
-    def test_3(self, bash):
+    def test_3(self, bash, functions):
         """a b|c should set args to 1"""
         output = self._test(bash, "(a bc)", 1, "a bc", 3)
         assert output == "1"
 
-    def test_4(self, bash):
+    def test_4(self, bash, functions):
         """a b c| should set args to 2"""
         output = self._test(bash, "(a b c)", 2, "a b c", 4)
         assert output == "2"
 
-    def test_5(self, bash):
+    def test_5(self, bash, functions):
         """a b| c should set args to 1"""
         output = self._test(bash, "(a b c)", 1, "a b c", 3)
         assert output == "1"
 
-    def test_6(self, bash):
+    def test_6(self, bash, functions):
         """a b -c| d should set args to 2"""
         output = self._test(bash, "(a b -c d)", 2, "a b -c d", 6)
         assert output == "2"
 
-    def test_7(self, bash):
+    def test_7(self, bash, functions):
         """a b -c d e| with -c arg excluded should set args to 2"""
         output = self._test(
             bash, "(a b -c d e)", 4, "a b -c d e", 10, arg='-a "@(-c|--foo)"'
         )
         assert output == "2"
 
-    def test_8(self, bash):
+    def test_8(self, bash, functions):
         """a -b -c d e| with -c arg excluded
         and -b included should set args to 1"""
         output = self._test(
@@ -60,7 +69,7 @@ def test_8(self, bash):
         )
         assert output == "2"
 
-    def test_9(self, bash):
+    def test_9(self, bash, functions):
         """a -b -c d e| with -b included should set args to 3"""
         output = self._test(
             bash, "(a -b -c d e)", 4, "a -b -c d e", 11, arg='-i "-b"'
