-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Improve TOML decoding error messages #3794
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
src/cargo/util/toml.rs
Outdated
{ | ||
Ok(PathValue::String(String::deserialize(deserializer)?)) | ||
} | ||
} |
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.
If we are writing custom deserialization anyway, maybe we can remove String
variant from the enum?
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.
Indeed!
Unfortunately while `#[serde(untagged)]` is precisely what we want in terms of semantics it leaves a little to be desired in terms of error messages. This commit updates to remove the usage of that attribute in favor of implementing `Deserialize` directly, which is quite simple in these few cases. Closes rust-lang#3790
e5561d5
to
cb53e1b
Compare
@bors: r+ |
📌 Commit cb53e1b has been approved by |
Improve TOML decoding error messages Unfortunately while `#[serde(untagged)]` is precisely what we want in terms of semantics it leaves a little to be desired in terms of error messages. This commit updates to remove the usage of that attribute in favor of implementing `Deserialize` directly, which is quite simple in these few cases. Closes #3790
☀️ Test successful - status-appveyor, status-travis |
Unfortunately while
#[serde(untagged)]
is precisely what we want in terms ofsemantics it leaves a little to be desired in terms of error messages. This
commit updates to remove the usage of that attribute in favor of implementing
Deserialize
directly, which is quite simple in these few cases.Closes #3790