From 1098b624b41a06035ae84ce202fb526cef06922e Mon Sep 17 00:00:00 2001 From: criss717 Date: Sun, 1 Nov 2026 18:40:36 +0100 Subject: [PATCH] fix(i18n): preserve language selection for combined short flags --- strix/interface/cli_args.py | 34 ++++++++---- strix/locales/en.json | 1 + strix/locales/es.json | 1 + tests/test_i18n.py | 104 ++++++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 9 deletions(-) diff --git a/strix/interface/cli_args.py b/strix/interface/cli_args.py index f6b28ebfb..4b51c099a 100644 --- a/strix/interface/cli_args.py +++ b/strix/interface/cli_args.py @@ -42,6 +42,7 @@ _ARGPARSE_MESSAGE_KEYS: dict[str, str] = { "expected at most one argument": "cli.argparse_expected_at_most", "expected at least one argument": "cli.argparse_expected_at_least", "unexpected option string: %s": "cli.argparse_unexpected_option", + "ambiguous option: %(option)s could match %(matches)s": "cli.argparse_ambiguous_option", } @@ -58,13 +59,21 @@ argparse._ = _translate_argparse # type: ignore[attr-defined] # A minimal parser used to resolve --language/--config BEFORE the full parser -# renders help text. It reuses argparse's own abbreviation rules (e.g. -# ``--conf`` -> ``--config``), so language resolution accepts exactly what the -# real parse accepts. ``exit_on_error=False`` makes a malformed known option -# raise instead of printing, so the full parser reports it once. +# renders help or error text. Registering every short option from the full +# parser with matching arity (plus ``nargs="?"`` on value-taking options) lets +# argparse split valid short-flag clusters (e.g. ``-nl es`` -> ``-n -l es``), +# avoid misreading ``-rl es`` as ``-l``, and survive missing or ambiguous other +# flags (e.g. ``strix -l es --config`` or ``strix --max 10 -l es``) so the full +# parser reports the error in the resolved language. _ABBREV_PARSER = argparse.ArgumentParser(add_help=False, allow_abbrev=True, exit_on_error=False) -_ABBREV_PARSER.add_argument("-l", "--language") -_ABBREV_PARSER.add_argument("--config") +_ABBREV_PARSER.add_argument("-h", action="store_true") +_ABBREV_PARSER.add_argument("-v", action="store_true") +_ABBREV_PARSER.add_argument("-n", action="store_true") +_ABBREV_PARSER.add_argument("-t", nargs="?", action="append") +_ABBREV_PARSER.add_argument("-m", nargs="?") +_ABBREV_PARSER.add_argument("-r", nargs="?") +_ABBREV_PARSER.add_argument("-l", "--language", nargs="?") +_ABBREV_PARSER.add_argument("--config", nargs="?") def _pre_resolve_language() -> None: @@ -72,7 +81,8 @@ def _pre_resolve_language() -> None: Argparse evaluates help text at parse time, so we must resolve the language BEFORE parse_args() is called. A minimal parser resolves the same option - abbreviations the real parse accepts (e.g. ``--conf`` -> ``--config``). + abbreviations and short-flag clusters the real parse accepts (e.g. + ``--conf`` -> ``--config``, ``-nl es`` -> ``-n -l es``). Priority matches ``_detect_language``: --language > STRIX_LANGUAGE env > --config file > default config > system locale. """ @@ -89,7 +99,6 @@ def _pre_resolve_language() -> None: try: namespace, _ = _ABBREV_PARSER.parse_known_args(pre_args) except argparse.ArgumentError: - # A malformed --language/--config; the full parser reports it. return if namespace.language: @@ -353,8 +362,15 @@ Strix Cloud: args.diff_scope = {"active": False} args.run_name = None + from strix.i18n import set_config_path, set_language + + if args.language: + set_language(args.language) + if args.config: - apply_config_override(validate_config_file(args.config)) + config_file = validate_config_file(args.config) + apply_config_override(config_file) + set_config_path(config_file) if args.mcp_config: mcp_config_path = Path(args.mcp_config).expanduser() diff --git a/strix/locales/en.json b/strix/locales/en.json index 7f4064dfb..73972aa00 100644 --- a/strix/locales/en.json +++ b/strix/locales/en.json @@ -32,6 +32,7 @@ "cli.argparse_expected_at_most": "expected at most one argument", "cli.argparse_expected_at_least": "expected at least one argument", "cli.argparse_unexpected_option": "unexpected option string: %s", + "cli.argparse_ambiguous_option": "ambiguous option: %(option)s could match %(matches)s", "cli.invalid_float": "invalid float value: {value!r}", "cli.finite_number": "must be a finite number greater than 0", "cli.invalid_int": "invalid int value: {value!r}", diff --git a/strix/locales/es.json b/strix/locales/es.json index 8470f4edd..b761c0d33 100644 --- a/strix/locales/es.json +++ b/strix/locales/es.json @@ -32,6 +32,7 @@ "cli.argparse_expected_at_most": "se espera como máximo un argumento", "cli.argparse_expected_at_least": "se espera al menos un argumento", "cli.argparse_unexpected_option": "opción inesperada: %s", + "cli.argparse_ambiguous_option": "opción ambigua: %(option)s podría coincidir con %(matches)s", "cli.invalid_float": "valor float inválido: {value!r}", "cli.finite_number": "debe ser un número finito mayor que 0", "cli.invalid_int": "valor entero inválido: {value!r}", diff --git a/tests/test_i18n.py b/tests/test_i18n.py index c64b428b9..e0e198901 100644 --- a/tests/test_i18n.py +++ b/tests/test_i18n.py @@ -501,6 +501,110 @@ class TestArgparseBuiltins: assert cli_args._translate_argparse("usage: ") == "usage: " assert cli_args._translate_argparse("options") == "options" + @pytest.mark.parametrize( + "cluster_argv", + [ + ["strix", "-nl", "es", "--target", "https://example.com"], + ["strix", "-nles", "--target", "https://example.com"], + ], + ) + def test_combined_short_flags_set_spanish_and_parse_end_to_end( + self, + monkeypatch: pytest.MonkeyPatch, + cluster_argv: list[str], + ) -> None: + monkeypatch.setattr(sys, "argv", cluster_argv) + cli_args._pre_resolve_language() + assert get_language() == "es" + + args = cli_args.parse_arguments() + assert args.non_interactive is True + assert args.language == "es" + assert get_language() == "es" + + def test_vnl_cluster_sets_spanish_in_pre_resolve_and_exits_with_version( + self, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + monkeypatch.setattr(sys, "argv", ["strix", "-vnl", "es"]) + cli_args._pre_resolve_language() + assert get_language() == "es" + + with pytest.raises(SystemExit) as exc_info: + cli_args.parse_arguments() + assert exc_info.value.code == 0 + assert capsys.readouterr().out.startswith("strix ") + + @pytest.mark.parametrize( + "help_argv", + [ + ["strix", "-nhl", "es"], + ["strix", "-hl", "es"], + ["strix", "-hvl", "es"], + ], + ) + def test_combined_short_help_flags_render_spanish( + self, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + help_argv: list[str], + ) -> None: + monkeypatch.setattr(sys, "argv", help_argv) + with pytest.raises(SystemExit) as exc_info: + cli_args.parse_arguments() + assert exc_info.value.code == 0 + out = capsys.readouterr().out + assert "uso: " in out + assert "opciones:" in out + + @pytest.mark.parametrize("cluster_flag", ["-rl", "-tl", "-ml"]) + def test_option_value_taking_short_flags_do_not_falsely_trigger_spanish( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + cluster_flag: str, + ) -> None: + # `-rl es`, `-tl es`, `-ml es` pass `"l"` as the value of `-r`/`-t`/`-m`, NOT `-l es`! + monkeypatch.setattr(sys, "argv", ["strix", cluster_flag, "es"]) + with patch.dict(os.environ, {}, clear=True): + cli_args._pre_resolve_language() + set_config_path(tmp_path / "missing.json") + assert get_language() == "en" + + with pytest.raises(SystemExit) as exc_info: + cli_args.parse_arguments() + assert exc_info.value.code == 2 + err = capsys.readouterr().err + assert "usage: " in err + + def test_valid_language_followed_by_missing_config_value_emits_spanish_error( + self, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + monkeypatch.setattr(sys, "argv", ["strix", "-l", "es", "--config"]) + with pytest.raises(SystemExit) as exc_info: + cli_args.parse_arguments() + assert exc_info.value.code == 2 + err = capsys.readouterr().err + assert "uso: " in err + assert "argumento --config: se espera un argumento" in err + + def test_ambiguous_option_before_language_flag_emits_spanish_error( + self, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + monkeypatch.setattr(sys, "argv", ["strix", "--max", "10", "-l", "es"]) + with pytest.raises(SystemExit) as exc_info: + cli_args.parse_arguments() + assert exc_info.value.code == 2 + err = capsys.readouterr().err + assert "uso: " in err + assert "opción ambigua: --max podría coincidir con" in err + class TestPyInstallerSpecPackaging: def test_strix_spec_bundles_locales_and_hiddenimport(self) -> None: