-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
doc: Propose a JsonUtils replacement by writing documentation #5875
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
# New Json Utility API | ||
|
||
## Raw value conversion (GetValue) | ||
|
||
`GetValue` is a convenience helper that will either read a value into existing storage (type-deduced) or | ||
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. | ||
|
||
```c++ | ||
std::string one; | ||
std::optional<std::string> two; | ||
|
||
JsonUtils::GetValue(json, one); | ||
// one is populated or unchanged. | ||
|
||
JsonUtils::GetValue(json, two); | ||
// two is populated, nullopt or unchanged | ||
|
||
auto three = JsonUtils::GetValue<std::string>(json); | ||
// three is populated or zero-initialized | ||
|
||
auto four = JsonUtils::GetValue<std::optional<std::string>>(json); | ||
// four is populated or nullopt | ||
``` | ||
|
||
## Key lookup (GetValueForKey) | ||
|
||
`GetValueForKey` follows the same rules as `GetValue`, but takes an additional key. | ||
It is assumed that the JSON value passed to GetValueForKey is of `object` type. | ||
|
||
```c++ | ||
std::string one; | ||
std::optional<std::string> two; | ||
|
||
JsonUtils::GetValueForKey(json, "firstKey", one); | ||
// one is populated or unchanged. | ||
|
||
JsonUtils::GetValueForKey(json, "secondKey", two); | ||
// two is populated, nullopt or unchanged | ||
|
||
auto three = JsonUtils::GetValueForKey<std::string>(json, "thirdKey"); | ||
// three is populated or zero-initialized | ||
|
||
auto four = JsonUtils::GetValueForKey<std::optional<std::string>>(json, "fourthKey"); | ||
// four is populated or nullopt | ||
``` | ||
|
||
## Converting User-Defined Types | ||
|
||
All conversions are done using specializations of `JsonUtils::ConversionTrait<T>`. | ||
To implement a converter for a user-defined type, you must implement a specialization of `JsonUtils::ConversionTrait<T>`. | ||
|
||
Every specialization over `T` must implement `static T FromJson(const Json::Value&)` and | ||
`static bool CanConvert(const Json::Value&)`. | ||
|
||
```c++ | ||
template<> | ||
struct ConversionTrait<MyCustomType> | ||
{ | ||
// This trait converts a string of the format "[0-9]" to a value of type MyCustomType. | ||
|
||
static MyCustomType FromJson(const Json::Value& json) | ||
{ | ||
return MyCustomType{ json.asString()[0] - '0' }; | ||
} | ||
|
||
static bool CanConvert(const Json::Value& json) | ||
{ | ||
return json.isString(); | ||
} | ||
}; | ||
``` | ||
|
||
For your "convenience" (;P), if you need to provide name-value enum mapping, | ||
there's the `EnumMapper<>` base template. It is somewhat verbose. | ||
|
||
```c++ | ||
template<> | ||
struct JsonUtils::ConversionTrait<CursorStyle> : public JsonUtils::EnumMapper<CursorStyle, JsonUtils::ConversionTrait<CursorStyle>> | ||
DHowett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
// Unfortunately, you need to repeat the enum type three whole times ^ | ||
|
||
// pair_type is provided by EnumMapper to make your life easier. | ||
// Unfortunately, you need to provide v- there a count of the values in the enum. | ||
static constexpr std::array<pair_type, 5> mappings = { | ||
pair_type{ "bar", CursorStyle::Bar }, // DEFAULT | ||
pair_type{ "vintage", CursorStyle::Vintage }, | ||
pair_type{ "underscore", CursorStyle::Underscore }, | ||
pair_type{ "filledBox", CursorStyle::FilledBox }, | ||
pair_type{ "emptyBox", CursorStyle::EmptyBox } | ||
}; | ||
}; | ||
``` | ||
|
||
### Advanced Use | ||
|
||
`GetValue` and `GetValueForKey` can be passed, as their final arguments, any value whose type implements the same | ||
interface as `ConversionTrait<T>`--that is, `FromJson(const Json::Value&)` and `CanConvert(const Json::Value&)`. | ||
|
||
This allows for one-off conversions without a specialization of `ConversionTrait` or even stateful converters. | ||
|
||
#### Stateful Converter Sample | ||
|
||
```c++ | ||
struct MultiplyingConverter { | ||
int BaseValue; | ||
|
||
bool CanConvert(const Json::Value&) { return true; } | ||
|
||
int FromJson(const Json::Value& value) | ||
{ | ||
return value.asInt() * BaseValue; | ||
} | ||
}; | ||
|
||
... | ||
|
||
Json::Value jv = /* a json value containing 66 */; | ||
MultiplyingConverter conv{10}; | ||
|
||
auto v = JsonUtils::GetValue<int>(jv); | ||
DHowett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// v is equal to 660. | ||
``` | ||
|
||
## Behavior Chart | ||
|
||
### GetValue(T&) (type-deducing) | ||
|
||
-|json type invalid|json null|valid | ||
-|-|-|- | ||
`T`|❌ exception|🔵 unchanged|✔ converted | ||
`std::optional<T>`|❌ exception|🟨 `nullopt`|✔ converted | ||
|
||
### GetValue<T>() (returning) | ||
DHowett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
-|json type invalid|json null|valid | ||
-|-|-|- | ||
`T`|❌ exception|🟨 `T{}` (zero value)|✔ converted | ||
`std::optional<T>`|❌ exception|🟨 `nullopt`|✔ converted | ||
|
||
### GetValueForKey(T&) (type-deducing) | ||
|
||
GetValueForKey builds on the behavior set from GetValue by adding | ||
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_ | ||
|
||
### GetValueForKey<T>() (return value) | ||
|
||
val type|key not found|_json type invalid_|_json null_|_valid_ | ||
-|-|-|-|- | ||
`T`|🟨 `T{}` (zero value)|_❌ exception_|_🟨 `T{}` (zero value)_|_✔ converted_ | ||
`std::optional<T>`|🟨 `nullopt`|_❌ exception_|_🟨 `nullopt`_|_✔ converted_ | ||
|
||
### Future Direction | ||
|
||
Perhaps it would be reasonable to supply a default CanConvert that returns true to reduce boilerplate. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for settings that accept multiple types (i.e.
closeOnExit
accepts bool and string), this is where that gets defined?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With regards to adopting this, would we have to create a converter for each of our settings?
The enum ones would use the
JSON_ENUM_MAPPER
below.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to create a converter for each setting that requires special handling. I've done all this work, but yeah. It's a big one.
The straight enum ones will be
JSON_ENUM_MAPPER
s, the custom enums will be (some hybrid)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that we generically specify how a specific type (NOT A SPECIFIC OPTION (!)) is converted to/from json to increase standardization across the codebase
Any time we need to serialize or deserialize, one of these type-specific serializers will be used automatically, seamlessly.