cli: absorb jsonOut bool into outputModeWriter; parseFlagSet promotes at parse boundary#212
Conversation
… at parse boundary (links-va-001-jsonout-b4u.1) Remove the redundant `jsonOut bool` parameter from printValue and all 20+ call sites. The outputModeWriter already carries the mode; parseFlagSet now promotes it to JSON when --json is parsed, so consumers read the mode from the writer. Adds JSONFlag() on cobraFlagSet as the single parse-boundary declaration; newOutputModeWriter wraps a *outputMode so promotion propagates through interface copies. Test harnesses updated to pass newOutputModeWriter instead of raw *bytes.Buffer when exercising --json paths.
brandon-fryslie
left a comment
There was a problem hiding this comment.
One blocking finding; everything else is correct.
What I verified as sound
Pointer cell design is correct. newOutputModeWriter takes mode by value, takes its address (&mode), and Go heap-escapes it. Every copy of the outputModeWriter struct shares the same *outputMode cell, so promoteToJSON() through any interface copy (including the one extracted by stdout.(jsonPromoter)) mutates the shared cell and is visible to all subsequent readers. The value-receiver on promoteToJSON() is not a bug: the mutation is through the pointer, not the struct value.
Sequencing is correct. In both runRank and runRankSet, parseFlagSet runs before the outputModeFromWriter(stdout) checks that guard the substitution notices. The promotion happens at the right time.
Global --json + subcommand --json interaction is correct. Run() wraps the writer in JSON mode when global --json is present. If parseFlagSet then sees a subcommand --json, it promotes JSON to JSON, which is idempotent.
GetBool("json") on unregistered flag is safe. If a subcommand omits JSONFlag(), GetBool returns an error and the err == nil guard skips the promotion block. Passing --json to such a subcommand correctly errors at fs.Parse() with "unknown flag."
Blocking: internal error path is untested
See inline comment on output_mode_test.go:115. The guard "internal: --json parsed but output writer cannot carry JSON mode" has no test coverage. [LAW:verifiable-goals]
…SON mode [LAW:verifiable-goals] The guard that refuses --json on a non-promotable writer had no behavioral test; add the failing-case assertion to TestSubcommandJSONFlagPromotesWriterMode so the contract is machine-checkable.
Summary
Closes links-va-001-jsonout-b4u.1 — absorption of the redundant
jsonOut boolparam threaded through 20+printValuecall sites.jsonOut boolfromprintValuesignature and all call sites acrossbackup.go,bulk.go,cli.go,dependency.go,doctor.go,hooks.go,issue_relations.go,sync.go, and moreparseFlagSetnow absorbs--jsonflag directly into theoutputModeWriterat the parse boundary — the bool never escapes flag parsingprintValuereads output mode from the writer viaoutputModeFromWriter; no parallel signalTestSubcommandJSONFlagPromotesWriterModeto pin the promoted-mode behaviorTest plan
go test ./...greenTestSubcommandJSONFlagPromotesWriterModeverifies--jsonflag correctly promotes writer mode at the parse boundaryoutput_mode_test.gotests unaffected