-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Display meaningful errors when JSON types don't match #7241
Conversation
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.
Yea okay these are nits
class TerminalApp::SettingsTypedDeserializationException final : public std::runtime_error | ||
{ | ||
public: | ||
SettingsTypedDeserializationException(const std::string_view description) : | ||
runtime_error(description.data()) {} | ||
}; | ||
|
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.
There's already TerminalWarnings.h
- is that maybe a better place for this to be defined (I suppose this is a nit)
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.
Yeah... it is a better place but I was somewhat expecting that we would come plowing through here with a refactor eventually when we moved it to another dll
src/cascadia/TerminalApp/TerminalSettingsSerializationHelpers.h
Outdated
Show resolved
Hide resolved
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.
suggestions
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
🎉 Handy links: |
This pull request completes (and somewhat rewrites) the JsonUtils error
handling arc. Deserialization errors, no longer represented by trees of
exceptions that must be rethrown and caught, are now transformed at
catch time into a message explaining what we expected and where we
expected it.
Instead of exception trees, a deserialization failure will result in a
single type of exception with the originating JSON object from which we
can determine the contents and location of the failure.
Because most of the error message actually comes from the JSON schema
or the actual supported types, and the other jsoncpp errors are not
localized I've made the decision to not localize these messages.