Skip to content
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

Custom Debug impl for io::Error #26416

Merged
merged 2 commits into from Jun 21, 2015
Merged

Custom Debug impl for io::Error #26416

merged 2 commits into from Jun 21, 2015

Conversation

retep998
Copy link
Member

Fixes #26408

Example output when using Result::unwrap:

thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Os { code: 2, message: "The system cannot find the file specified.\r\n" } }', src/libcore\result.rs:731

This could technically be considered a breaking change for any code that depends on the format of the Debug output for io::Error but I sincerely hope nobody wrote code like that.

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@sfackler
Copy link
Member

👍

@reem
Copy link
Contributor

reem commented Jun 19, 2015

+1 from me as well, definitely addresses the concerns from #26408

@sfackler
Copy link
Member

It may be a bit nicer to look at if we output Os { code: 6, message: "foobar" } instead of Os(6, "foobar") imo.

Fixes rust-lang#26408

Signed-off-by: Peter Atashian <retep998@gmail.com>
@reem
Copy link
Contributor

reem commented Jun 19, 2015

Agreed. I like the setup in the latest commit.

@arielb1
Copy link
Contributor

arielb1 commented Jun 19, 2015

Test please.

Signed-off-by: Peter Atashian <retep998@gmail.com>
@retep998
Copy link
Member Author

@arielb1 Test added

@sfackler
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 19, 2015

📌 Commit c8aec53 has been approved by sfackler

@bors
Copy link
Contributor

bors commented Jun 20, 2015

⌛ Testing commit c8aec53 with merge 8ca4351...

@bors
Copy link
Contributor

bors commented Jun 20, 2015

💔 Test failed - auto-linux-32-opt

@sfackler
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Jun 21, 2015

⌛ Testing commit c8aec53 with merge b53c6cc...

@bors
Copy link
Contributor

bors commented Jun 21, 2015

💔 Test failed - auto-win-gnu-64-opt

@retep998
Copy link
Member Author

shakes fist at spurious failures

@sfackler
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Jun 21, 2015

⌛ Testing commit c8aec53 with merge ecfcd2a...

bors added a commit that referenced this pull request Jun 21, 2015
Fixes #26408

Example output when using `Result::unwrap`:
```
thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Os { code: 2, message: "The system cannot find the file specified.\r\n" } }', src/libcore\result.rs:731
```

This could technically be considered a breaking change for any code that depends on the format of the `Debug` output for `io::Error` but I sincerely hope nobody wrote code like that.
@bors bors merged commit c8aec53 into rust-lang:master Jun 21, 2015
@pnkfelix
Copy link
Member

/me cant believe that we still havent solved the problem of including the requested path in the error information here (for the specific example of "no such file or directory")

@retep998
Copy link
Member Author

@pnkfelix That would involve storing the requested path in the io::Error and since that won't fit into the i32 we'd have to Box it in the Custom variant. And we'd have to add code to every function that could potentially return a no such file error to check for that error code and instead return the fat custom error with the requested path.

@nobody4t
Copy link

nobody4t commented Oct 8, 2018

why I still got this error when I run the code from Mac 10.13.6 with rustc version 1.29.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants