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

Expand examples on str functions #54520

Open
SoniEx2 opened this issue Sep 24, 2018 · 18 comments
Open

Expand examples on str functions #54520

SoniEx2 opened this issue Sep 24, 2018 · 18 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@SoniEx2
Copy link
Contributor

SoniEx2 commented Sep 24, 2018

Functions like contains, starts_with, ends_with, match_indices, etc could use more examples, for example foo.contains('\n') (check if a string contains newlines).

find, split, matches, trim_matches, etc all have them.

@SoniEx2 SoniEx2 changed the title Add more examples to more str functions Expand examples on str functions Sep 24, 2018
@hellow554
Copy link
Contributor

Why don't you submit a PR ;) It's a very good and easy topic to work on

@frewsxcv frewsxcv added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Sep 25, 2018
@frewsxcv
Copy link
Member

Functions like contains, starts_with, ends_with, match_indices, etc could use more examples, for example foo.contains('\n') (check if a string contains newlines).

https://doc.rust-lang.org/std/primitive.str.html#method.contains

Here's the current example we have:

let bananas = "bananas";

assert!(bananas.contains("nana"));
assert!(!bananas.contains("apples"));

What does foo.contains('\n') introduce conceptually that the existing example doesn't?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Sep 25, 2018

it shows you can use single characters and stuff?

@shepmaster
Copy link
Member

shepmaster commented Sep 25, 2018

Do you also suggest adding examples using a closure?

I think it would be better to highlight that these functions take a Pattern, link to that documentation, and show examples inside of Pattern of all the standard library provided implementations. This reduces redundancy.

@shepmaster
Copy link
Member

Also worth noting that Pattern is unstable and there's an RFC out there to improve it /cc @kennytm .

@kennytm kennytm added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Sep 26, 2018
@frewsxcv
Copy link
Member

it shows you can use single characters and stuff?

the documentation for this is in Pattern. extending the docs for all APIs that use Pattern demonstrating all the possible Pattern variations seems unrelated to the APIs we're documenting, in my opinion.

@pm215
Copy link

pm215 commented Dec 19, 2018

The most salient feature of the Pattern documentation is all the "nightly only", "nightly only" warnings in it. I think it would be better to be able to understand the documentation for non-nightly features without having to look at documentation that is so marked. Also in general, if you're relatively new to Rust, I think it's much easier to understand a concrete example than to follow a link to a page documenting an abstract Trait and then have to figure out how the trait applies to the particular situation you were interested in. More examples in the Pattern page itself would probably help too.

@hellow554
Copy link
Contributor

hellow554 commented Dec 19, 2018

https://doc.rust-lang.org/std/primitive.str.html#method.split

The pattern can be a &str, char, or a closure that determines the split.

Examples

  • Simple patterns:
  • A more complex pattern, using a closure:
  • If a string contains multiple contiguous separators, you will end up with empty strings in the output:
  • Contiguous separators are separated by the empty string.
  • ...

I honstely think there are more than enough code examples all with explanation. If you think we are missing something, please feel free to suggest one or open a PR and add one. We are looking forward to it!

@pm215
Copy link

pm215 commented Dec 19, 2018

Yes, the split method has a good set of examples. I'm here because I started at the trim_start_matches method, which doesn't so much. I raised a bug (#56891); it was merged with this one; estebank filed a PR #56898 which I think would be fine as a fix for it; shepmaster commented on that PR saying they were against merging it; so I'm here trying to argue the case for being a bit more verbose with the examples.

@shepmaster
Copy link
Member

shepmaster commented on that PR saying they were against merging it

To be clear, I'm only one voice; others may (and probably do) disagree with me.

I'm mostly arguing from a maintenance perspective; this request is the equivalent of "inline the Pattern docs into every API function that takes a Pattern argument". We don't do that for Iterator, a much more common argument (I believe). It makes it harder to explain things once and keep something up to date. Once Pattern stabilizes, any crate can implement it (think Regex), but cannot add itself to the str::split documentation after the fact.

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Dec 19, 2018

The problem is that Pattern is nightly-only, and the docs aren't written to reflect Pattern matching but instead give examples based on concrete impls.

@shepmaster
Copy link
Member

shepmaster commented Dec 19, 2018

The most salient feature of the Pattern documentation is all the "nightly only", "nightly only" warnings in it. I think it would be better to be able to understand the documentation for non-nightly features without having to look at documentation that is so marked.

I agree with this in general, but it's not an uncommon occurrence. Examples off the top of my head...

  1. Try is unstable, but ? is stable and relies on Try.

  2. Termination is unstable, but returning Result from main is stable and relies on Termination.

  3. Step is unstable, but the implementation of Iterator for Range are both stable and rely on Step.

I'd prefer to have some solution that can address all cases of this.

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Dec 20, 2018

All of those cases are used once or only a few times, and as such are allowed to rely on concrete stuff for the docs.

The problem with Pattern is that it's used MANY times...

Hmm. I wonder if we could boil everything down to one or two core Pattern-using things and describe everything else as a combination of them, so that inlining Pattern's docs only needs to happen a handful of times...

@steveklabnik steveklabnik added the P-medium Medium priority label Dec 27, 2018
@DevQps
Copy link
Contributor

DevQps commented Mar 30, 2019

shepmaster commented on that PR saying they were against merging it

To be clear, I'm only one voice; others may (and probably do) disagree with me.

I'm mostly arguing from a maintenance perspective; this request is the equivalent of "inline the Pattern docs into every API function that takes a Pattern argument". We don't do that for Iterator, a much more common argument (I believe). It makes it harder to explain things once and keep something up to date. Once Pattern stabilizes, any crate can implement it (think Regex), but cannot add itself to the str::split documentation after the fact.

Personally I think that making users aware that a Pattern is passed is the right way. Pattern is pretty powerful, and once people realize that everything that implements Pattern (such as &str and char) everything will probably become much more clear for them without making it seem like it only works on &str and chars.

Maybe we can find some middle ground by linking the examples to Pattern more clearly but also stating that Pattern is implemented by chars and &str? That would probably only add a single line of clarification without having to provide many examples all over the place. I am curious to what you think of this! I don't mind picking this up, if @SoniEx2 don't want to! I probably need some extra pointers on which docs to edit however :)

@shepmaster
Copy link
Member

Also be aware of RFC 2005, which enhances Patternand renames it to Needle.

@DevQps
Copy link
Contributor

DevQps commented Apr 1, 2019

@shepmaster Thanks for the advice, do you think it would be wise to wait with writing any documentation as that might soon be replaced with the Needle API? Or do you suspect that that will still take a while?

@DevQps
Copy link
Contributor

DevQps commented May 4, 2019

@shepmaster I hope you're still willing to respond to my previous comment here! :) Then I can either start writing some documentation or we can maybe wait for Needle to come through. (Y)

@shepmaster
Copy link
Member

wait with writing any documentation as that might soon be replaced with the Needle API

I don't expect it to be anytime soon — @kennytm do you have any feelings about when it might occur?

@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants