diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9fee7f57..9675b898 100644 --- a/CHANGELOG.rst +++ b/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 `__). +- ``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 + `__). +- 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 + `__). Changed ^^^^^^^ diff --git a/jsonargparse/_completions.py b/jsonargparse/_completions.py index 94ec8c51..10459054 100644 --- a/jsonargparse/_completions.py +++ b/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,27 +249,25 @@ 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 @@ -276,8 +275,7 @@ def shtab_prepare_action(action, parser) -> None: 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} diff --git a/jsonargparse_tests/test_shtab.py b/jsonargparse_tests/test_shtab.py index bdd87e4b..5d4db154 100644 --- a/jsonargparse_tests/test_shtab.py +++ b/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, diff --git a/pyproject.toml b/pyproject.toml index af576dfe..4048d1e9 100644 --- a/pyproject.toml +++ b/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",