review fixes: PR URL refactoring, target branch selection prompt

3 files changed, 56 insertions, 26 deletions

diff --git a/pkg/commands/pull_request.go b/pkg/commands/pull_request.go
index c06a27e2e..a8c96e328 100644
--- a/pkg/commands/pull_request.go
+++ b/pkg/commands/pull_request.go
@@ -11,8 +11,9 @@ import (
 
 // Service is a service that repository is on (Github, Bitbucket, ...)
 type Service struct {
-	Name           string
-	PullRequestURL func(owner string, repository string, from string, to string) string
+	Name                            string
+	pullRequestURLIntoDefaultBranch func(owner string, repository string, from string) string
+	pullRequestURLIntoTargetBranch func(owner string, repository string, from string, to string) string
 }
 
 // PullRequest opens a link in browser to create new pull request
@@ -36,34 +37,31 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 	case "github":
 		service = &Service{
 			Name: repositoryDomain,
-			PullRequestURL: func(owner string, repository string, from string, to string) string {
-				if to == "" {
-					return fmt.Sprintf("https://%s/%s/%s/compare/%s?expand=1", siteDomain, owner, repository, from)
-				} else {
-					return fmt.Sprintf("https://%s/%s/%s/compare/%s...%s?expand=1", siteDomain, owner, repository, to, from)
-				}
+			pullRequestURLIntoDefaultBranch: func(owner string, repository string, from string) string {
+				return fmt.Sprintf("https://%s/%s/%s/compare/%s?expand=1", siteDomain, owner, repository, from)
+			},
+			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)
 			},
 		}
 	case "bitbucket":
 		service = &Service{
 			Name: repositoryDomain,
-			PullRequestURL: func(owner string, repository string, from string, to string) string {
-				if to == "" {
-					return fmt.Sprintf("https://%s/%s/%s/pull-requests/new?source=%s&t=1", siteDomain, owner, repository, from)
-				} else {
-					return fmt.Sprintf("https://%s/%s/%s/pull-requests/new?source=%s&dest=%s&t=1", siteDomain, owner, repository, from, to)
-				}
+			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)
+			},
+			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)
 			},
 		}
 	case "gitlab":
 		service = &Service{
 			Name: repositoryDomain,
-			PullRequestURL: func(owner string, repository string, from string, to string) string {
-				if to == "" {
-					return fmt.Sprintf("https://%s/%s/%s/merge_requests/new?merge_request[source_branch]=%s", siteDomain, owner, repository, from)
-				} else {
-					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)
-				}
+			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)
+			},
+			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)
 			},
 		}
 	}
@@ -71,6 +69,14 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 	return service
 }
 
+func (s *Service) PullRequestURL(owner string, repository string, from string, to string) string {
+	if to == "" {
+		return s.pullRequestURLIntoDefaultBranch(owner, repository, from)
+	} else {
+		return s.pullRequestURLIntoTargetBranch(owner, repository, from, to)
+	}
+}
+
 func getServices(config config.AppConfigurer) []*Service {
 	services := []*Service{
 		NewService("github", "github.com", "github.com"),
diff --git a/pkg/gui/branches_panel.go b/pkg/gui/branches_panel.go
index da55c9df8..867566a24 100644
--- a/pkg/gui/branches_panel.go
+++ b/pkg/gui/branches_panel.go
@@ -545,6 +545,16 @@ func (gui *Gui) getBranchNames() []string {
 	return result
 }
 
+func (gui *Gui) getBranchByName(name string) *models.Branch {
+	for _, branch := range gui.State.Branches {
+		if branch.Name == name {
+			return branch
+		}
+	}
+
+	return nil
+}
+
 func (gui *Gui) findBranchNameSuggestions(input string) []*types.Suggestion {
 	branchNames := gui.getBranchNames()
 
diff --git a/pkg/gui/pull_request_menu_panel.go b/pkg/gui/pull_request_menu_panel.go
index d76752e36..9412b71d6 100644
--- a/pkg/gui/pull_request_menu_panel.go
+++ b/pkg/gui/pull_request_menu_panel.go
@@ -8,19 +8,19 @@ import (
 )
 
 func (gui *Gui) createPullRequestMenu(selectedBranch *models.Branch, checkedOutBranch *models.Branch) error {
-	menuItems := make([]*menuItem, 0, 2)
+	menuItems := make([]*menuItem, 0, 4)
 
 	if selectedBranch != checkedOutBranch {
 		menuItems = append(menuItems, &menuItem{
 			displayStrings: []string{
-				fmt.Sprintf("%s -> default branch", selectedBranch.Name),
+				fmt.Sprintf("%s → default branch", selectedBranch.Name),
 			},
 			onPress: func() error {
 				return createPullRequest(selectedBranch, nil, gui)
 			},
 		}, &menuItem{
 			displayStrings: []string{
-				fmt.Sprintf("%s -> %s", checkedOutBranch.Name, selectedBranch.Name),
+				fmt.Sprintf("%s → %s", checkedOutBranch.Name, selectedBranch.Name),
 			},
 			onPress: func() error {
 				return createPullRequest(checkedOutBranch, selectedBranch, gui)
@@ -30,19 +30,33 @@ func (gui *Gui) createPullRequestMenu(selectedBranch *models.Branch, checkedOutB
 
 	menuItems = append(menuItems, &menuItem{
 		displayStrings: []string{
-			fmt.Sprintf("%s -> default branch", checkedOutBranch.Name),
+			fmt.Sprintf("%s → default branch", checkedOutBranch.Name),
 		},
 		onPress: func() error {
 			return createPullRequest(checkedOutBranch, nil, gui)
 		},
+	}, &menuItem{
+		displayStrings: []string{
+			fmt.Sprintf("%s → select branch", checkedOutBranch.Name),
+		},
+		onPress: func() error {
+			return gui.prompt(promptOpts{
+				title:               checkedOutBranch.Name + " →",
+				findSuggestionsFunc: gui.findBranchNameSuggestions,
+				handleConfirm: func(response string) error {
+					targetBranch := gui.getBranchByName(response)
+					return createPullRequest(checkedOutBranch, targetBranch, gui)
+				}},
+			)
+		},
 	})
 
 	return gui.createMenu(fmt.Sprintf(gui.Tr.CreatePullRequestOptions), menuItems, createMenuOptions{showCancel: true})
 }
 
-func createPullRequest(checkedOutBranch *models.Branch, selectedBranch *models.Branch, gui *Gui) error {
+func createPullRequest(from *models.Branch, to *models.Branch, gui *Gui) error {
 	pullRequest := commands.NewPullRequest(gui.GitCommand)
-	url, err := pullRequest.Create(checkedOutBranch, selectedBranch)
+	url, err := pullRequest.Create(from, to)
 	if err != nil {
 		return gui.surfaceError(err)
 	}