Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Support to repository name including dots ( fixes #13 ) #17

Merged
merged 5 commits into from
Aug 1, 2017
Merged

Support to repository name including dots ( fixes #13 ) #17

merged 5 commits into from
Aug 1, 2017

Conversation

wraith13
Copy link
Contributor

I fixed #13 .
Let me know if there are any problem.

@msftclas
Copy link

@wraith13,
Thanks for having already signed the Contribution License Agreement. Your agreement has not been validated yet. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@chrmarti
Copy link
Contributor

Would it be simpler to just match .git and then drop the following [^\s]*?

@chrmarti chrmarti self-assigned this Jul 17, 2017
@wraith13
Copy link
Contributor Author

Yes, I think so.

@chrmarti
Copy link
Contributor

If you get a chance to update the PR, I'll be happy to merge it.

@wraith13
Copy link
Contributor Author

Thanks!

@wraith13
Copy link
Contributor Author

Sorry! I misunderstood that you said. ( My close friend informed to me. )
I will fix it later.

@wraith13
Copy link
Contributor Author

I fixed. How about this?

@chrmarti
Copy link
Contributor

Don't all GitHub remote's have the .git suffix? I thought /[^\s]*github\.com[/:]([^/]+)\/([^ ]+)\.git/ might do it.

@wraith13
Copy link
Contributor Author

Sorry, I am not familiar with git.
So, I took a safe way.

@wraith13
Copy link
Contributor Author

https://stackoverflow.com/questions/2514859/regular-expression-for-git-repository
There seems to must be .git at the end of the path of the git repository.

@wraith13
Copy link
Contributor Author

How about this time?

@chrmarti
Copy link
Contributor

Looks good, except I just found a remote without the '.git' suffix. Maybe that has become optional at some point?

@wraith13
Copy link
Contributor Author

I do not know. However, I think other tools also expect(need) '.git'.

First of all, in accordance with KISS principle, adopt this simple code.
We will respond if the complaint actually comes.
How about such a plan?

@chrmarti
Copy link
Contributor

Let's remove \.git from the regex and then remove .git from the owner variable later if it is there. That's simple and I have a case where .git is missing myself.

@wraith13
Copy link
Contributor Author

It's a repo instead of an owner, is it?
I fixed, how is this?

@chrmarti chrmarti merged commit 7411dc5 into microsoft:master Aug 1, 2017
@chrmarti
Copy link
Contributor

chrmarti commented Aug 1, 2017

Thanks @wraith13 !

kieferrm pushed a commit to kieferrm/vscode-github-issues-prs that referenced this pull request Apr 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It does not work in a repository that contains dots in the path/name.
3 participants