Repository navigation
refactor(cmd/morphic)!: run the CLI on cobra - #815
Merged
Merged
Conversation
The CLI parsed its own arguments: a hand-written dispatcher, a loop that
re-ran the standard flag package once per operand to allow flags after
the spec file, and a help renderer. None of that could complete shell
input, wrap help to the terminal or style a misuse, and each would have
meant more hand-written code.
Build the commands on cobra and let fang render help and errors. The
command table, args.go and help.go go away; compile and validate become
cobra commands that share one flag-binding helper. Help text moves into
the commands as paragraphs and runnable examples, which fang wraps and
highlights.
Kept as before: exit 0 for help, 1 for diagnostics at --fail-on, 2 for
misuse and I/O; a "--" ends flag parsing and a flag's value of "--" is
a value; every help spelling prints the same text; "-h" before a command
that does not exist reports it instead of printing help; diagnostics
are still plain lines on stderr.
Errors are classified by type, not by message: a misuse carries the
command it was typed to, so the hint points at that command's help.
Off a terminal they are the plain "morphic: ..." lines the CLI always
wrote; on one, fang's styled block. fang's own handler cannot tell the
two apart, so redirected stderr would otherwise get the padded block.
cobra's own help command answers an unknown topic with exit 0; the
help command is replaced so "morphic help bogus" stays exit 2.
BREAKING CHANGE: long flags take two dashes. -fail-on, -skip-validate,
-explain, -pretty, -opt and -help are no longer accepted; -o stays and
gains --output, and --o is gone. A help flag after the help command
("help compile --help") now asks for help of help, and "--help compile
extra" prints compile's help. A top-level "--" no longer introduces a
command: "morphic -- compile spec" is refused rather than run. A misuse
no longer prints a usage block; it prints the reason and a pointer to the
command's --help. fang adds --version, completion and a hidden man
command.
--version printed fang's "unknown (built from source)" for every binary built from a checkout. The Go toolchain does stamp such a binary with the commit's pseudo-version, and "+dirty" when the tree had changes, but fang only trusts a module checksum. Read the stamped version directly and print the IR schema version on a second line: it is what a consumer storing morphic's output has to pin. Shell completion offers .yaml, .yml and .json files for the one spec operand compile and validate take and nothing for a second, .json files for --output, and the commands help can describe. Completion of --fail-on's two values is left out. cobra keeps flag-value completions in a package-level map keyed by flag and reports a registration mistake as an error, which would put error plumbing through every command constructor for two words.
--explain reports on stdout instead of writing the document, so an accompanying --output was accepted, ignored, and answered with exit 0. A script that passed both believed it had written a file and had not. Refuse the combination as a misuse (exit 2) before any work happens, so the destination is neither created nor replaced. An empty --output is the default and does not conflict. BREAKING CHANGE: "compile --explain <pointer> --output <file>" exits 2 instead of 0.
Diagnostics are what the CLI exists to show, and every line looked the same: a warning that fails a gate read like an info note. On a terminal the severity is now drawn, error and warning in bold red and yellow, info in cyan, and the location is dimmed so the code and the message stand out. Sixteen-colour ANSI rather than a palette: it follows the terminal's own theme, so it reads on light and dark backgrounds without asking the terminal which it is. The location is dimmed rather than coloured because bright black is the background colour in some themes. Nothing changes off a terminal. The styled text is written through a colorprofile writer, which strips it for a stream that is not one and honours NO_COLOR and CLICOLOR_FORCE, so a redirected stderr, a log and a problem matcher still read "<severity> <code> <location>: <message>" byte for byte. The message, the spec's own words, is never styled. The rendering moves from compile.go to diagnostics.go with the location helpers, unchanged.
A leading "--" used to end flag parsing for morphic itself, so "morphic -- compile spec" ran compile. cobra reads the operand as nothing and prints help with exit 0, which makes that invocation look like a compile that succeeded and produced no IR. Make the root runnable and refuse any operand that reaches it: they can only arrive after a "--", which names no command. It exits 2 with the operand in the message. A bare "morphic" and "morphic --" still print help.
On a terminal, fang title-cases the whole first word of an error, which turned "--explain writes no document" into "--Explain writes no document" and would turn "OpenAPI.yaml: ..." into "Openapi.yaml: ...". The plain form that logs and pipes get was never affected, so the tests that covered it could not see this. Drop fang's transform from the error text and raise only the first letter, leaving a message that starts with a flag, a quote or a digit exactly as written.
The layering table in the README and the package list in CLAUDE.md said the CLI imports only the engine and the IR, which stopped being true when it moved onto its command framework; every other row already names the libraries its package reaches. Three comments still spelled --pretty and --opt with one dash. Nothing outside cmd/morphic depends on the old command model: no script, workflow or other package runs the CLI with a removed flag or reads its old help or error text, and the only script that runs it uses "compile <spec> -o <file>", which is unchanged.
pflag reads a single-dash token as a cluster of shorthands. Most long flags
spelled that way already failed to parse, but on compile "-opt=k=v", the
spelling the earlier flag package accepted, is "-o pt=k=v": the option was
dropped, the document written to a file named "pt=k=v", and the run exited
0. The spaced form "-opt k=v" failed, but as "requires exactly one spec
file".
Before cobra parses, each argument of the command it will run is checked:
one that spells a long flag of that command after a single dash is refused
with exit 2 and the two-dash spelling to use. Which arguments are flag
values, and where "--" ends the flags, is read by pflag itself on a probe
flag set that discards values, so the check keeps no copy of its grammar.
An argument pflag takes as a value ("-o -opt") is still a value, and one
after an argument pflag refuses is left to that refusal.
The root's version is now set when the tree is built, with -v/--version
defined beside -h, so the check sees "-version" too; fang no longer sets
it. As with -h, defining -v before cobra resolves the command makes
"morphic -v compile spec" refuse -v on compile instead of naming the spec
as an unknown command. Both exit 2.
On a terminal the "Try ... --help" line takes the message's margin from fang's error style. Rendering it bare dropped the indent with every test green: the unit tests compare lines only under hand-built styles, which have no margin. The test that runs through fang's own styles now asserts the message and hint lines as drawn.
Rewrap the compile paragraph the --output edit left over-long, and say that a long flag typed with one dash is refused with the spelling to use.
fang styled cobra's help and errors, and most of what it cost was code written around it: an error handler of our own, because fang's drew its terminal block on redirected stderr too; a fix for its title-casing of a message's first word; a version it replaced with "unknown" for any build from a checkout; environment pinning for its width and colour detection in the tests; and a terminal colour query that made help and every misuse wait about 4 s on a terminal that never answers. Help is now cobra's plain template, with the descriptions wrapped by hand and the examples indented as cobra indents a section. Errors are the plain "morphic: <reason>" line and, for a misuse, the pointer to that command's help, on a terminal as off one. A one-dash long flag's refusal is reported directly rather than through a stand-in command. The hidden man command fang added is gone.
Colour on a terminal was the last thing lipgloss and colorprofile were here for, and they brought a dozen modules beneath them. The nicety cost more to keep than it gave: two styling functions, a test file holding the styled and plain forms apart, and a TestMain pinning the colour variables for every test in the package. Diagnostics are the plain lines main prints, on a terminal as off one.
The layering table in the README and the package list in CLAUDE.md named fang beside cobra; the CLI now imports cobra and pflag alone.
Name the flag package that accepted the spelling, say plainly that a flag's value and the arguments after "--" are not checked, and drop man from the commands that join the tree late now that fang no longer adds it.
OmarAlJarrah
changed the title
flagNamesIn's comment called a section title the only line cobra leaves unindented; the description and the closing pointer to --help are too. Each ends the section before it, which is what the code relies on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The CLI parsed its own arguments: a hand-written dispatcher, a loop that re-ran
the standard flag package once per operand so flags could follow the spec file,
and a help renderer. #790 proposed replacing the parsing with pflag and left the
framework alone. This takes cobra instead, because help, completion and version
output are where the hand-written code could not keep up. It stays plain text:
no styling layer and no colour, so the CLI's dependencies are cobra and pflag.
compileandvalidateare cobra commands sharing one flag-binding helper;cobra renders help.
args.go,help.goand the command table are gone.--fail-on, 2 formisuse and I/O.
help <unknown>stays exit 2 (cobra's own help commandreturns 0).
morphic: …lines, as before. A misuse also points at thehelp of the command that was misused.
--versionreports the build the toolchain stamped and the IR schema version..jsonfor--output, andcommand names for
help.--explaintogether with--outputis refused;--outputused to beaccepted, ignored and answered with exit 0.
Closes #790.
Breaking
-fail-on,-skip-validate,-explain,-pretty,-optand-helpare refused with exit 2 and the two-dashspelling to use. Left to pflag,
compile -opt=k=vwould read as-o pt=k=vand write the IR to a file namedpt=k=v.-ostays and gains--output;--ois gone.help compile --helpdescribeshelp;--help compile extraprintscompile's help.
morphic -- compile specexits 2 instead of running compile. A top-level--names no command.
compile --explain <pointer> --output <file>exits 2 instead of 0.--help, not a usage block.Usage:,Examples:,Flags:).--versionandcompletion.Not in scope
Diagnostics and
--explainprint spec-derived text verbatim, controlcharacters included, exactly as before this change; escaping them is #816.
Test plan
make gateequivalents: gofmt, vet, golangci-lint at the pinned version, the-racesuite, exact 100% statement coverage, the architecture test.scripts/verify-atomic-output.shagainst the new binary.compile,validateand--explainruns overtestdata/on the new andthe previous binary: identical exit codes, stdout and stderr.