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.

Sign up for GitHub

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

Jump to bottom

Update remote matching to be more general #6

Closed
calebwherry opened this issue Oct 10, 2021 · 2 comments
Closed

Update remote matching to be more general #6

calebwherry opened this issue Oct 10, 2021 · 2 comments
Assignees
@robertdfrench
Labels
enhancement New feature or request
Milestone
Git PR v1.0.0

Comments

@calebwherry
Copy link
Collaborator

Using origin can cause issues with others not using that wordage. Replace with "not slash" match as it can be anything.

Originally posted by @calebwherry in #1 (comment)
@calebwherry
Copy link
Collaborator Author

The current regex for this is:

Regex::new(r"^ * remotes/origin/")

But why can't it just be:

Regex::new(r"^ * remotes/")

@robertdfrench Does it even matter what comes after? Do we have to guarantee something is there? I can't think a case where the second one would fail.

@calebwherry calebwherry self-assigned this Oct 11, 2021
@calebwherry calebwherry added this to the Git PR v1.0.0 milestone Oct 11, 2021
@calebwherry calebwherry added the enhancement New feature or request label Oct 11, 2021
@robertdfrench
Copy link
Owner

That would work for matching, but because the current design (which has problems like #7 anyway) uses the same regex for matching and removal, that change wouldn't work. It would lead to pr names looking like "origin/pr-name".

@robertdfrench robertdfrench mentioned this issue Oct 12, 2021
Merged
calebwherry pushed a commit that referenced this issue Oct 17, 2021
@robertdfrench
Match anything but '/' for origin name. Fixes #6.
801950e 
As @calebwherry said in the original issue:

> Using origin can cause issues with others not using that wordage.
> Replace with "not slash" match as it can be anything.

I tried to be clever and match hyphenated words, but then I tried to
look up the actual definition of a remote name, and there does not seem
to be one: https://stackoverflow.com/questions/41461152/which-characters-are-illegal-within-a-git-remote-name/41462742

So, it seems like the best thing to do is detect whatever isn't a slash,
because we know those *can't* be allowed, since they are allowed in
branch names (and therefore any ref with slashes in the remote or
branch would parse ambiguously). Whatever else git tells us is obviously
valid, since git is the authority here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@robertdfrench robertdfrench

Labels
enhancement New feature or request
Projects
None yet
Milestone
Git PR v1.0.0
Development

No branches or pull requests
2 participants
@robertdfrench @calebwherry