[terminal-stylist] Terminal Stylist: Console Output Consistency Review #53945
Closed
Replies: 1 comment
|
This discussion has been marked as outdated by Terminal Stylist. A newer discussion is available at Discussion #54198. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
Analyzed all non-test
.gofiles inpkg/for console output patterns (fmt.Print*,console.*, Lipgloss, Huh). Overall the codebase is highly consistent and follows Charmbracelet ecosystem best practices well.Findings
✅ Consistent usage
pkg/console(257 call sites across the repo) is the standard output layer, wrappingfmt.Fprintln/Fprintfto stdout/stderr withconsole.FormatInfoMessage,FormatWarningMessage,FormatErrorMessage,RenderStruct, etc. No ad-hoc message formatting was found outside this package.pkg/styles/theme.go,pkg/styles/huh_theme.go,pkg/console/{render,console,banner}.go,pkg/cli/{engine_secrets,compile_schedule_calendar}.go,pkg/logger/logger.go) relies on adaptive colors (adaptiveColor/lipgloss.LightDark) — no hardcoded hex colors were found leaking outsidepkg/styles.console.NewForm/NewSelectForm/NewInputForm/NewConfirmForm(pkg/console/{prompt_form,input,list,confirm}.go), which applystyles.HuhThemeandWithAccessible(IsAccessibleMode())uniformly — every call site inpkg/cli(interactive.go, add_interactive_.go, run_interactive.go, engine_secrets.go, bootstrap_profile_.go) goes through these wrappers, so accessibility mode and theming are applied consistently.pkg/console/terminal.goand consumed viaconsole.*helpers rather than ad-hocisattychecks scattered through business logic.🟡 Minor / acceptable exceptions
pkg/cli/status_command.go:295andpkg/cli/view_command.go:168use rawfmt.Print— both print output that was already rendered byconsole.RenderStruct/renderUnifiedTimelineStream, so this is a reasonable exception (avoids adding a trailing newline) rather than an inconsistency.pkg/console/terminal.gocontains two raw ANSI escape sequences (\033[H\033[2J,\033[K") for clear-screen/clear-line. These are low-level terminal control operations without a direct Lipgloss equivalent, so keeping them as raw escapes here is appropriate — but they are isolated to this single file, which is good encapsulation.fmt.Print*hits are all in test fixtures/linter testdata (pkg/linters/**/testdata), not production code, so they don't affect real CLI output.No issues found in:
pkg/stylesconsole.RenderStruct)Recommendations
fmt.Printcall sites (status_command.go, view_command.go) noting why rawfmt.Printis used instead of aconsole.*helper, to prevent future contributors from "fixing" it inconsistently.terminal.gobehind named helper functions (e.g.,ClearScreen(),ClearLine()) if not already exposed, for discoverability — quick check showed they're already scoped as package constants, so this is a nice-to-have only.No functional defects were identified; this is a maintenance/consistency review only.
All reactions