fix(cli): let generate markdown write to stdout - #766
Conversation
`generate markdown` was the only generator with no way to reach stdout: `--out-file` was required and nothing else emitted the document. The workaround people reach for is `--out-file /dev/stdout`, which is a device file on unix but just a relative-looking path on Windows — `xx::file::write` creates the parent directory first, so it leaves a real `C:\dev\stdout` behind. This repo's own test suite did exactly that, and the resulting snapshot had a stray `writing to /dev/stdout` line baked into its first line. `--out-file` is now optional on markdown and defaults to stdout, matching manpage, fig, json and completion. `-` also means stdout on markdown, manpage and fig, mirroring the `-f -` convention already used for reading a spec from stdin; a file literally named `-` is spelled `./-`. Both spellings go through one `write_or_stdout` helper sitting next to `parse_file_or_stdin`. The `writing to ...` progress line moved from stdout to stderr on markdown, manpage, fig and sdk. On stdout it lands inside the document as soon as the document itself goes to stdout, which is what the snapshot was recording. Dropping the `--out-file` requirement would have made `--out-dir` without `--multi` silently print to stdout and ignore the directory, so `--out-dir` now requires `--multi`. `/dev/stdout` is deliberately not special-cased on Windows: it is a unix fd path, and emulating it honestly pulls in `/dev/stderr`, `/dev/null` and `/dev/fd/N` too. `-` is the portable spelling and is now in the help text.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughFig, manpage, and Markdown generation now support shared stdout and file output handling. Markdown single-file output defaults to stdout. Progress messages use stderr, with tests and documentation updated. ChangesGenerator output handling
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant GeneratorCommand
participant write_or_stdout
participant stdout
participant OutputFile
GeneratorCommand->>write_or_stdout: pass generated contents and output path
write_or_stdout->>stdout: write when path is missing or "-"
write_or_stdout->>OutputFile: write when path is a file
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
generate markdownis the only generator with no way to reach stdout —--out-fileis required and nothing else emits the document:markdown--out-dir/--out-fileonlymanpage,figjson,completion,completion-initSo the workaround is
--out-file /dev/stdout. On unix that happens to work, because it is a real device file. On Windows it is just a relative-looking path, andxx::file::writecreates the parent directory before writing — so it silently produces a realC:\dev\stdoutfile and prints nothing.This repo's own test suite does it, at
cli/tests/markdown.rs:202. On Windows that test leaves the stray file behind and fails; on unix it passes, but the snapshot it produced haswriting to /dev/stdoutbaked in as its first line, because the progress message is printed to stdout and therefore lands inside the document.Fix
generate markdown --out-fileis now optional and defaults to stdout, matchingmanpage,fig,jsonandcompletion. This is the actual fix — it removes the reason to reach for/dev/stdoutin the first place.--out-file -means stdout onmarkdown,manpageandfig, mirroring the-f -convention already used for reading a spec from stdin. It matters for callers that build the path in a variable, and it gives Windows users a portable answer. A file literally named-is spelled./-.write_or_stdouthelper placed directly belowparse_file_or_stdin, so the input and output conventions read as each other's mirror.writing to …line moved from stdout to stderr onmarkdown,manpage,figandsdk.--out-dirnow requires--multi. Without this, dropping the--out-filerequirement would have made--out-diron its own silently print to stdout and ignore the directory./dev/stdoutis deliberately not special-cased on Windows. It is a unix fd path rather than a portable filename, and emulating it honestly drags in/dev/stderr,/dev/null,/dev/fd/Nand append-vs-truncate semantics.-is the portable spelling and is now documented in the help text of all three flags.Also left alone:
generate figwraps the spec in a prescript/postscript when writing to a file but prints the bare spec object to stdout.--out-file -follows the file path — it means "the bytes a file would have received" — and bareusage g figis unchanged. Whether those two should agree is a pre-existing question, flagged in a comment rather than answered here.Compatibility
No existing valid invocation changes meaning. The one behaviour change is that
usage g md -f x.kdlwith no output flag goes from a clap error to printing markdown, which is the point.--out-filekeepseffect=writeon all three commands. The effect model is static per flag with no way to say "read when the value is-", and for a field that agents read to decide whether to ask permission, over-reporting is the safe direction.Verified
Windows 11. In an empty directory:
No
C:\devis created.cargo test -p usage-cli(markdown, manpage, clap_sort, lib) andcargo test -p usage-lib --all-featurespass;cargo clippy --all --all-features -- -D warningsandcargo fmt --all -- --checkare clean.New tests, all platform-independent so they hold the line on the Linux CI:
test_markdown_out_file_dash_writes_no_file— runs with--out-file -in a fresh empty directory and asserts nothing was written. Before the fix this leaves a file named-, which is the same bug that producedC:\dev\stdout.test_markdown_stdout_when_out_file_omitted— the two spellings of stdout produce identical bytes.test_generate_markdown_basicnow asserts the progress line is on stderr and not on stdout.test_generate_manpage_out_file_dash_is_stdout—-o -matches omitting-o.The existing snapshot test switched from
/dev/stdoutto-; the only change tomarkdown__markdown_snapshot_with_examples.snapis the removal of the straywriting to /dev/stdoutline.Generated artifacts
Regenerated from the new help text:
cli/usage.usage.kdl,docs/cli/reference/commands.json,docs/cli/reference/generate/{markdown,manpage,fig}.md,cli/assets/usage.1,cli/assets/fig.ts. The diff in each is the three help strings and nothing else. Completion scripts are untouched — they carry no flag help.Making
--out-fileoptional produces no artifact diff on its own: clap'srequired_unless_presentnever setArgSettings::Required, andlib/src/spec/flag.rs:430readsis_required_set(), so the spec already recorded the flag as not required.This pull request was generated by Claude Code.
Summary by CodeRabbit
New Features
--out-file -.Documentation
Bug Fixes