Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: read fore/background colors on first access #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aymanbagabas
Copy link
Collaborator

@aymanbagabas aymanbagabas commented Aug 26, 2023

Caching the colors in OutputOption makes the order of options important. For example, if you're using WithUnsafe after WithColorCache, the colors wouldn't respect the unsafe option since the colors get cached during the execution of WithColorCache option.

Another example, is the use of a custom environ using the WithEnvironment option. If WithEnvironment comes after WithColorCache, reading the terminal colors won't respect the values in the environment.

Instead, don't read the colors in WithColorCache and make the user opt-in if they want to cache the colors right after creating a new output using ForegroundColor and BackgroundColor.

// To cache the colors after creating an output
output := NewOutput(os.Stderr)
_ = output.ForegroundColor()
_ = output.BackgroundColor()

Caching the colors in OutputOption makes the order of options important.
For example, if you're using WithUnsafe _after_ WithColorCache, the
colors wouldn't respect the `unsafe` option since the colors get cached
_during_ the execution of WithColorCache option.

Another example, is the use of a custom environ using the WithEnvironment
option. If WithEnvironment comes _after_ WithColorCache, reading the
terminal colors won't respect the values in the environment.

Instead, don't read the colors in WithColorCache and make the user
opt-in if they want to cache the colors right after creating a new
output using ForegroundColor and BackgroundColor.

```go
// To cache the colors after creating an output
output := NewOutput(os.Stderr)
_ = output.ForegroundColor()
_ = output.BackgroundColor()
```
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.

2 participants