Skip to content

Deduplicate identical inbox color schemes to heal user settings #12800

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.

Sign up for GitHub

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

Merged
DHowett merged 23 commits into from
Feb 27, 2024
Merged

Deduplicate identical inbox color schemes to heal user settings #12800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
924906a
It works?
DHowett Mar 25, 2022
a31b089
Move to FixupUserSettings, add a bunch of tests
DHowett Apr 24, 2022
56fad52
Migrate spelling-0.0.21 changes from main
DHowett Apr 24, 2022
29ff259
Merge remote-tracking branch 'origin/main' into dev/duhowett/fhl-2022…
DHowett Aug 20, 2023
8931ccf
Merge remote-tracking branch 'origin/main' into dev/duhowett/fhl-2022…
DHowett Sep 13, 2023
00a3f6e
Fix for main
DHowett Sep 27, 2023
6763baa
Merge remote-tracking branch 'origin/main' into dev/duhowett/fhl-2022…
DHowett Feb 23, 2024
1af99be
Merge remote-tracking branch 'origin/main' into dev/duhowett/fhl-2022…
DHowett Feb 23, 2024
2ada4fc
Force users to duplicate inbox schemes; make duplicate work
DHowett Feb 23, 2024
9086e69
Fix tests, Modified->modified
DHowett Feb 27, 2024
ff0bc39
Add collision detection
DHowett Feb 27, 2024
29d6eb1
reword
DHowett Feb 27, 2024
aac7f5a
comments
DHowett Feb 27, 2024
868aa1c
one spell
DHowett Feb 27, 2024
7f217e1
two spell
DHowett Feb 27, 2024
2b39c82
Merge remote-tracking branch 'origin/main' into dev/duhowett/fhl-2022…
DHowett Feb 27, 2024
50ddf7d
Fix an outdated test
DHowett Feb 27, 2024
f1341e8
mix some dark schemes in
DHowett Feb 27, 2024
3f8769f
Fix up code review nits and bugs!
DHowett Feb 27, 2024
f30e807
Add a test for a totally unused scheme too
DHowett Feb 27, 2024
36ee148
Add a comment for future generations
DHowett Feb 27, 2024
f2d3910
flip IsInBox to Editable, remove disclaimer for real
DHowett Feb 27, 2024
8440309
Track fixups made during load, rather than reverse engineering it
DHowett Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/actions/spelling/allow/allow.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ ptys
pwshw
qof
qps
Remappings
Retargets
rclt
reimplementation
reserialization
Expand Down
708 changes: 683 additions & 25 deletions src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp
View file
Open in desktop

Large diffs are not rendered by default.

306 changes: 306 additions & 0 deletions src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ namespace SettingsModelLocalTests
TEST_METHOD(RoundtripReloadEnvVars);
TEST_METHOD(DontRoundtripNoReloadEnvVars);

TEST_METHOD(RoundtripUserModifiedColorSchemeCollision);
TEST_METHOD(RoundtripUserModifiedColorSchemeCollisionUnusedByProfiles);
TEST_METHOD(RoundtripUserDeletedColorSchemeCollision);

private:
// Method Description:
// - deserializes and reserializes a json string representing a settings object model of type T
Expand Down Expand Up @@ -630,4 +634,306 @@ namespace SettingsModelLocalTests
VERIFY_IS_FALSE(newSettings->ProfileDefaults().HasReloadEnvironmentVariables(),
L"Ensure that the new settings object didn't find a reloadEnvironmentVariables");
}

void SerializationTests::RoundtripUserModifiedColorSchemeCollision()
{
static constexpr std::string_view oldSettingsJson{ R"(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}"
},
{
"name": "profile1",
"colorScheme": "Tango Dark",
"guid": "{d0a65a9d-8665-4128-97a4-a581aa747aa7}"
}
],
"schemes": [
{
"background": "#121314",
"black": "#121314",
"blue": "#121314",
"brightBlack": "#121314",
"brightBlue": "#121314",
"brightCyan": "#121314",
"brightGreen": "#121314",
"brightPurple": "#121314",
"brightRed": "#121314",
"brightWhite": "#121314",
"brightYellow": "#121314",
"cursorColor": "#121314",
"cyan": "#121314",
"foreground": "#121314",
"green": "#121314",
"name": "Campbell",
"purple": "#121314",
"red": "#121314",
"selectionBackground": "#121314",
"white": "#121314",
"yellow": "#121314"
},
{
"background": "#000000",
"black": "#000000",
"blue": "#3465A4",
"brightBlack": "#555753",
"brightBlue": "#729FCF",
"brightCyan": "#34E2E2",
"brightGreen": "#8AE234",
"brightPurple": "#AD7FA8",
"brightRed": "#EF2929",
"brightWhite": "#EEEEEC",
"brightYellow": "#FCE94F",
"cursorColor": "#FFFFFF",
"cyan": "#06989A",
"foreground": "#D3D7CF",
"green": "#4E9A06",
"name": "Tango Dark",
"purple": "#75507B",
"red": "#CC0000",
"selectionBackground": "#FFFFFF",
"white": "#D3D7CF",
"yellow": "#C4A000"
},
]
})" };

