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

Expose more of std::path #22208

Merged
merged 1 commit into from
Feb 18, 2015
Merged

Expose more of std::path #22208

merged 1 commit into from
Feb 18, 2015

Conversation

aturon
Copy link
Member

@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
Collaborator

r? @nikomatsakis

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

@aturon
Copy link
Member 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
}

/// The primary sperator for the current platform
pub const MAIN_SEP: char = platform::MAIN_SEP;
Copy link
Member

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
Member Author

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

@bors r+ d855febb1801dc2e9c5da80f30bf3d5bba8b5754

@huonw
Copy link
Member

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
Member Author

aturon commented Feb 13, 2015

Fixed!

@bors: r=nikomatsakis 949c37d

@aturon
Copy link
Member Author

aturon commented Feb 13, 2015

@bors: r=nikomatsakis a433c2f

@bors
Copy link
Contributor

bors commented Feb 13, 2015

⌛ Testing commit a433c2f with merge cdb941e...

@bors
Copy link
Contributor

bors commented Feb 13, 2015

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

@aturon
Copy link
Member Author

aturon commented Feb 13, 2015

@bors: r=nikomatsakis 66e4c2f

@bors
Copy link
Contributor

bors commented Feb 14, 2015

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

@bors
Copy link
Contributor

bors commented Feb 14, 2015

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

@aturon
Copy link
Member 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
Contributor

bors commented Feb 16, 2015

⌛ Testing commit f2330ca with merge 2b151cb...

@bors
Copy link
Contributor

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
Member Author

aturon commented Feb 16, 2015

@bors: r=nikomatsakis 4a9dd3f

@bors
Copy link
Contributor

bors commented Feb 17, 2015

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

@bors
Copy link
Contributor

bors commented Feb 17, 2015

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

@alexcrichton
Copy link
Member

@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
Contributor

bors commented Feb 18, 2015

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

@bors
Copy link
Contributor

bors commented Feb 18, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

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