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

Re-add theme as Solarized Dark - Patched #63

Merged
merged 2 commits into from
Jan 16, 2016
Merged

Conversation

jez
Copy link
Contributor

@jez jez commented Jan 12, 2016

I'm open to the name of this theme, but I chose "Solarized Dark - Patched" for
simplicity. I also added a note to the README to clarify the differences between these two schemes.

Fixes #62.

@jez
Copy link
Contributor Author

jez commented Jan 12, 2016

And for posterity's sake, here are some screenshots that illustrate the problem: the first is an issue in Vim using the patched theme that isn't present in the non-patched, and the second is a bug when the usage is flipped.

screenshot 2016-01-12 00 51 08
screenshot 2016-01-12 00 53 37

@mbadolato
Copy link
Owner

Fantastic, thank you! I'll take a look at it in depth tonight when I get home. Thanks Jake!

mbadolato added a commit that referenced this pull request Jan 16, 2016
Re-add theme as Solarized Dark - Patched
@mbadolato mbadolato merged commit b307737 into mbadolato:master Jan 16, 2016
@kevin-smets
Copy link
Contributor

Hmmm, why provide a patched version and not fix the original one? Just wondering...

@jez
Copy link
Contributor Author

jez commented Jan 21, 2016

That was precisely my point, and the point made by altercation; neither solution perfectly "fixes" the issue. The "fix" would be to rewrite ANSI escape codes to handle colors in a sane way instead of forcing people into a 16-color palette.

If you look at the screenshots above, you can see that using the patched version makes Vim look bad, and using the non-patched version makes other programs look bad.

Consider reading this thread for more information.

@kevin-smets
Copy link
Contributor

Alright, thanks for the clarification and the info :).

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.

3 participants