From cef1f8fc2c416d32cc407ce4f0a2c28d44236831 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 25 Apr 2026 12:55:28 +0200 Subject: [PATCH] If multiple remotes exist but only one is a Github remote, pick it without prompting If the repo has multiple remotes, but only one of them is on Github (the others might for example point to a self-hosted Critic server or something like that), lazygit would still present a menu to choose the remote for pull requests, but it would contain only that single entry. That's pointless, pick it automatically without prompting. We add some tests while we're at it; these wouldn't have caught the problem, because they only test getGithubBaseRemote which already takes the filtered github remotes. It's still better than not having any tests; the real issue could only have been caught with an integration test, which we don't bother adding. --- pkg/gui/controllers/helpers/refresh_helper.go | 64 +++++++++------- .../helpers/refresh_helper_test.go | 73 +++++++++++++++++++ 2 files changed, 111 insertions(+), 26 deletions(-) create mode 100644 pkg/gui/controllers/helpers/refresh_helper_test.go diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index ec2ad0d19..f2d5a9a49 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -817,13 +817,14 @@ func (self *RefreshHelper) refreshGithubPullRequests() { return } - baseRemote := self.getGithubBaseRemote() + githubRemotes := self.getGithubRemotes() + baseRemote := getGithubBaseRemote(githubRemotes, self.c.Git().GitHub.ConfiguredBaseRemoteName()) if baseRemote == nil { self.c.Model().PullRequests = nil self.c.Model().PullRequestsMap = nil - if !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] { - self.promptForBaseGithubRepo(authToken) + if len(githubRemotes) > 0 && !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] { + self.promptForBaseGithubRepo(authToken, githubRemotes) } return } @@ -833,22 +834,41 @@ func (self *RefreshHelper) refreshGithubPullRequests() { } } -func (self *RefreshHelper) getGithubBaseRemote() *models.Remote { - remotes := self.c.Model().Remotes +type githubRemoteInfo struct { + remote *models.Remote + repoName string +} +func (self *RefreshHelper) getGithubRemotes() []githubRemoteInfo { + return lo.FilterMap(self.c.Model().Remotes, func(remote *models.Remote, _ int) (githubRemoteInfo, bool) { + if len(remote.Urls) == 0 { + return githubRemoteInfo{}, false + } + repoName, err := self.c.Git().HostingService.GetRepoNameFromRemoteURL(remote.Urls[0]) + if err != nil { + return githubRemoteInfo{}, false + } + return githubRemoteInfo{remote: remote, repoName: repoName}, true + }) +} + +func getGithubBaseRemote(githubRemotes []githubRemoteInfo, configuredRemoteName string) *models.Remote { findRemoteByName := func(name string) *models.Remote { - remote, _ := lo.Find(remotes, func(remote *models.Remote) bool { - return remote.Name == name + info, ok := lo.Find(githubRemotes, func(info githubRemoteInfo) bool { + return info.remote.Name == name }) - return remote + if !ok { + return nil + } + return info.remote } - if configuredRemote := self.c.Git().GitHub.ConfiguredBaseRemoteName(); configuredRemote != "" { - return findRemoteByName(configuredRemote) + if configuredRemoteName != "" { + return findRemoteByName(configuredRemoteName) } - if len(remotes) == 1 { - return remotes[0] + if len(githubRemotes) == 1 { + return githubRemotes[0].remote } // Not sure if "upstream" is really a common convention for the name of the remote that PRs are @@ -860,31 +880,23 @@ func (self *RefreshHelper) getGithubBaseRemote() *models.Remote { return nil } -func (self *RefreshHelper) promptForBaseGithubRepo(authToken string) { - menuItems := lo.FilterMap(self.c.Model().Remotes, func(remote *models.Remote, _ int) (*types.MenuItem, bool) { - if len(remote.Urls) == 0 { - return nil, false - } - repoName, err := self.c.Git().HostingService.GetRepoNameFromRemoteURL(remote.Urls[0]) - if err != nil { - return nil, false - } - +func (self *RefreshHelper) promptForBaseGithubRepo(authToken string, githubRemotes []githubRemoteInfo) { + menuItems := lo.Map(githubRemotes, func(info githubRemoteInfo, _ int) *types.MenuItem { return &types.MenuItem{ - LabelColumns: []string{remote.Name, style.FgCyan.Sprint(repoName)}, + LabelColumns: []string{info.remote.Name, style.FgCyan.Sprint(info.repoName)}, OnPress: func() error { return self.c.WithWaitingStatus(self.c.Tr.FetchingPullRequests, func(gocui.Task) error { - if err := self.c.Git().GitHub.SetConfiguredBaseRemoteName(remote.Name); err != nil { + if err := self.c.Git().GitHub.SetConfiguredBaseRemoteName(info.remote.Name); err != nil { self.c.Log.Error(err) } - if err := self.setGithubPullRequests(authToken, remote); err != nil { + if err := self.setGithubPullRequests(authToken, info.remote); err != nil { self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error())) } return nil }) }, - }, true + } }) _ = self.c.Menu(types.CreateMenuOptions{ diff --git a/pkg/gui/controllers/helpers/refresh_helper_test.go b/pkg/gui/controllers/helpers/refresh_helper_test.go new file mode 100644 index 000000000..dea3b8b81 --- /dev/null +++ b/pkg/gui/controllers/helpers/refresh_helper_test.go @@ -0,0 +1,73 @@ +package helpers + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/samber/lo" + "github.com/stretchr/testify/assert" +) + +func TestGetGithubBaseRemote(t *testing.T) { + cases := []struct { + name string + githubRemotes []githubRemoteInfo + configuredRemote string + expected string + }{ + { + name: "configured remote wins", + githubRemotes: makeGithubRemoteInfoList("origin", "upstream", "fork"), + configuredRemote: "fork", + expected: "fork", + }, + { + name: "configured remote not in github remotes returns nil", + githubRemotes: makeGithubRemoteInfoList("origin"), + configuredRemote: "missing", + expected: "", + }, + { + name: "single github remote is auto-picked", + githubRemotes: makeGithubRemoteInfoList("myremote"), + configuredRemote: "", + expected: "myremote", + }, + { + name: "upstream is preferred when multiple github remotes exist", + githubRemotes: makeGithubRemoteInfoList("origin", "upstream", "fork"), + configuredRemote: "", + expected: "upstream", + }, + { + name: "no upstream and multiple remotes returns nil", + githubRemotes: makeGithubRemoteInfoList("origin", "fork"), + configuredRemote: "", + expected: "", + }, + { + name: "empty list returns nil", + githubRemotes: nil, + configuredRemote: "", + expected: "", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + result := getGithubBaseRemote(c.githubRemotes, c.configuredRemote) + if c.expected == "" { + assert.Nil(t, result) + } else { + assert.NotNil(t, result) + assert.Equal(t, c.expected, result.Name) + } + }) + } +} + +func makeGithubRemoteInfoList(names ...string) []githubRemoteInfo { + return lo.Map(names, func(name string, _ int) githubRemoteInfo { + return githubRemoteInfo{remote: &models.Remote{Name: name}, repoName: name} + }) +}