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

(v2) feat: query background color by default #1195

Open
wants to merge 4 commits into
base: v2-exp
Choose a base branch
from

Conversation

aymanbagabas
Copy link
Member

This commit makes the program query the terminal for its background color by default. This is useful if you want to enable automatic background color detection for your program.

It also adds a new option WithoutBackgroundColor to disable this behavior.

This commit makes the program query the terminal for its background
color by default. This is useful if you want to enable automatic
background color detection for your program.

It also adds a new option `WithoutBackgroundColor` to disable this
behavior.
@meowgorithm
Copy link
Member

For the alpha, why don't we keep it opt in? If we find the friction is too high we can merge this. To your point, it would be good to avoid the heavy query when we can.

@bashbunni
Copy link
Member

Aren't users used to this behaviour by default though? I do agree that less is more overall though. Just might feel weird at first if they're used to a certain behaviour

@aymanbagabas aymanbagabas changed the title feat: query background color by default (v2) feat: query background color by default Oct 23, 2024
@aymanbagabas aymanbagabas added this to the v2.0.0 milestone Oct 23, 2024
@aymanbagabas
Copy link
Member Author

I agree with @bashbunni tbh. Since we're moving to pure lipgloss and removing the lipgloss "hack" in bubbletea@v2 that queries the terminal on start, we need to send the terminal bg color on start similar to tea.WindowSizeMsg.

@meowgorithm
Copy link
Member

Because checking for the background color is so much more involved now (you need to listen for tea.BackgroundColorMsg in the same way you listen for tea.WindowSizeMsg) I don't think it's to much to ask to have users explicitly fire off the request. That said, if we find during the alpha/beta that people are getting confused we can enable it by default. It's easier to add versus take it away.

@bashbunni
Copy link
Member

What's the benefit of having the user fire off that request? Aren't they querying the terminal for its environment info for the WindowSizeMsg anyway?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants