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 support for centaur-tabs, doom-modeline, and solaire-mode #330

Merged
merged 1 commit into from
Jul 15, 2019
Merged

Add support for centaur-tabs, doom-modeline, and solaire-mode #330

merged 1 commit into from
Jul 15, 2019

Conversation

ianyepan
Copy link
Contributor

Seeing that Centaur Tabs is a growing package (I personally love it!), I've added the following color support for it.

;;;;; Centaur Tabs
   `(centaur-tabs-default ((t (:background ,zenburn-bg :foreground ,zenburn-fg+1))))
   `(centaur-tabs-selected ((t (:background ,zenburn-bg :foreground "white"))))
   `(centaur-tabs-unselected ((t (:background ,zenburn-bg-1 :foreground "#909090"))))
   `(centaur-tabs-selected-modified ((t (:background ,zenburn-bg :foreground ,"#bbbbbb"))))
   `(centaur-tabs-unselected-modified ((t (:background ,zenburn-bg-1 :foreground ,"#909090"))))
   `(centaur-tabs-active-bar-face ((t (:background ,zenburn-yellow))))
   `(centaur-tabs-modified-marker-selected ((t (:inherit 'centaur-tabs-selected-modified :foreground ,zenburn-yellow-1))))
   `(centaur-tabs-modified-marker-unselected ((t (:inherit 'centaur-tabs-unselected-modified :foreground ,zenburn-yellow-2))))

@ema2159
Copy link

ema2159 commented Jul 12, 2019

Just 2 things:

  • I think you have to only use the theme colors, so probably you'd need to use the zenburn-fg and one of the other Zenburn colors like zenburn cyan instead of the #909090, the #bbbbbb or the "white"

  • Could you post a screenshot to see how it looks?

@ianyepan
Copy link
Contributor Author

I've thought of that, but couldn't find a suitable colour for it. Perhaps I'll try again. Right now the screenshot looks like:

Screenshot 2019-07-13 at 03 07 17

@ianyepan
Copy link
Contributor Author

ianyepan commented Jul 12, 2019

[Update]: I have edited the snippet to be:

;;;;; Centaur Tabs
   `(centaur-tabs-default ((t (:background ,zenburn-bg :foreground ,zenburn-fg+1))))
   `(centaur-tabs-selected ((t (:background ,zenburn-bg :foreground "white"))))
   `(centaur-tabs-unselected ((t (:background ,zenburn-bg-1 :foreground "#909090"))))
   `(centaur-tabs-selected-modified ((t (:background ,zenburn-bg :foreground ,zenburn-yellow-1))))
   `(centaur-tabs-unselected-modified ((t (:background ,zenburn-bg-1 :foreground ,zenburn-yellow-2))))
   `(centaur-tabs-active-bar-face ((t (:background ,zenburn-yellow))))
   `(centaur-tabs-modified-marker-selected ((t (:inherit 'centaur-tabs-selected-modified :foreground ,zenburn-yellow-1))))
   `(centaur-tabs-modified-marker-unselected ((t (:inherit 'centaur-tabs-unselected-modified :foreground ,zenburn-yellow-2))))

However, for the unselected tab foreground, there seems to be only dark grey in the variable that the original author defined (to dark to be visible). Therefore I suggest that we stick to "#909090"

zenburn-theme.el Outdated
@@ -1535,6 +1535,16 @@ Also bind `class' to ((class color) (min-colors 89))."
;;;;; yascroll
`(yascroll:thumb-text-area ((t (:background ,zenburn-bg-1))))
`(yascroll:thumb-fringe ((t (:background ,zenburn-bg-1 :foreground ,zenburn-bg-1))))

;;;;; Centaur Tabs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The list of packages is sorted by lexicographic order, so centaur-tabs should come after calfw and before cider.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the comment should be centaur-tabs right?

@ema2159
Copy link

ema2159 commented Jul 13, 2019

@ianpan870102 I propose this one, what do you think?:

;;;;; centaur-tabs
   `(centaur-tabs-default ((t (:background ,zenburn-bg :foreground ,zenburn-fg :box nil))))
   `(centaur-tabs-selected ((t (:background ,zenburn-bg :foreground ,zenburn-fg :box nil))))
   `(centaur-tabs-unselected ((t (:background ,zenburn-bg-1 :foreground ,zenburn-fg-1 :box nil))))
   `(centaur-tabs-selected-modified ((t (:background ,zenburn-bg :foreground ,zenburn-orange :box nil))))
   `(centaur-tabs-unselected-modified ((t (:background ,zenburn-bg-1 :foreground ,zenburn-orange :box nil))))
   `(centaur-tabs-active-bar-face ((t (:background ,zenburn-yellow :box nil))))
   `(centaur-tabs-modified-marker-selected ((t (:inherit 'centaur-tabs-selected-modified :foreground ,zenburn-yellow :box nil))))
   `(centaur-tabs-modified-marker-unselected ((t (:inherit 'centaur-tabs-unselected-modified :foreground ,zenburn-yellow :box nil))))

The :box nil part is completely necessary because if not the icons will look bad, and the zenburn-fg-1 will do it instead of the "#909090" and the zenburn-fg will do it instead of the "white"

It looks like this:
DeepinScreenshot_select-area_20190712202255

Also, could you pleas add the following configuration for doom-modeline so it matches centaur-tabs?

;;;;; doom-modeline
`(doom-modeline-bar  ((t (:background ,zenburn-yellow))))
`(doom-modeline-inactive-bar  ((t (:background nil))))

@ianyepan
Copy link
Contributor Author

ianyepan commented Jul 13, 2019

Ah thanks for the :box nil trick, I have been tracking down the problem for a while haha. Though I think it looks great, it's a touch too dim IMHO. I wonder if we could add our own variables with consistent naming, something like zenburn-fg+2 for "white", and zenburnfg-fg-05 for "#909090". But I think this will be up to the author @bbatsov

If not, I agree to go with your proposal, using variables is always better than static coding.

@ianyepan ianyepan changed the title Add support for centaur tabs Add support for centaur tabs and doom modeline Jul 13, 2019
@ianyepan ianyepan changed the title Add support for centaur tabs and doom modeline Add support for centaur-tabs, doom-modeline, and solaire-mode Jul 13, 2019
@ianyepan
Copy link
Contributor Author

@basil-conto @ema2159 @bbatsov
In addition to centaur-tabs and doom-modeline as @ema2159 suggested, I've also added support for solaire-mode. Now a comprehensive screenshot with (too) many windows open would look something like this:

Screenshot 2019-07-13 at 11 52 02 AM

@ema2159
Copy link

ema2159 commented Jul 13, 2019

Looking really good!

@ianyepan ianyepan mentioned this pull request Jul 13, 2019
zenburn-theme.el Outdated Show resolved Hide resolved
zenburn-theme.el Outdated Show resolved Hide resolved
@bbatsov
Copy link
Owner

bbatsov commented Jul 13, 2019

Thanks for that PR. A bit of small feedback from me:

  • group the related commits into 3-4 by squashing them together
  • ideally we shouldn't introduce new colours to the palette unless there's a very strong reason to do so
  • capitalize commit messages

add support for centaur tabs

use zenburn-yellow as modified tab indication

remove old centaur tabs config

add custom local variable

to be squashed

add support for solaire-mode

rename colour variable
@ianyepan
Copy link
Contributor Author

ianyepan commented Jul 13, 2019

@bbatsov Thanks for the feedback! I have made my commits significantly cleaner, clearing diverging branches and squashing them to an overview commit. I have also capitalized the message and placed fg+2 after fg+1.

Regarding bg-07, it's created solely for solaire-mode, as I experimented with the existing bg-variants and they were either too bright or too dark IMO. I thought about calculating the number 07 precisely when I realized the use of bg-2, bg-1, bg-05, etc were not created with linear proportion either, so I picked a number somewhere in the middle and went with 07. I have now changed it to 08 as it's mathematically closer to the result obtained by the base-16 conversion between bg-1 and bg-05. What do you think?

@ema2159
Copy link

ema2159 commented Jul 14, 2019

If not new colors should be introduced then you could set the solaire background to the same color as the centaur tabs default background and use the already existing zenburn-fgs, all depends on mr @bbatsov opinion

@bbatsov bbatsov merged commit 7e22834 into bbatsov:master Jul 15, 2019
@bbatsov
Copy link
Owner

bbatsov commented Jul 15, 2019

Regarding bg-07, it's created solely for solaire-mode, as I experimented with the existing bg-variants and they were either too bright or too dark IMO. I thought about calculating the number 07 precisely when I realized the use of bg-2, bg-1, bg-05, etc were not created with linear proportion either, so I picked a number somewhere in the middle and went with 07. I have now changed it to 08 as it's mathematically closer to the result obtained by the base-16 conversion between bg-1 and bg-05. What do you think?

Let's give the new colour a shot. We can always change it down the line. Thanks for the in depth explanation of your reasoning!

@ianyepan
Copy link
Contributor Author

Let's give the new colour a shot. We can always change it down the line. Thanks for the in depth explanation of your reasoning!

Sure! Thanks a lot

@ianyepan ianyepan deleted the feature/centaur-tabs-support branch July 16, 2019 10:21
archit-rastogi pushed a commit to archit-rastogi/zenburn-emacs that referenced this pull request May 24, 2020
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.

4 participants