Skip to content

Expose more of std::path#22208

Merged
huonw merged 1 commit intorust-lang:masterfrom
aturon:expose-more-path
Feb 18, 2015
Merged

Expose more of std::path#22208
huonw merged 1 commit intorust-lang:masterfrom
aturon:expose-more-path

Conversation

@aturon
Copy link
Contributor

@aturon aturon commented Feb 12, 2015

This commit exposes the is_sep function and MAIN_SEP constant, as
well as Windows path prefixes. The path prefix enum is safely exposed on
all platforms, but it only yielded as a component for Windows.

Exposing the prefix enum as part of prefix components involved changing
the type from OsStr to the Prefix enum, which is a:

[breaking-change]

@rust-highfive
Copy link
Contributor

rust-highfive commented Feb 12, 2015

r? @nikomatsakis

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

@aturon
Copy link
Contributor Author

aturon commented Feb 12, 2015

cc @blaenk

@blaenk
Copy link
Contributor

blaenk commented Feb 12, 2015

It looks good! Thank you very much.

alexcrichton
alexcrichton reviewed Feb 12, 2015
View reviewed changes
src/libstd/path.rs Outdated
Copy link
Member

@alexcrichton alexcrichton Feb 12, 2015

Choose a reason for hiding this comment

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

Is it worth exposing both of these? Wouldn't is_sep be the same as foo == MAIN_SEP?

Copy link
Contributor Author

@aturon aturon Feb 12, 2015

Choose a reason for hiding this comment

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

No, Windows supports both / and \ as separators.

@aturon aturon force-pushed the expose-more-path branch 2 times, most recently from 30ec019 to d855feb Compare February 12, 2015 06:38
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 13, 2015

@bors r+ d855febb1801dc2e9c5da80f30bf3d5bba8b5754

@huonw
Copy link
Contributor

huonw commented Feb 13, 2015

These seem used rarely enough that expanding sep to separator may be clearer?

@blaenk
Copy link
Contributor

blaenk commented Feb 13, 2015

Yeah, it was originally proposed as is_separator iirc, so if it's no trouble we might as well expand it, since it will essentially be set in stone after this. I won't die over it though.

@aturon
Copy link
Contributor Author

aturon commented Feb 13, 2015

Fixed!

@bors: r=nikomatsakis 949c37d

@aturon
Copy link
Contributor Author

aturon commented Feb 13, 2015

@bors: r=nikomatsakis a433c2f

@bors
Copy link
Collaborator

bors commented Feb 13, 2015

⌛ Testing commit a433c2f with merge cdb941e...

@bors
Copy link
Collaborator

bors commented Feb 13, 2015

💔 Test failed - auto-win-32-nopt-t

@aturon
Copy link
Contributor Author

aturon commented Feb 13, 2015

@bors: r=nikomatsakis 66e4c2f

@bors
Copy link
Collaborator

bors commented Feb 14, 2015

⌛ Testing commit 66e4c2f with merge 44792ef...

@bors
Copy link
Collaborator

bors commented Feb 14, 2015

💔 Test failed - auto-win-32-nopt-t

@aturon
Copy link
Contributor Author

aturon commented Feb 15, 2015

@bors: r=nikomatsakis f2330ca

@blaenk
Copy link
Contributor

blaenk commented Feb 15, 2015

Any reason we're not exposing the Prefix methods? rust-lang/glob seems like it could use the is_verbatim. I could do the match manually, but if it's already there it seems like it'd be nice if it were exposed.

@bors
Copy link
Collaborator

bors commented Feb 16, 2015

⌛ Testing commit f2330ca with merge 2b151cb...

@bors
Copy link
Collaborator

bors commented Feb 16, 2015

💔 Test failed - auto-win-32-nopt-t

@aturon
Expose more of std::path
4a9dd3f 
This commit exposes the `is_sep` function and `MAIN_SEP` constant, as
well as Windows path prefixes. The path prefix enum is safely exposed on
all platforms, but it only yielded as a component for Windows.

Exposing the prefix enum as part of prefix components involved changing
the type from `OsStr` to the `Prefix` enum, which is a:

[breaking-change]
@aturon
Copy link
Contributor Author

aturon commented Feb 16, 2015

@bors: r=nikomatsakis 4a9dd3f

@bors
Copy link
Collaborator

bors commented Feb 17, 2015

⌛ Testing commit 4a9dd3f with merge 4361aee...

@bors
Copy link
Collaborator

bors commented Feb 17, 2015

💔 Test failed - auto-win-32-nopt-t

@alexcrichton
Copy link
Member

alexcrichton commented Feb 17, 2015

@bors: retry

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 17, 2015
@alexcrichton
rollup merge of rust-lang#22208: aturon/expose-more-path
eca2453 
This commit exposes the `is_sep` function and `MAIN_SEP` constant, as
well as Windows path prefixes. The path prefix enum is safely exposed on
all platforms, but it only yielded as a component for Windows.

Exposing the prefix enum as part of prefix components involved changing
the type from `OsStr` to the `Prefix` enum, which is a:

[breaking-change]
@bors
Copy link
Collaborator

bors commented Feb 18, 2015

⌛ Testing commit 4a9dd3f with merge 8d158c5...

@bors
Copy link
Collaborator

bors commented Feb 18, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton
Copy link
Member

alexcrichton commented Feb 18, 2015

@bors: retry

@bors
Copy link
Collaborator

bors commented Feb 18, 2015

⌛ Testing commit 4a9dd3f with merge 8c0b78e...

@huonw huonw merged commit 4a9dd3f into rust-lang:master Feb 18, 2015
@aturon aturon mentioned this pull request Feb 18, 2015
Closed
91 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nikomatsakis nikomatsakis

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@aturon @rust-highfive @blaenk @nikomatsakis @huonw @bors @alexcrichton