Rework of the URL subtraction feature #1392
Conversation
psf-chronographer
bot
added
the
bot:chronographer:provided
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodSpeed Performance ReportMerging #1392 will not alter performanceComparing Summary
Benchmarks breakdown
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1392 +/- ##
==========================================
+ Coverage 96.14% 96.20% +0.05%
==========================================
Files 31 31
Lines 5970 6053 +83
Branches 364 370 +6
==========================================
+ Hits 5740 5823 +83
Misses 204 204
Partials 26 26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I'm going to do a 1.17.0 release so we can start preparing aiohttp 3.11.x and I want do to some more downstream cleanups. We can do a 1.18.0 for this once its ready |
|
Some conflicts happened. I think I've resolved them correctly |
| ("path/to", "spam/", "../path/to"), | ||
| ("path/../to", "path/", "../to"), | ||
| ("path/..", ".", "path/.."), | ||
| ("path/../replace/me", "path/../replace", "replace/me"), |
There was a problem hiding this comment.
How is this different to the one below?
("path/../replace/me", "path/../replace/", "me"),
There was a problem hiding this comment.
If the last segment of the path does not have a / at the end, it is simply ignored when calculating the relative path
There was a problem hiding this comment.
In fact, we can say that if the path ends with a /, then its last segment is treated as a directory, otherwise - as a file. At least that's how it works for now 🤷♂️
There was a problem hiding this comment.
pathlib PosixPath can not be used to relativize URL pathes as URL pathes can have empty segments, and PosixPath strips trailing / anyway.
There was a problem hiding this comment.
Yeah, I've already noticed that...
There was a problem hiding this comment.
I'll think about what to replace it with
There was a problem hiding this comment.
I think I've managed to solve it
| ("http://example.com", "http://example.com/", "."), | ||
| ("//example.com", "//example.com", "."), | ||
| ("/path/to", "/spam/", "../path/to"), | ||
| ("path/to", "spam/", "../path/to"), |
There was a problem hiding this comment.
no path
("/", "/to", ".."),
("/", "/path/to", "../.."),
normal
("/path", "/path/to", ".."),
trailing / - empy segment at the end
("/path", "/path/", ".."),
("/path", "/path/to/", "../.."),
There was a problem hiding this comment.
While I support the intention I propose more conservative approach.
Let's provide a method, name it, instead of __sub__ operator overloading.
Add tests, doc, etc.
Publish new yarl release, wait for feedback.
Tune and polish.
After some time we can return to adding __sub__.
|
I sincerely apologize for such a long break. I haven't forgotten about this pull request, I've just been really busy lately. Now I need some time to remember everything and get back to work... |
for more information, see https://pre-commit.ci
What do these changes do?
Rework of the URL subtraction feature (added in #1340; removed in #1391)
Are there changes in behavior for the user?
Being able to calculate the relative path between two URLs using the subtraction syntax:
Related issue number
Resolves #1183
Checklist
Known issues