maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Remove the JSON reserializers (except for AKB) (#5918)

Browse source 
The AKB serializer is used in the tracelogging pipeline.

Chatted with @zadjii-msft about ganking the deserializers. The form
they'll take in the future is probably very different from this.

We'll need to have some better tracking of the _source_ or _pass_ a
setting was read during so that we can accurately construct an internal
settings attribution model. Diffing was very extremely cool, but we
didn't end up needing it.

This apparently drops our binary size by a whopping _zero bytes_ because
the optimizer was smarter than us and actually totally deleted it.
This commit is contained in:
Dustin L. Howett (MSFT) 2020-05-15 14:51:57 -07:00 committed by GitHub
parent 71e29b1617
commit 6394e5d70b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 2 additions and 298 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file

@ -783,7 +783,7 @@ namespace TerminalAppLocalTests
VERIFY_IS_FALSE(profile0._guid.has_value());
const auto serialized0Profile = profile0.ToJson();
const auto serialized0Profile = profile0.GenerateStub();
const auto profile1 = Profile::FromJson(serialized0Profile);
VERIFY_IS_FALSE(profile0._guid.has_value());
VERIFY_ARE_EQUAL(profile1._guid.has_value(), profile0._guid.has_value());
@ -794,7 +794,7 @@ namespace TerminalAppLocalTests
VERIFY_IS_TRUE(settings._profiles.at(0)._guid.has_value());
const auto serialized1Profile = settings._profiles.at(0).ToJson();
const auto serialized1Profile = settings._profiles.at(0).GenerateStub();
const auto profile2 = Profile::FromJson(serialized1Profile);
VERIFY_IS_TRUE(settings._profiles.at(0)._guid.has_value());

26
src/cascadia/TerminalApp/ColorScheme.cpp
View file

@ -82,32 +82,6 @@ void ColorScheme::ApplyScheme(TerminalSettings terminalSettings) const
}
}
// Method Description:
// - Serialize this object to a JsonObject.
// Arguments:
// - <none>
// Return Value:
// - a JsonObject which is an equivalent serialization of this object.
Json::Value ColorScheme::ToJson() const
{
Json::Value root;
root[JsonKey(NameKey)] = winrt::to_string(_schemeName);
root[JsonKey(ForegroundKey)] = Utils::ColorToHexString(_defaultForeground);
root[JsonKey(BackgroundKey)] = Utils::ColorToHexString(_defaultBackground);
root[JsonKey(SelectionBackgroundKey)] = Utils::ColorToHexString(_selectionBackground);
root[JsonKey(CursorColorKey)] = Utils::ColorToHexString(_cursorColor);
int i = 0;
for (const auto& colorName : TableColors)
{
auto& colorValue = _table.at(i);
root[JsonKey(colorName)] = Utils::ColorToHexString(colorValue);
i++;
}
return root;
}
// Method Description:
// - Create a new instance of this class from a serialized JsonObject.
// Arguments:

1
src/cascadia/TerminalApp/ColorScheme.h
View file

@ -40,7 +40,6 @@ public:
void ApplyScheme(winrt::Microsoft::Terminal::Settings::TerminalSettings terminalSettings) const;
Json::Value ToJson() const;
static ColorScheme FromJson(const Json::Value& json);
bool ShouldBeLayered(const Json::Value& json) const;
void LayerJson(const Json::Value& json);

34
src/cascadia/TerminalApp/GlobalAppSettings.cpp
View file

