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

Don't use the word 'unwrap' to describe core 'unwrap' functions #68849

Closed
wants to merge 2 commits into from

Conversation

brson
Copy link
Contributor

@brson brson commented Feb 5, 2020

It's tautological, and "unwrap" is essentially Rust-specific jargon.

I was teaching a newbie some Rust, and doing the usual hand-waving about error handling using unwrap. They asked what 'unwrap' means. I said look it up in the docs. The docs read (paraphrased) "unwrap unwraps". I was embarrassed.

This patch just changes the language to "consume", which is at least more general than "unwrap", has a clear English-language meaning, and is not the same word as the function names. It has the downside that "consuming" in the type system is still pretty Rust-specific and may not have obvious meaning in this context.

@rust-highfive
Copy link
Collaborator

r? @rkruppe

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 5, 2020
@brson
Copy link
Contributor Author

brson commented Feb 5, 2020

Oh, note that the "main" Option::unwrap function says "Move the value ... ", so it already has better docs, but Result::unwrap uses the "unwrap" language. It could be nicer to mimic that functions docs for all the other unwraps.

@brson
Copy link
Contributor Author

brson commented Feb 5, 2020

OK, I'm taking another pass at this to try to make all the unwrap docs more consistent.

@brson
Copy link
Contributor Author

brson commented Feb 5, 2020

I pushed another commit that tries to make the various unwrap methods across Option and Result consistent in their language. It uses the "Move" language from the existing Option::unwrap docs, though I think it reads a bit awkwardly.

I also changed two argument names in the Result methods, optb -> default, f -> op, for consistency with Option.

I didn't touch the unwrap_or_default docs.

@brson
Copy link
Contributor Author

brson commented Feb 5, 2020

OK, realizing their are various non-unwrap methods that also use the "unwrap" language, I'm feeling a bit frustrated and indecisive. I think none of them should say "unwrap" (unless they are explicitly referencing the unwrap method) since a newbie could encounter any of them and have no idea what unwrap means.

But I also think the "Moves the value v out of the Option<T> ..." language is verbose and doesn't read great.

I'm going to close this and rethink the whole thing. BBL...

@brson brson closed this Feb 5, 2020
@Ixrec
Copy link
Contributor

Ixrec commented Feb 5, 2020

FWIW, "unwrap" also has a regular English meaning separate from its jargon meaning, i.e. to remove a wrapper / layer of wrapping around the thing you're really interested in. The idea is that types like Box<T>, Option<T>, Result<T, E> are often just wrapping a T. The similar word "unbox" also comes to mind, but that's more easily mistaken for a Box-specific operation (std::num::Wrapping exists, but is far less common). I've also heard this usage of "unwrapping" in the context of several other languages to talk about smart pointers, monadic wrapper types, Java's autoboxing/autounboxing, or even just user-defined structs with one field, so IMO it's not even "Rust jargon"; at worst it's programming jargon. And finally, at least to my ear, the gap between "consume"'s English meaning and Rust meaning is significantly wider than it is for "unwrap."

But I do agree that there are some "unwrap() unwraps" traps worth fixing here. When someone is looking at the docs specifically for method foo(), it's reasonable to address the possibility that they have no idea what the English word foo means.

@mati865
Copy link
Contributor

mati865 commented Feb 5, 2020

I think extract would fit here as well and it doesn't collide with other names.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 8, 2020
Don't use the word "unwrap" to describe "unwrap" methods

It's tautological, and "unwrap" is essentially Rust-specific jargon.

I was teaching a newbie some Rust, and doing the usual hand-waving about error handling using unwrap. They asked what 'unwrap' means. I said look it up in the docs. The docs read (paraphrased) "unwrap unwraps". I was embarrassed.

This changes all the Option/Result functions with unwrapping behavior to use a variation on a single description:

> "Returns the contained `Some/Ok` value [or ...]."

It also renames the closure of `Result::unwrap_or_else` to `default` for consistency with `Option`, and perhaps makes a few other small tweaks.

Previous: rust-lang#68849
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 8, 2020
Don't use the word "unwrap" to describe "unwrap" methods

It's tautological, and "unwrap" is essentially Rust-specific jargon.

I was teaching a newbie some Rust, and doing the usual hand-waving about error handling using unwrap. They asked what 'unwrap' means. I said look it up in the docs. The docs read (paraphrased) "unwrap unwraps". I was embarrassed.

This changes all the Option/Result functions with unwrapping behavior to use a variation on a single description:

> "Returns the contained `Some/Ok` value [or ...]."

It also renames the closure of `Result::unwrap_or_else` to `default` for consistency with `Option`, and perhaps makes a few other small tweaks.

Previous: rust-lang#68849
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 8, 2020
Don't use the word "unwrap" to describe "unwrap" methods

It's tautological, and "unwrap" is essentially Rust-specific jargon.

I was teaching a newbie some Rust, and doing the usual hand-waving about error handling using unwrap. They asked what 'unwrap' means. I said look it up in the docs. The docs read (paraphrased) "unwrap unwraps". I was embarrassed.

This changes all the Option/Result functions with unwrapping behavior to use a variation on a single description:

> "Returns the contained `Some/Ok` value [or ...]."

It also renames the closure of `Result::unwrap_or_else` to `default` for consistency with `Option`, and perhaps makes a few other small tweaks.

Previous: rust-lang#68849
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 8, 2020
Don't use the word "unwrap" to describe "unwrap" methods

It's tautological, and "unwrap" is essentially Rust-specific jargon.

I was teaching a newbie some Rust, and doing the usual hand-waving about error handling using unwrap. They asked what 'unwrap' means. I said look it up in the docs. The docs read (paraphrased) "unwrap unwraps". I was embarrassed.

This changes all the Option/Result functions with unwrapping behavior to use a variation on a single description:

> "Returns the contained `Some/Ok` value [or ...]."

It also renames the closure of `Result::unwrap_or_else` to `default` for consistency with `Option`, and perhaps makes a few other small tweaks.

Previous: rust-lang#68849
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants