-
Notifications
You must be signed in to change notification settings - Fork 251
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
Fix for git subdirectories #288
Conversation
@finswimmer @abn Any thoughts on this? |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Pinging @abn @finswimmer |
1 similar comment
Pinging @abn @finswimmer |
Hi @ashnair1 sorry about the latency. :) I need to check a couple of things downstream before reviewing this. But in the meantime can you add some test coverage? |
No problem. I've added the test case. Cheers. |
As mentioned in the companion PR, I've set the default language version for pre-commit to be 3.8 due to flake8-type-checking not supporting Python<3.8 |
src/poetry/core/vcs/git.py
Outdated
@@ -226,7 +226,7 @@ def normalize_url(cls, url: str) -> GitUrl: | |||
|
|||
formatted = re.sub(r"^git\+", "", url) | |||
if parsed.rev: | |||
formatted = re.sub(rf"[#@]{parsed.rev}$", "", formatted) | |||
formatted = re.sub(rf"[#@]{parsed.rev}", "", formatted) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a bit too greedy?
For example @subdirectory#subdirectory=library
will be formatted to =library
. Granted this is an edge case, but it is not beyond the possible scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this only happen if rev=subdirectory? We could just add a guard for it, but I am open to suggestions.
Oh I just got what you meant. You're right, this is a problem. Positive lookahead should fix this
formatted = re.sub(rf"[#@]{parsed.rev}(?=[#&]?)", "", formatted)
This should work except when rev="subdirectory". As I understand rev is either a commit hash or branch name so it's unlikely this would happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abn Needed a negative lookahead too to address the rev=subdirectory problem. Now it's good.
formatted = re.sub(rf"[#@]{parsed.rev}(?=[#&]?)(?!\=)", "", formatted)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some additional tests to ensure expression works as expected.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
This fix is required for correctly handling git urls with subdirectory. For example consider the following url
Since the regex pattern is looking for
@rev
at the end, it will not strip out the revision in the above url since rev is not at the end of the string resulting in a url like this:The correct url should be the following