diff --git a/drydock/catalog/cli.py b/drydock/catalog/cli.py index ee2fb33..305e8bd 100644 --- a/drydock/catalog/cli.py +++ b/drydock/catalog/cli.py @@ -211,6 +211,23 @@ def _build_parser() -> argparse.ArgumentParser: help="Catalog root (default: ~/.drydock; can also use DRYDOCK_CATALOG env var)", ) + # One vocabulary, applied after every subparser is built rather than + # inside each one. Adding these per-subcommand is exactly how they drifted: + # no catalog subcommand accepted `-o` at all, and `--quiet` reached only + # `ingest` while `--markdown` reached only the others. A user meets that in + # their first minute, because `-o` works on all ten analyzers. + for sub in subparsers.choices.values(): + have = {a.dest for a in sub._actions} + if "output" not in have: + sub.add_argument("--output", "-o", metavar="FILE", + help="Write to FILE instead of stdout") + if "markdown" not in have: + sub.add_argument("--markdown", "-m", action="store_true", + help="Render for humans (default: JSON)") + if "quiet" not in have: + sub.add_argument("--quiet", "-q", action="store_true", + help="Suppress progress output on stderr") + return parser @@ -297,6 +314,19 @@ def _format_search(hits: list) -> str: return "\n".join(lines) + +def _emit(args, text: str) -> None: + """Send a payload to --output if given, otherwise stdout. + + Every handler goes through here. Adding `--output` to the parsers without + this is the dead-flag bug in miniature: the flag parsed, was accepted, and + did nothing -- which is worse than not offering it, because the caller + believes the file was written. + """ + write_output(text, getattr(args, "output", None), + Reporter(quiet=getattr(args, "quiet", False))) + + def _cmd_ingest(args) -> int: """Execute the ingest subcommand.""" catalog = Catalog(args.catalog or None) @@ -334,7 +364,7 @@ def _cmd_ingest(args) -> int: for phase, error in result.failures.items(): reporter.warn(f"{phase}: {error}") - print(dumps({ + _emit(args, dumps({ "repo_id": result.repo_id, "components_recorded": result.components_recorded, "components_stored": result.components_stored, @@ -355,7 +385,7 @@ def _cmd_list(args) -> int: try: repos = catalog.repos() if not args.markdown: - print(dumps([{ + _emit(args, dumps([{ "id": r.id, "name": r.name, "source": r.source, @@ -365,7 +395,7 @@ def _cmd_list(args) -> int: "file_count": r.file_count, } for r in repos])) else: - print(_format_list(repos)) + _emit(args, _format_list(repos)) return EXIT_OK except Exception as exc: print(f"error: {exc}", file=sys.stderr) @@ -393,7 +423,7 @@ def _cmd_show(args) -> int: facts = catalog.facts(repo_id=repo.id) if not args.markdown: - print(dumps({ + _emit(args, dumps({ "repo": { "id": repo.id, "name": repo.name, @@ -465,7 +495,7 @@ def _cmd_components(args) -> int: ) if not args.markdown: - print(dumps([{ + _emit(args, dumps([{ "id": c.id, "repo_id": c.repo_id, "name": c.name, @@ -478,7 +508,7 @@ def _cmd_components(args) -> int: "summary": c.summary, } for c in components])) else: - print(_format_components(components, catalog.root)) + _emit(args, _format_components(components, catalog.root)) return EXIT_OK except Exception as exc: print(f"error: {exc}", file=sys.stderr) @@ -498,7 +528,7 @@ def _cmd_search(args) -> int: ) if not args.markdown: - print(dumps([{ + _emit(args, dumps([{ "subject_id": h.subject_id, "subject_kind": h.subject_kind, "repo_id": h.repo_id, @@ -507,7 +537,7 @@ def _cmd_search(args) -> int: "rank": h.rank, } for h in hits])) else: - print(_format_search(hits)) + _emit(args, _format_search(hits)) return EXIT_OK except Exception as exc: print(f"error: {exc}", file=sys.stderr) @@ -546,7 +576,7 @@ def _cmd_diagram(args) -> int: ) if not args.markdown: - print(dumps({ + _emit(args, dumps({ "repo_id": repo.id, "diagram_kind": args.kind, "mermaid": mermaid_text, @@ -568,9 +598,9 @@ def _cmd_stats(args) -> int: try: stats = catalog.stats() if not args.markdown: - print(dumps(stats)) + _emit(args, dumps(stats)) else: - print(_format_stats(stats)) + _emit(args, _format_stats(stats)) return EXIT_OK except Exception as exc: print(f"error: {exc}", file=sys.stderr) @@ -613,7 +643,7 @@ def _cmd_forget(args) -> int: result = catalog.forget(repo.id, delete_content=delete_content) if not args.markdown: - print(dumps({ + _emit(args, dumps({ "repo_id": repo.id, "repo_name": repo.name, "components_removed": result["components"], diff --git a/tests/test_cli.py b/tests/test_cli.py index a7f55cd..98b698f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -346,3 +346,64 @@ def test_no_tests_filters_by_role(self, simple_python_project: Path) -> None: num_with_tests = len(files_with_tests) assert num_no_tests <= num_with_tests + + +# ============================================================================ +# The shared output vocabulary must reach EVERY command, catalog included. +# ============================================================================ + +def test_catalog_subcommands_share_the_output_vocabulary(tmp_path, monkeypatch): + """Every catalog subcommand accepts -o, --markdown and --quiet, and honours them. + + These had drifted: no catalog subcommand accepted `-o` at all, `--quiet` + reached only `ingest`, and `--markdown` reached only the others. A user + meets that immediately, because `-o` works on all ten analyzers. + + Behaviour is asserted, not the help text. Accepting a flag and honouring it + are different things -- the first attempt at this fix added the flags to + the parsers without wiring them, so `-o` parsed cleanly and wrote nothing, + which is worse than not offering it. + """ + import json + import subprocess + import sys + + monkeypatch.setenv("DRYDOCK_CATALOG", str(tmp_path / "cat")) + project = tmp_path / "proj" + project.mkdir() + (project / "m.py").write_text("def f():\n return 1\n") + + subprocess.run([sys.executable, "-m", "drydock.cli", "catalog", "ingest", + str(project)], capture_output=True, timeout=180) + + for sub in (["list"], ["stats"], ["components"], ["search", "f"]): + name = " ".join(sub) + + plain = subprocess.run([sys.executable, "-m", "drydock.cli", "catalog", *sub], + capture_output=True, text=True, timeout=180) + assert plain.returncode == 0, f"{name}: {plain.stderr}" + json.loads(plain.stdout) # JSON by default, like every other command + + target = tmp_path / f"{sub[0]}.json" + piped = subprocess.run( + [sys.executable, "-m", "drydock.cli", "catalog", *sub, "-o", str(target)], + capture_output=True, text=True, timeout=180) + assert piped.returncode == 0, f"{name}: {piped.stderr}" + assert target.exists() and target.stat().st_size > 0, ( + f"{name}: -o was accepted but wrote nothing" + ) + assert not piped.stdout.strip(), ( + f"{name}: payload went to stdout as well as the file" + ) + json.loads(target.read_text()) + + rendered = subprocess.run( + [sys.executable, "-m", "drydock.cli", "catalog", *sub, "--markdown"], + capture_output=True, text=True, timeout=180) + assert rendered.returncode == 0, f"{name}: {rendered.stderr}" + try: + json.loads(rendered.stdout) + except json.JSONDecodeError: + pass # correct: markdown is not JSON + else: + raise AssertionError(f"{name}: --markdown still produced JSON")