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 icon colors of sorin theme in other terminal themes. #584

Closed
wants to merge 1 commit into from
Closed

fix icon colors of sorin theme in other terminal themes. #584

wants to merge 1 commit into from

Conversation

alexanderjeurissen
Copy link

By default the sorin theme colors don't render correctly in several terminal themes for example solarized dark & light.

the screenshot below shows how the sorin theme looks by default in Iterm 2 + solarized dark:
screen shot 2014-03-30 at 15 49 31

as you can see, both the color of second and third arrow and the git icons don't render correctly.

the screenshot below shows the result after applying this commit:
screen shot 2014-03-30 at 15 52 36

As you can see after applying this commit both the icons and arrows render correctly.

@pablox-cl
Copy link

See #382... as Sorin have pointed in a lot of problems about solarized:

  • It works for him, so there's no real issue. The issue it's because every terminal is faulty except the one that he uses.
  • Solarized is overrated and a pain in the rectum (it seems that PITA it's not enough).

(I finally had to agree with the second point, the first one, not at all)

FWIW, the discussion on that issue explains which is the problem.

@alexanderjeurissen
Copy link
Author

I have to agree with you to a certain extend.
I think the first point is kind of bs, the "works on my machine so there is no real issue" statement is ignorant and naive and besides that untrue.

The second point I can understand it's hard to make a theme work for all different terminal configurations. but to state that Solarized ( a very popular theme ) is overrated is exaggerated and personal opinion.

I'm aware of the issue, im just not sure why it's a big deal to support a major terminal theme/configuration.

But this issue/pr can be closed i see there are similar pr's so that makes this pr unnecessary.
I will keep an eye on the discussion off issue #382

thx for the explanation.

@pablox-cl
Copy link

I was just saying what I believe @sorin-ionescu was I going to say. I don't agree with the first statement at all.

Solarized is a PITA, though I agree that being so popular it deserves more attention. In this particular case, I strongly believe it shouldn't use "bold" colors (what your patch it's a about).

Anyway, if you think those changes should be merged (I do), I think you need to insist on that PR ;)

@sorin-ionescu
Copy link
Owner

This issue is a duplicate of #382. Your iTerm2 Solarized theme is broken. The screenshots bellow are Solarized in iTerm2.

Solarized Dark
Solarized Light

@alexanderjeurissen
Copy link
Author

@sorin-ionescu I'm curious how that can be, i have the official solarized theme for Iterm 2, i just reapplied the preset to a new profile and still have the same issues, my patch fixes it but the bold colors don't seem to work..

screen shot 2014-03-30 at 21 22 20
screen shot 2014-03-30 at 21 22 32

could you link the solarized theme your using ?

@sorin-ionescu
Copy link
Owner

I am using my own.

@alexanderjeurissen
Copy link
Author

@sorin-ionescu i applied your version of solarized and that fixed the issue i was having.

@pablox-cl
Copy link

That means you are "fixing" the issue by changing your terminal colours, and then every other terminal is faulty?. From what I understand from your commit, you are fixing the sort of "wrong" colours you had in your prompt first.

You use "bright" colours (a feature of the terminal emulator itself), and it's "emulating" a brighter "bold red", instead showing the shades of grey (pun intended) that the original solarized pretended (16 colors, 8 grey and 8 accent colors; you are virtually using more colors, 8 greys, 8 original accent and 8 "brighter" accent).

@sorin-ionescu
Copy link
Owner

@pablox-cl The iTerm2 Solarized theme is faulty. Hence my pull request. Other Solarized implementations for other terminals may or may not be faulty.

@alexanderjeurissen
Copy link
Author

i have to agree with @pablox-cl on this one, i took another look at my iterm2 configuration and it seemed i had "use bold colors" checked which screwed up the colors.

however i still think that his or my PR or Pablo's PR should be merged because now several terminal configurations and themes need workarounds just to render correctly while removing the bold colors (IMO a way easier fix) doesn't affect other themes as much as the current theme affects Solarized.

I tested my PR on several terminal themes including several base 16 variants and even though i removed the bold font it still renders correctly.

therefore I'm not sure of the added value of the bold colors.

@sorin-ionescu
Copy link
Owner

You presume that themes in Prezto ought to revolve around Solarized, a colour scheme that requires hacks in all of its implementations, whether a terminal, a shell, or a text editor, when in fact, that is not the case.

@alexanderjeurissen
Copy link
Author

I'm just saying that in my tests the bold colors don't add much in several other terminal themes and configurations while not rendering correctly in solarized. hence the question why do we use bold colors anyway.

@sorin-ionescu
Copy link
Owner

Bold colours add emphases. It depends on the terminal colour scheme on whether they are useful, of which, there are many out there.

@pablox-cl
Copy link

Most terminals supports 16 colours. Solarized is meant to work with 16 colours (same with base16). If your terminal is properly configured solarized works fine. You built the sorin theme using a feature exclusive to iterm2. I already explained that in the solarized colourscheme bold red doesn't exist at all:

Solarized 16 colours palette

Bold red is equivalent is "base01". Not the bold red that only iTerm2 can render. I'm not saying prezto themes should revolve around solarized, but when solarized didn't worked in your terminal emulator, you changed the wrong thing: iterm conf instead your prompt colours.

@alexanderjeurissen , btw, likely your PR it's better because you remove every called to "bold colours". Anyway, you can always can copy your own version of your sorin theme or you can use a prompt like pure or colour palette like base16.

@sorin-ionescu
Copy link
Owner

I have changed the iTerm2 Solarized colour scheme based on the Terminal.app Solarized 256-colour scheme because colours were not showing up properly in Vim and elsewhere.

I don't use Solarized anymore. Between the terminal, the shell, the multiplexer, and the editor, it's a pain in the rectum.

There are other themes in Prezto and Zsh that use bold colours, not just my own. They are not broken for using bold colours. It's Solarized that's the problem. So take whatever theme you like, and remove bold colours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants