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.

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

Bump rustc-serialize and use std::path #84

Merged
merged 1 commit into from Feb 26, 2015
Merged

Conversation

@alexcrichton
Copy link
Contributor

alexcrichton commented Feb 25, 2015

This commit bumps the dependency on rustc-serialize to 0.3 and allows
Url::from_file_path to work with std::path::Path as well (transitively
working for PathBuf too).

@alexcrichton
Copy link
Contributor Author

alexcrichton commented Feb 25, 2015

Er sorry, forgot to fill out some other portions, adding tests and such now.

@alexcrichton alexcrichton force-pushed the alexcrichton:update branch 10 times, most recently from 5fa4222 to e36390e Feb 26, 2015
This commit bumps the dependency on rustc-serialize to 0.3 and allows
`Url::from_file_path` to work with `std::path::Path` as well (transitively
working for `PathBuf` too).
@alexcrichton
Copy link
Contributor Author

alexcrichton commented Feb 26, 2015

Ok! Filled out tests and verified that it's working on both unix and windows

@SimonSapin
Copy link
Member

SimonSapin commented Feb 26, 2015

Is it still useful to have both Url::form_file_path and Url::form_directory_path, in the new std::path, or does Path now record enough information to tell them apart? (This can be a follow up.)

It's unfortunate that we now can't manipulate/test paths for platforms other than the one we build for (compared to using e.g. std::old_path::windows:: Path directly). But I guess the complexity of multiple types and traits is not worth it.

SimonSapin added a commit that referenced this pull request Feb 26, 2015
Bump rustc-serialize and use std::path
@SimonSapin SimonSapin merged commit 8b63a3f into servo:master Feb 26, 2015
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@alexcrichton
Copy link
Contributor Author

alexcrichton commented Feb 26, 2015

Currently yes I believe that the distinction is available. Very little normalization happens by default so a is a distinct path from a/ (which I think is what you want, right?).

For example: http://is.gd/l1SS8I

@alexcrichton alexcrichton deleted the alexcrichton:update branch Feb 26, 2015
@SimonSapin
Copy link
Member

SimonSapin commented Feb 26, 2015

Right, this is what I meant. But I don’t know if we can rely on things like std::env::current_dir (when it’s ported to new path), or other user code that constructs a path to a directory, to include that CurDir component.

@alexcrichton
Copy link
Contributor Author

alexcrichton commented Feb 26, 2015

Ah yes that I would not rely on (I doubt current_dir returns paths with trailing slashes)

@SimonSapin
Copy link
Member

SimonSapin commented Feb 26, 2015

Ok, so we’ll keep the separate from_*_path methods.

Thanks for the PR!

@alexcrichton alexcrichton restored the alexcrichton:update branch Mar 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.