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 tab_bar_background not being respected #2480

Closed
wants to merge 1 commit into from
Closed

Fix tab_bar_background not being respected #2480

wants to merge 1 commit into from

Conversation

dmitmel
Copy link

@dmitmel dmitmel commented Mar 25, 2020

Hi! After update to v0.17.0 my tab bar changed from:

image

to:

image

By looking through the changelog I found that it was commit 0d87b8f which broke my configuration. Looks like while that commit did fix #2464, it broke powerline and separator tab bar styles. Fortunately, the fix was simple enough and now tab_bar_background is treated as specified in the documentation.

P. S. I fixed my specific configuration by copying the color from inactive_tab_background to tab_bar_background. Since my changes to tab bar rendering were insignificant (in other words: part of my config for tab bar isn't much different from the default configuration) I think that this fix is worthwhile including in the default configuration.

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

Your commit doesn't fix the problem. Now the tab bar looks like (with and without tab_bar_background because now it makes no difference whatsoever):

image

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

A few more screenshots:

  • tab_bar_style = powerline + tab_bar_background #ff0000
    image
  • tab_bar_style = fade + tab_bar_background #ff0000
    image

Though the powerline screenshot shows undoubtedly incorrect behavior, you might consider the fade one somewhat correct.

Is there anything wrong with my fix?

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

I also have to mention that I tested my changes on various tab_bar_style and tab_bar_background combinations, I can send screenshots of actual/expected behavior if necessary.

@kovidgoyal
Copy link
Owner

Your patch broke changing the colors, via patch_colors. See my latest commit

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

I can revert changes to patch_colors - they aren't necessary. The point is - your commit doesn't fix the problems (see screenshots).

@kovidgoyal
Copy link
Owner

No, see d9d4199

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

This one fixes powerline and fade, but separator doesn't look quite right yet:

image

I'll try to fix it as well.

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

Here you go:

diff --git a/kitty/tab_bar.py b/kitty/tab_bar.py
index be2866ba..c5eb6068 100644
--- a/kitty/tab_bar.py
+++ b/kitty/tab_bar.py
@@ -82,6 +82,7 @@ def draw_tab_with_separator(draw_data: DrawData, screen: Screen, tab: TabBarData
     end = screen.cursor.x
     screen.cursor.bold = screen.cursor.italic = False
     screen.cursor.fg = 0
+    screen.cursor.bg = as_rgb(color_as_int(draw_data.inactive_bg))
     screen.draw(draw_data.sep)
     screen.cursor.bg = 0
     return end

@kovidgoyal
Copy link
Owner

That was deliberate, I prefer the separator for the active tab to use the active background. Having one cell of inactive bg after the last tab if the last tab is active looks weird.

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

image

Looks weird when some tab in the middle is selected.

@kovidgoyal
Copy link
Owner

Looks OK to me, though if you want to change that, I am OK either way. make a separate PR for it, and use is_last to set the bg to inactive_tab_bg only for non-last tabs.

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

Nevermind. I switched to powerline mode and I hope get used to it soon.

@dmitmel
Copy link
Author

dmitmel commented Mar 25, 2020

Having thought about it, as a compromise I can either remove the last separator, similarly to powerline:

image

@kovidgoyal
Copy link
Owner

No last separator looks bad when the tab bar bbackground is set to something other than inactive background. Like I said, I am OK with unising inactive background for all separators except the last one.

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.

Bottom tab border issue with 0.17.0
2 participants