@ -249,40 +249,6 @@ void GlobalAppSettings::ApplyToSettings(TerminalSettings& settings) const noexce
settings.SoftwareRendering(_softwareRendering);
}
// Method Description:
// - Serialize this object to a JsonObject.
// Arguments:
// - <none>
// Return Value:
// - a JsonObject which is an equivalent serialization of this object.
Json::Value GlobalAppSettings::ToJson() const
{
Json::Value jsonObject;
jsonObject[JsonKey(DefaultProfileKey)] = winrt::to_string(Utils::GuidToString(_defaultProfile));
jsonObject[JsonKey(InitialRowsKey)] = _initialRows;
jsonObject[JsonKey(InitialColsKey)] = _initialCols;
jsonObject[JsonKey(RowsToScrollKey)] = _rowsToScroll;
jsonObject[JsonKey(InitialPositionKey)] = _SerializeInitialPosition(_initialX, _initialY);
jsonObject[JsonKey(AlwaysShowTabsKey)] = _alwaysShowTabs;
jsonObject[JsonKey(ShowTitleInTitlebarKey)] = _showTitleInTitlebar;
jsonObject[JsonKey(ShowTabsInTitlebarKey)] = _showTabsInTitlebar;
jsonObject[JsonKey(WordDelimitersKey)] = winrt::to_string(_wordDelimiters);
jsonObject[JsonKey(CopyOnSelectKey)] = _copyOnSelect;
jsonObject[JsonKey(CopyFormattingKey)] = _copyFormatting;
jsonObject[JsonKey(LaunchModeKey)] = winrt::to_string(_SerializeLaunchMode(_launchMode));
jsonObject[JsonKey(ThemeKey)] = winrt::to_string(_SerializeTheme(_theme));
jsonObject[JsonKey(TabWidthModeKey)] = winrt::to_string(_SerializeTabWidthMode(_tabWidthMode));
jsonObject[JsonKey(KeybindingsKey)] = _keybindings->ToJson();
jsonObject[JsonKey(ConfirmCloseAllKey)] = _confirmCloseAllTabs;
jsonObject[JsonKey(SnapToGridOnResizeKey)] = _SnapToGridOnResize;
jsonObject[JsonKey(ForceFullRepaintRenderingKey)] = _forceFullRepaintRendering;
jsonObject[JsonKey(SoftwareRenderingKey)] = _softwareRendering;
jsonObject[JsonKey(DebugFeaturesKey)] = _debugFeatures;
return jsonObject;
}
// Method Description:
// - Create a new instance of this class from a serialized JsonObject.
// Arguments:

1
src/cascadia/TerminalApp/GlobalAppSettings.h
View file

@ -82,7 +82,6 @@ public:
bool DebugFeaturesEnabled() const noexcept;
Json::Value ToJson() const;
static GlobalAppSettings FromJson(const Json::Value& json);
void LayerJson(const Json::Value& json);

177
src/cascadia/TerminalApp/Profile.cpp
View file

