-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Better error 305 #1221
Better error 305 #1221
Conversation
84766e6
to
ad053ef
Compare
This obviously doesn't give as much information as possible. I don't dynamically get the type of the argument. I don't really think that information would serve the user very well. That said, "key-style" is, in my opinion, somewhat ambiguous. I wanted to say "string" or "string-like", but I don't know if that covers all the possible types it could be. |
I think having string in the error would be fine, since JSON says that object keys are strings. |
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.
I agree with @gregmarr: please use with a string argument
instead of with a key-style argument
.
I have one question regarding this PR: Do the explanatory strings of exceptions belong to the public API of the library? If so, wouldn't changing be only allowed with a major release? |
@rivertam Could you update the PR with respect to #1221 (review)? |
@nlohmann Whoops, just saw this. Will do! |
Thanks! |
Improve error 305 to address #1220
closes #1220
Pull request checklist
Read the Contribution Guidelines for detailed information.
include/nlohmann
directory, runmake amalgamate
to create the single-header filesingle_include/nlohmann/json.hpp
. The whole process is described here.Please don't
#ifdef
s or other means.