// Key differences: one fewer color scheme (Tango Dark has been deleted) and defaults.colorScheme is set.
static constexpr std::string_view newSettingsJson{ R"-(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles":
{
"defaults": {
"colorScheme": "Campbell (modified)"
},
"list":
[
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}"
},
{
"name": "profile1",
"colorScheme": "Tango Dark",
"guid": "{d0a65a9d-8665-4128-97a4-a581aa747aa7}"
}
]
},
"actions": [ ],
"schemes": [
{
"background": "#121314",
"black": "#121314",
"blue": "#121314",
"brightBlack": "#121314",
"brightBlue": "#121314",
"brightCyan": "#121314",
"brightGreen": "#121314",
"brightPurple": "#121314",
"brightRed": "#121314",
"brightWhite": "#121314",
"brightYellow": "#121314",
"cursorColor": "#121314",
"cyan": "#121314",
"foreground": "#121314",
"green": "#121314",
"name": "Campbell",
"purple": "#121314",
"red": "#121314",
"selectionBackground": "#121314",
"white": "#121314",
"yellow": "#121314"
}
]
})-" };

implementation::SettingsLoader oldLoader{ oldSettingsJson, DefaultJson };
oldLoader.MergeInboxIntoUserSettings();
oldLoader.FinalizeLayering();
VERIFY_IS_TRUE(oldLoader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
const auto oldSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(oldLoader));
const auto oldResult{ oldSettings->ToJson() };

implementation::SettingsLoader newLoader{ newSettingsJson, DefaultJson };
newLoader.MergeInboxIntoUserSettings();
newLoader.FinalizeLayering();
newLoader.FixupUserSettings();
const auto newSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(newLoader));
const auto newResult{ newSettings->ToJson() };

VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult));
}

void SerializationTests::RoundtripUserModifiedColorSchemeCollisionUnusedByProfiles()
{
static constexpr std::string_view oldSettingsJson{ R"(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}"
}
],
"schemes": [
{
"background": "#111111",
"black": "#111111",
"blue": "#111111",
"brightBlack": "#111111",
"brightBlue": "#111111",
"brightCyan": "#111111",
"brightGreen": "#111111",
"brightPurple": "#111111",
"brightRed": "#111111",
"brightWhite": "#111111",
"brightYellow": "#111111",
"cursorColor": "#111111",
"cyan": "#111111",
"foreground": "#111111",
"green": "#111111",
"name": "Tango Dark",
"purple": "#111111",
"red": "#111111",
"selectionBackground": "#111111",
"white": "#111111",
"yellow": "#111111"
},
]
})" };

// Key differences: Tango Dark has been renamed; nothing else has changed
static constexpr std::string_view newSettingsJson{ R"-(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles":
{
"list":
[
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}"
}
]
},
"actions": [ ],
"schemes": [
{
"background": "#111111",
"black": "#111111",
"blue": "#111111",
"brightBlack": "#111111",
"brightBlue": "#111111",
"brightCyan": "#111111",
"brightGreen": "#111111",
"brightPurple": "#111111",
"brightRed": "#111111",
"brightWhite": "#111111",
"brightYellow": "#111111",
"cursorColor": "#111111",
"cyan": "#111111",
"foreground": "#111111",
"green": "#111111",
"name": "Tango Dark (modified)",
"purple": "#111111",
"red": "#111111",
"selectionBackground": "#111111",
"white": "#111111",
"yellow": "#111111"
},
]
})-" };

implementation::SettingsLoader oldLoader{ oldSettingsJson, DefaultJson };
oldLoader.MergeInboxIntoUserSettings();
oldLoader.FinalizeLayering();
VERIFY_IS_TRUE(oldLoader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
const auto oldSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(oldLoader));
const auto oldResult{ oldSettings->ToJson() };

implementation::SettingsLoader newLoader{ newSettingsJson, DefaultJson };
newLoader.MergeInboxIntoUserSettings();
newLoader.FinalizeLayering();
newLoader.FixupUserSettings();
const auto newSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(newLoader));
const auto newResult{ newSettings->ToJson() };

VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult));
}

