Repository: jesseduffield/lazygit (language: go)

Issue:
Only remote branches should be suggested when creating a PR against selected branch
**Is your feature request related to a problem? Please describe.**

When creating a PR against a selected branch (via ` O` = "create pull request options"), the suggestion area is populated with all local branches. I'd say that only remote branches should be listed there - it doesn't really make sense to create a PR against a branch that doesn't exist on the remote (at least for Github).

**To Reproduce**

Steps to reproduce the behavior:

1. Checkout local branch
2. Press 'O'
3. Select '... -> select branch' option

**Describe the solution you'd like**

Only remote branches are listed as suggestions.

**Describe alternatives you've considered**

n/a

**Additional context**

The suggestion function used in [createPullRequestMenu()](https://github.com/jesseduffield/lazygit/blob/a34bdf1a046c90c22a1c0b653241b8107e89c7f9/pkg/gui/controllers/branches_controller.go#L378) returns all local branches. Simply replacing it with [GetRemoteBranchesSuggestionsFunc()](https://github.com/jesseduffield/lazygit/blob/a34bdf1a046c90c22a1c0b653241b8107e89c7f9/pkg/gui/controllers/helpers/suggestions_helper.go#L173) doesn't work since   the corresponding remote is then included in the branch name (i.e.  `<remote>/<branch>`) - breaking the generated link.

I'd be happy to have a go at fixing this, but I'd need some pointers: I haven't fully grasped yet how the whole "create PR" feature deals with multiple remotes, for example - and whether that must be taken into account. Would a new suggestion function that lists remote branches without the `<remote>/` prefix (and possibly for `origin` only) do the trick?





Discussion / hints:
I believe at the moment the assumption is that you'll only be dealing with 'origin's remote branches, given that the github git repo is usually what origin points to.

So yeah feel free to only show those for origin (and whether to include the origin/ prefix or not just depends on whatever works)
I've started to work on this, hope to be able to file a PR in the next days.

Fix patch:
diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go
index a97168fc15f..4bed5c6e39c 100644
--- a/pkg/gui/controllers/branches_controller.go
+++ b/pkg/gui/controllers/branches_controller.go
@@ -730,11 +730,23 @@ func (self *BranchesController) createPullRequestMenu(selectedBranch *models.Bra
 			{
 				LabelColumns: fromToLabelColumns(branch.Name, self.c.Tr.SelectBranch),
 				OnPress: func() error {
+					if !branch.IsTrackingRemote() {
+						return errors.New(self.c.Tr.PullRequestNoUpstream)
+					}
+
+					if len(self.c.Model().Remotes) == 1 {
+						toRemote := self.c.Model().Remotes[0].Name
+						self.c.Log.Debugf("PR will target the only existing remote '%s'", toRemote)
+						return self.promptForTargetBranchNameAndCreatePullRequest(branch, toRemote)
+					}
+
 					self.c.Prompt(types.PromptOpts{
-						Title:               branch.Name + " →",
-						FindSuggestionsFunc: self.c.Helpers().Suggestions.GetRemoteBranchesSuggestionsFunc("/"),
-						HandleConfirm: func(targetBranchName string) error {
-							return self.createPullRequest(branch.Name, targetBranchName)
+						Title:               self.c.Tr.SelectTargetRemote,
+						FindSuggestionsFunc: self.c.Helpers().Suggestions.GetRemoteSuggestionsFunc(),
+						HandleConfirm: func(toRemote string) error {
+							self.c.Log.Debugf("PR will target remote '%s'", toRemote)
+
+							return self.promptForTargetBranchNameAndCreatePullRequest(branch, toRemote)
 						},
 					})
 
@@ -764,6 +776,26 @@ func (self *BranchesController) createPullRequestMenu(selectedBranch *models.Bra
 	return self.c.Menu(types.CreateMenuOptions{Title: fmt.Sprint(self.c.Tr.CreatePullRequestOptions), Items: menuItems})
 }
 
+func (self *BranchesController) promptForTargetBranchNameAndCreatePullRequest(fromBranch *models.Branch, toRemote string) error {
+	remoteDoesNotExist := lo.NoneBy(self.c.Model().Remotes, func(remote *models.Remote) bool {
+		return remote.Name == toRemote
+	})
+	if remoteDoesNotExist {
+		return fmt.Errorf(self.c.Tr.NoValidRemoteName, toRemote)
+	}
+
+	self.c.Prompt(types.PromptOpts{
+		Title:               fmt.Sprintf("%s → %s/", fromBranch.UpstreamBranch, toRemote),
+		FindSuggestionsFunc: self.c.Helpers().Suggestions.GetRemoteBranchesForRemoteSuggestionsFunc(toRemote),
+		HandleConfirm: func(toBranch string) error {
+			self.c.Log.Debugf("PR will target branch '%s' on remote '%s'", toBranch, toRemote)
+			return self.createPullRequest(fromBranch.UpstreamBranch, toBranch)
+		},
+	})
+
+	return nil
+}
+
 func (self *BranchesController) createPullRequest(from string, to string) error {
 	url, err := self.c.Helpers().Host.GetPullRequestURL(from, to)
 	if err != nil {
diff --git a/pkg/gui/controllers/helpers/suggestions_helper.go b/pkg/gui/controllers/helpers/suggestions_helper.go
index 441a488b52e..e5e933a8c7c 100644
--- a/pkg/gui/controllers/helpers/suggestions_helper.go
+++ b/pkg/gui/controllers/helpers/suggestions_helper.go
@@ -162,10 +162,26 @@ func (self *SuggestionsHelper) getRemoteBranchNames(separator string) []string {
 	})
 }
 
+func (self *SuggestionsHelper) getRemoteBranchNamesForRemote(remoteName string) []string {
+	remote, ok := lo.Find(self.c.Model().Remotes, func(remote *models.Remote) bool {
+		return remote.Name == remoteName
+	})
+	if ok {
+		return lo.Map(remote.Branches, func(branch *models.RemoteBranch, _ int) string {
+			return branch.Name
+		})
+	}
+	return nil
+}
+
 func (self *SuggestionsHelper) GetRemoteBranchesSuggestionsFunc(separator string) func(string) []*types.Suggestion {
 	return FilterFunc(self.getRemoteBranchNames(separator), self.c.UserConfig().Gui.UseFuzzySearch())
 }
 
+func (self *SuggestionsHelper) GetRemoteBranchesForRemoteSuggestionsFunc(remoteName string) func(string) []*types.Suggestion {
+	return FilterFunc(self.getRemoteBranchNamesForRemote(remoteName), self.c.UserConfig().Gui.UseFuzzySearch())
+}
+
 func (self *SuggestionsHelper) getTagNames() []string {
 	return lo.Map(self.c.Model().Tags, func(tag *models.Tag, _ int) string {
 		return tag.Name
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index fa28130a94a..aa942093b8b 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -686,6 +686,8 @@ type TranslationSet struct {
 	CreatePullRequestOptions                 string
 	DefaultBranch                            string
 	SelectBranch                             string
+	SelectTargetRemote                       string
+	NoValidRemoteName                        string
 	CreatePullRequest                        string
 	SelectConfigFile                         string
 	NoConfigFileFoundErr                     string
@@ -1676,6 +1678,8 @@ func EnglishTranslationSet() *TranslationSet {
 		CreatePullRequestOptions:                 "View create pull request options",
 		DefaultBranch:                            "Default branch",
 		SelectBranch:                             "Select branch",
+		SelectTargetRemote:                       "Select target remote",
+		NoValidRemoteName:                        "A remote named '%s' does not exist",
 		SelectConfigFile:                         "Select config file",
 		NoConfigFileFoundErr:                     "No config file found",
 		LoadingFileSuggestions:                   "Loading file suggestions",


Test patch:
diff --git a/pkg/integration/tests/branch/open_pull_request_invalid_target_remote_name.go b/pkg/integration/tests/branch/open_pull_request_invalid_target_remote_name.go
new file mode 100644
index 00000000000..ab5e36d048a
--- /dev/null
+++ b/pkg/integration/tests/branch/open_pull_request_invalid_target_remote_name.go
@@ -0,0 +1,54 @@
+package branch
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var OpenPullRequestInvalidTargetRemoteName = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Open up a pull request, specifying a non-existing target remote",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		// Create an initial commit ('git branch set-upstream-to' bails out otherwise)
+		shell.CreateFileAndAdd("file", "content1")
+		shell.Commit("one")
+
+		// Create a new branch
+		shell.NewBranch("branch-1")
+
+		// Create a couple of remotes
+		shell.CloneIntoRemote("upstream")
+		shell.CloneIntoRemote("origin")
+
+		// To allow a pull request to be created from a branch, it must have an upstream set.
+		shell.SetBranchUpstream("branch-1", "origin/branch-1")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		// Open a PR for the current branch (i.e. 'branch-1')
+		t.Views().
+			Branches().
+			Focus().
+			Press(keys.Branches.ViewPullRequestOptions)
+
+		t.ExpectPopup().
+			Menu().
+			Title(Equals("View create pull request options")).
+			Select(Contains("Select branch")).
+			Confirm()
+
+		// Verify that we're prompted to enter the remote and enter the name of a non-existing one.
+		t.ExpectPopup().
+			Prompt().
+			Title(Equals("Select target remote")).
+			Type("non-existing-remote").
+			Confirm()
+
+		// Verify that this leads to an error being shown (instead of progressing to branch selection).
+		t.ExpectPopup().Alert().
+			Title(Equals("Error")).
+			Content(Contains("A remote named 'non-existing-remote' does not exist")).
+			Confirm()
+	},
+})
diff --git a/pkg/integration/tests/branch/open_pull_request_select_remote_and_target_branch.go b/pkg/integration/tests/branch/open_pull_request_select_remote_and_target_branch.go
new file mode 100644
index 00000000000..ac744210f80
--- /dev/null
+++ b/pkg/integration/tests/branch/open_pull_request_select_remote_and_target_branch.go
@@ -0,0 +1,74 @@
+package branch
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var OpenPullRequestSelectRemoteAndTargetBranch = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Open up a pull request, specifying a remote and target branch",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig: func(config *config.AppConfig) {
+		config.GetUserConfig().OS.OpenLink = "echo {{link}} > /tmp/openlink"
+	},
+	SetupRepo: func(shell *Shell) {
+		// Create an initial commit ('git branch set-upstream-to' bails out otherwise)
+		shell.CreateFileAndAdd("file", "content1")
+		shell.Commit("one")
+
+		// Create a new branch and a remote that has that branch
+		shell.NewBranch("branch-1")
+		shell.CloneIntoRemote("upstream")
+
+		// Create another branch and a second remote. The first remote doesn't have this branch.
+		shell.NewBranch("branch-2")
+		shell.CloneIntoRemote("origin")
+
+		// To allow a pull request to be created from a branch, it must have an upstream set.
+		shell.SetBranchUpstream("branch-2", "origin/branch-2")
+
+		shell.RunCommand([]string{"git", "remote", "set-url", "origin", "https://github.com/my-personal-fork/lazygit"})
+		shell.RunCommand([]string{"git", "remote", "set-url", "upstream", "https://github.com/jesseduffield/lazygit"})
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		// Open a PR for the current branch (i.e. 'branch-2')
+		t.Views().
+			Branches().
+			Focus().
+			Press(keys.Branches.ViewPullRequestOptions)
+
+		t.ExpectPopup().
+			Menu().
+			Title(Equals("View create pull request options")).
+			Select(Contains("Select branch")).
+			Confirm()
+
+		// Verify that we're prompted to enter the remote
+		t.ExpectPopup().
+			Prompt().
+			Title(Equals("Select target remote")).
+			SuggestionLines(
+				Equals("origin"),
+				Equals("upstream")).
+			ConfirmSuggestion(Equals("upstream"))
+
+		// Verify that we're prompted to enter the target branch and that only those branches
+		// present in the selected remote are listed as suggestions (i.e. 'branch-2' is not there).
+		t.ExpectPopup().
+			Prompt().
+			Title(Equals("branch-2 → upstream/")).
+			SuggestionLines(
+				Equals("branch-1"),
+				Equals("master")).
+			ConfirmSuggestion(Equals("master"))
+
+		// Verify that the expected URL is used (by checking the openlink file)
+		//
+		// Please note that when targeting a different remote - like it's done here in this test -
+		// the link is not yet correct. Thus, this test is expected to fail once this is fixed.
+		t.FileSystem().FileContent(
+			"/tmp/openlink",
+			Equals("https://github.com/my-personal-fork/lazygit/compare/master...branch-2?expand=1\n"))
+	},
+})
diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go
index 5c46695430b..2f60f3a47ab 100644
--- a/pkg/integration/tests/test_list.go
+++ b/pkg/integration/tests/test_list.go
@@ -48,7 +48,9 @@ var tests = []*components.IntegrationTest{
 	branch.NewBranchAutostash,
 	branch.NewBranchFromRemoteTrackingDifferentName,
 	branch.NewBranchFromRemoteTrackingSameName,
+	branch.OpenPullRequestInvalidTargetRemoteName,
 	branch.OpenPullRequestNoUpstream,
+	branch.OpenPullRequestSelectRemoteAndTargetBranch,
 	branch.OpenWithCliArg,
 	branch.Rebase,
 	branch.RebaseAbortOnConflict,