Skip to content

fix(cli): make -- terminate flag parsing for all later args - #330

Merged
OmarAlJarrah merged 2 commits into
mainfrom
fix/cli-double-dash-terminates-flags
Aug 9, 2026
Merged

fix(cli): make -- terminate flag parsing for all later args#330
OmarAlJarrah merged 2 commits into
mainfrom
fix/cli-double-dash-terminates-flags

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

-- is supposed to end flag parsing, but morphic only ever shielded one argument with it.
parseArgs calls flag.Parse in a loop so flags may appear on either side of the spec path, and
Parse consumes a -- while reporting nothing about having seen one — so the next round of the
loop turns flag parsing straight back on. morphic compile -- spec.yaml -o out.json therefore
exits 0 and writes out.json, where three operands should be a usage error, and
morphic compile -- -a.yaml -b.yaml answers flag provided but not defined: -b.yaml about a file
the user had explicitly marked as not a flag.

The linked issue's motivating case does not hold, and this PR does not repeat it. It says a spec
file whose name begins with - is "awkward or impossible to pass"; it is neither, because
compile takes exactly one operand and one shielded argument is enough — morphic compile -- -dash.yaml compiles today. The actual defect is the inverse: -- is too weak rather than
useless. Arguments the user marked as operands are still read as flags, so a misuse succeeds
silently and a dash-named extra operand is reported as a bad flag instead of as an extra operand.

The absence is the same at all three places morphic reads an argument list, so all three are
fixed here rather than only the one the issue names:

  • parseArgs splits at the terminator before the parse loop starts, so everything after it is
    an operand for the whole invocation instead of for one round.
  • Root dispatchmorphic -- compile spec.yaml used to report unknown command "--"; the
    marker is now consumed and compile is the command. morphic -- -h reports
    unknown command "-h" rather than printing help, since past a -- nothing is a flag. help
    still routes to help, because it is a command word and never was a flag.
  • filterHelpTokensmorphic help -- --help used to report unknown command "--"; it now
    names --help, and morphic help -- compile prints compile's help.

The split tracks which flags read their value from the following argument, so a -- that a flag
asked for stays that flag's value instead of becoming a marker. That is the same distinction
TestRun_HelpFlagAsFlagValue already pins for --help, and it is why the split takes the
FlagSet rather than searching argv for the token.

compile's help text gains a paragraph describing the terminator; testdata/compile-help.txt is
regenerated to match and changes by exactly those four lines. The README's CLI section is left
alone — it documents the flag table, and -- is not a flag.

Test plan

  • TestRun_TerminatorEndsFlagParsing (new, cmd/morphic/args_test.go) drives run over nine
    argument lists covering all three sites. Six were red before the fix; the three that were
    already correct are regression guards against over-correcting.
  • Each site was reverted on its own with the tests kept, and only that site's cases went red:
    parseArgs → the two operand cases; root dispatch → the two -- before a command-word cases;
    filterHelpTokens → the two help -- cases. No case covers more than one site.
  • TestRun_TerminatorAsFlagValue is the guard for the other half of the rule: morphic compile -o -- spec.yaml --skip-validate must write a file literally named -- and still parse the flag
    after the spec. It passes before and after the fix, and goes red when the split is degraded to a
    plain scan for the token — which is how the FlagSet-aware walk earns its place.
  • TestTakesNextValue_FlagSpellings and TestSplitAtTerminator_Cases are flat tables over the
    split's own edges: inline =value, boolean flags, undefined flags, -, ---o, -=x, a
    terminator with nothing after it, and a second terminator that must stay an operand.
  • Full gate green in a clean worktree: gofmt, go vet, golangci-lint, go build, and
    scripts/check-coverage.sh at 100%.

Ordering

Branched from main and intended to land after #312. cmd/morphic/compile.go and
cmd/morphic/main.go merge cleanly with it. The one conflict is filterHelpTokens's doc comment,
where #312 rewrites the sentence naming runCompile and this PR appends a sentence about the
terminator — take both. Nothing here touches parseArgs' callers, so validate inherits the
terminator through the shared bind path with no further change.

Closes #231

One conflict, the one this branch predicted: filterHelpTokens' doc comment.
#312 rewrote the sentence naming runCompile, since dispatch now owns the
help-request detection, and this branch appends a paragraph on what a "--" does
to the filtering. Both are kept — they describe different halves of the same
comment and neither supersedes the other.

Everything else merged clean, and the merged tree passes: the terminator work
touches parseArgs, root dispatch and filterHelpTokens, none of which #312 moved.
@OmarAlJarrah
OmarAlJarrah merged commit 0302e01 into main Aug 9, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/cli-double-dash-terminates-flags branch August 9, 2026 10:58
OmarAlJarrah added a commit that referenced this pull request Aug 9, 2026
The three conflicts this branch predicted, resolved as it said: compile.go keeps
#312's specOptions split and gains the pretty field, command_test.go keeps
#312's two-list block and gains "pretty", and the README keeps #312's
two-command flag table with the -o wording changed and a --pretty row added.
#335 also landed on the README since, so its diagnostics and exit-code prose is
kept whole rather than reverted to this branch's older paragraph.

One interaction the branch could not predict: #330's
TestRun_TerminatorAsFlagValue asserts the artifact contains `"name": "Tiny"` —
the indented spelling. That test is about which argument "--" became, not about
formatting, and -o is compact now, so it matched text it never meant to pin. It
decodes the artifact instead, which is what it was always asking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cmd/morphic: -- does not terminate flag parsing after the first positional

1 participant