francma / everforest-jetbrains

🌲 Everforest theme for JetBrains
ISC License
19 stars 4 forks source link

Add console color support. #8

Closed frere-jacques closed 1 year ago

francma commented 1 year ago

Did you try to build the theme with these changed colors and use it? I see that you are using some variables that do not exist - for example bg_dim. Also I don't get why some colors use bg variant and some something else lie bg5 or bg_visual.

<option name="CONSOLE_BLUE_OUTPUT">
  <value>
    <option name="FOREGROUND" value="${blue}" />
    <option name="BACKGROUND" value="${bg_blue}" />
  </value>
</option>
<option name="CONSOLE_CYAN_BRIGHT_OUTPUT">
  <value>
    <option name="FOREGROUND" value="${aqua}" />
    <option name="BACKGROUND" value="${bg5}" />
  </value>
</option>
frere-jacques commented 1 year ago

Hmm, I have to confess that I didn't try that. I thought it's straight forward, but I see I should do this, sry.

Regarding bg_dim I just assumed you have the complete color palette available. Is there a reason why you don't use it? Otherwise I would add it.

I used bg5 and bg_visual because these colors fit foreground colors best and there is not a background color for each foreground color. What is your opinion on that?

francma commented 1 year ago

Regarding bg_dim I just assumed you have the complete color palette available. Is there a reason why you don't use it? Otherwise I would add it.

The color palette was updated recently in original Everforest theme. Now it should be updated https://github.com/francma/everforest-jetbrains/commit/62c6639fda3e5de06ad495f78a63d0939877df9b

I used bg5 and bg_visual because these colors fit foreground colors best and there is not a background color for each foreground color. What is your opinion on that?

I would define colors just like in official alacritty theme https://gist.github.com/sainnhe/6432f83181c4520ea87b5211fed27950

frere-jacques commented 1 year ago

Can you add grey2 too or at least accept it if I add it? I can looked now in the notebook and jupyter fields too. I can try to compile and test it. And make a new pull request. There is one thing I couldn;t figure out how to set which is a minor inconvenience when looking in table output of jupyter notebooks and selecting a line. There is a bright highlight and font stays fg. I guess this highlight comes from another point of config.

You mean I should take background and foreground identical? I don't know if that will look good for intellij. There you have eg. Blue BG and FG and Bright Blue BG and FG. I can set Blue and Bright Blue identical, but I guess it's better to have BG and FG different. So I picked the BG colors and for Purple and Aqua the visual and bg5. But I can try it out, if you want.

frere-jacques commented 1 year ago

Okay so I updated my stuff to remove grey2. I also updated the general foreground highlighing to be fg. I didn't update the backgrounds yet. I guess the background and foreground are there for a purpose and I am afraid it would break something. Have a look at this screen. I tested it with my community version. I couldn't test the notebook stuff with that. 20230109Jan011673286042

Edit: And sry, since I am really a noob I have no clue, if I have to do a new pull request, since I am now some commits further.

frere-jacques commented 1 year ago

Did you found time to think about my proposal?

Btw. I found two really annoying colors that appear not to be configurable via ui. I asked on jetbrains page for the labels to replace via theme. Checkout here: https://intellij-support.jetbrains.com/hc/en-us/community/posts/9648252123922-Help-identifying-color-settings

francma commented 1 year ago

Please don't use bg_visual. It would make selecting text invisible.

Also please reword the commit messages and maybe even split it to 2 commits (console support, jupyter support).

And please try to test the colors with some cli programs such as htop, to make sure that everything is working correctly.

frere-jacques commented 1 year ago

Thanks for clarification. I can understand your decision on background visual. I will check how other themes deal with this foreground background topic. And test with htop.

I can put my consolidated edits in two new commits and make a new push release.

frere-jacques commented 1 year ago

Hi Martin,

I fetched freshly from your upstream repo and made three distinct clean commits. Sry for the many small steps till this point. If you don't see any problem with the contributions, could you upload the new version to jetbrains plugins too?

If I find further optimization I will make new updates :).

frere-jacques commented 1 year ago

Is anything still wrong with my contribution?

francma commented 1 year ago

Looks good. Thank you.

I'll upload a new version soon, in the meantime you can use locally build version.