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

Add powerline faces #27

Merged
merged 1 commit into from
Oct 1, 2016
Merged

Add powerline faces #27

merged 1 commit into from
Oct 1, 2016

Conversation

colonelpanic8
Copy link
Contributor

No description provided.

@belak
Copy link
Member

belak commented Sep 30, 2016

Thanks for your contribution!

Any chance you could provide a screenshot? I'm not as familiar with the powerline faces, so I'd like to make sure it still looks good.

Also, looks like it uses different settings from the normal modeline. Is that on purpose?

     (mode-line                                    :foreground base04 :background base02 :box nil)
     (mode-line-buffer-id                          :foreground base0B :background nil)
     (mode-line-emphasis                           :foreground base06 :slant italic)
     (mode-line-highlight                          :foreground base0E :box nil :weight bold)
     (mode-line-inactive                           :foreground base03 :background base01 :box nil)

@colonelpanic8
Copy link
Contributor Author

Thanks for your contribution!

Any chance you could provide a screenshot? I'm not as familiar with the powerline faces, so I'd like to make sure it still looks good.

Sure I'll take before/after screenshots

Also, looks like it uses different settings from the normal modeline. Is that on purpose?

Yes. powerline still uses the modeline settings for the "background" portion of the line, but it uses these parts for the segments in the modeline

@colonelpanic8
Copy link
Contributor Author

Before http://imgur.com/l7EC0si

The problem with this is that the lighter active window "looks" inactive, and the darker "inactive" windows look active.

After http://imgur.com/HGui14Q

Its much more obvious that the active center window is active

@colonelpanic8
Copy link
Contributor Author

The screenshots are in base16-default-dark. If you'd like to see something else, let me know.

@colonelpanic8
Copy link
Contributor Author

Hmmm. I took a look at some of the other themes and some of them don't look great. They didn't really look good with powerline before either. It's sort of hard to pick colors that work with all of the different themes here.

@belak
Copy link
Member

belak commented Sep 30, 2016

I've tested your changes on default-dark, and tomorrow (on tomorrow, it's hard to see contrast, but it looks better than before)... it looks good on those. However, on ones like github it looks pretty bad. Writing for a bunch of themes like this can be a massive pain sometimes.

I know when I'm not really sure what to do, I sometimes look at the vim theme, or hybrid as it's a variation on tomorrow-dark. The project page also provides some basic guidelines, but doesn't account for everything (http://chriskempson.com/projects/base16/).

screenshot from 2016-09-30 16-34-42

That looks good to me because it's very clear which is active and which is inactive.

I'm mostly happy with how this looks, so I can merge this as-is and iterate on it later, or if you want I can hold off if you want to tweak it.

@colonelpanic8
Copy link
Contributor Author

Lets merge for now. I might take a look at it again later this weekend

@belak belak merged commit 4a50d7f into tinted-theming:master Oct 1, 2016
@belak
Copy link
Member

belak commented Oct 1, 2016

Merged! I've opened an issue so I remember to take a look at this later.

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