refactor to use regex for matching git service URL

pkg/commands/pull_request.go

@@ -2,26 +2,30 @@ package commands
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/go-errors/errors"
-	"github.com/jesseduffield/lazygit/pkg/config"
+	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
+// if you want to make a custom regex for a given service feel free to test it out
+// at regoio.herokuapp.com
+var defaultUrlRegexStrings = []string{
+	`^https?://.*/(?P<owner>.*)/(?P<repo>.*?)(\.git)?$`,
+	`^git@.*:(?P<owner>.*)/(?P<repo>.*?)(\.git)?$`,
+}
+
 // Service is a service that repository is on (Github, Bitbucket, ...)
 type Service struct {
 	Name                            string
 	pullRequestURLIntoDefaultBranch func(owner string, repository string, from string) string
 	pullRequestURLIntoTargetBranch  func(owner string, repository string, from string, to string) string
+	URLRegexStrings                 []string
 }
 
-// NewService builds a Service based on the host type
+func NewGithubService(repositoryDomain string, siteDomain string) *Service {
-func NewService(typeName string, repositoryDomain string, siteDomain string) *Service {
+	return &Service{
-	var service *Service
-
-	switch typeName {
-	case "github":
-		service = &Service{
 		Name: repositoryDomain,
 		pullRequestURLIntoDefaultBranch: func(owner string, repository string, from string) string {
 			return fmt.Sprintf("https://%s/%s/%s/compare/%s?expand=1", siteDomain, owner, repository, from)
@@ -29,9 +33,12 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 		pullRequestURLIntoTargetBranch: func(owner string, repository string, from string, to string) string {
 			return fmt.Sprintf("https://%s/%s/%s/compare/%s...%s?expand=1", siteDomain, owner, repository, to, from)
 		},
+		URLRegexStrings: defaultUrlRegexStrings,
 	}
-	case "bitbucket":
+}
-		service = &Service{
+
+func NewBitBucketService(repositoryDomain string, siteDomain string) *Service {
+	return &Service{
 		Name: repositoryDomain,
 		pullRequestURLIntoDefaultBranch: func(owner string, repository string, from string) string {
 			return fmt.Sprintf("https://%s/%s/%s/pull-requests/new?source=%s&t=1", siteDomain, owner, repository, from)
@@ -39,9 +46,12 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 		pullRequestURLIntoTargetBranch: func(owner string, repository string, from string, to string) string {
 			return fmt.Sprintf("https://%s/%s/%s/pull-requests/new?source=%s&dest=%s&t=1", siteDomain, owner, repository, from, to)
 		},
+		URLRegexStrings: defaultUrlRegexStrings,
 	}
-	case "gitlab":
+}
-		service = &Service{
+
+func NewGitLabService(repositoryDomain string, siteDomain string) *Service {
+	return &Service{
 		Name: repositoryDomain,
 		pullRequestURLIntoDefaultBranch: func(owner string, repository string, from string) string {
 			return fmt.Sprintf("https://%s/%s/%s/merge_requests/new?merge_request[source_branch]=%s", siteDomain, owner, repository, from)
@@ -49,24 +59,23 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 		pullRequestURLIntoTargetBranch: func(owner string, repository string, from string, to string) string {
 			return fmt.Sprintf("https://%s/%s/%s/merge_requests/new?merge_request[source_branch]=%s&merge_request[target_branch]=%s", siteDomain, owner, repository, from, to)
 		},
+		URLRegexStrings: defaultUrlRegexStrings,
 	}
 }
 
-	return service
+func (s *Service) PullRequestURL(repoURL string, from string, to string) string {
-}
+	repoInfo := s.getRepoInfoFromURL(repoURL)
 
-func (s *Service) PullRequestURL(owner string, repository string, from string, to string) string {
 	if to == "" {
-		return s.pullRequestURLIntoDefaultBranch(owner, repository, from)
+		return s.pullRequestURLIntoDefaultBranch(repoInfo.Owner, repoInfo.Repository, from)
 	} else {
-		return s.pullRequestURLIntoTargetBranch(owner, repository, from, to)
+		return s.pullRequestURLIntoTargetBranch(repoInfo.Owner, repoInfo.Repository, from, to)
 	}
 }
 
 // PullRequest opens a link in browser to create new pull request
 // with selected branch
 type PullRequest struct {
-	GitServices []*Service
 	GitCommand *GitCommand
 }
 
@@ -76,42 +85,53 @@ type RepoInformation struct {
 	Repository string
 }
 
-func getServices(config config.AppConfigurer) []*Service {
+// NewPullRequest creates new instance of PullRequest
-	services := []*Service{
+func NewPullRequest(gitCommand *GitCommand) *PullRequest {
-		NewService("github", "github.com", "github.com"),
+	return &PullRequest{
-		NewService("bitbucket", "bitbucket.org", "bitbucket.org"),
+		GitCommand: gitCommand,
-		NewService("gitlab", "gitlab.com", "gitlab.com"),
+	}
 }
 
-	configServices := config.GetUserConfig().Services
+func (pr *PullRequest) getServices() []*Service {
+	services := []*Service{
+		NewGithubService("github.com", "github.com"),
+		NewBitBucketService("bitbucket.org", "bitbucket.org"),
+		NewGitLabService("gitlab.com", "gitlab.com"),
+	}
+
+	configServices := pr.GitCommand.Config.GetUserConfig().Services
+
+	if len(configServices) > 0 {
+		serviceFuncMap := map[string]func(repositoryDomain string, siteDomain string) *Service{
+			"github":    NewGithubService,
+			"bitbucket": NewBitBucketService,
+			"gitlab":    NewGitLabService,
+		}
 
 		for repoDomain, typeAndDomain := range configServices {
 			splitData := strings.Split(typeAndDomain, ":")
 			if len(splitData) != 2 {
-			// TODO log this misconfiguration
+				pr.GitCommand.Log.Errorf("Unexpected format for git service: '%s'. Expected something like 'github.com:github.com'", typeAndDomain)
 				continue
 			}
 
-		service := NewService(splitData[0], repoDomain, splitData[1])
+			serviceFunc := serviceFuncMap[splitData[0]]
-		if service == nil {
+			if serviceFunc == nil {
-			// TODO log this unsupported service
+				serviceNames := []string{}
+				for serviceName := range serviceFuncMap {
+					serviceNames = append(serviceNames, serviceName)
+				}
+				pr.GitCommand.Log.Errorf("Unknown git service type: '%s'. Expected one of %s", splitData[0], strings.Join(serviceNames, ", "))
 				continue
 			}
 
-		services = append(services, service)
+			services = append(services, serviceFunc(repoDomain, splitData[1]))
+		}
 	}
 
 	return services
 }
 
-// NewPullRequest creates new instance of PullRequest
-func NewPullRequest(gitCommand *GitCommand) *PullRequest {
-	return &PullRequest{
-		GitServices: getServices(gitCommand.Config),
-		GitCommand:  gitCommand,
-	}
-}
-
 // Create opens link to new pull request in browser
 func (pr *PullRequest) Create(from string, to string) (string, error) {
 	pullRequestURL, err := pr.getPullRequestURL(from, to)
@@ -142,7 +162,7 @@ func (pr *PullRequest) getPullRequestURL(from string, to string) (string, error)
 	repoURL := pr.GitCommand.GetRemoteURL()
 	var gitService *Service
 
-	for _, service := range pr.GitServices {
+	for _, service := range pr.getServices() {
 		if strings.Contains(repoURL, service.Name) {
 			gitService = service
 			break
@@ -153,34 +173,22 @@ func (pr *PullRequest) getPullRequestURL(from string, to string) (string, error)
 		return "", errors.New(pr.GitCommand.Tr.UnsupportedGitService)
 	}
 
-	repoInfo := getRepoInfoFromURL(repoURL)
+	pullRequestURL := gitService.PullRequestURL(repoURL, from, to)
-
-	pullRequestURL := gitService.PullRequestURL(repoInfo.Owner, repoInfo.Repository, from, to)
 
 	return pullRequestURL, nil
 }
 
-func getRepoInfoFromURL(url string) *RepoInformation {
+func (s *Service) getRepoInfoFromURL(url string) *RepoInformation {
-	isHTTP := strings.HasPrefix(url, "http")
+	for _, regexStr := range s.URLRegexStrings {
-
+		re := regexp.MustCompile(regexStr)
-	if isHTTP {
+		matches := utils.FindNamedMatches(re, url)
-		splits := strings.Split(url, "/")
+		if matches != nil {
-		owner := strings.Join(splits[3:len(splits)-1], "/")
-		repo := strings.TrimSuffix(splits[len(splits)-1], ".git")
-
 			return &RepoInformation{
-			Owner:      owner,
+				Owner:      matches["owner"],
-			Repository: repo,
+				Repository: matches["repo"],
+			}
 		}
 	}
 
-	tmpSplit := strings.Split(url, ":")
+	return nil
-	splits := strings.Split(tmpSplit[1], "/")
-	owner := strings.Join(splits[0:len(splits)-1], "/")
-	repo := strings.TrimSuffix(splits[len(splits)-1], ".git")
-
-	return &RepoInformation{
-		Owner:      owner,
-		Repository: repo,
-	}
 }

pkg/commands/pull_request_test.go

@@ -9,6 +9,7 @@ import (
 // TestGetRepoInfoFromURL is a function.
 func TestGetRepoInfoFromURL(t *testing.T) {
 	type scenario struct {
+		service  *Service
 		testName string
 		repoURL  string
 		test     func(*RepoInformation)
@@ -16,6 +17,7 @@ func TestGetRepoInfoFromURL(t *testing.T) {
 
 	scenarios := []scenario{
 		{
+			NewGithubService("github.com", "github.com"),
 			"Returns repository information for git remote url",
 			"git@github.com:petersmith/super_calculator",
 			func(repoInfo *RepoInformation) {
@@ -24,6 +26,16 @@ func TestGetRepoInfoFromURL(t *testing.T) {
 			},
 		},
 		{
+			NewGithubService("github.com", "github.com"),
+			"Returns repository information for git remote url, trimming trailing '.git'",
+			"git@github.com:petersmith/super_calculator.git",
+			func(repoInfo *RepoInformation) {
+				assert.EqualValues(t, repoInfo.Owner, "petersmith")
+				assert.EqualValues(t, repoInfo.Repository, "super_calculator")
+			},
+		},
+		{
+			NewGithubService("github.com", "github.com"),
 			"Returns repository information for http remote url",
 			"https://my_username@bitbucket.org/johndoe/social_network.git",
 			func(repoInfo *RepoInformation) {
@@ -35,7 +47,7 @@ func TestGetRepoInfoFromURL(t *testing.T) {
 
 	for _, s := range scenarios {
 		t.Run(s.testName, func(t *testing.T) {
-			s.test(getRepoInfoFromURL(s.repoURL))
+			s.test(s.service.getRepoInfoFromURL(s.repoURL))
 		})
 	}
 }