Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 42 additions & 12 deletions drydock/catalog/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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"],
Expand Down
61 changes: 61 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Loading