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

Update for Show/String reform #4

Closed
wants to merge 2 commits into from

Conversation

renato-zannon
Copy link
Contributor

Fallout from the landing on rust-lang/rust#21457

@SimonSapin
Copy link
Collaborator

Result<_, ()> doesn't implement unwrap anymore

Does it really not? rust-lang/rust#21457 says:

The unwrap methods on Result now require Display instead of Debug

@renato-zannon
Copy link
Contributor Author

Does it really not? rust-lang/rust#21457 says:

The unwrap methods on Result now require Display instead of Debug

Yes, that is the reason it does not - () only implements Debug, not Display. The following program does not compile on rustc 1.0.0-dev (8160fc478 2015-01-22 16:50:17 +0000)

fn main() {
    let res: Result<String, ()> = Err(());
    res.unwrap();
}

error:

foo.rs:3:9: 3:17 error: type `core::result::Result<collections::string::String, ()>` does not implement any method in scope named `unwrap`
foo.rs:3     res.unwrap();
                 ^~~~~~~~
error: aborting due to previous error

@SimonSapin
Copy link
Collaborator

Oh, I see. For what it’s worth, there is some discussion in rust-lang/rfcs#565 on reverting that change.

@renato-zannon
Copy link
Contributor Author

Seems like it will be reverted. Good to know!

I think my first commit is still necessary though. Would you like me to submit another PR?

@SimonSapin
Copy link
Collaborator

No need. I’m in the middle of building Rust, since these changes are apparently not in Nightly yet.

@SimonSapin
Copy link
Collaborator

Alright, I’ve published v0.2.17 with your first commit. Tests are failing, but I expect this will be resolved when the Result::unwrap change is reverted.

@SimonSapin SimonSapin closed this Jan 23, 2015
@SimonSapin
Copy link
Collaborator

Oops, sorry, I got confused with rust-url.

@SimonSapin SimonSapin reopened this Jan 23, 2015
@tomprogrammer
Copy link
Owner

The proposed changes in this PR were merged with 97087a8 and published in version 0.2.4

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.

3 participants