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

ch09-02: ? may be used with Result or Option #1852

Closed
wants to merge 1 commit into from
Closed

ch09-02: ? may be used with Result or Option #1852

wants to merge 1 commit into from

Conversation

BooleanCat
Copy link

@BooleanCat BooleanCat commented Mar 9, 2019

ch09-02 makes (what seems to me, I'm still learning!) to be a false claim that ? may only be used with Result.

Yet when I tried this it compiled just fine:

fn hello() -> Option<String> {
    Some(String::from("hello"))?;
    Some(String::from("bye"))
}

I'm a little uncomfortable with the change I'm suggesting here since it seems strange to claim that ? works with Option without demonstrating it as such - however this section of the book doesn't seem like the right place for that. Any advice?

@carols10cents
Copy link
Member

to be a false claim that ? may only be used with Result.

Yes, it used to be the case that ? could only be used with Result-- the ability to use it with Option was only recently added and we haven't gotten around to documenting it yet :)

Thank you for the suggested fix -- I think we're going to fix it a different way though, because the rest of this paragraph only talks about Result and doesn't really explain what using ? on Option does, exactly.

This already has a bug to track it: #1679 so thank you for the reminder, and we'll fix this soon!

@UnHumbleBen
Copy link
Contributor

Seeing as how the other pull request for this issue (#1713) was rejected for being too large, this pull request should be merge as a temporary fix instead, to at least let the reader know that ? can be used on Option, even if the explanation doesn't exist yet.

@UnHumbleBen
Copy link
Contributor

Alternatively I made my own pull request with a more subtle change which may be easier to incorporate as a temporary fix.
#2047

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