attempts to fix ANSI sequences leaking#42
Merged
nxtcoder17 merged 1 commit intomasterfrom Feb 17, 2026
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideCentralizes terminal theme detection at process startup using termenv and an environment variable, then switches downstream rendering logic to rely on that environment variable instead of querying termenv directly, promoting creack/pty and x/sync to direct dependencies. Sequence diagram for centralized terminal theme detection and usagesequenceDiagram
actor User
participant RunMain as cmd_run_main
participant Termenv as termenv_Output
participant OS as os_Stdout
participant Env as os_Environ
participant Resolver as pkg_runfile_resolver_task
User->>RunMain: start run command
RunMain->>Env: getenv RUNFILE_THEME
alt RUNFILE_THEME not set
RunMain->>Termenv: NewOutput(os_Stdout)
Termenv->>OS: query background
OS-->>Termenv: dark or light
Termenv-->>RunMain: HasDarkBackground result
RunMain->>Env: setenv RUNFILE_THEME dark_or_light
else RUNFILE_THEME already set
RunMain-->>RunMain: skip theme detection
end
User->>Resolver: execute task that prints command
Resolver->>Env: getenv RUNFILE_THEME
alt RUNFILE_THEME == light
Resolver-->>Resolver: set borderColor light
Resolver-->>Resolver: set colorscheme xcode
else RUNFILE_THEME != light
Resolver-->>Resolver: set borderColor dark
Resolver-->>Resolver: set colorscheme catppuccin_macchiato
end
Resolver-->>User: themed command output
Flow diagram for theme configuration via environment variableflowchart LR
subgraph Startup
A[cmd_run_main] --> B{RUNFILE_THEME set?}
B -- No --> C[termenv NewOutput os_Stdout]
C --> D[HasDarkBackground]
D --> E{Background is dark?}
E -- Yes --> F[theme = dark]
E -- No --> G[theme = light]
F --> H[setenv RUNFILE_THEME]
G --> H[setenv RUNFILE_THEME]
B -- Yes --> I[skip detection]
end
subgraph Runtime
J[pkg_runfile_resolver_task printCommand]
J --> K[getenv RUNFILE_THEME]
K --> L{RUNFILE_THEME == light?}
L -- Yes --> M[borderColor light\ncolorscheme xcode]
L -- No --> N[borderColor dark\ncolorscheme catppuccin_macchiato]
end
H --> K
I --> K
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
RUNFILE_THEMElogic is now duplicated betweenprintCommandandmain; consider centralizing theme detection/access in a small helper (e.g.,GetTheme()orIsLightTheme()) to avoid string/env-name drift. - Automatically mutating
RUNFILE_THEMEas a process-wide environment variable inmaincould be surprising for callers; if this is used as a library entry point elsewhere, consider passing the resolved theme through configuration or context instead of setting env.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `RUNFILE_THEME` logic is now duplicated between `printCommand` and `main`; consider centralizing theme detection/access in a small helper (e.g., `GetTheme()` or `IsLightTheme()`) to avoid string/env-name drift.
- Automatically mutating `RUNFILE_THEME` as a process-wide environment variable in `main` could be surprising for callers; if this is used as a library entry point elsewhere, consider passing the resolved theme through configuration or context instead of setting env.
## Individual Comments
### Comment 1
<location> `cmd/run/main.go:43-48` </location>
<code_context>
+ // Detect terminal theme early, before any subprocesses run.
+ // This prevents ANSI response sequences from leaking onto stdin.
+ if os.Getenv("RUNFILE_THEME") == "" {
+ theme := "dark"
+ if !termenv.NewOutput(os.Stdout).HasDarkBackground() {
+ theme = "light"
+ }
+ os.Setenv("RUNFILE_THEME", theme)
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Using os.Stdout for theme detection can misbehave when stdout is redirected or piped.
`termenv.NewOutput(os.Stdout).HasDarkBackground()` sends probe escape sequences to stdout. If the command’s stdout is piped or redirected, those sequences will pollute the output and the theme may not match the user’s actual terminal. Prefer probing a TTY-backed stream (e.g., stderr) or first checking `term.IsTerminal(int(os.Stdout.Fd()))` and skipping detection when stdout is non-interactive.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ground color check
d0e9f97 to
a36773b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves #41
attempts to fix ANSI sequences leaking because of termenv's background color check
Summary by Sourcery
Detect terminal background once at startup and use an environment variable to control theming, preventing ANSI escape sequence leakage.
New Features:
Bug Fixes:
Enhancements:
Build: