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

Document that Result.unwrap prints the Err's value #15632

Merged
merged 1 commit into from
Jul 14, 2014
Merged

Document that Result.unwrap prints the Err's value #15632

merged 1 commit into from
Jul 14, 2014

Conversation

masklinn
Copy link
Contributor

I saw that it was bounded by Show but the implication is no guarantee (and had only 0.10 to test, where this behavior has been added to 0.11)

@alexcrichton
Copy link
Member

Could you tweak the documentation to say that instead of printing the task fails with E's Show implementation? You can suppress printing with set_stderr or redirect to any location.

@masklinn
Copy link
Contributor Author

Sure. Maybe model the precise message after Option.expect's, e.g.

Fails if the value is an Err, with a custom failure message provided by the Err's value.

would that work?

Should I squash the new commit atop the old one?

@alexcrichton
Copy link
Member

That sounds good to me! And yes, I'm a fan of squashing :)

It is implied by the Show bound, but that implication can be missed.
@masklinn
Copy link
Contributor Author

Here's a version with error messages matching that of Option.expect, I also added a Failure section title since that seems to be pretty common.

@steveklabnik
Copy link
Member

:shipit:

bors added a commit that referenced this pull request Jul 14, 2014
I saw that it was bounded by `Show` but the implication is no guarantee (and had only 0.10 to test, where this behavior has been added to 0.11)
@bors bors closed this Jul 14, 2014
@bors bors merged commit ded48c5 into rust-lang:master Jul 14, 2014
@masklinn masklinn deleted the patch-1 branch July 15, 2014 19:59
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2023
scip: Use load_workspace_at.

This honors the build script config, and is also simpler.
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.

4 participants