erikw / tmux-powerline

⚡️ A tmux plugin giving you a hackable status bar consisting of dynamic & beautiful looking powerline segments, written purely in bash.
BSD 3-Clause "New" or "Revised" License
3.36k stars 508 forks source link

Fix issue with air quality rendering a white background. #397

Closed shyce closed 2 months ago

shyce commented 3 months ago

This pull request fixes the default and bubble theme so that including air works properly by default.

xx4h commented 3 months ago

Hi @shyce, thanks for pointing that out and already providing a solution!

The way i see it, air_color.sh should not be a segment at all. Overriding the color for a specific symbol of a segment should be done directly in the segment. Something like we did here: https://github.com/erikw/tmux-powerline/blob/fd480c2d6ae5e24a18589c4ef8e3685bc576e186/segments/vcs_branch.sh#L9 and then use it like this: https://github.com/erikw/tmux-powerline/blob/fd480c2d6ae5e24a18589c4ef8e3685bc576e186/segments/vcs_branch.sh#L68

What do you think? Do you want to go for this refactor?

shyce commented 3 months ago

Yeah let me take a look at this tonight after a bit of a break from the pc. I didn't like it either, just didn't want to commit a big change as a new contributor. Thanks for the feedback!

xx4h commented 3 months ago

Yeah let me take a look at this tonight after a bit of a break from the pc. I didn't like it either, just didn't want to commit a big change as a new contributor. Thanks for the feedback!

Cool, appreciate it! Let me know if you have any questions.

shyce commented 3 months ago

I gave this a shot last night and it looks like it will need a bit more work. No modules are using #[bg=] currently because it doesn't color the powerline symbols as well, since it allows you to have multiple background colors in one echo. I'm still thinking about the best way to do this while adhering to the previous theme api.

I didn't want to change the way #[bg=] works because it might break peoples custom themes.

xx4h commented 3 months ago

Oh, i guess i got i wrong yesterday. So it is not about the color for the symbol, it's about the color of the whole segment, right? And based on the air-quality the color of the segment should change?

shyce commented 3 months ago

Yeah the air quality index changes the whole segment color based on the quality of the air I guess. I tried the echo approach instead of sourcing and for some reason, the output would not concat to the string. No idea why.

shyce commented 3 months ago

I just tried again, and I guess I made a typo because it did work this time. Let me update it.

shyce commented 3 months ago

Stand by, I guess my editor tab stops do not match the project.

shyce commented 3 months ago

That's much cleaner, thanks. I'll try again at my dynamic approach that lets each segment setup their own colors in a later PR.