-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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(util): Allow git tag and branch name with hash #4881
Conversation
I'm a bit concerned by the ambiguity this change would add, which could cause issues in other parts of our stack that make the assumption that there can only be a single |
I agree. We can keep the old function as it is and handle multiple |
@arcanis I don't agree, if yarn is supposed to support git tag, it must support all forms of valid tag. |
@arcanis ☝️ |
Sorry @torifat, I missed your comment! Problem with that is that it breaks a very common assumption about URLs, so I'm not sure it would work :/ Is it common to have hashes inside a branch name? I understand the "it's valid git" argument, but it's quite inconvenient nevertheless. |
I think any legit |
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.
Agree with @arcanis here. I'm okay if we decode the part after the #
sign.
I opened #5960 before realizing that this PR existed. I'll close mine since it's basically the same as this one. My opinion is that unless we can prove that this does break something elsewhere in yarn, I don't see a reason to not support it. I don't think the branch/tag is used in a URL so I don't think it would need URL encoded. It's just passed through |
Summary
In Git
#
is a valid character fortag
andbranch
Fixes #4880