void SerializationTests::RoundtripUserDeletedColorSchemeCollision()
{
static constexpr std::string_view oldSettingsJson{ R"(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}"
}
],
"schemes": [
{
"name": "Tango Dark",
"foreground": "#D3D7CF",
"background": "#000000",
"cursorColor": "#FFFFFF",
"black": "#000000",
"red": "#CC0000",
"green": "#4E9A06",
"yellow": "#C4A000",
"blue": "#3465A4",
"purple": "#75507B",
"cyan": "#06989A",
"white": "#D3D7CF",
"brightBlack": "#555753",
"brightRed": "#EF2929",
"brightGreen": "#8AE234",
"brightYellow": "#FCE94F",
"brightBlue": "#729FCF",
"brightPurple": "#AD7FA8",
"brightCyan": "#34E2E2",
"brightWhite": "#EEEEEC"
}
]
})" };

// Key differences: Tango Dark has been deleted, as it was identical to the inbox one.
static constexpr std::string_view newSettingsJson{ R"-(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles":
{
"list":
[
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}"
}
]
},
"actions": [ ],
"schemes": [ ]
})-" };

implementation::SettingsLoader oldLoader{ oldSettingsJson, DefaultJson };
oldLoader.MergeInboxIntoUserSettings();
oldLoader.FinalizeLayering();
VERIFY_IS_TRUE(oldLoader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
const auto oldSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(oldLoader));
const auto oldResult{ oldSettings->ToJson() };

implementation::SettingsLoader newLoader{ newSettingsJson, DefaultJson };
newLoader.MergeInboxIntoUserSettings();
newLoader.FinalizeLayering();
newLoader.FixupUserSettings();
const auto newSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(newLoader));
const auto newResult{ newSettings->ToJson() };

VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult));
}
}
13 changes: 13 additions & 0 deletions src/cascadia/TerminalSettingsEditor/ColorSchemeViewModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_NotifyChanges(L"IsDefaultScheme");
}

bool ColorSchemeViewModel::IsEditable() const
{
return _scheme.Origin() == Model::OriginTag::User;
}

bool ColorSchemeViewModel::RequestRename(winrt::hstring newName)
{
if (const auto parentPageVM{ _parentPageVM.get() })
Expand Down Expand Up @@ -126,6 +131,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}
}

void ColorSchemeViewModel::Duplicate_Click(const IInspectable& /*sender*/, const Windows::UI::Xaml::RoutedEventArgs& /*e*/)
{
if (const auto parentPageVM{ _parentPageVM.get() })
{
return parentPageVM.RequestDuplicateCurrentScheme();
}
}

void ColorSchemeViewModel::DeleteConfirmation_Click(const IInspectable& /*sender*/, const Windows::UI::Xaml::RoutedEventArgs& /*e*/)
{
if (const auto parentPageVM{ _parentPageVM.get() })
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettingsEditor/ColorSchemeViewModel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
Editor::ColorTableEntry ColorEntryAt(uint32_t index);
bool IsDefaultScheme();
void RefreshIsDefault();
bool IsEditable() const;

void DeleteConfirmation_Click(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e);
void SetAsDefault_Click(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e);
void Duplicate_Click(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e);

// DON'T YOU DARE ADD A `WINRT_CALLBACK(PropertyChanged` TO A CLASS DERIVED FROM ViewModelHelper. Do this instead:
using ViewModelHelper<ColorSchemeViewModel>::PropertyChanged;

WINRT_PROPERTY(bool, IsInBoxScheme);
WINRT_PROPERTY(Windows::Foundation::Collections::IVector<Editor::ColorTableEntry>, NonBrightColorTable, nullptr);
WINRT_PROPERTY(Windows::Foundation::Collections::IVector<Editor::ColorTableEntry>, BrightColorTable, nullptr);

Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettingsEditor/ColorSchemeViewModel.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Microsoft.Terminal.Settings.Editor
runtimeclass ColorSchemeViewModel : Windows.UI.Xaml.Data.INotifyPropertyChanged, Windows.Foundation.IStringable
{
String Name;
Boolean IsInBoxScheme;
Boolean IsEditable { get; };
Boolean IsDefaultScheme { get; };

Boolean RequestRename(String newName);
Expand All @@ -27,6 +27,7 @@ namespace Microsoft.Terminal.Settings.Editor

void DeleteConfirmation_Click(IInspectable sender, Windows.UI.Xaml.RoutedEventArgs args);
void SetAsDefault_Click(IInspectable sender, Windows.UI.Xaml.RoutedEventArgs args);
void Duplicate_Click(IInspectable sender, Windows.UI.Xaml.RoutedEventArgs args);
}

[default_interface] runtimeclass ColorTableEntry : Windows.UI.Xaml.Data.INotifyPropertyChanged
Expand Down
Loading