Add more ways to create a PathBuf to docs #41531

steveklabnik · 2017-04-25T10:40:11Z

The best way to do this wasn't in the documentation, and the ways that
were there needed some extra text to elaborate.

Fixes #40159

/cc @nagisa

rust-highfive · 2017-04-25T10:40:21Z

r? @BurntSushi

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

GuillaumeGomez · 2017-04-25T10:42:20Z

src/libstd/path.rs

@@ -1036,14 +1044,40 @@ impl<'a> cmp::Ord for Components<'a> {
 ///
 /// # Examples
 ///
+/// You can use `push` to build up a `PathBuf` from


Missing link to push method.

GuillaumeGomez · 2017-04-25T10:42:30Z

src/libstd/path.rs

 /// path.set_extension("dll");
 /// ```
+///
+/// However, `push` is best used for dynamic situations. This is a better way


Missing link to push method.

GuillaumeGomez · 2017-04-25T10:42:40Z

src/libstd/path.rs

+/// let path = PathBuf::from("c:\\windows\system32.dll");
+/// ```
+///
+/// Which method works best depends on what kind of sitaution you're in.


"sitaution"

Mark-Simulacrum · 2017-04-25T12:34:41Z

It'd be nice to document how to create a PathBuf (or Path) from Components, since as far as I know it's currently not trivial, since there's no way to directly collect an iterator with Item=Component into Path/PathBuf.

mzji · 2017-04-25T18:20:17Z

src/libstd/path.rs

+/// ```
+/// use std::path::PathBuf;
+///
+/// let path = PathBuf::from("c:\\windows\system32.dll");


Shouldn't the path be c:\\windows\\system32.dll ?
BTW, on windows we prefer to write the drive letters as uppercase. How about change the path to C:\\Windows\\system32\\user32.dll ? It's a real file path which exists on almost all Windows installations today.

on windows we prefer to write the drive letters as uppercase.

I thought so too (I'm a Windows user now) but since it was small before, I didn't change it. Happy to do so

Also, if I check in explorer.exe, C:\\windows\ does not load, but C:\windows\ does, so maybe it should be the other way? C:\\windows\\ doesn't load either.

Since Rust is a language which works on multiple platforms, I think that following the conventions on different platform is useful and important.
Here the \\ is a escape sequence in the path string, so the real path is C:\Windows\ (note the path fragment Windows is capitalized since on Windows the name of this directory is capitailized by default).

I was thinking of { as escapes in format strings, not \\ as an escape, whoops, heh 😓

nagisa · 2017-04-26T09:57:39Z

I would suggest using raw string to avoid escapes.

…

On Apr 26, 2017 12:49 PM, "mzji" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/libstd/path.rs <#41531 (comment)>: > +/// However, `push` is best used for dynamic situations. This is a better way +/// to do this when you know all of the components ahead of time: +/// +/// ``` +/// use std::path::PathBuf; +/// +/// let path: PathBuf = ["c:\\", "windows", "system32.dll"].iter().collect(); +/// ``` +/// +/// We can still do better than this! Since these are all strings, we can use +/// `From::from`: +/// +/// ``` +/// use std::path::PathBuf; +/// +/// let path = PathBuf::from("c:\\windows\system32.dll"); Here the \\ is a escape sequence in the path string, so the real path *is* C:\Windows\ (note the path fragment Windows is capitalized since on Windows the name of this directory is capitailized by default). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#41531 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApc0rOkpDhRbXAT7voA5o4N1LyTJDM4ks5rzxNEgaJpZM4NHTJ_> .

steveklabnik · 2017-05-03T09:59:30Z

I plan on updating this PR early next week; sorry about the delay.

carols10cents · 2017-05-08T13:26:56Z

Looks like there's a test failure for you to fix when you get back to this o.O:

[01:01:42] failures:
[01:01:42] 
[01:01:42] ---- path.rs - path::PathBuf (line 1076) stdout ----
[01:01:42] 	error: unknown character escape: s
[01:01:42]  --> <anon>:6:39
[01:01:42]   |
[01:01:42] 6 | let path = PathBuf::from("c:\\windows\system32.dll");
[01:01:42]   |                                       ^
[01:01:42] 
[01:01:42] error: aborting due to previous error(s)
[01:01:42] 
[01:01:42] thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:216
[01:01:42] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:01:42] 
[01:01:42] 
[01:01:42] failures:
[01:01:42]     path.rs - path::PathBuf (line 1076)
[01:01:42] 
[01:01:42] test result: FAILED. 783 passed; 1 failed; 22 ignored; 0 measured
[01:01:42] 
[01:01:42] error: test failed
[01:01:42]

steveklabnik · 2017-05-09T16:42:47Z

I believe I've fixed up everything; let's see what Travis says.

It'd be nice to document how to create a PathBuf (or Path) from Components, since as far as I know it's currently not trivial, since there's no way to directly collect an iterator with Item=Component into Path/PathBuf.

I agree this would be neat, but I don't know how either. I guess .map(as_os_str) and then collect from there? @rust-lang/libs any advice here?

The best way to do this wasn't in the documentation, and the ways that were there needed some extra text to elaborate. Fixes rust-lang#40159

alexcrichton · 2017-05-09T21:22:53Z

I'd recommend Components::as_path

Mark-Simulacrum · 2017-05-09T21:24:23Z

@alexcrichton The problem there is that it doesn't work as soon as you've filtered or done any other similar iterator operation on Components as far as I can tell; it'll only work if you have the "raw" Components iterator, which is non-ideal.

alexcrichton · 2017-05-09T21:25:53Z

Ah in that case we have impl<P: AsRef<Path>> FromIterator<P> for PathBuf, so that could be used I believe?

nagisa · 2017-05-09T21:26:26Z

src/libstd/path.rs

+/// ```
+/// use std::path::PathBuf;
+///
+/// let path: PathBuf = [r"C:\", "windows", "system32.dll"].iter().collect();


The \ after C: should not be necessary here?

Mark-Simulacrum · 2017-05-09T21:29:46Z

@alexcrichton No, that doesn't work either (https://is.gd/yG5UPk), since Component doesn't implement AsRef<Path> (it isn't a path, really, so this is understandable)--it feels like we need impl<P: AsRef<OsStr>> FromIterator<P> for PathBuf? Should I open a new issue about this?

nagisa · 2017-05-09T21:31:28Z

it isn't a path, really, so this is understandable

Component of a path is always a valid path.

The changes to me seem good and I’d r+; it seems to me like discussion about Components could be separate.

nagisa · 2017-05-09T21:36:28Z

@bors r+ rollup

@Mark-Simulacrum could you please fill a separate issue for Component?

bors · 2017-05-09T21:36:32Z

📌 Commit 23382e6 has been approved by nagisa

Mark-Simulacrum · 2017-05-09T22:14:52Z

Filed #41866.

@nagisa

Add more ways to create a PathBuf to docs The best way to do this wasn't in the documentation, and the ways that were there needed some extra text to elaborate. Fixes rust-lang#40159 /cc @nagisa

@nagisa

Add more ways to create a PathBuf to docs The best way to do this wasn't in the documentation, and the ways that were there needed some extra text to elaborate. Fixes rust-lang#40159 /cc @nagisa

Rollup of 5 pull requests - Successful merges: #41531, #41536, #41809, #41854, #41886 - Failed merges:

rust-highfive assigned BurntSushi Apr 25, 2017

GuillaumeGomez reviewed Apr 25, 2017

View reviewed changes

frewsxcv added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Apr 25, 2017

mzji reviewed Apr 25, 2017

View reviewed changes

aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 26, 2017

Add more ways to create a PathBuf to docs

23382e6

The best way to do this wasn't in the documentation, and the ways that were there needed some extra text to elaborate. Fixes rust-lang#40159

steveklabnik force-pushed the gh40159 branch from 89f7aec to 23382e6 Compare May 9, 2017 16:44

nagisa reviewed May 9, 2017

View reviewed changes

Mark-Simulacrum mentioned this pull request May 9, 2017

Collecting into a PathBuf from Iterator<Item=Component> is hard. #41866

Closed

frewsxcv mentioned this pull request May 10, 2017

Rollup of 11 pull requests #41869

Closed

frewsxcv mentioned this pull request May 10, 2017

Rollup of 9 pull requests #41872

Closed

frewsxcv mentioned this pull request May 10, 2017

Rollup of 8 pull requests #41881

Closed

steveklabnik mentioned this pull request May 10, 2017

Rollup of 5 pull requests #41887

Merged

bors added a commit that referenced this pull request May 10, 2017

Auto merge of #41887 - steveklabnik:rollup, r=steveklabnik

978d2cf

Rollup of 5 pull requests - Successful merges: #41531, #41536, #41809, #41854, #41886 - Failed merges:

bors merged commit 23382e6 into rust-lang:master May 10, 2017

steveklabnik deleted the gh40159 branch October 25, 2017 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more ways to create a PathBuf to docs #41531

Add more ways to create a PathBuf to docs #41531

steveklabnik commented Apr 25, 2017

rust-highfive commented Apr 25, 2017

GuillaumeGomez Apr 25, 2017

GuillaumeGomez Apr 25, 2017

GuillaumeGomez Apr 25, 2017

Mark-Simulacrum commented Apr 25, 2017

mzji Apr 25, 2017 •

edited

Loading

GuillaumeGomez Apr 25, 2017

steveklabnik Apr 26, 2017

mzji Apr 26, 2017 •

edited

Loading

steveklabnik Apr 26, 2017

nagisa commented Apr 26, 2017 via email

steveklabnik commented May 3, 2017

carols10cents commented May 8, 2017

steveklabnik commented May 9, 2017 •

edited

Loading

alexcrichton commented May 9, 2017

Mark-Simulacrum commented May 9, 2017

alexcrichton commented May 9, 2017

nagisa May 9, 2017 •

edited

Loading

Mark-Simulacrum commented May 9, 2017

nagisa commented May 9, 2017 •

edited

Loading

nagisa commented May 9, 2017

bors commented May 9, 2017

Mark-Simulacrum commented May 9, 2017

Add more ways to create a PathBuf to docs #41531

Add more ways to create a PathBuf to docs #41531

Conversation

steveklabnik commented Apr 25, 2017

rust-highfive commented Apr 25, 2017

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

Mark-Simulacrum commented Apr 25, 2017

mzji Apr 25, 2017 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

steveklabnik Apr 26, 2017

Choose a reason for hiding this comment

mzji Apr 26, 2017 • edited Loading

Choose a reason for hiding this comment

steveklabnik Apr 26, 2017

Choose a reason for hiding this comment

nagisa commented Apr 26, 2017 via email

steveklabnik commented May 3, 2017

carols10cents commented May 8, 2017

steveklabnik commented May 9, 2017 • edited Loading

alexcrichton commented May 9, 2017

Mark-Simulacrum commented May 9, 2017

alexcrichton commented May 9, 2017

nagisa May 9, 2017 • edited Loading

Choose a reason for hiding this comment

Mark-Simulacrum commented May 9, 2017

nagisa commented May 9, 2017 • edited Loading

nagisa commented May 9, 2017

bors commented May 9, 2017

Mark-Simulacrum commented May 9, 2017

mzji Apr 25, 2017 •

edited

Loading

mzji Apr 26, 2017 •

edited

Loading

steveklabnik commented May 9, 2017 •

edited

Loading

nagisa May 9, 2017 •

edited

Loading

nagisa commented May 9, 2017 •

edited

Loading