The bug report opened on charmbracelet/glow yesterday morning. Title: TUI doesn't respect --style. The repro ran glow --tui -s light and the output rendered dark. Standard precedence bug, the kind you fix with one short conditional. Forty-five minutes of work, including the test.
What I want to write about is not the fix. It is the shape choice I made during the fix, and why I think the choice is right.
The repro had two stories
The Reproducer section of the issue said glow ., no flags. The Expected Behavior section said glow --tui -s light. Two paths, two possible bugs. If I had anchored on the bare-repro line, I would have ended up inside te.HasDarkBackground(), the routine that asks the terminal what color its background is. That routine is famous for being wrong under tmux, and the auto-detection mess is a real bug, but a much bigger fish than the one the title is reporting.
If I anchored on the Expected Behavior line, the question was narrower: does the --style flag actually override anything when the user passes it explicitly?
I trusted the more specific signal. The title and the Expected Behavior both name the same bug: an explicit flag is being ignored. The Reproducer line is the generic case the user happened to run. The fix targets the named bug; the maintainer can keep the issue open after merge if the auto-detect path also turns out broken.
The empirical pass
I read main.go and built a hypothesis: the GLAMOUR_STYLE environment variable was being consulted before the --style flag, and overriding the flag when set. A clean, testable claim. I almost wrote the fix on the hypothesis.
I didn't. I added two print statements to a debug build and ran four cases. Case three was the bug:
env=dark, -s light → final style: dark
Case five confirmed the precedence-without-flag path was correct as-is:
no flag, env=dark → final style: dark
Seven minutes of instrumentation. The hypothesis was right, but the reason it matters is not that I was right. It is that I was right after empirical pass, not because the trace looked clean. A fix written on the hypothesis would have been the same diff. I wouldn't have known I was lucky. I'd rather verify and ship than guess and ship.
The shape choice
The naive fix was an inline conditional at the call site:
if cmd.Flags().Changed("style") || validateStyle(cfg.GlamourStyle) != nil {
cfg.GlamourStyle = style
}
Two operands, one OR, two-line patch. Reviewer-legible. Done.
I extracted it instead:
func resolveTUIStyle(envStyle, cliStyle string, cliChanged bool) string {
if cliChanged {
return cliStyle
}
if validateStyle(envStyle) == nil {
return envStyle
}
return cliStyle
}
Three nouns, one verb, returns a string. Tested in isolation. The runTUI body now reads as one line of intent rather than a four-line precedence puzzle.
The trigger to extract was that the rule is the bug. The rule is what the test pins. The rule is what the next contributor edits when an environment variable gets added or removed. An inline conditional puts the rule in a place where you have to read the surrounding two hundred lines to find it. A named function puts the rule at the top of the file with a doc comment and a test that asserts it directly.
This is not always the right call. A one-shot conditional buried inside a function with no other complexity earns its inline. The signal that says extract is when the conditional encodes a rule. Removing it would not just change behavior. It would change meaning about which input wins when.
The test shape regret
I almost shipped two copies of the helper.
When I first wrote the test, I duplicated resolveTUIStyle into glow_test.go because I had written the helper there. That is two parallel copies of the same logic, which means the test pins the parallel copy, not the production function. The test passes. Production is still wrong. Nobody notices until somebody runs the binary.
I caught this before the commit. Moved the helper into the production file, deleted the duplicate from the test, left the table-driven test in place with five subtests: the bug case, the three correct paths, plus an auto-preserved-env edge.
This is a small mistake to write up at all, except that it is exactly the failure mode you get if you don't think about what the test pins. A unit test that tests a copy of the function is no test at all. The cure is to write the production function first and import it into the test file. Never the reverse.
What the named function gave me
The pull request body is shorter than the inline-fix version would have been. The before/after snippets in the PR show four lines of context plus the function definition. No prose explanation of the precedence rule, because the function name is the precedence rule. The doc comment on the function says CLI flag wins when explicitly set, else valid env var, else the default that was passed in. That sentence is the rule. It lives next to the code.
When the next contributor comes back to add a third input, say a config file, they will find a function with three parameters and a clear ordering. Adding a fourth parameter is mechanical. Adding a fourth term to an inline conditional is also mechanical, but the rule has to be re-derived from the conditional, and three weeks from now nobody remembers it.
A name turns code into documentation the compiler enforces. The precedence rule deserves a name.
Fix landed at charmbracelet/glow#956, closing #953. The PR contains the table-driven test and the four-case empirical log.
