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

Missing link to push method.

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

src/libstd/path.rs

Missing link to push method.

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

src/libstd/path.rs

"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

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

src/libstd/path.rs Outdated

Copy link

Member

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

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

Missing link to push method.

GuillaumeGomez reviewed Apr 25, 2017

View reviewed changes

src/libstd/path.rs Outdated

Copy link

Member

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

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

Missing link to push method.

GuillaumeGomez reviewed Apr 25, 2017

View reviewed changes

src/libstd/path.rs Outdated

Copy link

Member

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

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

"sitaution"

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

Uh oh!

Conversation

steveklabnik commented Apr 25, 2017

Uh oh!

rust-highfive commented Apr 25, 2017

Uh oh!

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum commented Apr 25, 2017

Uh oh!

mzji Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez Apr 25, 2017

Choose a reason for hiding this comment

Uh oh!

steveklabnik Apr 26, 2017

Choose a reason for hiding this comment

Uh oh!

mzji Apr 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steveklabnik Apr 26, 2017

Choose a reason for hiding this comment

Uh oh!

nagisa commented Apr 26, 2017 via email

Uh oh!

steveklabnik commented May 3, 2017

Uh oh!

carols10cents commented May 8, 2017

Uh oh!

steveklabnik commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexcrichton commented May 9, 2017

Uh oh!

Mark-Simulacrum commented May 9, 2017

Uh oh!

alexcrichton commented May 9, 2017

Uh oh!

nagisa May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum commented May 9, 2017

Uh oh!

nagisa commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nagisa commented May 9, 2017

Uh oh!

bors commented May 9, 2017

Uh oh!

Mark-Simulacrum commented May 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

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