Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/ApplicationState.idl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Microsoft.Terminal.Settings.Model
{
enum InfoBarMessage
{
// We need a sentinel value to map deleted keys to, as we can't actually delete them (GH#16874)
// Unknown and deprecated dismissed message names map to this sentinel value.
IgnoredDeprecatedEntry = 0,
CloseOnExitInfo,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,16 +588,57 @@ JSON_FLAG_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::IntenseStyle)

JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::InfoBarMessage)
{
JSON_MAPPINGS(4) = {
pair_type{ "deprecated", ValueType::IgnoredDeprecatedEntry },
pair_type{ "closeOnExitInfo", ValueType::CloseOnExitInfo },
ValueType FromJson(const Json::Value& json)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for fixing the deserializer. What about ToJson? We should be able to prevent serializing IgnoredDeprecatedEntry altogether.

{
const auto name{ Detail::GetStringView(json) };
for (const auto& pair : mappings)
{
if (pair.first == name)
{
return pair.second;
}
}

// Keep deprecated keys last, so when they get serialized again they aren't written out
pair_type{ "keyboardServiceWarning", ValueType::IgnoredDeprecatedEntry },
pair_type{ "setAsDefault", ValueType::IgnoredDeprecatedEntry },
// Unknown names may come from another version of Terminal. They must not prevent startup.
return ValueType::IgnoredDeprecatedEntry;
}

JSON_MAPPINGS(1) = {
pair_type{ "closeOnExitInfo", ValueType::CloseOnExitInfo },
};
};

template<>
struct ::Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<winrt::Windows::Foundation::Collections::IVector<winrt::Microsoft::Terminal::Settings::Model::InfoBarMessage>> :
ConversionTrait<std::vector<winrt::Microsoft::Terminal::Settings::Model::InfoBarMessage>>
{
using ValueType = winrt::Microsoft::Terminal::Settings::Model::InfoBarMessage;
using Base = ConversionTrait<std::vector<ValueType>>;

winrt::Windows::Foundation::Collections::IVector<ValueType> FromJson(const Json::Value& json)
{
return winrt::single_threaded_vector<ValueType>(Base::FromJson(json));
}

Json::Value ToJson(const winrt::Windows::Foundation::Collections::IVector<ValueType>& val)
{
Json::Value json{ Json::arrayValue };
if (val)
{
ConversionTrait<ValueType> trait;
for (const auto& message : val)
{
// Drop unknown and deprecated dismissals instead of writing the sentinel back to state.json.
if (message != ValueType::IgnoredDeprecatedEntry)
{
json.append(trait.ToJson(message));
}
}
}
return json;
}
};

template<>
struct ::Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<winrt::Microsoft::Terminal::Settings::Model::ThemeColor>
{
Expand Down
20 changes: 17 additions & 3 deletions src/cascadia/UnitTests_SettingsModel/ApplicationStateTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;

namespace SettingsModelUnitTests
{
// Covers the workspace-persistence APIs added to ApplicationState:
// SaveWorkspace / RemoveWorkspace / RenameWorkspace / TakeWorkspace /
// AllPersistedWorkspaces.
// All tests operate on a throw-away ApplicationState instance pointed at
// a temp directory, so they don't touch the real user state.
class ApplicationStateTests
Expand All @@ -29,6 +26,7 @@ namespace SettingsModelUnitTests
TEST_METHOD(RenameWorkspaceNoOpForMissingEntry);
TEST_METHOD(TakeWorkspaceRemovesAndReturns);
TEST_METHOD(TakeWorkspaceReturnsNullWhenMissing);
TEST_METHOD(IgnoreUnknownDismissedMessages);

private:
static std::filesystem::path _tempRoot()
Expand Down Expand Up @@ -56,6 +54,22 @@ namespace SettingsModelUnitTests
}
};

void ApplicationStateTests::IgnoreUnknownDismissedMessages()
{
Log::Comment(L"GH #16874 - Unknown dismissed message names must not terminate state loading");

Json::Value json{ Json::objectValue };
json["dismissedMessages"].append("unknownMessage");
json["dismissedMessages"].append("closeOnExitInfo");

auto state = _make();
state->FromJson(json, implementation::FileSource::Shared);

const auto serialized = state->ToJson(implementation::FileSource::Shared);
VERIFY_ARE_EQUAL(1u, serialized["dismissedMessages"].size());
VERIFY_ARE_EQUAL("closeOnExitInfo", serialized["dismissedMessages"][0].asString());
}

void ApplicationStateTests::SaveAndLookupWorkspace()
{
auto state = _make();
Expand Down
Loading