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.

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

Support same name repository #14

Merged
merged 4 commits into from
Jun 9, 2014

Conversation

Sixeight
Copy link
Contributor

@Sixeight Sixeight commented Jun 7, 2014

Support same name repository like Bundler. (see also: http://bundler.io/v1.6/git.html)
We can ghq get ruby then clone https://github.com/ruby/ruby now.

(I'm sorry for new last line...)


Do I support /ruby ?

diff --git a/url.go b/url.go
index 3912537..9cbb802 100644
--- a/url.go
+++ b/url.go
@@ -29,8 +29,8 @@ func NewURL(ref string) (*url.URL, error) {
        }

        if !url.IsAbs() {
-               if !strings.Contains(url.Path, "/") {
-                       url.Path = url.Path + "/" + url.Path
+               if strings.Index(url.Path, "/") <= 0 {
+                       url.Path = url.Path + "/" + strings.Replace(url.Path, "/", "", 1)
                }
                url.Scheme = "https"
                url.Host = "github.com"
diff --git a/url_test.go b/url_test.go
index e99c46e..e97b6a2 100644
--- a/url_test.go
+++ b/url_test.go
@@ -34,6 +34,11 @@ func TestNewURL(t *testing.T) {
        Expect(sameNameRepository.String()).To(Equal("https://github.com/same-name-ghq/same-name-ghq"))
        Expect(sameNameRepository.Host).To(Equal("github.com"))
        Expect(err).To(BeNil())
+
+       sameNameRepositoryWithSlash, err := NewURL("/same-name-ghq")
+       Expect(sameNameRepositoryWithSlash.String()).To(Equal("https://github.com/same-name-ghq/same-name-ghq"))
+       Expect(sameNameRepositoryWithSlash.Host).To(Equal("github.com"))
+       Expect(err).To(BeNil())
 }

 func TestConvertGitHubURLHTTPToSSH(t *testing.T) {

@motemen
Copy link
Member

motemen commented Jun 7, 2014

gofmt please? (and then the last blank line should be removed)

@motemen
Copy link
Member

motemen commented Jun 7, 2014

I don't think we should support paths starting with "/" (e.g. "/ruby" expands to "https://github.com/ruby/ruby). You can fail with error or leave the behavior undefined in that case.

@Sixeight
Copy link
Contributor Author

Sixeight commented Jun 7, 2014

Oh, I'm sorry. fix it soon.

@Sixeight
Copy link
Contributor Author

Sixeight commented Jun 7, 2014

@motemen Fixed. Please review again.

Thank you for your mention of paths starting with "/". I decided to avoid "/ruby" case. Current behavior /ruby => https://github.com/ruby, /ruby/ruby => https://github.com/ruby/ruby.

}
isSSH := c.Bool("p")
if isSSH {
fmt.Println(url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debugging line remained?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very very sorry..

@Sixeight
Copy link
Contributor Author

Sixeight commented Jun 8, 2014

refs #4

@motemen
Copy link
Member

motemen commented Jun 9, 2014

Thanks 😄

motemen added a commit that referenced this pull request Jun 9, 2014
@motemen motemen merged commit 00f1ca1 into x-motemen:master Jun 9, 2014
@Sixeight
Copy link
Contributor Author

Sixeight commented Jun 9, 2014

Thank you for your patient review. my PR is badly made... 🙇

@Sixeight Sixeight deleted the patch/same-name-repository branch June 9, 2014 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants