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

Add a warning when a profile has an unknown color scheme #3033

Merged
merged 5 commits into from
Oct 15, 2019

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

Add a warning when the user sets their colorScheme to a scheme that doesn't exists. When that occurs, we'll set their color table to the campbell scheme, to prevent it from being just entirely black.

PR Checklist

@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Oct 2, 2019
src/cascadia/TerminalApp/CascadiaSettings.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/CascadiaSettings.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Profile.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Profile.cpp Outdated Show resolved Hide resolved
@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Oct 2, 2019
src/cascadia/TerminalApp/CascadiaSettings.cpp Show resolved Hide resolved
@@ -590,13 +539,13 @@ ColorScheme* CascadiaSettings::_FindMatchingColorScheme(const Json::Value& schem
{
for (auto& scheme : _globals.GetColorSchemes())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: file a followup here. now that we have a map, no need to iterate all of them. key=name=able to find layering scheme

src/cascadia/TerminalApp/GlobalAppSettings.cpp Outdated Show resolved Hide resolved
src/cascadia/ut_app/JsonTests.cpp Show resolved Hide resolved
src/types/inc/utils.hpp Outdated Show resolved Hide resolved
@cinnamon-msft cinnamon-msft added the Needs-Second It's a PR that needs another sign-off label Oct 14, 2019
@ghost ghost requested review from miniksa and carlos-zamora October 14, 2019 20:26
Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with this with Dustin's comments. @DHowett-MSFT since @zadjii-msft is out, do you want to fix your own nits so we can get this into 1910? Do you want me to fix them for him?

@DHowett-MSFT
Copy link
Contributor

I'll do it and resolve the conflicts.

@miniksa miniksa removed the Needs-Second It's a PR that needs another sign-off label Oct 14, 2019
@ghost
Copy link

ghost commented Oct 23, 2019

🎉Windows Terminal Preview v0.6.2951.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure that all the profiles's color scheme names are actually defined
5 participants