Skip to content

Add a doctest for the std::string::as_string method. #19628

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

Merged
merged 1 commit into from
Dec 10, 2014

Conversation

jbranchaud
Copy link
Contributor

No description provided.

@jbranchaud
Copy link
Contributor Author

Sure thing!

@jbranchaud
Copy link
Contributor Author

@steveklabnik Is "Examples" supposed to be the standard convention? I am noticing the occurrence of "Example" a number of places throughout the API docs. If so, I can try to update some of these other offending instances in a separate PR as well.

@steveklabnik
Copy link
Member

We haven't officially made a convention because I haven't had the chance to write up the RFC, but you're correct that they're scattered. I wouldn't mind a PR updating them to all say "Examples," even if we eventually decide on a different convention, it's easier to change if they're all the same.

@jbranchaud jbranchaud force-pushed the add-string-as-string-doctest branch from 60f916e to 0beb7d8 Compare December 8, 2014 00:37
@jbranchaud
Copy link
Contributor Author

Updated.

@bluss
Copy link
Member

bluss commented Dec 8, 2014

Intrusive comment! :-)

I'm worried about "Doctest". Providing examples in documentation is great, it's a good goal. Testing functions through it is not -- there are much better ways to do that using the testing infrastructure.

So they should be useful examples first. The reason they are tested is just to make sure they continue to be up to date and correct.

With this in mind, the example in the PR is not very instructive to the reader of the documentation.

@bluss
Copy link
Member

bluss commented Dec 8, 2014

I should be constructive too, It's hard to find a good example truly, what do you think about this?

use std::string::as_string;

fn stringconsumer(s: &String) {
    assert_eq!(s, &"foo");
}

stringconsumer(&*as_string("foo"));

@jbranchaud jbranchaud force-pushed the add-string-as-string-doctest branch from 0beb7d8 to 6dc9828 Compare December 9, 2014 02:08
@jbranchaud
Copy link
Contributor Author

Updated based on feedback from @bluss.

@bluss Thanks for your suggestion and for providing an example, I've incorporated your feedback.

/// ```
/// use std::string::as_string;
///
/// fn string_ref_consumer(s: &String) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking a &String is almost never what we want, can we make this a String and

@steveklabnik
Copy link
Member

r=me with the updated example changes

Change Example to Examples.

Add a doctest that better demonstrates the utility of as_string.

Update the doctest example to use String instead of &String.
@jbranchaud jbranchaud force-pushed the add-string-as-string-doctest branch from 6dc9828 to de3fcee Compare December 9, 2014 17:12
@jbranchaud
Copy link
Contributor Author

Updated.

@steveklabnik
Copy link
Member

Thank you so much!

bors added a commit that referenced this pull request Dec 10, 2014
@bors bors closed this Dec 10, 2014
@bors bors merged commit de3fcee into rust-lang:master Dec 10, 2014
@bluss
Copy link
Member

bluss commented Dec 10, 2014

Friends, the example misses the point of as_string: it allows creating a &String using no allocation at all. It's a special case, half-obsoleted by BorrowFrom solutions in the HashMap instead, but that's the usecase of as_string.

The reason any example is contrieved is that there are no use cases I can think of, but what it used to be was that a certain interface needed &String, and this was a way to supply it without allocation. Creating a plain allocated String is easy using other means.

@jbranchaud jbranchaud deleted the add-string-as-string-doctest branch December 10, 2014 15:54
lnicola pushed a commit to lnicola/rust that referenced this pull request Apr 28, 2025
don't ignore config values that fail to parse
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