Fix shtab choices escaping and failing tests

CHANGELOG.rst

@@ -57,6 +57,14 @@ Fixed
   dataclass-like types that are not added as a group, including when nested in
   lists and dicts (`#939
   <https://github.com/mauvilsa/jsonargparse/pull/939>`__).
+- ``shtab`` bash completion scripts not escaping choices and type messages, such
+  that a value containing a single quote, e.g. a ``Literal`` type, produced a
+  script with invalid syntax (`#497
+  <https://github.com/mauvilsa/jsonargparse/pull/497>`__).
+- Tests for ``shtab`` completions failing with ``shtab>=1.9.1`` due to a change
+  in how it quotes the elements of the generated bash arrays. The completion
+  scripts themselves were not affected (`#497
+  <https://github.com/mauvilsa/jsonargparse/pull/497>`__).
 
 Changed
 ^^^^^^^

jsonargparse/_completions.py

@@ -3,6 +3,7 @@
 import locale
 import os
 import re
+import shlex
 from collections import defaultdict
 from contextlib import contextmanager, suppress
 from contextvars import ContextVar
@@ -153,7 +154,7 @@ def get_shtab_script(parser, shell: str, preambles: list[str] | None = None) ->
     if not preambles:
         preambles = []
     if shell == "bash":
-        preambles += [bash_compgen_typehint.strip().replace("%s", prog)]
+        preambles += [bash_compgen_typehint.strip().replace("{prog}", prog)]
     with prepare_actions_context(shell, prog, preambles):
         shtab_prepare_actions(parser)
     return shtab.complete(parser, shell, preamble="\n".join(preambles))
@@ -248,36 +249,33 @@ def shtab_prepare_action(action, parser) -> None:
         action.choices = choices
 
 
-bash_compgen_typehint_name = "_jsonargparse_%s_compgen_typehint"
+# "{prog}" is a placeholder replaced with the normalized prog name, see get_shtab_script.
+bash_compgen_typehint_name = "_jsonargparse_{prog}_compgen_typehint"
 bash_compgen_typehint = """
-_jsonargparse_%%s_matched_choices() {
-  local TOTAL=$(echo "$1" | wc -w | tr -d " ")
-  if [ "$TOTAL" != 0 ]; then
-    local MATCH=$(echo "$2" | wc -w | tr -d " ")
-    printf "; $MATCH/$TOTAL matched choices"
-  fi
-}
 %(name)s() {
-  local REQUIRE_PREFIX="$4"
+  local CHOICES="$1" WORD="$2" MESSAGE="$3" REQUIRE_PREFIX="$4" TOTAL="$5"
+  local IFS=$'\\n'  # choices may contain spaces, so split matches on newline only
   local MATCH=()
-  if [ "$REQUIRE_PREFIX" = 1 ] && [ -z "$2" ]; then
+  if [ "$REQUIRE_PREFIX" = 1 ] && [ -z "$WORD" ]; then
     MATCH=()
   else
-    MATCH=( $(IFS=" " compgen -W "$1" "$2") )
+    MATCH=( $(IFS=" " compgen -W "$CHOICES" "$WORD") )
+  fi
+  local MATCHED=""
+  if [ "$TOTAL" != 0 ]; then
+    MATCHED="; ${#MATCH[@]}/$TOTAL matched choices"
   fi
   if [ ${#MATCH[@]} = 0 ]; then
     if [ "$COMP_TYPE" = 63 ]; then
-      MATCHED=$(_jsonargparse_%%s_matched_choices "$1" "${MATCH[*]}")
-      printf "%(b)s\\n$3$MATCHED\\n%(n)s" >&2
+      printf "%(b)s\\n%%s%%s\\n%(n)s" "$MESSAGE" "$MATCHED" >&2
       kill -WINCH $$
     fi
   else
     for match in "${MATCH[@]}"; do
       echo "$match"
     done
     if [ "$COMP_TYPE" = 63 ]; then
-      MATCHED=$(_jsonargparse_%%s_matched_choices "$1" "${MATCH[*]}")
-      printf "%(b)s\\n$3$MATCHED%(n)s" >&2
+      printf "%(b)s\\n%%s%%s%(n)s" "$MESSAGE" "$MATCHED" >&2
     fi
   fi
 }
@@ -289,15 +287,19 @@ def shtab_prepare_action(action, parser) -> None:
 
 
 def add_bash_typehint_completion(parser, action, message, choices, require_prefix=False) -> None:
-    fn_typehint = norm_name(bash_compgen_typehint_name % shtab_prog.get())
+    fn_typehint = norm_name(bash_compgen_typehint_name.replace("{prog}", shtab_prog.get()))
     fn_name = parser.prog.replace(" [options] ", "_")
     fn_name = norm_name(f"_jsonargparse_{fn_name}_{action.dest}_typehint")
-    fn = '{fn_name}(){{ {fn_typehint} "{choices}" "$1" "{message}" {require_prefix}; }}'.format(
+    # choices are quoted twice: once so that compgen -W splits them into the intended words,
+    # and once so that the whole word list reaches the function as a single argument.
+    wordlist = shlex.quote(" ".join(shlex.quote(c) for c in choices))
+    fn = '{fn_name}(){{ {fn_typehint} {choices} "$1" {message} {require_prefix} {total}; }}'.format(
         fn_name=fn_name,
         fn_typehint=fn_typehint,
-        choices=" ".join(choices),
-        message=message,
+        choices=wordlist,
+        message=shlex.quote(message),
         require_prefix=1 if require_prefix else 0,
+        total=len(choices),
     )
     shtab_preambles.get().append(fn)
     action.complete = {"bash": fn_name}

jsonargparse_tests/test_shtab.py

@@ -1,4 +1,5 @@
 import re
+import shlex
 import subprocess
 import tempfile
 from enum import Enum
@@ -47,6 +48,13 @@ def parser() -> ArgumentParser:
     return ArgumentParser(exit_on_error=False, prog="tool")
 
 
+def get_bash_array(shtab_script, name):
+    """Elements of a bash array assignment, independent of how shtab quotes them."""
+    match = re.search(rf"^{re.escape(name)}=\((.*)\)$", shtab_script, re.MULTILINE)
+    assert match, f"{name} array not found in shtab script"
+    return shlex.split(match.group(1))
+
+
 def is_positional(dest, parser):
     if parser is not None:
         action = next(a for a in parser._actions if a.dest == dest)
@@ -69,7 +77,7 @@ def assert_bash_typehint_completions(subtests, shtab_script, completions):
                 sh = f'source {shtab_script_path}; COMP_TYPE=63 _jsonargparse_tool_{norm_name(dest)}_typehint "{word}"'
                 popen = subprocess.Popen(["bash", "-c", sh], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
                 out, err = popen.communicate()
-                assert list(out.decode().split()) == choices
+                assert out.decode().splitlines() == choices
                 if extra is None:
                     assert f"Expected type: {typehint}" in err.decode()
                 elif re.match(r"^\d/\d$", extra):
@@ -182,6 +190,23 @@ def test_bash_literal(parser, subtests):
     )
 
 
+def test_bash_literal_special_characters(parser, subtests):
+    typehint = Literal["one two", "three", "it's"]
+    parser.add_argument("--literal", type=typehint)
+    shtab_script = get_shtab_script(parser, "bash")
+    syntax_check = subprocess.run(["bash", "-n"], input=shtab_script.encode(), capture_output=True)
+    assert syntax_check.returncode == 0, syntax_check.stderr.decode()
+    assert_bash_typehint_completions(
+        subtests,
+        shtab_script,
+        [
+            ("literal", typehint, "", ["one two", "three", "it's"], "3/3"),
+            ("literal", typehint, "o", ["one two"], "1/3"),
+            ("literal", typehint, "i", ["it's"], "1/3"),
+        ],
+    )
+
+
 def test_bash_literal_none(parser, subtests):
     typehint = Literal[None]
     parser.add_argument("--literal", type=typehint)
@@ -364,18 +389,20 @@ def get_params_patch(cls, method, logger):
     parser.add_argument("--cls", type=Base)
     with capture_logs(logger) as logs, patch("jsonargparse._completions.get_signature_parameters", get_params_patch):
         shtab_script = get_shtab_script(parser, "bash")
-    assert "'--cls' '--cls.p1' '--cls.p2'" in shtab_script
-    assert f"'{__name__}.SubB'" in shtab_script
-    assert "'--cls.p3'" not in shtab_script
+    options = get_bash_array(shtab_script, "_shtab_tool_option_strings")
+    assert options == ["-h", "--help", "--cls.help", "--cls", "--cls.p1", "--cls.p2"]
+    assert f"{__name__}.SubB" in get_bash_array(shtab_script, "_shtab_tool___cls_help_choices")
+    assert "--cls.p3" not in shtab_script
     assert "test_shtab.SubB': test get params failure" in logs.getvalue()
 
 
 def test_bash_subclasses_help(parser):
     parser.add_argument("--cls", type=Base)
     shtab_script = get_shtab_script(parser, "bash")
-    assert "'--cls.help' '--cls' '--cls.p1' '--cls.p2' '--cls.p3'" in shtab_script
-    classes = f"'{__name__}.Base' '{__name__}.SubA' '{__name__}.SubB'"
-    assert f"_cls_help_choices=({classes})" in shtab_script
+    options = get_bash_array(shtab_script, "_shtab_tool_option_strings")
+    assert options == ["-h", "--help", "--cls.help", "--cls", "--cls.p1", "--cls.p2", "--cls.p3"]
+    classes = [f"{__name__}.Base", f"{__name__}.SubA", f"{__name__}.SubB"]
+    assert get_bash_array(shtab_script, "_shtab_tool___cls_help_choices") == classes
 
 
 def test_bash_subclasses(parser, subtests):
@@ -402,9 +429,10 @@ def __init__(self, o1: bool):
 def test_bash_union_subclasses(parser, subtests):
     parser.add_argument("--cls", type=Union[Base, Other])
     shtab_script = get_shtab_script(parser, "bash")
-    assert "'--cls.help' '--cls' '--cls.p1' '--cls.p2' '--cls.p3' '--cls.o1'" in shtab_script
-    classes = f"'{__name__}.Base' '{__name__}.SubA' '{__name__}.SubB' '{__name__}.Other'"
-    assert f"_cls_help_choices=({classes})" in shtab_script
+    options = get_bash_array(shtab_script, "_shtab_tool_option_strings")
+    assert options == ["-h", "--help", "--cls.help", "--cls", "--cls.p1", "--cls.p2", "--cls.p3", "--cls.o1"]
+    classes = [f"{__name__}.Base", f"{__name__}.SubA", f"{__name__}.SubB", f"{__name__}.Other"]
+    assert get_bash_array(shtab_script, "_shtab_tool___cls_help_choices") == classes
     assert_bash_typehint_completions(
         subtests,
         shtab_script,
@@ -427,7 +455,8 @@ def __init__(self, s1: Optional[Base]):
 def test_bash_nested_subclasses(parser, subtests):
     parser.add_argument("--cls", type=SupBase)
     shtab_script = get_shtab_script(parser, "bash")
-    assert "'--cls.help' '--cls' '--cls.s1' '--cls.s1.p1' '--cls.s1.p2' '--cls.s1.p3'" in shtab_script
+    options = get_bash_array(shtab_script, "_shtab_tool_option_strings")
+    assert options == ["-h", "--help", "--cls.help", "--cls", "--cls.s1", "--cls.s1.p1", "--cls.s1.p2", "--cls.s1.p3"]
     assert_bash_typehint_completions(
         subtests,
         shtab_script,
@@ -440,7 +469,8 @@ def test_bash_nested_subclasses(parser, subtests):
 def test_bash_callable_return_class(parser, subtests):
     parser.add_argument("--cls", type=Callable[[int], Base])
     shtab_script = get_shtab_script(parser, "bash")
-    assert "_option_strings=('-h' '--help' '--cls.help' '--cls' '--cls.p2' '--cls.p3')" in shtab_script
+    options = get_bash_array(shtab_script, "_shtab_tool_option_strings")
+    assert options == ["-h", "--help", "--cls.help", "--cls", "--cls.p2", "--cls.p3"]
     assert "--cls.p1" not in shtab_script
     classes = f"{__name__}.Base {__name__}.SubA {__name__}.SubB".split()
     assert_bash_typehint_completions(
@@ -480,12 +510,13 @@ def test_bash_subcommands(parser, subparser, subtests):
     assert "--print_completion" not in help_str
 
     shtab_script = get_shtab_script(parser, "bash")
-    assert "_subparsers=('s1' 's2')" in shtab_script
+    assert get_bash_array(shtab_script, "_shtab_tool_subparsers") == ["s1", "s2"]
 
-    assert "_s1_option_strings=('-h' '--help' '--enum')" in shtab_script
-    assert "_s2_option_strings=('-h' '--help' '--cls.help' '--cls' '--cls.p1' '--cls.p2' '--cls.p3')" in shtab_script
-    classes = f"'{__name__}.Base' '{__name__}.SubA' '{__name__}.SubB'"
-    assert f"_s2___cls_help_choices=({classes})" in shtab_script
+    assert get_bash_array(shtab_script, "_shtab_tool_s1_option_strings") == ["-h", "--help", "--enum"]
+    options = get_bash_array(shtab_script, "_shtab_tool_s2_option_strings")
+    assert options == ["-h", "--help", "--cls.help", "--cls", "--cls.p1", "--cls.p2", "--cls.p3"]
+    classes = [f"{__name__}.Base", f"{__name__}.SubA", f"{__name__}.SubB"]
+    assert get_bash_array(shtab_script, "_shtab_tool_s2___cls_help_choices") == classes
 
     assert_bash_typehint_completions(
         subtests,

pyproject.toml

@@ -68,7 +68,8 @@ fsspec = [
     "fsspec>=0.8.4",
 ]
 shtab = [
-    "shtab>=1.7.1",
+    # 1.8.2 and 1.9.0 generate corrupted *_COMPGEN values, fixed in 1.9.1
+    "shtab>=1.7.1,!=1.8.2,!=1.9.0",
 ]
 argcomplete = [
     "argcomplete>=3.5.1",