Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 :).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants