-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 Debug impl for io::Error. #47120
Conversation
r? @dtolnay (rust_highfive has picked a reviewer for you, use r? to override) |
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.
Thanks!
@bors r+ |
📌 Commit f058dce has been approved by |
f058dce
to
418d050
Compare
I forgot a stability attribute so you'll have to |
@bors r+ |
📌 Commit 418d050 has been approved by |
@bors rollup |
@bors r- |
src/libstd/io/error.rs
Outdated
#[stable(feature = "rust1", since = "1.0.0")] | ||
pub struct Error { | ||
repr: Repr, | ||
} | ||
|
||
#[stable(feature = "io_error_debug", since = "1.24.0")] |
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.
As Debug
has been implemented for io::Error
since 1.0.0 #[stable(feature = "rust1", since = "1.0.0")]
would make more sense here.
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.
Will do
418d050
to
fc87b6f
Compare
I fixed the test, changed the stability attribute, and also added a |
fc87b6f
to
bcc2368
Compare
Could you update your comment at the top of this thread with an accurate before and after? The after |
f84a6f2
to
cb25cd5
Compare
@dtolnay done! I also modified the failing test with one that's closer to the one listed in the OP. |
cb25cd5
to
42a6a40
Compare
src/libstd/io/error.rs
Outdated
} | ||
}; | ||
let expected = format!( | ||
"Custom { \ |
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.
Should be {{
here.
[01:18:41] error: invalid format string: expected `'}'`, found `'k'`
[01:18:41] --> /checkout/src/libstd/io/error.rs:590:13
[01:18:41] |
[01:18:41] 590 | / "Custom { \
[01:18:41] 591 | | kind: InvalidInput, \
[01:18:41] 592 | | error: Os {{ \
[01:18:41] 593 | | code: {:?}, \
[01:18:41] ... |
[01:18:41] 596 | | }} \
[01:18:41] 597 | | }}",
[01:18:41] | |_______________^
d011822
to
fe3e8e2
Compare
I'm fairly certain that I've fixed the build errors. Waiting on CI. |
Travis says:
|
Ping from triage, @clarcharr! Will you have time to address the build issues? |
fe3e8e2
to
52e074e
Compare
Build issue seems to be addressed, CI is green, so I'm gonna go ahead and @bors r=dtolnay |
📌 Commit 52e074e has been approved by |
Better Debug impl for io::Error. This PR includes the below changes: 1. The former impl wrapped the entire thing in `Error { repr: ... }` which was unhelpful; this has been removed. 2. The `Os` variant of `io::Error` included the code and message, but not the kind; this has been fixed. 3. The `Custom` variant of `io::Error` included a `Custom(Custom { ... })`, which is now just `Custom { ... }`. Example of previous impl: ```rust Error { repr: Custom( Custom { kind: InvalidData, error: Error { repr: Os { code: 2, message: "no such file or directory" } } } ) } ``` Example of new impl: ```rust Custom { kind: InvalidData, error: Os { code: 2, kind: NotFound, message: "no such file or directory" } } ```
Better Debug impl for io::Error. This PR includes the below changes: 1. The former impl wrapped the entire thing in `Error { repr: ... }` which was unhelpful; this has been removed. 2. The `Os` variant of `io::Error` included the code and message, but not the kind; this has been fixed. 3. The `Custom` variant of `io::Error` included a `Custom(Custom { ... })`, which is now just `Custom { ... }`. Example of previous impl: ```rust Error { repr: Custom( Custom { kind: InvalidData, error: Error { repr: Os { code: 2, message: "no such file or directory" } } } ) } ``` Example of new impl: ```rust Custom { kind: InvalidData, error: Os { code: 2, kind: NotFound, message: "no such file or directory" } } ```
This PR includes the below changes:
Error { repr: ... }
which was unhelpful; this has been removed.Os
variant ofio::Error
included the code and message, but not the kind; this has been fixed.Custom
variant ofio::Error
included aCustom(Custom { ... })
, which is now justCustom { ... }
.Example of previous impl:
Example of new impl: