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.

Sign up for GitHub

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

Jump to bottom

std::env::SplitPaths does not document its Item type #59543

Closed
brettcannon opened this issue Mar 30, 2019 · 5 comments · Fixed by #60022
Closed

std::env::SplitPaths does not document its Item type #59543

brettcannon opened this issue Mar 30, 2019 · 5 comments · Fixed by #60022
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. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@brettcannon
Copy link
Contributor

Reading both https://doc.rust-lang.org/std/env/struct.SplitPaths.html and https://doc.rust-lang.org/std/env/fn.split_paths.html there's no specification that the Item type for the iterator is PathBuf. To me it seemed ambiguous what an "iterator that splits an environment variable into paths" or "an iterator over the paths" returns since it could be the string representing the paths -- which is OsStr -- or an actual Path object.

I'm happy to submit a PR if this is makes sense enough to fix (since according #30093 it used to specify this detail and now it doesn't with a quick perusal of https://github.com/rust-lang/rust/blame/master/src/libstd/env.rs not turning up anything obvious as to why the change).
@Centril Centril added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Mar 30, 2019
@jonas-schievink jonas-schievink added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Mar 30, 2019
@DevQps
Copy link
Contributor

DevQps commented Apr 3, 2019 •
edited
Loading

@brettcannon If you unfold impl<'a> Iterator for SplitPaths<'a> at the Trait Implementation section in SplitPaths you can see that type Item = PathBuf. Did you mean that this should be more clearly mentioned in the description of SplitPaths (or maybe split_paths as well)?

I do think it would be nice if some extra text on SplitPaths would be added that clarifies the item type of the iterator!

@brettcannon
Copy link
Contributor Author

brettcannon commented Apr 6, 2019 •
edited
Loading

@DevQps since I didn't even think of expanding impl<'a> Iterator for SplitPaths<'a> for the info (which in hindsight makes sense), my answer is "yes" 😉 , mentioning it explicitly as part of the main summary would be handy".

As I said, if this is deemed reasonable I'm happy to do the PR to add the change.

@DevQps
Copy link
Contributor

DevQps commented Apr 6, 2019

@brettcannon That would be great! Let us know what you come up with!

@nathankleyn nathankleyn mentioned this issue Apr 16, 2019
Merged
nathankleyn added a commit to nathankleyn/rust that referenced this issue Apr 17, 2019
@nathankleyn
Document Item type in std::env::SplitPaths iterator.
8951eea 
Previously there wasn't any documentation to show what the type of
`Item` was inside `std::env::SplitPaths`. Now, in the same format as
other examples of docs in `srd` for `Iterator#Item`, we mention the
type.

This fixes rust-lang#59543.
@brettcannon
Copy link
Contributor Author

Looks like someone snagged fixing this before I had a chance to get to it, so I'll let #60022 and its author handle fixing this.

@DevQps
Copy link
Contributor

DevQps commented Apr 27, 2019

@brettcannon Nice :) Let's wait for that then :)

Centril added a commit to Centril/rust that referenced this issue Apr 28, 2019
@Centril
Rollup merge of rust-lang#60022 - nathankleyn:fix-issue-59543, r=Centril
95abeb0 
Document `Item` type in `std::env::SplitPaths` iterator.

Previously there wasn't any documentation to show what the type of
`Item` was inside `std::env::SplitPaths`. Now, in the same format as
other examples of docs in `std` for `Iterator#Item`, we mention the
type.

This fixes rust-lang#59543.

r? @steveklabnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Document Item type in std::env::SplitPaths iterator. nathankleyn/rust
4 participants
@brettcannon @Centril @jonas-schievink @DevQps