Rework JsonUtils' optional handling to let Converters see null (#8175)
The JsonUtils changes in #8018 revealed that we need more robust, configurable optional handling. We learned that there's a class of values that was previously underrepresented in our API: _strings that have an explicit empty value_. The Settings model supports starting directory, icon, background image et al values that are empty. That emptiness _overrides_ a value set in a lower layer, so it is not sufficient to represent the empty value for any one of those fields as an unset optional. There are a couple other settings for which we've implemented a hand-rolled option type (for roughly the same reason): foreground, background, any color fields that override values from the color scheme _or_ the lower layer profile. These requirements are best fulfilled by better optional support in JsonUtils. Where the library would originally detect known types of optional and pre-filter them out during `GetValue` and `SetValue`, it will now defer to another conversion trait. This commit introduces a helper conversion trait and an "option oracle". The conversion trait will use the option oracle to detect emptiness, generate empty option values, and read values out of option types. In so doing, the trait is insulated from the implementation details of any specific option type. Any special logic for handling JSON null and option types has been stripped from GetValue. Due to this, there is an express change in behavior for some converters: * `GetValue<T>(jsonNull)` where `T` is **not** an option type[1] has been upgraded from a silent no-op to an exception. Further, I took the opportunity to replace NullableSetting with std::optional<std::optional<T>>, which accurately represents "setting that the user might explicitly clear". I've added a test to JsonUtilsTests to make sure it can serialize/deserialize double optionals the way we expect it to. Tests (Local, Unit for TerminalApp/SettingsModel): Summary: Total=140, Passed=140, Failed=0, Blocked=0, Not Run=0, Skipped=0 [1]: Explicitly, if `T` is not an option type _and the converter does not support null_.
This commit is contained in:
parent
0c0830b2f2
commit
3a5c33b005
|
@ -2277,6 +2277,7 @@ tcome
|
|||
tcommandline
|
||||
tcommands
|
||||
tcon
|
||||
TDelegated
|
||||
TDP
|
||||
TEAMPROJECT
|
||||
tearoff
|
||||
|
|
|
@ -8,19 +8,20 @@ return a JSON value coerced into the specified type.
|
|||
When reading into existing storage, it returns a boolean indicating whether that storage was modified.
|
||||
|
||||
If the JSON value cannot be converted to the specified type, an exception will be generated.
|
||||
For non-nullable type conversions (most POD types), `null` is considered to be an invalid type.
|
||||
|
||||
```c++
|
||||
std::string one;
|
||||
std::optional<std::string> two;
|
||||
|
||||
JsonUtils::GetValue(json, one);
|
||||
// one is populated or unchanged.
|
||||
// one is populated or an exception is thrown.
|
||||
|
||||
JsonUtils::GetValue(json, two);
|
||||
// two is populated, nullopt or unchanged
|
||||
// two is populated, nullopt or an exception is thrown
|
||||
|
||||
auto three = JsonUtils::GetValue<std::string>(json);
|
||||
// three is populated or zero-initialized
|
||||
// three is populated or an exception is thrown
|
||||
|
||||
auto four = JsonUtils::GetValue<std::optional<std::string>>(json);
|
||||
// four is populated or nullopt
|
||||
|
@ -225,14 +226,14 @@ auto v = JsonUtils::GetValue<int>(json, conv);
|
|||
|
||||
-|json type invalid|json null|valid
|
||||
-|-|-|-
|
||||
`T`|❌ exception|🔵 unchanged|✔ converted
|
||||
`T`|❌ exception|❌ exception|✔ converted
|
||||
`std::optional<T>`|❌ exception|🟨 `nullopt`|✔ converted
|
||||
|
||||
### GetValue<T>() (returning)
|
||||
|
||||
-|json type invalid|json null|valid
|
||||
-|-|-|-
|
||||
`T`|❌ exception|🟨 `T{}` (zero value)|✔ converted
|
||||
`T`|❌ exception|❌ exception|✔ converted
|
||||
`std::optional<T>`|❌ exception|🟨 `nullopt`|✔ converted
|
||||
|
||||
### GetValueForKey(T&) (type-deducing)
|
||||
|
@ -242,14 +243,14 @@ a "key not found" state. The remaining three cases are the same.
|
|||
|
||||
val type|key not found|_json type invalid_|_json null_|_valid_
|
||||
-|-|-|-|-
|
||||
`T`|🔵 unchanged|_❌ exception_|_🔵 unchanged_|_✔ converted_
|
||||
`std::optional<T>`|_🔵 unchanged_|_❌ exception_|_🟨 `nullopt`_|_✔ converted_
|
||||
`T`|🔵 unchanged|_❌ exception_|_❌ exception_|_✔ converted_
|
||||
`std::optional<T>`|🔵 unchanged|_❌ exception_|_🟨 `nullopt`_|_✔ converted_
|
||||
|
||||
### GetValueForKey<T>() (return value)
|
||||
|
||||
val type|key not found|_json type invalid_|_json null_|_valid_
|
||||
-|-|-|-|-
|
||||
`T`|🟨 `T{}` (zero value)|_❌ exception_|_🟨 `T{}` (zero value)_|_✔ converted_
|
||||
`T`|🟨 `T{}` (zero value)|_❌ exception_|_❌ exception_|_✔ converted_
|
||||
`std::optional<T>`|🟨 `nullopt`|_❌ exception_|_🟨 `nullopt`_|_✔ converted_
|
||||
|
||||
### Future Direction
|
||||
|
|
|
@ -144,7 +144,6 @@ namespace SettingsModelLocalTests
|
|||
// of from keybindings.
|
||||
|
||||
const std::string commands0String{ R"([
|
||||
{ "name": "command0", "command": { "action": "splitPane", "split": null } },
|
||||
{ "name": "command1", "command": { "action": "splitPane", "split": "vertical" } },
|
||||
{ "name": "command2", "command": { "action": "splitPane", "split": "horizontal" } },
|
||||
{ "name": "command4", "command": { "action": "splitPane" } },
|
||||
|
@ -157,18 +156,8 @@ namespace SettingsModelLocalTests
|
|||
VERIFY_ARE_EQUAL(0u, commands.Size());
|
||||
auto warnings = implementation::Command::LayerJson(commands, commands0Json);
|
||||
VERIFY_ARE_EQUAL(0u, warnings.size());
|
||||
VERIFY_ARE_EQUAL(5u, commands.Size());
|
||||
VERIFY_ARE_EQUAL(4u, commands.Size());
|
||||
|
||||
{
|
||||
auto command = commands.Lookup(L"command0");
|
||||
VERIFY_IS_NOT_NULL(command);
|
||||
VERIFY_IS_NOT_NULL(command.Action());
|
||||
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action());
|
||||
const auto& realArgs = command.Action().Args().try_as<SplitPaneArgs>();
|
||||
VERIFY_IS_NOT_NULL(realArgs);
|
||||
// Verify the args have the expected value
|
||||
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
|
||||
}
|
||||
{
|
||||
auto command = commands.Lookup(L"command1");
|
||||
VERIFY_IS_NOT_NULL(command);
|
||||
|
|
|
@ -322,7 +322,6 @@ namespace SettingsModelLocalTests
|
|||
void KeyBindingsTests::TestSplitPaneArgs()
|
||||
{
|
||||
const std::string bindings0String{ R"([
|
||||
{ "keys": ["ctrl+c"], "command": { "action": "splitPane", "split": null } },
|
||||
{ "keys": ["ctrl+d"], "command": { "action": "splitPane", "split": "vertical" } },
|
||||
{ "keys": ["ctrl+e"], "command": { "action": "splitPane", "split": "horizontal" } },
|
||||
{ "keys": ["ctrl+g"], "command": { "action": "splitPane" } },
|
||||
|
@ -335,17 +334,8 @@ namespace SettingsModelLocalTests
|
|||
VERIFY_IS_NOT_NULL(keymap);
|
||||
VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size());
|
||||
keymap->LayerJson(bindings0Json);
|
||||
VERIFY_ARE_EQUAL(5u, keymap->_keyShortcuts.size());
|
||||
VERIFY_ARE_EQUAL(4u, keymap->_keyShortcuts.size());
|
||||
|
||||
{
|
||||
KeyChord kc{ true, false, false, static_cast<int32_t>('C') };
|
||||
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
|
||||
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
|
||||
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
|
||||
VERIFY_IS_NOT_NULL(realArgs);
|
||||
// Verify the args have the expected value
|
||||
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
|
||||
}
|
||||
{
|
||||
KeyChord kc{ true, false, false, static_cast<int32_t>('D') };
|
||||
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
|
||||
|
|
|
@ -72,11 +72,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
|||
|
||||
// This is like std::optional, but we can use it in inheritance to determine whether the user explicitly cleared it
|
||||
template<typename T>
|
||||
struct NullableSetting
|
||||
{
|
||||
std::optional<T> setting{ std::nullopt };
|
||||
bool set{ false };
|
||||
};
|
||||
using NullableSetting = std::optional<std::optional<T>>;
|
||||
}
|
||||
|
||||
// Use this macro to quickly implement both getters and the setter for an
|
||||
|
@ -93,7 +89,7 @@ public: \
|
|||
bool Has##name() const \
|
||||
{ \
|
||||
return _##name.has_value(); \
|
||||
}; \
|
||||
} \
|
||||
\
|
||||
/* Returns the resolved value for this setting */ \
|
||||
/* fallback: user set value --> inherited value --> system set value */ \
|
||||
|
@ -101,19 +97,19 @@ public: \
|
|||
{ \
|
||||
const auto val{ _get##name##Impl() }; \
|
||||
return val ? *val : type{ __VA_ARGS__ }; \
|
||||
}; \
|
||||
} \
|
||||
\
|
||||
/* Overwrite the user set value */ \
|
||||
void name(const type& value) \
|
||||
{ \
|
||||
_##name = value; \
|
||||
}; \
|
||||
} \
|
||||
\
|
||||
/* Clear the user set value */ \
|
||||
void Clear##name() \
|
||||
{ \
|
||||
_##name = std::nullopt; \
|
||||
}; \
|
||||
} \
|
||||
\
|
||||
private: \
|
||||
std::optional<type> _##name{ std::nullopt }; \
|
||||
|
@ -137,7 +133,7 @@ private: \
|
|||
\
|
||||
/*no value was found*/ \
|
||||
return std::nullopt; \
|
||||
};
|
||||
}
|
||||
|
||||
// This macro is similar to the one above, but is reserved for optional settings
|
||||
// like Profile.Foreground (where null is interpreted
|
||||
|
@ -148,51 +144,51 @@ public: \
|
|||
/* Returns true if the user explicitly set the value, false otherwise*/ \
|
||||
bool Has##name() const \
|
||||
{ \
|
||||
return _##name.set; \
|
||||
}; \
|
||||
return _##name.has_value(); \
|
||||
} \
|
||||
\
|
||||
/* Returns the resolved value for this setting */ \
|
||||
/* fallback: user set value --> inherited value --> system set value */ \
|
||||
winrt::Windows::Foundation::IReference<type> name() const \
|
||||
{ \
|
||||
const auto val{ _get##name##Impl() }; \
|
||||
if (val.set) \
|
||||
if (val) \
|
||||
{ \
|
||||
if (val.setting) \
|
||||
if (*val) \
|
||||
{ \
|
||||
return *val.setting; \
|
||||
return **val; \
|
||||
} \
|
||||
return nullptr; \
|
||||
} \
|
||||
return winrt::Windows::Foundation::IReference<type>{ __VA_ARGS__ }; \
|
||||
}; \
|
||||
} \
|
||||
\
|
||||
/* Overwrite the user set value */ \
|
||||
void name(const winrt::Windows::Foundation::IReference<type>& value) \
|
||||
{ \
|
||||
if (value) /*set value is different*/ \
|
||||
{ \
|
||||
_##name.setting = value.Value(); \
|
||||
_##name = std::optional<type>{ value.Value() }; \
|
||||
} \
|
||||
else \
|
||||
{ \
|
||||
_##name.setting = std::nullopt; \
|
||||
/* note we're setting the _inner_ value */ \
|
||||
_##name = std::optional<type>{ std::nullopt }; \
|
||||
} \
|
||||
_##name.set = true; \
|
||||
}; \
|
||||
} \
|
||||
\
|
||||
/* Clear the user set value */ \
|
||||
void Clear##name() \
|
||||
{ \
|
||||
_##name.set = false; \
|
||||
}; \
|
||||
_##name = std::nullopt; \
|
||||
} \
|
||||
\
|
||||
private: \
|
||||
NullableSetting<type> _##name{}; \
|
||||
NullableSetting<type> _get##name##Impl() const \
|
||||
{ \
|
||||
/*return user set value*/ \
|
||||
if (Has##name()) \
|
||||
if (_##name) \
|
||||
{ \
|
||||
return _##name; \
|
||||
} \
|
||||
|
@ -201,12 +197,12 @@ private: \
|
|||
/*iterate through parents to find a value*/ \
|
||||
for (auto parent : _parents) \
|
||||
{ \
|
||||
auto val{ parent->_get##name##Impl() }; \
|
||||
if (val.set) \
|
||||
if (auto val{ parent->_get##name##Impl() }) \
|
||||
{ \
|
||||
return val; \
|
||||
} \
|
||||
} \
|
||||
\
|
||||
/*no value was found*/ \
|
||||
return { std::nullopt, false }; \
|
||||
};
|
||||
return std::nullopt; \
|
||||
}
|
||||
|
|
|
@ -60,36 +60,34 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
const std::string_view zeroCopyString{ begin, gsl::narrow_cast<size_t>(end - begin) };
|
||||
return zeroCopyString;
|
||||
}
|
||||
|
||||
template<typename T>
|
||||
struct DeduceOptional
|
||||
{
|
||||
using Type = typename std::decay<T>::type;
|
||||
static constexpr bool IsOptional = false;
|
||||
};
|
||||
|
||||
template<typename TOpt>
|
||||
struct DeduceOptional<std::optional<TOpt>>
|
||||
{
|
||||
using Type = typename std::decay<TOpt>::type;
|
||||
static constexpr bool IsOptional = true;
|
||||
};
|
||||
|
||||
template<typename TOpt>
|
||||
struct DeduceOptional<::winrt::Windows::Foundation::IReference<TOpt>>
|
||||
{
|
||||
using Type = typename std::decay<TOpt>::type;
|
||||
static constexpr bool IsOptional = true;
|
||||
};
|
||||
|
||||
template<>
|
||||
struct DeduceOptional<::winrt::hstring>
|
||||
{
|
||||
using Type = typename ::winrt::hstring;
|
||||
static constexpr bool IsOptional = true;
|
||||
};
|
||||
}
|
||||
|
||||
template<typename T>
|
||||
struct OptionOracle
|
||||
{
|
||||
template<typename U> // universal parameter
|
||||
static constexpr bool HasValue(U&&)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
};
|
||||
|
||||
template<typename T>
|
||||
struct OptionOracle<std::optional<T>>
|
||||
{
|
||||
static constexpr std::optional<T> EmptyV() { return std::nullopt; }
|
||||
static constexpr bool HasValue(const std::optional<T>& o) { return o.has_value(); }
|
||||
static constexpr auto&& Value(const std::optional<T>& o) { return *o; }
|
||||
};
|
||||
|
||||
template<typename T>
|
||||
struct OptionOracle<::winrt::Windows::Foundation::IReference<T>>
|
||||
{
|
||||
static constexpr ::winrt::Windows::Foundation::IReference<T> EmptyV() { return nullptr; }
|
||||
static constexpr bool HasValue(const ::winrt::Windows::Foundation::IReference<T>& o) { return static_cast<bool>(o); }
|
||||
static constexpr auto&& Value(const ::winrt::Windows::Foundation::IReference<T>& o) { return o.Value(); }
|
||||
};
|
||||
|
||||
class DeserializationError : public std::runtime_error
|
||||
{
|
||||
public:
|
||||
|
@ -177,13 +175,27 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
// Leverage the wstring converter's validation
|
||||
winrt::hstring FromJson(const Json::Value& json)
|
||||
{
|
||||
if (json.isNull())
|
||||
{
|
||||
return winrt::hstring{};
|
||||
}
|
||||
return winrt::hstring{ til::u8u16(Detail::GetStringView(json)) };
|
||||
}
|
||||
|
||||
Json::Value ToJson(const winrt::hstring& val)
|
||||
{
|
||||
if (val == winrt::hstring{})
|
||||
{
|
||||
return Json::Value::nullSingleton();
|
||||
}
|
||||
return til::u16u8(val);
|
||||
}
|
||||
|
||||
bool CanConvert(const Json::Value& json)
|
||||
{
|
||||
// hstring has a specific behavior for null, so it can convert it
|
||||
return ConversionTrait<std::wstring>::CanConvert(json) || json.isNull();
|
||||
}
|
||||
};
|
||||
#endif
|
||||
|
||||
|
@ -419,6 +431,56 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
};
|
||||
#endif
|
||||
|
||||
template<typename T, typename TDelegatedConverter = ConversionTrait<typename std::decay<T>::type>, typename TOpt = std::optional<typename std::decay<T>::type>>
|
||||
struct OptionalConverter
|
||||
{
|
||||
using Oracle = OptionOracle<TOpt>;
|
||||
TDelegatedConverter delegatedConverter{};
|
||||
|
||||
TOpt FromJson(const Json::Value& json)
|
||||
{
|
||||
if (!json && !delegatedConverter.CanConvert(json))
|
||||
{
|
||||
// If the nested converter can't deal with null, emit an empty optional
|
||||
// If it can, it probably has specific null behavior that it wants to use.
|
||||
return Oracle::EmptyV();
|
||||
}
|
||||
TOpt val{ delegatedConverter.FromJson(json) };
|
||||
return val;
|
||||
}
|
||||
|
||||
bool CanConvert(const Json::Value& json)
|
||||
{
|
||||
return json.isNull() || delegatedConverter.CanConvert(json);
|
||||
}
|
||||
|
||||
Json::Value ToJson(const TOpt& val)
|
||||
{
|
||||
if (!Oracle::HasValue(val))
|
||||
{
|
||||
return Json::Value::nullSingleton();
|
||||
}
|
||||
return delegatedConverter.ToJson(Oracle::Value(val));
|
||||
}
|
||||
|
||||
std::string TypeDescription() const
|
||||
{
|
||||
return delegatedConverter.TypeDescription();
|
||||
}
|
||||
};
|
||||
|
||||
template<typename T>
|
||||
struct ConversionTrait<std::optional<T>> : public OptionalConverter<T, ConversionTrait<T>, std::optional<T>>
|
||||
{
|
||||
};
|
||||
|
||||
#ifdef WINRT_Windows_Foundation_H
|
||||
template<typename T>
|
||||
struct ConversionTrait<::winrt::Windows::Foundation::IReference<T>> : public OptionalConverter<T, ConversionTrait<T>, ::winrt::Windows::Foundation::IReference<T>>
|
||||
{
|
||||
};
|
||||
#endif
|
||||
|
||||
template<typename T, typename TBase>
|
||||
struct EnumMapper
|
||||
{
|
||||
|
@ -586,31 +648,15 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
template<typename T, typename Converter>
|
||||
bool GetValue(const Json::Value& json, T& target, Converter&& conv)
|
||||
{
|
||||
if constexpr (Detail::DeduceOptional<T>::IsOptional)
|
||||
if (!conv.CanConvert(json))
|
||||
{
|
||||
// FOR OPTION TYPES
|
||||
// - If the json object is set to `null`, then
|
||||
// we'll instead set the target back to the empty optional.
|
||||
if (json.isNull())
|
||||
{
|
||||
target = T{}; // zero-construct an empty optional
|
||||
return true;
|
||||
}
|
||||
DeserializationError e{ json };
|
||||
e.expectedType = conv.TypeDescription();
|
||||
throw e;
|
||||
}
|
||||
|
||||
if (json)
|
||||
{
|
||||
if (!conv.CanConvert(json))
|
||||
{
|
||||
DeserializationError e{ json };
|
||||
e.expectedType = conv.TypeDescription();
|
||||
throw e;
|
||||
}
|
||||
|
||||
target = conv.FromJson(json);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
target = conv.FromJson(json);
|
||||
return true;
|
||||
}
|
||||
|
||||
// GetValue, forced return type, manual converter
|
||||
|
@ -654,7 +700,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
template<typename T>
|
||||
bool GetValue(const Json::Value& json, T& target)
|
||||
{
|
||||
return GetValue(json, target, ConversionTrait<typename Detail::DeduceOptional<T>::Type>{});
|
||||
return GetValue(json, target, ConversionTrait<typename std::decay<T>::type>{});
|
||||
}
|
||||
|
||||
// GetValue, forced return type, with automatic converter
|
||||
|
@ -662,7 +708,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
std::decay_t<T> GetValue(const Json::Value& json)
|
||||
{
|
||||
std::decay_t<T> local{};
|
||||
GetValue(json, local, ConversionTrait<typename Detail::DeduceOptional<T>::Type>{});
|
||||
GetValue(json, local, ConversionTrait<typename std::decay<T>::type>{});
|
||||
return local; // returns zero-initialized or value
|
||||
}
|
||||
|
||||
|
@ -670,14 +716,14 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
template<typename T>
|
||||
bool GetValueForKey(const Json::Value& json, std::string_view key, T& target)
|
||||
{
|
||||
return GetValueForKey(json, key, target, ConversionTrait<typename Detail::DeduceOptional<T>::Type>{});
|
||||
return GetValueForKey(json, key, target, ConversionTrait<typename std::decay<T>::type>{});
|
||||
}
|
||||
|
||||
// GetValueForKey, forced return type, with automatic converter
|
||||
template<typename T>
|
||||
std::decay_t<T> GetValueForKey(const Json::Value& json, std::string_view key)
|
||||
{
|
||||
return GetValueForKey<T>(json, key, ConversionTrait<typename Detail::DeduceOptional<T>::Type>{});
|
||||
return GetValueForKey<T>(json, key, ConversionTrait<typename std::decay<T>::type>{});
|
||||
}
|
||||
|
||||
// Get multiple values for keys (json, k, &v, k, &v, k, &v, ...).
|
||||
|
@ -696,15 +742,19 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
|
|||
template<typename T, typename Converter>
|
||||
void SetValueForKey(Json::Value& json, std::string_view key, const T& target, Converter&& conv)
|
||||
{
|
||||
// demand guarantees that it will return a value or throw an exception
|
||||
*json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target);
|
||||
// We don't want to write any empty optionals into JSON (right now).
|
||||
if (OptionOracle<T>::HasValue(target))
|
||||
{
|
||||
// demand guarantees that it will return a value or throw an exception
|
||||
*json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target);
|
||||
}
|
||||
}
|
||||
|
||||
// SetValueForKey, type-deduced, with automatic converter
|
||||
template<typename T>
|
||||
void SetValueForKey(Json::Value& json, std::string_view key, const T& target)
|
||||
{
|
||||
SetValueForKey(json, key, target, ConversionTrait<typename Detail::DeduceOptional<T>::Type>{});
|
||||
SetValueForKey(json, key, target, ConversionTrait<typename std::decay<T>::type>{});
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
@ -293,10 +293,10 @@ void Profile::LayerJson(const Json::Value& json)
|
|||
JsonUtils::GetValueForKey(json, HiddenKey, _Hidden);
|
||||
|
||||
// Core Settings
|
||||
_Foreground.set = JsonUtils::GetValueForKey(json, ForegroundKey, _Foreground.setting);
|
||||
_Background.set = JsonUtils::GetValueForKey(json, BackgroundKey, _Background.setting);
|
||||
_SelectionBackground.set = JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground.setting);
|
||||
_CursorColor.set = JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor.setting);
|
||||
JsonUtils::GetValueForKey(json, ForegroundKey, _Foreground);
|
||||
JsonUtils::GetValueForKey(json, BackgroundKey, _Background);
|
||||
JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground);
|
||||
JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor);
|
||||
JsonUtils::GetValueForKey(json, ColorSchemeKey, _ColorSchemeName);
|
||||
|
||||
// TODO:MSFT:20642297 - Use a sentinel value (-1) for "Infinite scrollback"
|
||||
|
@ -320,18 +320,11 @@ void Profile::LayerJson(const Json::Value& json)
|
|||
|
||||
// Padding was never specified as an integer, but it was a common working mistake.
|
||||
// Allow it to be permissive.
|
||||
JsonUtils::GetValueForKey(json, PaddingKey, _Padding, JsonUtils::PermissiveStringConverter<std::wstring>{});
|
||||
JsonUtils::GetValueForKey(json, PaddingKey, _Padding, JsonUtils::OptionalConverter<hstring, JsonUtils::PermissiveStringConverter<std::wstring>>{});
|
||||
|
||||
JsonUtils::GetValueForKey(json, ScrollbarStateKey, _ScrollState);
|
||||
|
||||
// StartingDirectory is "nullable". But we represent a null starting directory as the empty string
|
||||
// When null is set in the JSON, we empty initialize startDir (empty string), and set StartingDirectory to that
|
||||
// Without this, we're accidentally setting StartingDirectory to nullopt instead.
|
||||
hstring startDir;
|
||||
if (JsonUtils::GetValueForKey(json, StartingDirectoryKey, startDir))
|
||||
{
|
||||
_StartingDirectory = startDir;
|
||||
}
|
||||
JsonUtils::GetValueForKey(json, StartingDirectoryKey, _StartingDirectory);
|
||||
|
||||
JsonUtils::GetValueForKey(json, IconKey, _Icon);
|
||||
JsonUtils::GetValueForKey(json, BackgroundImageKey, _BackgroundImagePath);
|
||||
|
@ -341,7 +334,7 @@ void Profile::LayerJson(const Json::Value& json)
|
|||
JsonUtils::GetValueForKey(json, RetroTerminalEffectKey, _RetroTerminalEffect);
|
||||
JsonUtils::GetValueForKey(json, AntialiasingModeKey, _AntialiasingMode);
|
||||
|
||||
_TabColor.set = JsonUtils::GetValueForKey(json, TabColorKey, _TabColor.setting);
|
||||
JsonUtils::GetValueForKey(json, TabColorKey, _TabColor);
|
||||
|
||||
JsonUtils::GetValueForKey(json, BellStyleKey, _BellStyle);
|
||||
}
|
||||
|
|
|
@ -101,6 +101,38 @@ JSON_FLAG_MAPPER(JsonTestFlags)
|
|||
};
|
||||
};
|
||||
|
||||
struct hstring_like
|
||||
{
|
||||
std::string value;
|
||||
};
|
||||
template<>
|
||||
struct ConversionTrait<hstring_like>
|
||||
{
|
||||
hstring_like FromJson(const Json::Value& json)
|
||||
{
|
||||
return { ConversionTrait<std::string>{}.FromJson(json) };
|
||||
}
|
||||
|
||||
bool CanConvert(const Json::Value& json)
|
||||
{
|
||||
return json.isNull() || ConversionTrait<std::string>{}.CanConvert(json);
|
||||
}
|
||||
|
||||
Json::Value ToJson(const hstring_like& val)
|
||||
{
|
||||
if (val.value == "")
|
||||
{
|
||||
return Json::Value::nullSingleton();
|
||||
}
|
||||
return val.value;
|
||||
}
|
||||
|
||||
std::string TypeDescription() const
|
||||
{
|
||||
return "string";
|
||||
}
|
||||
};
|
||||
|
||||
namespace TerminalAppUnitTests
|
||||
{
|
||||
class JsonUtilsTests
|
||||
|
@ -119,6 +151,11 @@ namespace TerminalAppUnitTests
|
|||
TEST_METHOD(FlagMapper);
|
||||
|
||||
TEST_METHOD(NestedExceptionDuringKeyParse);
|
||||
|
||||
TEST_METHOD(SetValueHStringLike);
|
||||
TEST_METHOD(GetValueHStringLike);
|
||||
|
||||
TEST_METHOD(DoubleOptional);
|
||||
};
|
||||
|
||||
template<typename T>
|
||||
|
@ -136,9 +173,8 @@ namespace TerminalAppUnitTests
|
|||
//// 1.a. Type Invalid - Exception ////
|
||||
VERIFY_THROWS_SPECIFIC(GetValue<int>(object), DeserializationError, _ReturnTrueForException);
|
||||
|
||||
//// 1.b. JSON NULL - Zero Value ////
|
||||
std::string zeroValueString{};
|
||||
VERIFY_ARE_EQUAL(zeroValueString, GetValue<std::string>(Json::Value::nullSingleton()));
|
||||
//// 1.b. JSON NULL - Exception ////
|
||||
VERIFY_THROWS_SPECIFIC(GetValue<std::string>(Json::Value::nullSingleton()), DeserializationError, _ReturnTrueForException);
|
||||
|
||||
//// 1.c. Valid - Valid ////
|
||||
VERIFY_ARE_EQUAL(expected, GetValue<std::string>(object));
|
||||
|
@ -167,9 +203,8 @@ namespace TerminalAppUnitTests
|
|||
VERIFY_THROWS_SPECIFIC(GetValue(object, outputRedHerring), DeserializationError, _ReturnTrueForException);
|
||||
VERIFY_ARE_EQUAL(5, outputRedHerring); // unchanged
|
||||
|
||||
//// 1.b. JSON NULL - Unchanged ////
|
||||
VERIFY_IS_FALSE(GetValue(Json::Value::nullSingleton(), output)); // FALSE = storage not modified!
|
||||
VERIFY_ARE_EQUAL("sentinel", output); // unchanged
|
||||
//// 1.b. JSON NULL - Exception ////
|
||||
VERIFY_THROWS_SPECIFIC(GetValue(Json::Value::nullSingleton(), output), DeserializationError, _ReturnTrueForException);
|
||||
|
||||
//// 1.c. Valid ////
|
||||
VERIFY_IS_TRUE(GetValue(object, output));
|
||||
|
@ -208,14 +243,14 @@ namespace TerminalAppUnitTests
|
|||
//// 1.a. Type Invalid - Exception ////
|
||||
VERIFY_THROWS_SPECIFIC(GetValueForKey<int>(object, key), DeserializationError, _ReturnTrueForException);
|
||||
|
||||
//// 1.b. JSON NULL - Zero Value ////
|
||||
std::string zeroValueString{};
|
||||
VERIFY_ARE_EQUAL(zeroValueString, GetValueForKey<std::string>(object, nullKey));
|
||||
//// 1.b. JSON NULL - Exception ////
|
||||
VERIFY_THROWS_SPECIFIC(GetValueForKey<std::string>(object, nullKey), DeserializationError, _ReturnTrueForException);
|
||||
|
||||
//// 1.c. Valid - Valid ////
|
||||
VERIFY_ARE_EQUAL(expected, GetValueForKey<std::string>(object, key));
|
||||
|
||||
//// 1.d. Not Found - Zero Value ////
|
||||
std::string zeroValueString{};
|
||||
VERIFY_ARE_EQUAL(zeroValueString, GetValueForKey<std::string>(object, invalidKey));
|
||||
|
||||
//// 2. Optional ////
|
||||
|
@ -253,8 +288,7 @@ namespace TerminalAppUnitTests
|
|||
VERIFY_ARE_EQUAL(5, outputRedHerring); // unchanged
|
||||
|
||||
//// 1.b. JSON NULL - Unchanged ////
|
||||
VERIFY_IS_FALSE(GetValueForKey(object, nullKey, output)); // FALSE = storage not modified!
|
||||
VERIFY_ARE_EQUAL("sentinel", output); // unchanged
|
||||
VERIFY_THROWS_SPECIFIC(GetValueForKey(object, nullKey, output), DeserializationError, _ReturnTrueForException);
|
||||
|
||||
//// 1.c. Valid ////
|
||||
VERIFY_IS_TRUE(GetValueForKey(object, key, output));
|
||||
|
@ -505,4 +539,89 @@ namespace TerminalAppUnitTests
|
|||
VERIFY_THROWS_SPECIFIC(GetValueForKey<int>(object, key), DeserializationError, CheckKeyInException);
|
||||
}
|
||||
|
||||
void JsonUtilsTests::SetValueHStringLike()
|
||||
{
|
||||
// Terminal has a string type (hstring) where null/"" are the same, and
|
||||
// we want to make sure that optionals of that type serialize "properly".
|
||||
hstring_like first{ "" };
|
||||
hstring_like second{ "second" };
|
||||
std::optional<hstring_like> third{ { "" } };
|
||||
std::optional<hstring_like> fourth{ { "fourth" } };
|
||||
std::optional<hstring_like> fifth{};
|
||||
|
||||
Json::Value object{ Json::objectValue };
|
||||
|
||||
SetValueForKey(object, "first", first);
|
||||
SetValueForKey(object, "second", second);
|
||||
SetValueForKey(object, "third", third);
|
||||
SetValueForKey(object, "fourth", fourth);
|
||||
SetValueForKey(object, "fifth", fifth);
|
||||
|
||||
VERIFY_ARE_EQUAL(Json::Value::nullSingleton(), object["first"]); // real empty value serializes as null
|
||||
VERIFY_ARE_EQUAL("second", object["second"].asString()); // serializes as a string
|
||||
VERIFY_ARE_EQUAL(Json::Value::nullSingleton(), object["third"]); // optional populated with real empty value serializes as null
|
||||
VERIFY_ARE_EQUAL("fourth", object["fourth"].asString()); // serializes as a string
|
||||
VERIFY_IS_FALSE(object.isMember("fifth")); // does not serialize
|
||||
}
|
||||
|
||||
void JsonUtilsTests::GetValueHStringLike()
|
||||
{
|
||||
Json::Value object{ Json::objectValue };
|
||||
object["string"] = "string";
|
||||
object["null"] = Json::Value::nullSingleton();
|
||||
// object["nonexistent"] can't be set, clearly, to continue not existing
|
||||
|
||||
hstring_like v;
|
||||
VERIFY_IS_TRUE(GetValueForKey(object, "string", v));
|
||||
VERIFY_ARE_EQUAL("string", v.value); // deserializes as string
|
||||
VERIFY_IS_TRUE(GetValueForKey(object, "null", v));
|
||||
VERIFY_ARE_EQUAL("", v.value); // deserializes as real value, but empty
|
||||
VERIFY_IS_FALSE(GetValueForKey(object, "nonexistent", v)); // does not deserialize
|
||||
|
||||
std::optional<hstring_like> optionalV;
|
||||
// deserializes as populated optional containing string
|
||||
VERIFY_IS_TRUE(GetValueForKey(object, "string", optionalV));
|
||||
VERIFY_IS_TRUE(optionalV.has_value());
|
||||
VERIFY_ARE_EQUAL("string", optionalV->value);
|
||||
|
||||
optionalV = std::nullopt;
|
||||
// deserializes as populated optional containing real empty value
|
||||
VERIFY_IS_TRUE(GetValueForKey(object, "null", optionalV));
|
||||
VERIFY_IS_TRUE(optionalV.has_value());
|
||||
VERIFY_ARE_EQUAL("", optionalV->value);
|
||||
|
||||
optionalV = std::nullopt;
|
||||
// does not deserialize; optional remains nullopt
|
||||
VERIFY_IS_FALSE(GetValueForKey(object, "nonexistent", optionalV));
|
||||
VERIFY_ARE_EQUAL(std::nullopt, optionalV);
|
||||
}
|
||||
|
||||
void JsonUtilsTests::DoubleOptional()
|
||||
{
|
||||
const std::optional<std::optional<int>> first{ std::nullopt }; // no value
|
||||
const std::optional<std::optional<int>> second{ std::optional<int>{ std::nullopt } }; // outer has a value, inner is "no value"
|
||||
const std::optional<std::optional<int>> third{ std::optional<int>{ 3 } }; // outer has a value, inner is "no value"
|
||||
|
||||
Json::Value object{ Json::objectValue };
|
||||
|
||||
SetValueForKey(object, "first", first);
|
||||
SetValueForKey(object, "second", second);
|
||||
SetValueForKey(object, "third", third);
|
||||
|
||||
VERIFY_IS_FALSE(object.isMember("first"));
|
||||
VERIFY_IS_TRUE(object.isMember("second"));
|
||||
VERIFY_ARE_EQUAL(Json::Value::nullSingleton(), object["second"]);
|
||||
VERIFY_ARE_EQUAL(Json::Value{ 3 }, object["third"]);
|
||||
|
||||
std::optional<std::optional<int>> firstOut, secondOut, thirdOut;
|
||||
VERIFY_IS_FALSE(GetValueForKey(object, "first", firstOut));
|
||||
VERIFY_IS_TRUE(GetValueForKey(object, "second", secondOut));
|
||||
VERIFY_IS_TRUE(static_cast<bool>(secondOut));
|
||||
VERIFY_ARE_EQUAL(std::nullopt, *secondOut); // should have come back out as null
|
||||
|
||||
VERIFY_IS_TRUE(GetValueForKey(object, "third", thirdOut));
|
||||
VERIFY_IS_TRUE(static_cast<bool>(thirdOut));
|
||||
VERIFY_IS_TRUE(static_cast<bool>(*thirdOut));
|
||||
VERIFY_ARE_EQUAL(3, **thirdOut);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue