Refactor URL parsing so it's not being called twice

pkg/repo.go

@@ -1,6 +1,7 @@
 package pkg
 
 import (
+	urlpkg "net/url"
 	"os"
 	"path"
 
@@ -14,11 +15,8 @@ type Repo struct {
 	Status *RepoStatus
 }
 
-func CloneRepo(url string, repoRoot string) (path string, err error) {
+func CloneRepo(url *urlpkg.URL, repoRoot string) (path string, err error) {
-	repoPath, err := URLToPath(url)
+	repoPath := URLToPath(url)
-	if err != nil {
-		return path, err
-	}
 
 	path, err = MakeDir(repoRoot, repoPath)
 	if err != nil {
@@ -33,7 +31,7 @@ func CloneRepo(url string, repoRoot string) (path string, err error) {
 		RemoteCreateCallback: nil,
 	}
 
-	_, err = git.Clone(url, path, options)
+	_, err = git.Clone(url.String(), path, options)
 	if err != nil {
 		return path, errors.Wrap(err, "Failed cloning repo")
 	}

pkg/repo_test.go

@@ -1,6 +1,7 @@
 package pkg
 
 import (
+	urlpkg "net/url"
 	"os"
 	"testing"
 )
@@ -14,7 +15,9 @@ func TestFetch(t *testing.T) {
 
 	// Clone the origin repo
 	repoRoot := newTempDir(t)
-	path, err := CloneRepo(origin.Path(), repoRoot)
+	url, err := urlpkg.Parse(origin.Path())
+	checkFatal(t, err)
+	path, err := CloneRepo(url, repoRoot)
 	checkFatal(t, err)
 
 	// Open cloned repo and load its status

pkg/status_test.go

@@ -1,6 +1,7 @@
 package pkg
 
 import (
+	urlpkg "net/url"
 	"reflect"
 	"testing"
 
@@ -159,7 +160,9 @@ func TestStatusCloned(t *testing.T) {
 	origin := newTestRepo(t)
 	repoRoot := newTempDir(t)
 
-	path, err := CloneRepo(origin.Path(), repoRoot)
+	url, err := urlpkg.Parse(origin.Path())
+	checkFatal(t, err)
+	path, err := CloneRepo(url, repoRoot)
 	checkFatal(t, err)
 	repo, err := OpenRepo(path)
 	checkFatal(t, err)
@@ -213,7 +216,9 @@ func TestBranchCloned(t *testing.T) {
 	createBranch(t, origin, "branch")
 
 	repoRoot := newTempDir(t)
-	path, err := CloneRepo(origin.Path(), repoRoot)
+	url, err := urlpkg.Parse(origin.Path())
+	checkFatal(t, err)
+	path, err := CloneRepo(url, repoRoot)
 	checkFatal(t, err)
 	repo, err := OpenRepo(path)
 	checkFatal(t, err)

pkg/url.go

@@ -13,12 +13,35 @@ import (
 // See: https://golang.org/src/cmd/go/internal/get/vcs.go
 var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+)@([a-zA-Z0-9._-]+):(.*)$`)
 
-func URLToPath(rawurl string) (repoPath string, err error) {
+func ParseURL(rawURL string) (url *urlpkg.URL, err error) {
-	url, err := parseURL(rawurl)
+	// If rawURL matches SCP-like syntax, convert it into a URL.
-	if err != nil {
+	// eg, "git@github.com:user/repo" becomes "ssh://git@github.com/user/repo".
-		return "", err
+	if m := scpSyntax.FindStringSubmatch(rawURL); m != nil {
+		url = &urlpkg.URL{
+			Scheme: "ssh",
+			User:   urlpkg.User(m[1]),
+			Host:   m[2],
+			Path:   m[3],
+		}
+	} else {
+		url, err = urlpkg.Parse(rawURL)
+		if err != nil {
+			return nil, errors.Wrap(err, "Failed parsing URL")
+		}
 	}
 
+	if url.Host == "" && url.Path == "" {
+		return nil, errors.New("Parsed URL is empty")
+	}
+
+	if url.Scheme == "" || url.Scheme == "git+ssh" {
+		url.Scheme = "ssh"
+	}
+
+	return url, nil
+}
+
+func URLToPath(url *urlpkg.URL) (repoPath string) {
 	// remove port numbers from host
 	repoHost := strings.Split(url.Host, ":")[0]
 
@@ -29,29 +52,5 @@ func URLToPath(rawurl string) (repoPath string, err error) {
 	// remove tilde (~) char from username
 	repoPath = strings.ReplaceAll(repoPath, "~", "")
 
-	return repoPath, nil
+	return repoPath
-}
-
-func parseURL(rawurl string) (url *urlpkg.URL, err error) {
-	// If rawurl matches SCP-like syntax, convert it into a URL.
-	// eg, "git@github.com:user/repo" becomes "ssh://git@github.com/user/repo".
-	if m := scpSyntax.FindStringSubmatch(rawurl); m != nil {
-		url = &urlpkg.URL{
-			Scheme: "ssh",
-			User:   urlpkg.User(m[1]),
-			Host:   m[2],
-			Path:   m[3],
-		}
-	} else {
-		url, err = urlpkg.Parse(rawurl)
-		if err != nil {
-			return nil, errors.Wrap(err, "Failed parsing URL")
-		}
-	}
-
-	if url.Host == "" && url.Path == "" {
-		return nil, errors.New("Parsed URL is empty")
-	}
-
-	return url, nil
 }

pkg/url_test.go

@@ -1,6 +1,8 @@
 package pkg
 
-import "testing"
+import (
+	"testing"
+)
 
 // Following URLs are considered valid according to https://git-scm.com/docs/git-clone#_git_urls:
 // ssh://[user@]host.xz[:port]/path/to/repo.git
@@ -47,11 +49,13 @@ func TestURLParse(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		got, err := URLToPath(test.in)
+		url, err := ParseURL(test.in)
 		if err != nil {
 			t.Errorf("Error parsing URL: %+v", err)
 		}
 
+		got := URLToPath(url)
+
 		if got != test.want {
 			t.Errorf("Wrong result of parsing URL: %s, got: %s; want: %s", test.in, got, test.want)
 		}
@@ -68,11 +72,10 @@ func TestInvalidURLParse(t *testing.T) {
 		//"git@github.com:1234:grdl/git-get.git",
 	}
 
-	for _, url := range invalidURLs {
+	for _, in := range invalidURLs {
-		got, err := URLToPath(url)
+		got, err := ParseURL(in)
 		if err == nil {
-			t.Errorf("Wrong result of parsing invalid URL: %s, got: %s, want: error", url, got)
+			t.Errorf("Wrong result of parsing invalid URL: %s, got: %s, want: error", in, got)
 		}
 	}
-
 }