From e13b31ed8e30300bc04467d6589e519a9f957489 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:00:57 -0400 Subject: [PATCH 01/22] Keep shiv extractions of the test suite out of ~/.shiv Zipapps built by the tests and by the docs snippet check unpack themselves into SHIV_ROOT when they run, which defaults to ~/.shiv and is never cleaned up. A session fixture now points SHIV_ROOT at the pytest temp directory for every test, subprocess and xdist worker, and scripts/docs.py does the same for the snippet runner. Co-Authored-By: Claude Opus 5.5 --- scripts/docs.py | 7 ++++++- tests/conftest.py | 14 ++++++++++++++ tests/test_isolation.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/test_isolation.py diff --git a/scripts/docs.py b/scripts/docs.py index 2761516..9e2b3e4 100644 --- a/scripts/docs.py +++ b/scripts/docs.py @@ -882,11 +882,16 @@ def check_readme() -> list[str]: return [] +def snippet_environment(tmp: str | os.PathLike[str]) -> dict[str, str]: + """Environment for the snippet tests. Anything a snippet builds with shiv extracts into ``tmp``, not ``~/.shiv``.""" + return {**os.environ, "NO_COLOR": "1", "PYTHONWARNINGS": "ignore", "SHIV_ROOT": str(Path(tmp) / "shiv-root")} + + def check_snippets() -> list[str]: """Load, compile and test every manifest in docs/snippets from a scratch copy of that directory.""" errors = [] - env = {**os.environ, "NO_COLOR": "1", "PYTHONWARNINGS": "ignore"} with tempfile.TemporaryDirectory() as tmp: + env = snippet_environment(tmp) workdir = Path(tmp) / "snippets" shutil.copytree(SNIPPETS, workdir) previous = Path.cwd() diff --git a/tests/conftest.py b/tests/conftest.py index d40a281..7d43187 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -36,6 +36,20 @@ def pytest_sessionfinish(session, exitstatus): rmtree("test-manifest-builds-response", ignore_errors=True) +@pytest.fixture(scope="session", autouse=True) +def shiv_root(tmp_path_factory): + """Zipapps built by the tests extract themselves here instead of into ``~/.shiv``. + + A shiv zipapp unpacks its bundled packages into ``SHIV_ROOT`` (default ``~/.shiv``) on every run + of a new build, and nothing removes them. Set in ``os.environ`` so that it also reaches the + subprocesses the tests start; each xdist worker runs this fixture for itself. + """ + root = tmp_path_factory.mktemp("shiv-root") + with pytest.MonkeyPatch.context() as environment: + environment.setenv("SHIV_ROOT", str(root)) + yield root + + @pytest.fixture def isolated_fs(tmp_path, monkeypatch): """Run the test from an empty temporary working directory. diff --git a/tests/test_isolation.py b/tests/test_isolation.py new file mode 100644 index 0000000..4367fc4 --- /dev/null +++ b/tests/test_isolation.py @@ -0,0 +1,33 @@ +"""The test suite must not leave anything in the user's home directory.""" + +import importlib.util +import os +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent + + +def test_shiv_root_points_into_the_pytest_temp_directory(tmp_path_factory): + # zipapps built by the tests extract themselves into SHIV_ROOT when they run. Without it they + # go to ~/.shiv, one copy per build, and are never removed. + shiv_root = Path(os.environ["SHIV_ROOT"]).resolve() + assert shiv_root.is_relative_to(tmp_path_factory.getbasetemp().resolve()) + assert not shiv_root.is_relative_to((Path.home() / ".shiv").resolve()) + + +def test_shiv_root_reaches_subprocesses(): + code = "import os; print(os.environ.get('SHIV_ROOT', ''))" + result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, check=True) + assert result.stdout.strip() == os.environ["SHIV_ROOT"] + + +def test_docs_snippet_runner_uses_a_temporary_shiv_root(tmp_path): + spec = importlib.util.spec_from_file_location("cliffy_docs_script", ROOT / "scripts" / "docs.py") + assert spec and spec.loader + docs = importlib.util.module_from_spec(spec) + spec.loader.exec_module(docs) + + env = docs.snippet_environment(tmp_path) + assert Path(env["SHIV_ROOT"]).is_relative_to(tmp_path) From d920c990690b5887e9c01f7b7225d3caabc39c6e Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:02:17 -0400 Subject: [PATCH 02/22] Give the raw init template a command and a test `cli init --raw` wrote a manifest with no commands and no tests, so there was nothing to run and `cli test` exited with an error. Both templates are now checked end to end: validate, run, and pass their own tests. Co-Authored-By: Claude Opus 5.5 --- cliffy/manifest.py | 9 ++++++--- tests/test_cli.py | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/cliffy/manifest.py b/cliffy/manifest.py index 034072f..6e52de9 100644 --- a/cliffy/manifest.py +++ b/cliffy/manifest.py @@ -546,7 +546,8 @@ def get_raw_template(cls, cli_name: str, json_schema: bool) -> str: json_schema (bool): Flag to include JSON schema reference comment Returns: - str: A YAML-formatted template for a CLI manifest with default configuration + str: A YAML-formatted template for a CLI manifest with default configuration. It has one command + and one test, so that it runs and passes `cli test` as written. """ manifest = "" if json_schema: @@ -573,11 +574,13 @@ def get_raw_template(cls, cli_name: str, json_schema: bool) -> str: command_templates: {{}} -commands: {{}} +commands: + hello: print("Hello!") cli_options: {{}} -tests: [] +tests: + - hello: assert result_text == "Hello!" """ return manifest diff --git a/tests/test_cli.py b/tests/test_cli.py index 7f3c03e..7ea8ca5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,9 +1,12 @@ import os import re +import subprocess +import sys from datetime import datetime from pathlib import Path from unittest.mock import MagicMock, patch +import pytest from click.testing import CliRunner from cliffy.cli import ( @@ -18,6 +21,9 @@ update_command, validate_command, ) +from cliffy.cli import ( + test_command as manifest_test_command, +) from cliffy.homer import get_metadata from cliffy.loader import Loader from cliffy.manifest import CLIMetadata @@ -441,3 +447,24 @@ def test_build_failure_exit_code(isolated_fs): assert "fails build failed" in result.output assert result.exit_code == 1 + + +@pytest.mark.parametrize("flags", [[], ["--raw"]], ids=["commented", "raw"]) +def test_init_template_loads_runs_and_passes_its_own_tests(isolated_fs, flags): + # the commented template used to fail with "parameter without a default follows parameter with a + # default", and the raw template had nothing to run or test + runner = CliRunner() + assert runner.invoke(init_command, ["mycli", *flags]).exit_code == 0 + + validated = runner.invoke(validate_command, ["mycli.yaml"]) + assert validated.exit_code == 0, validated.output + + ran = subprocess.run( + [sys.executable, "-m", "cliffy", "run", "mycli.yaml", "--", "hello"], capture_output=True, text=True + ) + assert ran.returncode == 0, ran.stderr + assert "Hello" in ran.stdout + + tested = runner.invoke(manifest_test_command, ["mycli.yaml"]) + assert "All tests passed" in tested.output + assert tested.exit_code == 0 From 2492a0754edcb097673acee8fcddc6d9c188c390 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:06:45 -0400 Subject: [PATCH 03/22] Give generated functions and group variables collision-free names A root command and a group could map to the same Python name (command `db-app` and group `db` are both `db_app`), as could two groups, a command and its alias copy, or a command and a name from `vars`, `imports` or `functions`. The commander now reserves every name it emits and appends a numeric suffix to a name that is taken. A `vars` key that is not a Python identifier (`my-var`), is a keyword, or is a name the generated module defines itself is rejected with a manifest-level error instead of producing a SyntaxError in the generated code. Co-Authored-By: Claude Opus 5.5 --- cliffy/commander.py | 69 ++++++++++++++++++++++++++++++++++++-- cliffy/commanders/click.py | 21 ++++++++---- cliffy/commanders/typer.py | 8 +++-- cliffy/manifest.py | 27 ++++++++++++++- tests/test_codegen.py | 58 ++++++++++++++++++++++++++++++++ 5 files changed, 171 insertions(+), 12 deletions(-) diff --git a/cliffy/commander.py b/cliffy/commander.py index 0003746..3bf1172 100644 --- a/cliffy/commander.py +++ b/cliffy/commander.py @@ -1,5 +1,6 @@ from __future__ import annotations +import ast from abc import ABC, abstractmethod from collections import defaultdict from collections.abc import ItemsView, Iterator, ValuesView @@ -31,6 +32,8 @@ class CLI(BaseModel): code: str requires: list[str] = [] blueprint: str | None = None + # generated function name -> where it comes from in the manifest, i.e. {"db_create": "commands.db.create"} + functions: dict[str, str] = {} class BaseGroup(BaseModel): @@ -39,11 +42,13 @@ class BaseGroup(BaseModel): parent_group: BaseGroup | None = None commands: list[Command] = [] help: str = "" + # set by the Commander, which knows the other names of the generated module + var: str = "" @property def var_name(self) -> str: """Returns valid Python variable name for group app""" - return to_identifier(self.name + "_app") + return self.var or to_identifier(self.name + "_app") class Groups(BaseModel): @@ -101,13 +106,16 @@ class Commander(ABC): "base_imports", "callbacks_by_group", "cli", + "command_functions", "commands", + "functions", "greedy", "groups", "manifest", "parser", "root_commands", "root_group", + "taken_names", ) def __init__(self, manifest: CLIManifest) -> None: @@ -137,6 +145,57 @@ def __init__(self, manifest: CLIManifest) -> None: ] self.build_groups() + # Python names of the generated module. Every command function and group variable gets one that + # nothing else uses: two manifest names can map to the same identifier (`db-app` and the group `db`). + self.taken_names: set[str] = set(self.manifest.vars) | self.user_defined_names() + self.functions: dict[str, str] = {} + self.command_functions: dict[str, str] = {} + for group in self.groups.values(): + group.var = self.unique_name(to_identifier(group.name + "_app")) + + def user_defined_names(self) -> set[str]: + """Names bound by the `imports` and `functions` sections, as far as they can be read as Python.""" + names: set[str] = set() + for section in (self.manifest.imports, self.manifest.functions): + for block in [section] if isinstance(section, str) else section: + try: + tree = ast.parse(block) + except SyntaxError: + # i.e. a function with shell lines. Its name cannot be known before it is transformed + continue + for node in tree.body: + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef): + names.add(node.name) + elif isinstance(node, ast.Import | ast.ImportFrom): + names.update((alias.asname or alias.name).split(".")[0] for alias in node.names) + elif isinstance(node, ast.Assign): + names.update(target.id for target in node.targets if isinstance(target, ast.Name)) + return names + + def unique_name(self, name: str, source: str = "") -> str: + """Reserves a Python name for the generated module: `name`, or `name_2`, `name_3`, ... when it is taken. + + Args: + name (str): The identifier to reserve. + source (str): Where a function with this name comes from in the manifest, i.e. `commands.db.create`. + """ + unique, count = name, 1 + while unique in self.taken_names: + count += 1 + unique = f"{name}_{count}" + self.taken_names.add(unique) + if source: + self.functions[unique] = source + return unique + + def command_func_name(self, command: Command) -> str: + """Name of the Python function generated for a command""" + if command.name not in self.command_functions: + self.command_functions[command.name] = self.unique_name( + self.parser.get_command_func_name(command), source=f"commands.{command.name}" + ) + return self.command_functions[command.name] + def _merge_command_template(self, command: Command) -> None: """Merge command with its template if specified.""" if not command.template: @@ -387,4 +446,10 @@ def generate_cli(manifest: CLIManifest, commander_cls: type[Commander] = Command """ commander = commander_cls(manifest) commander.generate_cli() - return CLI(name=manifest.name, version=manifest.version, code=commander.cli, requires=manifest.requires) + return CLI( + name=manifest.name, + version=manifest.version, + code=commander.cli, + requires=manifest.requires, + functions=commander.functions, + ) diff --git a/cliffy/commanders/click.py b/cliffy/commanders/click.py index 9aedb8d..4f27e2e 100644 --- a/cliffy/commanders/click.py +++ b/cliffy/commanders/click.py @@ -45,6 +45,7 @@ def show_aliases(ctx, param, value): ctx.exit() """ callbacks = self.callbacks_for() + self.functions["cli"] = "callbacks" self.cli += """ @click.group(context_settings=CONTEXT_SETTINGS""" if "invoke_without_command" not in self.manifest.cli_options: @@ -93,6 +94,7 @@ def define_groups(self) -> None: for group in self.groups.values(): parsed_help = group.help.replace("\n", "") if group.help else "" callbacks = self.callbacks_for(group) + self.functions[group.var_name] = f"callbacks (group {group.name})" self.cli += f""" @click.group({self.parse_callback_options(callbacks)}){self.parse_callback_decorators(callbacks)} def {group.var_name}({self.parse_callback_param_names(callbacks)}): @@ -109,7 +111,7 @@ def add_root_command(self, command: Command) -> None: if not command.run: return - parsed_command_func_name = self.click_parser.get_command_func_name(command) + parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.click_parser.get_parsed_command_name(command) parsed_help = command.help.replace("\n", "") if command.help else "" @@ -125,13 +127,13 @@ def {parsed_command_func_name}({self.click_parser.get_param_names(command)}): self.cli += f""" @cli.command(name={py_str(alias)}, hidden=True) {self.click_parser.parse_params(command)} -def {self.click_parser.get_alias_func_name(command, alias)}({self.click_parser.get_param_names(command)}): +def {self.alias_func_name(command, alias)}({self.click_parser.get_param_names(command)}): {py_str("Alias for " + parsed_command_name)} {self.click_parser.parse_command_run(command)} """ def add_sub_command(self, command: Command, group: BaseGroup) -> None: - parsed_command_func_name = self.click_parser.get_command_func_name(command) + parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.click_parser.get_parsed_command_name(command) parsed_help = command.help.replace("\n", "") if command.help else "" @@ -147,11 +149,18 @@ def {parsed_command_func_name}({self.click_parser.get_param_names(command)}): self.cli += f""" @{group.var_name}.command(name={py_str(alias)}, hidden=True) {self.click_parser.parse_params(command)} -def {self.click_parser.get_alias_func_name(command, alias)}({self.click_parser.get_param_names(command)}): +def {self.alias_func_name(command, alias)}({self.click_parser.get_param_names(command)}): {py_str("Alias for " + parsed_command_name)} {self.click_parser.parse_command_run(command)} """ + def alias_func_name(self, command: Command, alias: str) -> str: + """Name of the function that is registered under an alias: Click needs one function per command name.""" + return self.unique_name( + self.click_parser.get_alias_func_name(self.command_func_name(command), alias), + source=f"commands.{command.name}", + ) + def add_main_block(self) -> None: self.cli += """ if __name__ == "__main__": @@ -270,7 +279,7 @@ def _build_click_option(self, param: CommandParam) -> str: return f"@click.option({', '.join(parts)})" - def get_alias_func_name(self, command: Command, alias: str) -> str: + def get_alias_func_name(self, command_func_name: str, alias: str) -> str: """Function name for the copy of a command that is registered under an alias""" alias_suffix = re.sub(r"\W", "_", alias) - return f"{self.get_command_func_name(command)}_{alias_suffix}" + return f"{command_func_name}_{alias_suffix}" diff --git a/cliffy/commanders/typer.py b/cliffy/commanders/typer.py index 4a0ff4a..b95cd09 100644 --- a/cliffy/commanders/typer.py +++ b/cliffy/commanders/typer.py @@ -49,6 +49,7 @@ def aliases_callback(value: bool): raise typer.Exit() """ callbacks = self.callbacks_for() + self.functions["main"] = "callbacks" self.cli += f""" @cli.callback({self.parse_callback_options(callbacks)}) def main({self.parse_callback_params(callbacks)}""" @@ -89,7 +90,7 @@ def add_root_command(self, command: Command) -> None: if not command.run: return - parsed_command_func_name = self.parser.get_command_func_name(command) + parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.parser.get_parsed_command_name(command) parsed_command_config = self.parser.get_parsed_config(command) parsed_help = command.help.replace("\n", "") if command.help else "" @@ -116,9 +117,10 @@ def define_groups(self) -> None: self.cli += f"""{group.var_name} = typer.Typer({empty_or_help}) """ if callbacks := self.callbacks_for(group): + callback_name = self.unique_name(f"{group.var_name}_callback", source=f"callbacks (group {group.name})") self.cli += f""" @{group.var_name}.callback({self.parse_callback_options(callbacks)}) -def {group.var_name}_callback({self.parse_callback_params(callbacks)} +def {callback_name}({self.parse_callback_params(callbacks)} ): {self.parse_callback_run(callbacks)} """ @@ -137,7 +139,7 @@ def add_sub_command(self, command: Command, group: BaseGroup) -> None: # a script-less command only carries the help of the group with the same name return - parsed_command_func_name = self.parser.get_command_func_name(command) + parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.parser.get_parsed_command_name(command) parsed_command_config = self.parser.get_parsed_config(command) parsed_help = command.help.replace("\n", "") if command.help else "" diff --git a/cliffy/manifest.py b/cliffy/manifest.py index 6e52de9..0cba144 100644 --- a/cliffy/manifest.py +++ b/cliffy/manifest.py @@ -1,3 +1,5 @@ +import keyword +import re import sys from collections.abc import ItemsView, Iterator from datetime import datetime @@ -6,7 +8,7 @@ from pydantic import BaseModel, Field, RootModel, ValidationInfo, field_validator, model_validator -from .helper import CLI_NAME_PATTERN, wrap_as_comment +from .helper import CLI_NAME_PATTERN, GENERATED_NAMES, wrap_as_comment LATEST_SCHEMA_VERSION = "v3" @@ -358,6 +360,29 @@ def is_safe_name(cls, value: str) -> str: ) return value + @field_validator("vars", mode="after") + @classmethod + def var_names_are_identifiers(cls, value: dict[str, Any]) -> dict[str, Any]: + """A var becomes a module-level Python variable of the generated CLI.""" + for name in value: + if name.isidentifier() and not keyword.iskeyword(name) and name not in GENERATED_NAMES: + continue + + if name in GENERATED_NAMES: + problem = "is a name the generated CLI uses itself" + elif keyword.iskeyword(name): + problem = "is a Python keyword" + else: + problem = "is not a valid Python identifier" + suggestion = re.sub(r"\W", "_", name) + if not suggestion.isidentifier() or keyword.iskeyword(suggestion) or suggestion in GENERATED_NAMES: + suggestion = f"{suggestion}_" if suggestion.isidentifier() else f"_{suggestion}" + raise ValueError( + f"vars.{name}: `{name}` {problem}. A var becomes a Python variable, so use letters, digits and `_` " + f"only, i.e. `{suggestion}`." + ) + return value + @field_validator("manifestVersion", mode="after") @classmethod def is_v3(cls, value: str) -> str: diff --git a/tests/test_codegen.py b/tests/test_codegen.py index e349658..32d3018 100644 --- a/tests/test_codegen.py +++ b/tests/test_codegen.py @@ -240,6 +240,64 @@ def test_names_become_valid_identifiers(run, manifest, args, expected_output): assert expected_output in result.output +COLLIDING_NAMES = [ + # a root command and a group that both used to be the Python name `db_app` + pytest.param("commands:\n db-app: print(1)\n db.x: print(2)\n", ["db-app"], "1", id="command_vs_group_var"), + pytest.param("commands:\n db-app: print(1)\n db.x: print(2)\n", ["db", "x"], "2", id="group_var_vs_command"), + pytest.param("commands:\n db.x: print(2)\n db-app: print(1)\n", ["db", "x"], "2", id="group_var_declared_first"), + pytest.param("commands:\n a-b: print(1)\n a_b: print(2)\n", ["a-b"], "1", id="dash_vs_underscore"), + pytest.param("commands:\n a.b: print(1)\n a_b: print(2)\n", ["a", "b"], "1", id="subcommand_vs_command"), + pytest.param("commands:\n a.b.x: print(1)\n a_b.y: print(2)\n", ["a", "b", "x"], "1", id="group_vs_group"), + pytest.param("commands:\n a.b.x: print(1)\n a_b.y: print(2)\n", ["a_b", "y"], "2", id="group_vs_group_second"), + pytest.param( + "imports: import json\ncommands:\n json: print(1)\n dump: print(json.dumps([2]))\n", + ["dump"], + "[2]", + id="command_vs_import", + ), + pytest.param( + "vars:\n status: up\ncommands:\n status: print(1)\n show: print(status)\n", + ["show"], + "up", + id="command_vs_var", + ), + pytest.param( + "functions:\n - |\n def helper():\n return 3\ncommands:\n helper: print(1)\n show: print(helper())\n", + ["show"], + "3", + id="command_vs_function", + ), + pytest.param( + "commands:\n rm|rm-x: print(1)\n rm-rm-x: print(2)\n rm_rm_x: print(3)\n", + ["rm-x"], + "1", + id="alias_vs_command", + ), +] + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +@pytest.mark.parametrize("manifest, args, expected_output", COLLIDING_NAMES) +def test_names_that_collide_get_distinct_identifiers(run, base, manifest, args, expected_output): + result = run(base + manifest, args) + assert result.exit_code == 0, result.output + assert result.output.strip() == expected_output + + +@pytest.mark.parametrize( + "name, hint", + [("my-var", "my_var"), ("2nd", ""), ("class", ""), ("cli", ""), ("typer", ""), ("has space", "")], +) +def test_var_name_must_be_a_python_identifier(generate, capsys, name, hint): + # a var becomes a module-level Python variable: `my-var = "x"` was a SyntaxError in the generated code + with pytest.raises(SystemExit): + generate(BASE + f"vars:\n {name}: x\ncommands:\n a: print(1)\n") + + error = capsys.readouterr() + assert f"vars.{name}" in error.out + error.err + assert hint in error.out + error.err + + @pytest.mark.parametrize("use_click", [False, True]) def test_keyword_param_name_is_rejected(generate, use_click): manifest = (CLICK if use_click else BASE) + "commands:\n a:\n params: [--from: str = 'x']\n run: print(1)\n" From cb29b3b8ad1f081a4682e71d9f89744e255825cd Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:08:05 -0400 Subject: [PATCH 04/22] Make cli validate compile and import the generated code A manifest passed validation even when the code generated from it was not valid Python or failed on import. `cli validate` now compiles the generated module and imports it (which defines the commands without running them) and reports a failure against the part of the manifest it came from, for example `commands.db.create: NameError: name 'N' is not defined`. Co-Authored-By: Claude Opus 5.5 --- cliffy/checker.py | 65 +++++++++++++++++++++++++++++++++ cliffy/cli.py | 10 +++-- tests/test_validate.py | 83 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 3 deletions(-) create mode 100644 cliffy/checker.py create mode 100644 tests/test_validate.py diff --git a/cliffy/checker.py b/cliffy/checker.py new file mode 100644 index 0000000..58523cb --- /dev/null +++ b/cliffy/checker.py @@ -0,0 +1,65 @@ +"""Checks the code generated for a manifest and reports problems in the manifest's terms.""" + +from __future__ import annotations + +import re +import traceback + +from cliffy.commander import CLI + +GENERATED_FILE = "" +FUNCTION_DEFINITION = re.compile(r"(?:async\s+)?def (\w+)\(") +MODULE_LEVEL = "the module level code (`imports`, `vars`, `types` or `functions`)" + + +class GeneratedCodeError(Exception): + """The code generated for a manifest does not compile or does not import. + + The message names the part of the manifest the failing code was generated from. + """ + + +def locate(cli: CLI, line_number: int) -> str: + """Part of the manifest that line `line_number` (1-based) of the generated code was generated from.""" + lines = cli.code.splitlines() + index = min(max(line_number, 1), len(lines)) - 1 + + # a decorator belongs to the function below it + while index < len(lines) - 1 and lines[index].startswith("@"): + index += 1 + + for line in reversed(lines[: index + 1]): + if match := FUNCTION_DEFINITION.match(line): + return cli.functions.get(match.group(1), MODULE_LEVEL) + return MODULE_LEVEL + + +def describe(cli: CLI, error: BaseException, line_number: int | None) -> str: + where = locate(cli, line_number) if line_number else MODULE_LEVEL + message = f"{where}: {type(error).__name__}: {error.msg if isinstance(error, SyntaxError) else error}" + lines = cli.code.splitlines() + if line_number and 0 < line_number <= len(lines): + message += f"\n {lines[line_number - 1].strip()}" + return message + + +def check_generated_code(cli: CLI) -> None: + """Compiles and imports the generated code. Importing defines the commands, it does not run them. + + Raises: + GeneratedCodeError: the code is not valid Python, or fails while it is imported. + """ + try: + # dont_inherit: this module's own `from __future__ import annotations` must not apply to the generated code + compiled = compile(cli.code, GENERATED_FILE, "exec", dont_inherit=True) + except SyntaxError as e: + raise GeneratedCodeError(describe(cli, e, e.lineno)) from e + + try: + exec(compiled, {"__name__": f"cliffy_check_{cli.name.replace('-', '_')}"}) + except (Exception, SystemExit) as e: + generated_frames = [ + frame for frame in traceback.extract_tb(e.__traceback__) if frame.filename == GENERATED_FILE + ] + line_number = generated_frames[-1].lineno if generated_frames else None + raise GeneratedCodeError(describe(cli, e, line_number)) from e diff --git a/cliffy/cli.py b/cliffy/cli.py index c22d3be..23b93fe 100644 --- a/cliffy/cli.py +++ b/cliffy/cli.py @@ -7,6 +7,7 @@ from cliffy.ai import ai from cliffy.blueprint import BlueprintError, installed_blueprints, resolve_blueprint from cliffy.builder import build_cli, build_cli_from_manifest, run_cli +from cliffy.checker import GeneratedCodeError, check_generated_code from cliffy.doc import DocGenerator from cliffy.helper import ( CLI_NAME_PATTERN, @@ -357,12 +358,15 @@ def test(manifest: str, exitfirst: bool, blueprint: str | None = None) -> None: @click.argument("manifest", type=click.File("rb"), required=True) @blueprint_option def validate(manifest: TextIO, blueprint: str | None = None) -> None: - """Validate the syntax and structure of a CLI manifest""" + """Validate a CLI manifest: its structure, its requirements and the code generated from it""" try: - Transformer(manifest, blueprint=blueprint) - out(f"Manifest {manifest.name} is valid", fg="green") + T = Transformer(manifest, blueprint=blueprint) + check_generated_code(T.cli) + except GeneratedCodeError as e: + exit_err(f"Manifest {manifest.name} is invalid: the generated code does not work\n {e}") except Exception as e: exit_err(f"Manifest {manifest.name} is invalid: {e}") + out(f"Manifest {manifest.name} is valid", fg="green") @click.argument("cli_or_manifest", type=ManifestOrCLI()) diff --git a/tests/test_validate.py b/tests/test_validate.py new file mode 100644 index 0000000..883a42d --- /dev/null +++ b/tests/test_validate.py @@ -0,0 +1,83 @@ +"""`cli validate` checks the code that is generated, not only the YAML.""" + +import pytest +from click.testing import CliRunner + +from cliffy.cli import validate_command + +HEADER = "name: checked\nversion: '1'\n" + + +def validate(tmp_path, body: str, header: str = HEADER): + manifest_path = tmp_path / "checked.yaml" + manifest_path.write_text(header + body) + return CliRunner().invoke(validate_command, [str(manifest_path)]) + + +@pytest.mark.parametrize("backend", ["", "use_click: true\n"], ids=["typer", "click"]) +def test_validate_accepts_working_manifest(tmp_path, backend): + result = validate(tmp_path, backend + "commands:\n hello:\n params: [name: str!]\n run: print(name)\n") + + assert result.exit_code == 0, result.output + assert "is valid" in result.output + + +@pytest.mark.parametrize( + "backend, code", + [ + ("", "x = = 1"), + ("use_click: true\n", "x = = 1"), + # pybash returns nothing for code it cannot tokenize, which leaves a function without a body. + # (With Click the docstring is a body, so there the command silently does nothing.) + ("", "print(("), + ], + ids=["typer", "click", "typer_unclosed_bracket"], +) +def test_validate_reports_syntax_error_against_the_command(tmp_path, backend, code): + result = validate(tmp_path, backend + f"commands:\n ok: print(1)\n db.broken: '{code}'\n after: print(2)\n") + + assert result.exit_code == 1 + assert "is invalid" in result.output + assert "commands.db.broken: " in result.output + assert "commands.after" not in result.output and "commands.ok" not in result.output + assert "Error: " in result.output + assert "Traceback" not in result.output + + +def test_validate_reports_params_that_do_not_compile(tmp_path): + # the error the commented `cli init` template used to have + params = " params:\n - 'loud: bool = False'\n - 'name: str'\n" + result = validate(tmp_path, "commands:\n hello:\n" + params + " run: print(name)\n") + + assert result.exit_code == 1 + assert "commands.hello" in result.output + assert "default" in result.output + + +@pytest.mark.parametrize("backend", ["", "use_click: true\n"], ids=["typer", "click"]) +def test_validate_reports_code_that_does_not_import(tmp_path, backend): + result = validate( + tmp_path, + backend + "commands:\n first: print(1)\n count:\n params:\n - --n: NoSuchType\n run: print(n)\n", + ) + + assert result.exit_code == 1 + assert "commands.count" in result.output + assert "NoSuchType" in result.output + assert "Traceback" not in result.output + + +def test_validate_reports_broken_imports_section(tmp_path): + result = validate(tmp_path, "imports: import no_such_module_for_cliffy\ncommands:\n a: print(1)\n") + + assert result.exit_code == 1 + assert "imports" in result.output + assert "no_such_module_for_cliffy" in result.output + + +def test_validate_does_not_run_commands(tmp_path): + marker = tmp_path / "ran" + result = validate(tmp_path, f"commands:\n touch: open({str(marker)!r}, 'w').close()\n") + + assert result.exit_code == 0, result.output + assert not marker.exists() From bfccdac33133e7dc774c12bb907fd881d6b62bb0 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:08:14 -0400 Subject: [PATCH 05/22] Print cliffy's own version for cli --version With the rich extra installed, the version lookup of `click.version_option()` ran inside rich-click and reported rich-click's version. Co-Authored-By: Claude Opus 5.5 --- cliffy/cli.py | 4 +++- cliffy/helper.py | 10 +++++++++- tests/test_cli.py | 3 +++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/cliffy/cli.py b/cliffy/cli.py index 23b93fe..89e749b 100644 --- a/cliffy/cli.py +++ b/cliffy/cli.py @@ -13,6 +13,7 @@ CLI_NAME_PATTERN, ManifestOrCLI, age_datetime, + cliffy_version, exit_err, indent_block, out, @@ -58,7 +59,8 @@ def show_aliases_callback(ctx: Any, param: Any, val: bool) -> None: @click.group(context_settings=CONTEXT_SETTINGS) -@click.version_option() +# the version is passed in: with rich-click installed, the automatic lookup finds rich-click's own version +@click.version_option(cliffy_version(), package_name="cliffy") @click.option("--aliases", type=bool, is_flag=True, is_eager=True, callback=show_aliases_callback) def cli(aliases: bool) -> None: pass diff --git a/cliffy/helper.py b/cliffy/helper.py index f850b15..694c2eb 100644 --- a/cliffy/helper.py +++ b/cliffy/helper.py @@ -7,7 +7,7 @@ import re import sys from datetime import datetime -from importlib.metadata import distributions +from importlib.metadata import PackageNotFoundError, distributions, version from importlib.resources import files from importlib.util import module_from_spec, spec_from_file_location from pathlib import Path @@ -143,6 +143,14 @@ def to_identifier(name: str) -> str: return identifier +def cliffy_version() -> str: + """Version of the installed cliffy package""" + try: + return version("cliffy") + except PackageNotFoundError: # pragma: no cover - cliffy is run from a source tree that is not installed + return "unknown" + + def get_installed_package_versions() -> dict[str, str]: """Installed distributions of the running interpreter, keyed by normalized (PEP 503) name.""" installed_packages: dict[str, str] = {} diff --git a/tests/test_cli.py b/tests/test_cli.py index 7ea8ca5..bf56235 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,6 +3,7 @@ import subprocess import sys from datetime import datetime +from importlib import metadata from pathlib import Path from unittest.mock import MagicMock, patch @@ -46,6 +47,8 @@ def test_cli_version(): runner = CliRunner() result = runner.invoke(cli, ["--version"]) # type: ignore assert result.exit_code == 0 + # with the rich extra installed this used to print the version of rich-click + assert result.output.strip() == f"cli, version {metadata.version('cliffy')}" def test_cli_init(): From 80033453532d7b72684c3007c8f3130002c3cb5e Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:09:13 -0400 Subject: [PATCH 06/22] Reject unsupported manifestVersion values The key was removed from the manifest before validation, so the version check never ran and `manifestVersion: v1` loaded as if it was v3. The version is now checked for every file the transformer reads (manifests, includes and blueprint specs) and the error says what to change. examples/town.yaml and pydev.yaml declared v1 and now declare v3. Co-Authored-By: Claude Opus 5.5 --- cliffy/manifest.py | 33 ++++++++++++++------ cliffy/transformer.py | 5 ++- examples/pydev.yaml | 2 +- examples/town.yaml | 2 +- tests/test_transformer.py | 65 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 13 deletions(-) diff --git a/cliffy/manifest.py b/cliffy/manifest.py index 0cba144..e4adf8d 100644 --- a/cliffy/manifest.py +++ b/cliffy/manifest.py @@ -11,6 +11,27 @@ from .helper import CLI_NAME_PATTERN, GENERATED_NAMES, wrap_as_comment LATEST_SCHEMA_VERSION = "v3" +MIGRATION_GUIDE = "https://jaykv.github.io/cliffy/project/migration/" +# schema version -> the cliffy release that stopped reading it +DEPRECATED_SCHEMA_VERSIONS = {"v1": "0.4.0", "v2": "0.5.0"} + + +def manifest_version_problem(value: object) -> str | None: + """Why a `manifestVersion` value cannot be used and what to change, or None when it is the current version.""" + version = str(value).strip() + if version == LATEST_SCHEMA_VERSION: + return None + + use_latest = f"set `manifestVersion: {LATEST_SCHEMA_VERSION}` or remove the line" + if version in DEPRECATED_SCHEMA_VERSIONS: + return ( + f"The {version} schema is deprecated since cliffy {DEPRECATED_SCHEMA_VERSIONS[version]} and is no longer " + f"read. Update the manifest to the {LATEST_SCHEMA_VERSION} schema ({MIGRATION_GUIDE}), then {use_latest}." + ) + return ( + f"Unrecognized manifest version `{version}`. This version of cliffy reads `{LATEST_SCHEMA_VERSION}` " + f"manifests: {use_latest}." + ) class Example(BaseModel): @@ -386,16 +407,8 @@ def var_names_are_identifiers(cls, value: dict[str, Any]) -> dict[str, Any]: @field_validator("manifestVersion", mode="after") @classmethod def is_v3(cls, value: str) -> str: - if value.strip() == "v1": - raise ValueError( - "v1 schema is deprecated with cliffy >= 0.4.0. Please upgrade the manifest to the v3 schema." - ) - if value.strip() == "v2": - raise ValueError( - "v2 schema is deprecated with cliffy >= 0.5.0. Please upgrade the manifest to the v3 schema." - ) - if value.strip() != "v3": - raise ValueError(f"Unrecognized manifest version {value}. Latest is {LATEST_SCHEMA_VERSION}.") + if problem := manifest_version_problem(value): + raise ValueError(problem) return value @model_validator(mode="after") diff --git a/cliffy/transformer.py b/cliffy/transformer.py index 9b1ebf2..6506c5e 100644 --- a/cliffy/transformer.py +++ b/cliffy/transformer.py @@ -17,7 +17,7 @@ from cliffy.commanders.click import ClickCommander from cliffy.commanders.typer import TyperCommander from cliffy.helper import check_requirement, exit_err, get_installed_package_versions, out -from cliffy.manifest import LATEST_SCHEMA_VERSION, CLIManifest, IncludeManifest +from cliffy.manifest import LATEST_SCHEMA_VERSION, CLIManifest, IncludeManifest, manifest_version_problem from cliffy.merger import cliffy_merger @@ -51,7 +51,10 @@ def __init__( # manifests that are being resolved right now, outermost first: used to detect include cycles self.include_chain = (*include_chain, self.manifest_path) self.command_config = self.load_manifest(manifest_io) + # checked here, for every kind of file: an include and a blueprint spec have no model field for it self.manifest_version = self.command_config.pop("manifestVersion", LATEST_SCHEMA_VERSION) + if problem := manifest_version_problem(self.manifest_version): + exit_err(f"~ error validating {manifest_io.name}: {problem}") self.blueprint: Blueprint | None = None self.blueprint_ref = None if as_include else self.find_blueprint_reference(blueprint) if not self.blueprint_ref and self.command_config.get("includes"): diff --git a/examples/pydev.yaml b/examples/pydev.yaml index 25d0adc..45e1741 100644 --- a/examples/pydev.yaml +++ b/examples/pydev.yaml @@ -1,6 +1,6 @@ # yaml-language-server: $schema=cliffy_schema.json -manifestVersion: v1 # optional (Default will be the latest) +manifestVersion: v3 # optional (default is the latest) name: pydev version: 0.1.0 help: Python developer utilities diff --git a/examples/town.yaml b/examples/town.yaml index 4c5e5a2..2e948e5 100644 --- a/examples/town.yaml +++ b/examples/town.yaml @@ -1,6 +1,6 @@ # yaml-language-server: $schema=cliffy_schema.json -manifestVersion: v1 # optional (Default will be the latest) +manifestVersion: v3 # optional (default is the latest) name: town version: 0.1.1 diff --git a/tests/test_transformer.py b/tests/test_transformer.py index 919128c..6906f9f 100644 --- a/tests/test_transformer.py +++ b/tests/test_transformer.py @@ -1,4 +1,6 @@ +import glob import os +import re import tempfile import pytest @@ -257,3 +259,66 @@ def test_requires_unmet(tmp_path, capsys, requirement, expected_error): error = capsys.readouterr().err assert f"requires `{requirement}` to be installed\n {expected_error}" in error + + +MANIFEST_BODY = "name: t\nversion: '1'\ncommands:\n a: print(1)\n" +SPEC_BLUEPRINT = ( + "from cliffy.blueprint import Blueprint\n\n\n" + "class Spec(Blueprint):\n" + " name: str\n\n" + " def to_manifest(self):\n" + " return {'name': self.name, 'version': '1', 'commands': {'a': 'print(1)'}}\n" +) + + +@pytest.mark.parametrize( + "version, expected_error", + [ + ("v1", "v1 schema is deprecated"), + ("v2", "v2 schema is deprecated"), + ("v4", "Unrecognized manifest version `v4`"), + ("3", "Unrecognized manifest version `3`"), + ], +) +@pytest.mark.parametrize("where", ["manifest", "include", "blueprint_spec"]) +def test_unsupported_manifest_version_is_an_error(tmp_path, capsys, where, version, expected_error): + # the key was removed before validation, so `manifestVersion: v1` loaded as if it was v3 + manifest_path = tmp_path / "main.yaml" + if where == "manifest": + manifest_path.write_text(f"manifestVersion: {version}\n" + MANIFEST_BODY) + elif where == "include": + (tmp_path / "shared.yaml").write_text(f"manifestVersion: {version}\ncommands:\n b: print(2)\n") + manifest_path.write_text("includes: [shared.yaml]\n" + MANIFEST_BODY) + else: + (tmp_path / "spec.py").write_text(SPEC_BLUEPRINT) + manifest_path.write_text(f"blueprint: spec.py\nmanifestVersion: {version}\nname: t\n") + + with pytest.raises(SystemExit): + load(manifest_path) + + error = capsys.readouterr().err + assert expected_error in error + assert "manifestVersion: v3" in error + assert ("shared.yaml" if where == "include" else "main.yaml") in error + + +@pytest.mark.parametrize("where", ["manifest", "include", "blueprint_spec"]) +def test_current_manifest_version_is_accepted(tmp_path, where): + manifest_path = tmp_path / "main.yaml" + if where == "manifest": + manifest_path.write_text("manifestVersion: v3\n" + MANIFEST_BODY) + elif where == "include": + (tmp_path / "shared.yaml").write_text("manifestVersion: v3\ncommands:\n b: print(2)\n") + manifest_path.write_text("includes: [shared.yaml]\n" + MANIFEST_BODY) + else: + (tmp_path / "spec.py").write_text(SPEC_BLUEPRINT) + manifest_path.write_text("blueprint: spec.py\nmanifestVersion: v3\nname: t\n") + + assert load(manifest_path).cli.name == "t" + + +def test_example_manifests_use_the_current_version(): + for path in sorted(glob.glob("examples/*.yaml")): + with open(path, encoding="utf-8") as manifest_file: + declared = re.findall(r"^manifestVersion:\s*(\S+)", manifest_file.read(), re.MULTILINE) + assert declared in ([], ["v3"]), path From a16e3af3d811b7de1f6f8524c7b1751e9d5dc5eb Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:09:21 -0400 Subject: [PATCH 07/22] Count only test cases in the cli test progress counter Shell steps advanced the counter too, so it could print "10 of 9". The message for a shell step without a prefix also described `$` and `>` the wrong way round: `$` runs the command directly, `>` runs it through a shell. Co-Authored-By: Claude Opus 5.5 --- cliffy/cli.py | 8 ++++++-- cliffy/tester.py | 6 +++--- tests/test_tester.py | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/cliffy/cli.py b/cliffy/cli.py index 89e749b..d599b7f 100644 --- a/cliffy/cli.py +++ b/cliffy/cli.py @@ -313,11 +313,15 @@ def test(manifest: str, exitfirst: bool, blueprint: str | None = None) -> None: out("āœØ Invoking tests āœØ", nl=False) total = tester.total_cases passed = 0 - for i, case in enumerate(tester.test_pipeline): + case_number = 0 + for case in tester.test_pipeline: if isinstance(case, ShellScript): + # a setup or cleanup step: it is run, but it is not one of the counted test cases tester.invoke_shell(case) continue + case_number += 1 + out(f"\n\nšŸŖ„ > {tester.T.cli.name} {case.command}\n") try: test = tester.invoke_test(case.command, case.assert_script) @@ -326,7 +330,7 @@ def test(manifest: str, exitfirst: bool, blueprint: str | None = None) -> None: if result.exception: out(str(result)) next(test, "") - out(f"\nāœ… {i + 1} of {total}") + out(f"\nāœ… {case_number} of {total}") passed += 1 except AssertionError: _, _, tb = sys.exc_info() diff --git a/cliffy/tester.py b/cliffy/tester.py index ecdffb2..ea01db0 100644 --- a/cliffy/tester.py +++ b/cliffy/tester.py @@ -30,9 +30,9 @@ def is_shelled(cls, value: str) -> str: return value raise ValueError( - "Shell script for test case must start with > or $. " - "Use > to indicate a safe subprocess call. " - "Use $ to execute the command through the shell. WARNING: this can be dangerous." + "A shell step in tests must start with $ or >. " + "Use $ to run the command directly, without a shell. " + "Use > to run it through a shell (pipes into shell builtins, globs). WARNING: this can be dangerous." ) diff --git a/tests/test_tester.py b/tests/test_tester.py index a6be277..2ce2d7a 100644 --- a/tests/test_tester.py +++ b/tests/test_tester.py @@ -1,11 +1,14 @@ import glob +import re import tempfile import pytest from click.testing import CliRunner +from pybash.transformer import transform as transform_bash from cliffy.cli import test_command as manifest_test_command from cliffy.helper import TEMP_FILES +from cliffy.tester import ShellScript from cliffy.tester import Tester as ManifestTester BROKEN_MANIFEST = "name: broken\nversion: '1'\nimports: import no_such_module_for_cliffy\ncommands:\n a: print(1)\n" @@ -75,3 +78,40 @@ def test_manifest_tests_exitfirst_exit_code(tmp_path): assert "AssertionError" in result.output assert "1 of 2" not in result.output and "2 of 2" not in result.output assert result.exit_code == 1 + + +def test_progress_counter_only_counts_test_cases(tmp_path): + # shell steps were numbered too, so two cases after a shell step were reported as "2 of 2" and "3 of 2" + manifest_path = write_manifest( + tmp_path, + " - $ echo setup\n - hello: assert 'hi' in result.output\n - $ echo between\n" + " - hello: assert 'hi' in result.output\n", + ) + + result = CliRunner().invoke(manifest_test_command, [manifest_path]) + + assert result.exit_code == 0 + assert re.findall(r"\d+ of \d+", result.output) == ["1 of 2", "2 of 2"] + + +def test_progress_counter_numbers_failed_cases_too(tmp_path): + manifest_path = write_manifest( + tmp_path, " - hello: assert 'bye' in result.output\n - hello: assert 'hi' in result.output\n" + ) + + result = CliRunner().invoke(manifest_test_command, [manifest_path]) + + assert result.exit_code == 1 + assert re.findall(r"\d+ of \d+", result.output) == ["2 of 2"] + + +def test_shell_step_error_describes_the_prefixes_correctly(): + # `$` is the direct subprocess call and `>` goes through a shell: the message had them the other way round + with pytest.raises(ValueError) as error: + ShellScript(command="echo hi") + + message = str(error.value) + assert re.search(r"Use \$ to run the command directly", message) + assert re.search(r"Use > to run it through a shell", message) + assert "subprocess.run([" in transform_bash("$ echo hi") + assert "shell=True" in transform_bash("> echo hi") From d3eb364731e303dce28f2b3f7369c017baca7bc0 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:10:02 -0400 Subject: [PATCH 08/22] Turn line breaks in command and group help into spaces Newlines were removed from help text without a replacement, so "Greet\nuser" became "Greetuser". A line break is now a space and a blank line stays a paragraph break, for commands and groups in both backends. Co-Authored-By: Claude Opus 5.5 --- cliffy/commanders/click.py | 8 ++++---- cliffy/commanders/typer.py | 10 +++++----- cliffy/helper.py | 10 ++++++++++ tests/test_codegen.py | 32 ++++++++++++++++++++++++++++++++ tests/test_manifest.py | 5 +++-- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/cliffy/commanders/click.py b/cliffy/commanders/click.py index 4f27e2e..2757e1a 100644 --- a/cliffy/commanders/click.py +++ b/cliffy/commanders/click.py @@ -3,7 +3,7 @@ import re from cliffy.commander import BaseGroup, Commander -from cliffy.helper import py_str +from cliffy.helper import clean_help, py_str from cliffy.manifest import ( Callback, CLIManifest, @@ -92,7 +92,7 @@ def parse_callback_run(self, callbacks: list[Callback]) -> str: def define_groups(self) -> None: for group in self.groups.values(): - parsed_help = group.help.replace("\n", "") if group.help else "" + parsed_help = clean_help(group.help) callbacks = self.callbacks_for(group) self.functions[group.var_name] = f"callbacks (group {group.name})" self.cli += f""" @@ -113,7 +113,7 @@ def add_root_command(self, command: Command) -> None: parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.click_parser.get_parsed_command_name(command) - parsed_help = command.help.replace("\n", "") if command.help else "" + parsed_help = clean_help(command.help) self.cli += f""" @cli.command(name={py_str(parsed_command_name)}) @@ -135,7 +135,7 @@ def {self.alias_func_name(command, alias)}({self.click_parser.get_param_names(co def add_sub_command(self, command: Command, group: BaseGroup) -> None: parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.click_parser.get_parsed_command_name(command) - parsed_help = command.help.replace("\n", "") if command.help else "" + parsed_help = clean_help(command.help) self.cli += f""" @{group.var_name}.command(name={py_str(parsed_command_name)}) diff --git a/cliffy/commanders/typer.py b/cliffy/commanders/typer.py index b95cd09..14ce654 100644 --- a/cliffy/commanders/typer.py +++ b/cliffy/commanders/typer.py @@ -1,7 +1,7 @@ import datetime from cliffy.commander import BaseGroup, Commander -from cliffy.helper import py_str +from cliffy.helper import clean_help, py_str from cliffy.manifest import Callback, CLIManifest, Command @@ -93,7 +93,7 @@ def add_root_command(self, command: Command) -> None: parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.parser.get_parsed_command_name(command) parsed_command_config = self.parser.get_parsed_config(command) - parsed_help = command.help.replace("\n", "") if command.help else "" + parsed_help = clean_help(command.help) empty_or_help = f"help={py_str(parsed_help)}," if parsed_help else "" command_args = f"{py_str(parsed_command_name)}, {empty_or_help}{parsed_command_config}" alias_epilog = py_str(f"Alias for {parsed_command_name}") @@ -112,7 +112,7 @@ def {parsed_command_func_name}({self.parser.parse_params(command)}): def define_groups(self) -> None: for group in self.groups.values(): - parsed_help = group.help.replace("\n", "") if group.help else "" + parsed_help = clean_help(group.help) empty_or_help = f"help={py_str(parsed_help)}," if parsed_help else "" self.cli += f"""{group.var_name} = typer.Typer({empty_or_help}) """ @@ -128,7 +128,7 @@ def {callback_name}({self.parse_callback_params(callbacks)} def add_group(self, group: BaseGroup) -> None: """Add a group to the CLI with proper nesting""" parent_group = group.parent_group.var_name if group.parent_group else "cli" - parsed_help = group.help.replace("\n", "") if group.help else "" + parsed_help = clean_help(group.help) group_args = f"name={py_str(group.short_name)}, help={py_str(parsed_help)}" self.cli += f"""{parent_group}.add_typer({group.var_name}, {group_args}) """ @@ -142,7 +142,7 @@ def add_sub_command(self, command: Command, group: BaseGroup) -> None: parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.parser.get_parsed_command_name(command) parsed_command_config = self.parser.get_parsed_config(command) - parsed_help = command.help.replace("\n", "") if command.help else "" + parsed_help = clean_help(command.help) empty_or_help = f"help={py_str(parsed_help)}," if parsed_help else "" command_args = f"{py_str(parsed_command_name)}, {empty_or_help}{parsed_command_config}" alias_epilog = py_str(f"Alias for {parsed_command_name}") diff --git a/cliffy/helper.py b/cliffy/helper.py index 694c2eb..32a160a 100644 --- a/cliffy/helper.py +++ b/cliffy/helper.py @@ -128,6 +128,16 @@ def py_str(value: object) -> str: return json.dumps(str(value), ensure_ascii=False) +def clean_help(text: str) -> str: + """Help text as one line per paragraph: a line break becomes a space, a blank line stays a paragraph break. + + Typer and Click reflow the lines of a paragraph and keep paragraphs apart, and both use the first + paragraph as the summary of a command in the command list. + """ + paragraphs = (" ".join(paragraph.split()) for paragraph in re.split(r"\n\s*\n", text or "")) + return "\n\n".join(paragraph for paragraph in paragraphs if paragraph) + + def to_identifier(name: str) -> str: """Turns name into a Python identifier that does not hide a keyword, a builtin or a generated name. diff --git a/tests/test_codegen.py b/tests/test_codegen.py index 32d3018..ac65787 100644 --- a/tests/test_codegen.py +++ b/tests/test_codegen.py @@ -240,6 +240,38 @@ def test_names_become_valid_identifiers(run, manifest, args, expected_output): assert expected_output in result.output +MULTILINE_HELP = "Greet\\nthe user.\\n\\nSecond\\n paragraph." + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +@pytest.mark.parametrize( + "commands, args", + [ + (f' a:\n help: "{MULTILINE_HELP}"\n run: print(1)\n', ["a", "-h"]), + (f' g:\n help: "{MULTILINE_HELP}"\n g.a: print(1)\n', ["g", "-h"]), + (f' g.a:\n help: "{MULTILINE_HELP}"\n run: print(1)\n', ["g", "a", "-h"]), + ], + ids=["command", "group", "subcommand"], +) +def test_help_line_breaks_become_spaces_and_paragraphs_are_kept(run, base, commands, args): + result = run(base + "commands:\n" + commands, args) + + assert result.exit_code == 0, result.output + lines = [line.strip(" ā”‚") for line in result.output.splitlines()] + assert "Greet the user." in lines + assert "Second paragraph." in lines + # each paragraph is rendered on its own (Typer puts a blank line in between, rich-click does not) + assert lines.index("Second paragraph.") > lines.index("Greet the user.") + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +def test_help_overview_shows_the_first_paragraph(run, base): + result = run(base + f'commands:\n a:\n help: "{MULTILINE_HELP}"\n run: print(1)\n', ["-h"]) + + assert "Greet the user." in result.output + assert "Greetthe" not in result.output + + COLLIDING_NAMES = [ # a root command and a group that both used to be the Python name `db_app` pytest.param("commands:\n db-app: print(1)\n db.x: print(2)\n", ["db-app"], "1", id="command_vs_group_var"), diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 413b196..b5bc38b 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -275,7 +275,7 @@ def test_command_config_partial(): assert "rich_help_panel=" not in cmdr.cli # Other options should not be present -def test_command_config_help_newline_removed(): +def test_command_help_newline_becomes_a_space(): manifest = CLIManifest( name="test", version="0.1.0", @@ -292,7 +292,8 @@ def test_command_config_help_newline_removed(): cmdr = TyperCommander(manifest=manifest) cmdr.generate_cli() - assert 'help="Test commandwith newline",' in cmdr.cli + # the newline used to be removed, which glued the words together: "Test commandwith newline" + assert 'help="Test command with newline",' in cmdr.cli def test_command_pre_run_post_run(): From 7ea3e454dbb91477345ed91506366986b181549b Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:13:28 -0400 Subject: [PATCH 09/22] Bring the Click backend to parity with Typer for params, types and config With `use_click: true`: - an explicit param whose name has no `--` is a positional argument (it used to become an option), and argument help is passed on - a shorthand argument without `!` is optional, as it is for Typer - `types` are resolved: a plain type (`N: int`, `N: int = 5`) or a complete Click decorator. A Typer definition, a Typer signature string or an inline `typer.Option(...)` is rejected with an error that names the command - `Optional[X]`, `X | None` and `list[X]` are translated to Click types - command and template `config` is passed to `command()`; `rich_help_panel` becomes rich-click's `panel` - a help-only subcommand no longer generates an empty function For both backends a plain type in `types` is now an alias: the param keeps the kind its name gives it (`--n` is an option) and its own default and help. The Typer backend used to replace the whole definition, which turned `--n: N` into a positional argument and dropped the help of an explicit param. Code generation errors raised for a command are prefixed with `commands.`. Co-Authored-By: Claude Opus 5.5 --- cliffy/commander.py | 18 ++- cliffy/commanders/click.py | 163 ++++++++++++++------- cliffy/parser.py | 15 +- docs/snippets/click.yaml | 11 +- tests/test_cli.py | 2 +- tests/test_click_commander.py | 5 +- tests/test_click_parity.py | 268 ++++++++++++++++++++++++++++++++++ 7 files changed, 419 insertions(+), 63 deletions(-) create mode 100644 tests/test_click_parity.py diff --git a/cliffy/commander.py b/cliffy/commander.py index 3bf1172..bd38f8b 100644 --- a/cliffy/commander.py +++ b/cliffy/commander.py @@ -4,6 +4,7 @@ from abc import ABC, abstractmethod from collections import defaultdict from collections.abc import ItemsView, Iterator, ValuesView +from contextlib import contextmanager from pybash.transformer import transform as transform_bash from pydantic import BaseModel @@ -261,7 +262,16 @@ def add_subcommands(self) -> None: for group in self.groups.values(): self.add_group(group) for subcommand in group.commands: - self.add_sub_command(subcommand, group) + with self.reported_against(subcommand): + self.add_sub_command(subcommand, group) + + @contextmanager + def reported_against(self, command: Command) -> Iterator[None]: + """Names the command in the manifest that a code generation error belongs to""" + try: + yield + except ValueError as e: + raise ValueError(f"commands.{command.name}: {e}") from e def generate_cli(self) -> None: self.add_base_imports() @@ -277,7 +287,8 @@ def generate_cli(self) -> None: def add_root_commands(self) -> None: for root_command in self.root_group.commands: - self.add_root_command(root_command) + with self.reported_against(root_command): + self.add_root_command(root_command) def add_imports(self) -> None: if not self.manifest.imports: @@ -315,7 +326,8 @@ def add_functions(self) -> None: def add_command(self, command: Command) -> None: if "." in command.name: group = command.name.rsplit(".", 1)[0] - self.add_sub_command(command, self.groups[group]) + with self.reported_against(command): + self.add_sub_command(command, self.groups[group]) def add_greedy_commands(self) -> None: """Greedy commands get lazy-loaded. Only supported for group-commands currently""" diff --git a/cliffy/commanders/click.py b/cliffy/commanders/click.py index 2757e1a..343e2e0 100644 --- a/cliffy/commanders/click.py +++ b/cliffy/commanders/click.py @@ -116,7 +116,7 @@ def add_root_command(self, command: Command) -> None: parsed_help = clean_help(command.help) self.cli += f""" -@cli.command(name={py_str(parsed_command_name)}) +@cli.command(name={py_str(parsed_command_name)}{self.click_parser.get_parsed_config(command)}) {self.click_parser.parse_params(command)} def {parsed_command_func_name}({self.click_parser.get_param_names(command)}): {py_str(parsed_help)} @@ -133,12 +133,16 @@ def {self.alias_func_name(command, alias)}({self.click_parser.get_param_names(co """ def add_sub_command(self, command: Command, group: BaseGroup) -> None: + if not command.run: + # a script-less command only carries the help of the group with the same name + return + parsed_command_func_name = self.command_func_name(command) parsed_command_name = self.click_parser.get_parsed_command_name(command) parsed_help = clean_help(command.help) self.cli += f""" -@{group.var_name}.command(name={py_str(parsed_command_name)}) +@{group.var_name}.command(name={py_str(parsed_command_name)}{self.click_parser.get_parsed_config(command)}) {self.click_parser.parse_params(command)} def {parsed_command_func_name}({self.click_parser.get_param_names(command)}): {py_str(parsed_help)} @@ -202,20 +206,106 @@ def parse_params(self, command: Command) -> str: def parse_param_list(self, params: list[ParamBlock]) -> str: """Parse a list of parameters into Click decorators""" - decorators = [] - for param in params: - if isinstance(param, GenericCommandParam): - decorators.append(param.root.strip()) - elif isinstance(param, CommandParam): - decorators.append(self._build_click_option(param)) - elif isinstance(param, SimpleCommandParam): - decorators.append(self._parse_simple_param(param)) + return "\n".join(self.parse_param(param) for param in params) + + def parse_param(self, param: ParamBlock) -> str: # type: ignore[override] + """Parse one param into a Click decorator""" + if isinstance(param, GenericCommandParam): + decorator = param.root.strip() + if not decorator.startswith("@"): + raise ValueError( + f"param `{decorator}` is a function signature entry, which only the Typer backend can use. " + "With `use_click: true` a string param has to be a Click decorator, " + "i.e. \"@click.option('--name', ...)\"." + ) + return decorator + + if isinstance(param, SimpleCommandParam): + if "typer." in param.raw_type: + raise ValueError( + f"param `{param.name}` is a Typer definition (`{param.raw_type.strip()}`), which the Click " + "backend cannot use. Write it as a Click decorator string, " + "i.e. \"@click.option('--name', ...)\", or remove `use_click`." + ) + default, required, help_text = param.default, param.required, "" + else: + default, required, help_text = param.default, param.required, param.help + if default is not None: + default = str(default) + + param_type = param.type.strip() + if param_type in self.manifest.types: + definition = self.manifest.types[param_type].strip() + if definition.startswith("@click."): + # a complete Click decorator: used as written, like a Typer definition is for the Typer backend + return definition + if "typer." in definition: + raise ValueError( + f"param `{param.name}` uses types.{param_type}, which is a Typer definition (`{definition}`). " + f"With `use_click: true` a type is a plain type such as `{param_type}: int`, or a Click decorator " + f"such as `{param_type}: \"@click.option('--name', ...)\"`." + ) + # `N: int = 5` names a type and a default. A default on the param itself wins. + param_type, _, type_default = (part.strip() for part in definition.partition("=")) + if default is None and type_default: + default = type_default + + click_type, multiple = self._click_type(param_type) + if default is not None: + if isinstance(param, CommandParam): + # a str default of an explicit param is data, not an expression: emit it as a string literal + default = repr(default) if click_type == "str" else default + else: + default = self._simple_default(click_type, default) + + self.normalize_param_name(param.name) # validates + if param.is_option(): + parts = self._option_declarations(param.name, param.short, click_type) + if click_type and click_type != "bool": + parts.append(f"type={click_type}") + if multiple: + parts.append("multiple=True") + if required: + parts.append("required=True") + if default is not None: + parts.append(f"default={default}") + if help_text: + parts.append(f"help={help_text!r}") + return f"@click.option({', '.join(parts)})" + + parts = [repr(param.name.strip())] + if click_type: + parts.append(f"type={click_type}") + if multiple: + parts.append("nargs=-1") + if required: + parts.append("required=True") + elif not required and default is None: + # an argument is optional unless it is marked with `!` or `required`, as in Typer + parts.append("required=False") + if default is not None: + parts.append(f"default={default}") + if help_text: + parts.append(f"help={help_text!r}") + return f"@click.argument({', '.join(parts)})" - return "\n".join(decorators) + OPTIONAL_TYPE = re.compile(r"(?:typing\.)?Optional\[(.+)\]|(.+?)\s*\|\s*None|None\s*\|\s*(.+)") + LIST_TYPE = re.compile(r"(?:typing\.)?[Ll]ist\[(.+)\]") + + def _click_type(self, param_type: str) -> tuple[str, bool]: + """Annotation -> (Click type, takes several values). Optional[int] -> int, list[str] -> str, several.""" + click_type = param_type.strip() + if match := self.OPTIONAL_TYPE.fullmatch(click_type): + click_type = next(group for group in match.groups() if group).strip() + multiple = False + if match := self.LIST_TYPE.fullmatch(click_type): + click_type, multiple = match.group(1).strip(), True + if click_type in ("Any", "typing.Any"): + click_type = "" + return click_type, multiple def _option_declarations(self, name: str, short: str | None, param_type: str) -> list[str]: """--dry-run with short d -> ['--dry-run', '-d']. A bool is a `--flag/--no-flag` switch, like in Typer.""" - self.normalize_param_name(name) # validates long_name = name.strip().lstrip("-") declaration = f"--{long_name}/--no-{long_name}" if param_type == "bool" else f"--{long_name}" declarations = [repr(declaration)] @@ -225,28 +315,7 @@ def _option_declarations(self, name: str, short: str | None, param_type: str) -> def _parse_simple_param(self, param: SimpleCommandParam) -> str: """Parse SimpleCommandParam into Click decorator""" - param_type = param.type - default_value = param.default - - if param.is_option(): - parts = self._option_declarations(param.name, param.short, param_type) - if param_type and param_type != "bool": - parts.append(f"type={param_type}") - if param.required: - parts.append("required=True") - if default_value is not None: - parts.append(f"default={self._simple_default(param_type, default_value)}") - - return f"@click.option({', '.join(parts)})" - - self.normalize_param_name(param.name) # validates - parts = [repr(param.name.strip())] - if param_type: - parts.append(f"type={param_type}") - if default_value is not None: - parts.append(f"default={self._simple_default(param_type, default_value)}") - - return f"@click.argument({', '.join(parts)})" + return self.parse_param(param) @staticmethod def _simple_default(param_type: str, default_value: str) -> str: @@ -260,24 +329,16 @@ def _simple_default(param_type: str, default_value: str) -> str: return repr(default_value) return repr(literal) if isinstance(literal, str) else repr(default_value) - def _build_click_option(self, param: CommandParam) -> str: - """Build Click option decorator from CommandParam""" - parts = self._option_declarations(param.name, param.short, param.type) - - if param.type and param.type != "bool": - parts.append(f"type={param.type}") - - if param.required: - parts.append("required=True") - - if param.default is not None: - # a str default is data, not an expression: emit it as a string literal - parts.append(f"default={str(param.default)!r}" if param.type == "str" else f"default={param.default}") - - if param.help: - parts.append(f"help={param.help!r}") + def get_parsed_config(self, command: Command) -> str: + """Command `config` as keyword arguments of Click's `command()`, with a leading comma when there are any.""" + if not command.config: + return "" - return f"@click.option({', '.join(parts)})" + configured_options = command.config.model_dump(exclude_unset=True) + if "rich_help_panel" in configured_options: + # rich-click calls the panel a command is listed under `panel` + configured_options["panel"] = configured_options.pop("rich_help_panel") + return f",{self.to_args(configured_options)}" if configured_options else "" def get_alias_func_name(self, command_func_name: str, alias: str) -> str: """Function name for the copy of a command that is registered under an alias""" diff --git a/cliffy/parser.py b/cliffy/parser.py index 978967c..e629296 100644 --- a/cliffy/parser.py +++ b/cliffy/parser.py @@ -113,9 +113,6 @@ def parse_param(self, param: CommandParam | GenericCommandParam | SimpleCommandP # a full typer definition, i.e. `name: str = typer.Option(..., prompt=True)`, is passed through as-is return f"{norm_param_name}: {param.raw_type.strip()}, " - if param_type in self.manifest.types: - return f"{norm_param_name}: {self.manifest.types[param_type]}," - aliases = [param.short] if param.short else None default_val = param.default @@ -124,6 +121,18 @@ def parse_param(self, param: CommandParam | GenericCommandParam | SimpleCommandP # a str default is data, not an expression: emit it as a string literal default_val = py_str(default_val) + if param_type in self.manifest.types: + definition = self.manifest.types[param_type].strip() + if "typer." in definition: + # a complete Typer definition replaces everything after the param name + return f"{norm_param_name}: {definition}," + # `N: int` or `N: int = 5` names a type (and a default): the param keeps its own name, kind and help + param_type, _, type_default = (part.strip() for part in definition.partition("=")) + if default_val is None and type_default: + default_val = type_default + elif isinstance(param, CommandParam) and param.default is not None and param_type == "str": + default_val = py_str(param.default) + return self.build_param_type( param_name=norm_param_name, param_type=param_type, diff --git a/docs/snippets/click.yaml b/docs/snippets/click.yaml index ee4ca7b..f163b72 100644 --- a/docs/snippets/click.yaml +++ b/docs/snippets/click.yaml @@ -14,13 +14,18 @@ commands: print(f"hello {name}") resize: - help: Explicit params always become options + help: Explicit params work like in Typer params: - - name: width + - name: image + type: str + required: true + help: File to resize + - name: --width type: int + short: -w default: 800 help: Target width - run: print(width) + run: print(image, width) tag: help: Raw Click decorators diff --git a/tests/test_cli.py b/tests/test_cli.py index bf56235..fe4256d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -350,7 +350,7 @@ def test_validate_reports_generation_errors_with_exit_code(isolated_fs): result = CliRunner().invoke(validate_command, ["invalid.yaml"]) - assert "is invalid: Invalid command name: a b" in result.output + assert "is invalid: commands.a b: Invalid command name: a b" in result.output assert result.exit_code == 1 diff --git a/tests/test_click_commander.py b/tests/test_click_commander.py index 506e04c..4b56ada 100644 --- a/tests/test_click_commander.py +++ b/tests/test_click_commander.py @@ -57,8 +57,9 @@ def test_parse_command_with_params(): command = manifest.commands["greet"] parsed_params = parser.parse_params(command) - assert "--name" in parsed_params - assert "type=str" in parsed_params + # an explicit param without `--` is a positional argument, as it is for Typer (it used to become `--name`) + assert "@click.argument('name', type=str, help='Name to greet')" in parsed_params + assert "--name" not in parsed_params assert "--count" in parsed_params assert "type=int" in parsed_params diff --git a/tests/test_click_parity.py b/tests/test_click_parity.py new file mode 100644 index 0000000..01eb46e --- /dev/null +++ b/tests/test_click_parity.py @@ -0,0 +1,268 @@ +"""The Click backend accepts what the manifest reference documents, with the meaning it has for Typer.""" + +import textwrap + +import pytest +from click.testing import CliRunner as ClickCliRunner +from typer.testing import CliRunner as TyperCliRunner + +from cliffy.helper import import_module_from_path +from cliffy.transformer import Transformer + +BACKENDS = pytest.mark.parametrize("use_click", [False, True], ids=["typer", "click"]) + + +@pytest.fixture +def run(tmp_path): + """Generates a CLI from a manifest body for one backend, imports it and invokes it with args""" + counter = iter(range(1000)) + + def _run(body: str, args: list[str], use_click: bool = True): + number = next(counter) + manifest_path = tmp_path / f"manifest{number}.yaml" + header = f"name: t\nversion: '1'\nuse_click: {str(use_click).lower()}\n" + manifest_path.write_text(header + textwrap.dedent(body), encoding="utf-8") + with open(manifest_path, encoding="utf-8") as manifest_io: + T = Transformer(manifest_io, validate_requires=False) + module_path = tmp_path / f"generated{number}.py" + module_path.write_text(T.cli.code, encoding="utf-8") + module = import_module_from_path(str(module_path)) + runner = ClickCliRunner() if use_click else TyperCliRunner() + return runner.invoke(module.cli, args, env={"COLUMNS": "200", "NO_COLOR": "1", "TERM": "dumb"}) + + return _run + + +EXPLICIT_ARGUMENT = """ + commands: + a: + params: + - name: who + type: str + help: Person to greet + required: true + - name: greeting + type: str + default: hello + - name: suffix + type: str + run: print(greeting, who, suffix) +""" + + +@BACKENDS +@pytest.mark.parametrize( + "args, expected_output", + [(["Ada"], "hello Ada None"), (["Ada", "hi"], "hi Ada None"), (["Ada", "hi", "!"], "hi Ada !")], +) +def test_explicit_param_without_dashes_is_a_positional_argument(run, use_click, args, expected_output): + # the Click backend used to turn `name: who` into the option `--who` + result = run(EXPLICIT_ARGUMENT, ["a", *args], use_click) + + assert result.exit_code == 0, result.output + assert result.output.strip() == expected_output + + +@BACKENDS +def test_required_explicit_argument_is_required(run, use_click): + result = run(EXPLICIT_ARGUMENT, ["a"], use_click) + + assert result.exit_code == 2 + assert "who" in result.output.lower() + + +@BACKENDS +def test_explicit_argument_help_is_shown(run, use_click): + result = run(EXPLICIT_ARGUMENT, ["a", "-h"], use_click) + + assert "Person to greet" in result.output + assert "--who" not in result.output + + +@BACKENDS +@pytest.mark.parametrize("args, expected_output", [([], "None"), (["x"], "x")]) +def test_shorthand_argument_without_bang_is_optional(run, use_click, args, expected_output): + result = run("commands:\n a:\n params: [label: str]\n run: print(label)\n", ["a", *args], use_click) + + assert result.exit_code == 0, result.output + assert result.output.strip() == expected_output + + +@BACKENDS +def test_explicit_option_with_short_help_and_required(run, use_click): + body = """ + commands: + a: + params: + - name: --width + type: int + short: -w + help: Target width + required: true + run: print(width + 1) + """ + assert run(body, ["a", "-w", "2"], use_click).output.strip() == "3" + assert run(body, ["a"], use_click).exit_code == 2 + assert "Target width" in run(body, ["a", "-h"], use_click).output + + +@BACKENDS +@pytest.mark.parametrize( + "types, param, args, expected_output", + [ + ("N: int", "- --n: N", ["--n", "2"], "3"), + ("N: int = 5", "- --n: N", [], "6"), + ("N: int = 5", "- --n|-x: N", ["-x", "1"], "2"), + ("N: int", "- --n: N = 7", [], "8"), + ("N: int", "- n: N!", ["4"], "5"), + ("N: int", "- name: --n\n type: N\n default: 9", [], "10"), + ], + ids=["option", "type_default", "short", "param_default", "argument", "explicit"], +) +def test_types_section_names_a_type(run, use_click, types, param, args, expected_output): + # `--n: N` with `types: {N: int}` was a NameError with the Click backend + body = f"types:\n {types}\ncommands:\n a:\n params:\n {param}\n run: print(n + 1)\n" + result = run(body, ["a", *args], use_click) + + assert result.exit_code == 0, result.output + assert result.output.strip() == expected_output + + +def test_types_section_can_hold_a_click_decorator(run): + body = """ + types: + Loud: "@click.option('--loud', '-l', is_flag=True, help='Shout')" + global_params: + - loud: Loud + commands: + a: print(loud) + b: + params: [word: str!] + run: print(word.upper() if loud else word) + """ + assert run(body, ["a", "-l"]).output.strip() == "True" + assert run(body, ["b", "hi", "--loud"]).output.strip() == "HI" + assert "Shout" in run(body, ["b", "-h"]).output + + +@pytest.mark.parametrize( + "body, expected_error", + [ + ( + "types:\n Level: int = typer.Option(1, '--level')\n" + "commands:\n pack:\n params: [level: Level]\n run: print(level)\n", + r"commands\.pack: .*types\.Level.*Typer definition.*@click\.option", + ), + ( + "commands:\n pack:\n params:\n - level: int = typer.Option(1, '--level')\n run: print(level)\n", + r"commands\.pack: .*param `level`.*Typer definition.*@click\.option", + ), + ( + "commands:\n pack:\n params: ['level: int = 1']\n run: print(level)\n", + r"commands\.pack: .*`level: int = 1`.*Click decorator", + ), + ], + ids=["types", "shorthand", "generic_signature"], +) +def test_typer_definitions_are_rejected_with_a_manifest_error(run, body, expected_error): + # these used to be copied into the Click code and fail when the CLI was run + with pytest.raises(ValueError, match=expected_error): + run(body, ["pack"]) + + +@BACKENDS +@pytest.mark.parametrize( + "param, args, expected_output", + [ + ("--tag: list[str]", ["--tag", "a", "--tag", "b"], "['a', 'b']"), + ("names: list[str]", ["a", "b"], "['a', 'b']"), + ("--n: Optional[int]", ["--n", "2"], "[2]"), + ("--n: int | None", ["--n", "2"], "[2]"), + ], + ids=["list_option", "list_argument", "optional", "union_none"], +) +def test_optional_and_list_types(run, use_click, param, args, expected_output): + name = param.split(":")[0].lstrip("-") + body = f"commands:\n a:\n params:\n - '{param.split(':')[0]}': {param.split(': ')[1]}\n" + body += f" run: print(list({name}) if isinstance({name}, (list, tuple)) else [{name}])\n" + result = run(body, ["a", *args], use_click) + + assert result.exit_code == 0, result.output + assert result.output.strip() == expected_output + + +CONFIG = """ + command_templates: + admin: + config: + rich_help_panel: Admin + epilog: from the template + commands: + visible: print(1) + secret: + config: + hidden: true + run: print(2) + old: + help: Old command + config: + deprecated: true + epilog: See the new command + short_help: Short text + run: print(3) + reset: + template: admin + config: + no_args_is_help: true + context_settings: + max_content_width: 60 + params: [target: str] + run: print(target) +""" + + +@BACKENDS +def test_command_config_is_applied(run, use_click): + # `config` was ignored by the Click backend + overview = run(CONFIG, ["-h"], use_click).output + assert "visible" in overview + assert "secret" not in overview + assert "Short text" in overview + assert "Admin" in overview + + assert run(CONFIG, ["secret"], use_click).output.strip() == "2" + assert "See the new command" in run(CONFIG, ["old", "-h"], use_click).output + assert "eprecated" in run(CONFIG, ["old", "-h"], use_click).output + + no_args = run(CONFIG, ["reset"], use_click) + assert "Usage:" in no_args.output + assert "from the template" in no_args.output + assert run(CONFIG, ["reset", "db"], use_click).output.strip() == "db" + + +@BACKENDS +def test_help_only_subcommand_sets_the_help_of_a_nested_group(run, use_click): + body = "commands:\n a.b:\n help: Nested group help\n a.b.c: print(1)\n" + + assert run(body, ["a", "b", "c"], use_click).output.strip() == "1" + assert "Nested group help" in run(body, ["a", "-h"], use_click).output + + +@BACKENDS +def test_type_alias_keeps_the_help_of_an_explicit_param(run, use_click): + # a plain type in `types` used to replace the whole definition for Typer: the option became an argument + body = """ + types: + Token: str + commands: + a: + params: + - name: --token + type: Token + default: abc + help: Access token + run: print(token) + """ + assert run(body, ["a"], use_click).output.strip() == "abc" + assert run(body, ["a", "--token", "x"], use_click).output.strip() == "x" + assert "Access token" in run(body, ["a", "-h"], use_click).output From d4acdec2212ad2133050aca6521a61ac8a245a3e Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 12:16:00 -0400 Subject: [PATCH 10/22] Resolve vars transitively, allow scalar vars, report Jinja errors by line - A var that uses another computed var is now resolved, whatever order the vars are written in. A circular reference is an error that names the circle. - `vars` values can be numbers and booleans. They keep their type in `{{ }}` expressions and in the generated Python variable. A `{expression}` var with more than one key is rejected instead of silently dropping the rest. examples/cliffy_schema.json is regenerated and checked by a test. - A Jinja error is reported as manifest file, line and line text instead of a traceback. Comments stay part of the template on purpose: Jinja statements are written in comments so that the file stays valid YAML. The message says so and shows how to keep literal braces. Co-Authored-By: Claude Opus 5.5 --- cliffy/commander.py | 5 +- cliffy/manifest.py | 11 +++- cliffy/transformer.py | 105 +++++++++++++++++++++++++++---- examples/cliffy_schema.json | 9 +++ tests/test_transformer.py | 119 ++++++++++++++++++++++++++++++++++++ 5 files changed, 233 insertions(+), 16 deletions(-) diff --git a/cliffy/commander.py b/cliffy/commander.py index bd38f8b..35a0e6c 100644 --- a/cliffy/commander.py +++ b/cliffy/commander.py @@ -309,8 +309,11 @@ def add_vars(self) -> None: if isinstance(val, dict): # `name: {expression: null}` is the documented way to assign a Python expression self.cli += f"{var} = {next(iter(val))}\n" - else: + elif isinstance(val, str): self.cli += f"{var} = {py_str(val)}\n" + else: + # a number or a boolean is written as the Python literal of the same value + self.cli += f"{var} = {val!r}\n" self.cli += "\n" def add_functions(self) -> None: diff --git a/cliffy/manifest.py b/cliffy/manifest.py index e4adf8d..4efdc89 100644 --- a/cliffy/manifest.py +++ b/cliffy/manifest.py @@ -139,7 +139,9 @@ def is_option(self) -> bool: ParamBlock = CommandParam | SimpleCommandParam | GenericCommandParam -VarBlock = str | dict[str, None] +# a string, a number or a boolean; or `{expression}` (a one-key mapping without a value) for a Python expression. +# bool is listed before int: in Python a bool is an int, and `true` has to stay a boolean. +VarBlock = str | bool | int | float | dict[str, None] class CommandConfig(BaseModel): @@ -385,7 +387,12 @@ def is_safe_name(cls, value: str) -> str: @classmethod def var_names_are_identifiers(cls, value: dict[str, Any]) -> dict[str, Any]: """A var becomes a module-level Python variable of the generated CLI.""" - for name in value: + for name, var in value.items(): + if isinstance(var, dict) and len(var) != 1: + raise ValueError( + f"vars.{name}: a Python expression is written as `{{expression}}`, one expression in braces. " + f"Found {len(var)}." + ) if name.isidentifier() and not keyword.iskeyword(name) and name not in GENERATED_NAMES: continue diff --git a/cliffy/transformer.py b/cliffy/transformer.py index 6506c5e..f4407b2 100644 --- a/cliffy/transformer.py +++ b/cliffy/transformer.py @@ -1,8 +1,9 @@ import os +import traceback from typing import Any, Self, TextIO import yaml -from jinja2 import BaseLoader, Environment, FileSystemLoader +from jinja2 import BaseLoader, Environment, TemplateSyntaxError, meta from pydantic import ValidationError from cliffy.blueprint import ( @@ -145,10 +146,12 @@ def resolve_include(self, path: str) -> Self: @staticmethod def load_manifest(manifest_io: TextIO) -> dict[str, Any]: + """Reads a manifest file: it is rendered as a Jinja template with its own `vars`, then parsed as YAML.""" try: manifest_path = os.path.realpath(manifest_io.name) with open(manifest_path, encoding="utf-8") as manifest_file: - raw_manifest = yaml.safe_load(manifest_file) + source = manifest_file.read() + raw_manifest = yaml.safe_load(source) if not isinstance(raw_manifest, dict): exit_err(f"~ error loading manifest: {manifest_io.name} must be a YAML mapping") @@ -156,20 +159,96 @@ def load_manifest(manifest_io: TextIO) -> dict[str, Any]: if not isinstance(manifest_vars, dict): exit_err(f"~ error loading manifest: vars in {manifest_io.name} must be a mapping") - # `env` is passed to the templates as-is. Environment values are data: they are never - # rendered as templates themselves. - render_context = {**manifest_vars, "env": os.environ} - var_env = Environment(loader=BaseLoader()) - interpolated_vars: dict[str, Any] = { - var_env.from_string(str(k)).render(render_context): var_env.from_string(str(v)).render(render_context) - for k, v in manifest_vars.items() - } - interpolated_vars["env"] = os.environ - manifest_env = Environment(loader=FileSystemLoader(manifest_path)).get_template("") - manifest = yaml.safe_load(manifest_env.render(interpolated_vars)) + context = resolve_vars(manifest_vars, manifest_io.name) + manifest = yaml.safe_load(render_manifest(source, context, manifest_io.name)) if not isinstance(manifest, dict): exit_err(f"~ error loading manifest: {manifest_io.name} must be a YAML mapping") return manifest except yaml.YAMLError as e: out(f"{e}") exit_err("~ error loading manifest") + + +JINJA = Environment(loader=BaseLoader()) +TEMPLATE_FILE = "