@ -263,183 +263,6 @@ TerminalSettings Profile::CreateTerminalSettings(const std::unordered_map<std::w
return terminalSettings;
}
// Method Description:
// - Serialize this object to a JsonObject.
// Arguments:
// - <none>
// Return Value:
// - a JsonObject which is an equivalent serialization of this object.
Json::Value Profile::ToJson() const
{
Json::Value root = GenerateStub();
///// Profile-specific settings /////
// As of #2795, all profile-specific settings were moved to GenerateStub. If
// any new profiles-specific settings are added, they should probably be
// added here instead of in that method.
///// Core Settings /////
if (_defaultForeground)
{
root[JsonKey(ForegroundKey)] = Utils::ColorToHexString(_defaultForeground.value());
}
if (_defaultBackground)
{
root[JsonKey(BackgroundKey)] = Utils::ColorToHexString(_defaultBackground.value());
}
if (_selectionBackground)
{
root[JsonKey(SelectionBackgroundKey)] = Utils::ColorToHexString(_selectionBackground.value());
}
if (_cursorColor)
{
root[JsonKey(CursorColorKey)] = Utils::ColorToHexString(_cursorColor.value());
}
if (_schemeName)
{
const auto scheme = winrt::to_string(_schemeName.value());
root[JsonKey(ColorSchemeKey)] = scheme;
}
root[JsonKey(HistorySizeKey)] = _historySize;
root[JsonKey(SnapOnInputKey)] = _snapOnInput;
// Only add the cursor height property if we're a legacy-style cursor.
if (_cursorShape == CursorStyle::Vintage)
{
root[JsonKey(CursorHeightKey)] = _cursorHeight;
}
root[JsonKey(CursorShapeKey)] = winrt::to_string(_SerializeCursorStyle(_cursorShape));
root[JsonKey(CommandlineKey)] = winrt::to_string(_commandline);
root[JsonKey(FontFaceKey)] = winrt::to_string(_fontFace);
root[JsonKey(FontSizeKey)] = _fontSize;
root[JsonKey(AcrylicTransparencyKey)] = _acrylicTransparency;
root[JsonKey(UseAcrylicKey)] = _useAcrylic;
root[JsonKey(PaddingKey)] = winrt::to_string(_padding);
if (_connectionType)
{
root[JsonKey(ConnectionTypeKey)] = winrt::to_string(Utils::GuidToString(_connectionType.value()));
}
if (_scrollbarState)
{
const auto scrollbarState = winrt::to_string(_scrollbarState.value());
root[JsonKey(ScrollbarStateKey)] = scrollbarState;
}
if (_icon)
{
const auto icon = winrt::to_string(_icon.value());
root[JsonKey(IconKey)] = icon;
}
if (_tabTitle)
{
root[JsonKey(TabTitleKey)] = winrt::to_string(_tabTitle.value());
}
if (_suppressApplicationTitle)
{
root[JsonKey(SuppressApplicationTitleKey)] = _suppressApplicationTitle;
}
if (_startingDirectory)
{
root[JsonKey(StartingDirectoryKey)] = winrt::to_string(_startingDirectory.value());
}
if (_backgroundImage)
{
root[JsonKey(BackgroundImageKey)] = winrt::to_string(_backgroundImage.value());
}
if (_backgroundImageOpacity)
{
root[JsonKey(BackgroundImageOpacityKey)] = _backgroundImageOpacity.value();
}
if (_backgroundImageStretchMode)
{
root[JsonKey(BackgroundImageStretchModeKey)] = SerializeImageStretchMode(_backgroundImageStretchMode.value()).data();
}
if (_backgroundImageAlignment)
{
root[JsonKey(BackgroundImageAlignmentKey)] = SerializeImageAlignment(_backgroundImageAlignment.value()).data();
}
root[JsonKey(CloseOnExitKey)] = _SerializeCloseOnExitMode(_closeOnExitMode).data();
if (_retroTerminalEffect)
{
root[JsonKey(RetroTerminalEffectKey)] = _retroTerminalEffect.value();
}
root[JsonKey(AntialiasingModeKey)] = SerializeTextAntialiasingMode(_antialiasingMode).data();
return root;
}
// Method Description:
// - This generates a json object `diff` s.t.
// this = other.LayerJson(diff)
// So if:
// - this has a nullopt for an optional, diff will have null for that member
// - this has a value for an optional, diff will have our value. If the other
// did _not_ have a value, and we did, diff will have our value.
// Arguments:
// - other: the other profile object to use as the "base" for this diff. The
// result could be layered upon that json object to re-create this object's
// serialization.
// Return Value:
// - a diff between this and the other object, such that this could be recreated
// from the diff and the other object.
Json::Value Profile::DiffToJson(const Profile& other) const
{
auto otherJson = other.ToJson();
auto myJson = ToJson();
Json::Value diff;
// Iterate in two steps:
// - first over all the keys in the 'other' object's serialization.
// - then over all the keys in our serialization.
// In this way, we ensure all keys from both objects are present in the
// final object.
for (const auto& key : otherJson.getMemberNames())
{
if (myJson.isMember(key))
{
// Both objects have the key
auto otherVal = otherJson[key];
auto myVal = myJson[key];
if (otherVal != myVal)
{
diff[key] = myVal;
}
}
else
{
// key is not in this json object. Set to null, so that when the
// diff is layered upon the original object, we'll properly set
// nullopt for any optionals that weren't present in this object.
diff[key] = Json::Value::null;
}
}
for (const auto& key : myJson.getMemberNames())
{
if (otherJson.isMember(key))
{
// both objects have this key. Do nothing, this is handled above
}
else
{
// We have a key the other object did not. Add our value.
diff[key] = myJson[key];
}
}
return diff;
}
// Method Description:
// - Generates a Json::Value which is a "stub" of this profile. This stub will
// have enough information that it could be layered with this profile.

2
src/cascadia/TerminalApp/Profile.h
View file

@ -54,8 +54,6 @@ public:
winrt::Microsoft::Terminal::Settings::TerminalSettings CreateTerminalSettings(const std::unordered_map<std::wstring, ColorScheme>& schemes) const;
Json::Value ToJson() const;
Json::Value DiffToJson(const Profile& other) const;
Json::Value GenerateStub() const;
static Profile FromJson(const Json::Value& json);
bool ShouldBeLayered(const Json::Value& json) const;

55
src/cascadia/ut_app/JsonTests.cpp
View file

@ -27,8 +27,6 @@ namespace TerminalAppUnitTests
TEST_METHOD(ParseInvalidJson);
TEST_METHOD(ParseSimpleColorScheme);
TEST_METHOD(ProfileGeneratesGuid);
TEST_METHOD(DiffProfile);
TEST_METHOD(DiffProfileWithNull);
TEST_METHOD(TestWrongValueType);
@ -172,59 +170,6 @@ namespace TerminalAppUnitTests
VERIFY_ARE_EQUAL(profile4.GetGuid(), cmdGuid);
}
void JsonTests::DiffProfile()
{
Profile profile0;
Profile profile1;
Log::Comment(NoThrowString().Format(
L"Both these profiles are the same, their diff should have _no_ values"));
auto diff = profile1.DiffToJson(profile0);
Log::Comment(NoThrowString().Format(L"diff:%hs", Json::writeString(_builder, diff).c_str()));
VERIFY_ARE_EQUAL(0u, diff.getMemberNames().size());
profile1._name = L"profile1";
diff = profile1.DiffToJson(profile0);
Log::Comment(NoThrowString().Format(L"diff:%hs", Json::writeString(_builder, diff).c_str()));
VERIFY_ARE_EQUAL(1u, diff.getMemberNames().size());
}
void JsonTests::DiffProfileWithNull()
{
Profile profile0;
Profile profile1;
profile0._icon = L"foo";
Log::Comment(NoThrowString().Format(
L"Case 1: Base object has an optional that the derived does not - diff will have null for that value"));
auto diff = profile1.DiffToJson(profile0);
Log::Comment(NoThrowString().Format(L"diff:%hs", Json::writeString(_builder, diff).c_str()));
VERIFY_ARE_EQUAL(1u, diff.getMemberNames().size());
VERIFY_IS_TRUE(diff.isMember("icon"));
VERIFY_IS_TRUE(diff["icon"].isNull());
Log::Comment(NoThrowString().Format(
L"Case 2: Add an optional to the derived object that's not present in the root."));
profile0._icon = std::nullopt;
profile1._icon = L"bar";
diff = profile1.DiffToJson(profile0);
Log::Comment(NoThrowString().Format(L"diff:%hs", Json::writeString(_builder, diff).c_str()));
VERIFY_ARE_EQUAL(1u, diff.getMemberNames().size());
VERIFY_IS_TRUE(diff.isMember("icon"));
VERIFY_IS_TRUE(diff["icon"].isString());
VERIFY_IS_TRUE("bar" == diff["icon"].asString());
}
void JsonTests::TestWrongValueType()
{
// This json blob has a whole bunch of settings with the wrong value