Skip to content

fix: honor color profile when rendering table cells#369

Open
semihbkgr wants to merge 1 commit into
muesli:masterfrom
semihbkgr:fix/no-color-bar-escape-sequences
Open

fix: honor color profile when rendering table cells#369
semihbkgr wants to merge 1 commit into
muesli:masterfrom
semihbkgr:fix/no-color-bar-escape-sequences

Conversation

@semihbkgr

Copy link
Copy Markdown

Summary

  • The bar renderer uses the package-level termenv.String(...), which hardcodes the Style's profile to ANSI. When NO_COLOR is set (or stdout is non-TTY), env is Ascii and theme colors collapse to NoColor{} — but because the Style still claims ANSI, Foreground(NoColor{}).Background(NoColor{}) appends empty SGR slots and the renderer emits \x1b[;;m…\x1b[0m around the bar.
  • Switching to env.String(...) carries the detected profile into the Style, so Styled() short-circuits and emits plain text under Ascii. Behavior is unchanged for any non-Ascii profile (the Style's profile is only consulted for the Ascii short-circuit; actual color codes come from each Color.Sequence(...)).

Fixes #364

🤖 Generated with Claude Code

Use env.String(...) instead of the package-level termenv.String(...) so
the detected color profile is carried into the Style. The package-level
constructor hardcodes profile to ANSI, which causes empty SGR escape
sequences (\x1b[;;m ... \x1b[0m) to wrap the usage bar when NO_COLOR is
set and stdout is non-TTY.

Fixes muesli#364

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cschanhniem

Copy link
Copy Markdown

The env.String(...) vs termenv.String(...) distinction is subtle but important here. termenv.String(s) returns a termenv.Style with the global default profile (which is effectively ANSI unless explicitly changed), while env.String(s) carries the detected profile from the terminal output. When NO_COLOR is set or stdout is piped, env is Ascii and the String method on Ascii profiles still wraps in termenv.Style, but Style.Styled() correctly short-circuits to plain text for Ascii because it checks Style.profile == Ascii before emitting SGR sequences.

The deeper issue is that termenv v0.x stores the profile as a package-level global (termenv.ColorProfile), which means any code path that calls termenv.String() without first calling termenv.SetColorProfile(env.Profile) will use the wrong profile. A more structural fix for this pattern across the codebase would be:

func NewBarRenderer(output *termenv.Output) *BarRenderer {
    return &BarRenderer{out: output} // store Output, not just profile
}

func (r *BarRenderer) Render(v float64) string {
    // r.out.String(...) always uses the correct profile
}

This pattern (passing *termenv.Output around instead of calling package-level termenv.*) prevents the class of bug entirely, since the Output knows its own profile from construction time. The current fix is correct and minimal for the reported case, but if more rendering code is added (custom bar styles, sparkline-like elements), centralizing on the Output receiver pattern would be more maintainable.

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.

^[[;;m as well as ^[[0m are printed when NO_COLOR is set AND with piped → cat output

2 participants