-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Remove the Solarized color schemes from the defaults #6617
Conversation
If users want these, they can add them back themselves.
I'm very much in favour of getting these schemes out of the defaults, but I did wonder whether it might be necessary to include some sort of backwards-compatibility hack that auto-updated users that were already using Solarized, maybe patching their settings with a custom instance of the scheme? I'm not sure if that's feasible, or worth the effort, but perhaps it's worth checking the telemetry to get an idea of how many users this would actually affect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dunno, I think I agree that we can't just break people who are currently using solarized. I do agree that it needs to be taken to a farm upstate.
I think the "patch it in" solution isn't bad - remove it from the defaults, and just insert the theme into their settings.json manually if they've currently got it set.
If it counts for anything VS Code ships modified solarized themes VS Code Solarized Dark This is a snip of the terminal color portion. I'm not sure which background and foreground is used for the terminal. It could be "editor.background"?
And a reference to #4480 where more discussion on the topic occurred |
I agree with @soul4soul . It would be easier to fix the bug in the short term without breaking existing configurations if Terminal simply modified the Solarized themes so that none of the foreground colors are identical to the background color. The biggest problem with Solarized isn't that the theme is broken (which, to be clear, it is. There's no good reason to have matching foreground and background colors), the biggest problem is that Terminal treats "bold" text as being bright white specifically, which makes all of the light-background themes bad. I gathered some information for #109, and in doing so I found that most terminal emulators, in their default configuration, have at least one matching foreground and background color, producing invisible text. Yet those emulators don't (as far as I can tell) suffer the steady stream of bug reports that Windows Terminal does. I'm convinced that this bug is unique, and specific to Windows Terminal, simply because it is the only terminal emulator where "bold" means "white". |
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment. |
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment. |
I'd like to put a pin in this for now, and come back to it when we have the ability to patch it into a user's settings if she's using Solarized. Gonna close pending discussion in #6618. |
Solarized is not suitable for Terminal use, and as such, these themes are a bug factory.
If users want these, they can add them back themselves.
There's a number of discussions, both open and closed, about Solarized's insuitability as a default scheme. Here's my proposal: everyone who is using Solarized today will be using Campbell tomorrow. We will get an influx of issues complaining about the colors not working, but we will stop getting reports that "i can't see my text" and "you were mistaken for including these."
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed