Skip to content

Add Url::make_relative() as the inverse of Url::join() #668

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

Merged
merged 1 commit into from
May 3, 2021

Conversation

sdroege
Copy link
Contributor

@sdroege sdroege commented Jan 28, 2021

This creates a relative URL from a base URL and another absolute URL.

Fixes #666

@sdroege
Copy link
Contributor Author

sdroege commented Jan 28, 2021

The clippy failures are all unrelated to this PR. I can submit another PR for them if you agree that all those should be fixed, or let me know which of the lints you'd like to disable (personally I'm not a fan of clippy::manual-range-contains...)

@djc
Copy link
Contributor

djc commented Jan 28, 2021

Yup, sorry about the clippy failures. Another PR would be nice, though a separate commit in this PR is also fine. I think we should fix all of them except the ones about the unit errors, because those would break backwards compatibility.

@sdroege
Copy link
Contributor Author

sdroege commented Jan 28, 2021

clippy fixes are in #669

@sdroege
Copy link
Contributor Author

sdroege commented Jan 28, 2021

Rebased on top of #669

@valenting
Copy link
Collaborator

Thanks for this PR. It looks really nice.
Would you mind adding a couple of tests for the negative? ie? returning None?

@sdroege
Copy link
Contributor Author

sdroege commented Mar 26, 2021

Sure, will add a few tests for that later or tomorrow! Thanks for the review

@sdroege
Copy link
Contributor Author

sdroege commented Mar 28, 2021

Added tests for the 4 possible error cases.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #703) made this pull request unmergeable. Please resolve the merge conflicts.

This creates a relative URL from a base URL and another absolute URL.

Fixes servo#666
@sdroege
Copy link
Contributor Author

sdroege commented May 3, 2021

And rebased for the merge conflicts. @valenting anything else missing here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API for converting a base URL and another URL into a relative one
4 participants