Opening a GitHub PR to a specific branch fails since update

[matt-sanders]

Describe the bug
I've recently upgraded LazyGit from 0.38.2 to 0.43.1. On 0.38.2 I would create a pull request to a specific branch using O, select my branch, and press enter. This would open a url like this: https://github.com/<org>/<repo>/compare/<base>...<compare>?expand=1. Since the update, I've noticed that my options all contain origin/ at the beginning of the base. This results in the following URL: https://github.com/<org>/<repo>/compare/origin%2F<base>...<compare>?expand=1 and GitHub gives me a page saying there is nothing to compare. If I remove the origin%2F then it works as expected.

Is this maybe a config that's changed recently?

To Reproduce
Steps to reproduce the behavior:

1. Open LazyGit in a directory with an existing git repo with a remote named "origin" hosted on GitHub.
2. Create a new branch, push some changes
3. Press O to open the "View create pull request options" menu
4. Choose the second option "Select branch"
5. Select a branch, they should all be prefixed here with origin/ and press enter
6. Notice the page from GitHub saying "nothing to compare"

Expected behavior
It should open a PR creation page correctly.

Version info:
commit=, build date=, build source=homebrew, version=0.43.1, os=darwin, arch=arm64, git version=2.39.3 (Apple Git-145)
git version 2.39.3 (Apple Git-145)

[mark2185]

Just out of curiosity, is this a PR across forks?

The assumption is that the original repo is on the origin remote, whilst the fork is on upstream, if I'm not mistaken.

[matt-sanders]

No this is not across forks.

[ricardoprato]

I have the same problem with bitbucket

[ChrisMcD1]

After doing some bisecting, issue seems to be introduced by #2693

[ChrisMcD1]

This issue appears to have been resolved by #1889 and included in v0.45.0!

The change from using the local branches models.Branch.UpstreamBranch to models.RemoteBranch.Name seems to have changed the value from origin/master to master. I'm no Go expert, but that appears to be because the RemoteBranch Name field comes from this:

lazygit/pkg/commands/git_commands/remote_loader.go

Lines 106 to 111 in 40d6800

	split := strings.SplitN(line, "/", 2)
	if len(split) != 2 {
	return false, nil
	}
	remoteName := split[0]
	name := split[1]

so it does not include the remote's name, whereas the branch UpstreamBranch comes from

lazygit/pkg/commands/git_commands/branch_loader.go

Line 131 in 40d6800

branch.UpstreamBranch = match.Merge.Short()

which uses the config.Branch.Merge field passed to Short(), whose value is origin/master. Is it is unclear to me if this function Short() is meant to turn the underlying ReferenceNames string from origin/master to master, but it doesn't appear to do that.

lazygit/vendor/github.com/jesseduffield/go-git/v5/plumbing/reference.go

Lines 113 to 124 in 53f8249

	func (r ReferenceName) Short() string {
	s := string(r)
	res := s
	for _, format := range RefRevParseRules {
	_, err := fmt.Sscanf(s, format, &res)
	if err == nil {
	continue
	}
	}
	return res
	}

Regardless, this issue is now fixed, so I think it can be closed? @jesseduffield

[ChrisMcD1]

I kept digging a little bit because my initial justification for the bug didn't feel complete, and now I think the error came not from the definition of UpstreamBranch, but rather that the Suggestions produced by the ISuggestionsHelper are always going to have matching labels and values, since they are ultimately produced by this matchesToSuggestions.

lazygit/pkg/gui/controllers/helpers/suggestions_helper.go

Lines 57 to 64 in 40d6800

	func matchesToSuggestions(matches []string) []*types.Suggestion {
	return lo.Map(matches, func(match string, _ int) *types.Suggestion {
	return &types.Suggestion{
	Value: match,
	Label: match,
	}
	})
	}

The producer of string[] remote branch names in getRemoteBranchNames likely intended that the concatenated remote name and branch name would be used to identify the item by being the Suggestion's Label, but not its Value.

lazygit/pkg/gui/controllers/helpers/suggestions_helper.go

Lines 157 to 163 in 40d6800

	func (self *SuggestionsHelper) getRemoteBranchNames(separator string) []string {
	return lo.FlatMap(self.c.Model().Remotes, func(remote *models.Remote, _ int) []string {
	return lo.Map(remote.Branches, func(branch *models.RemoteBranch, _ int) string {
	return fmt.Sprintf("%s%s%s", remote.Name, separator, branch.Name)
	})
	})
	}

[ChrisMcD1]

I retract that last statement about the value produced by getRemoteBranchNames only being useful as an identifier, because in the current context of the diffing menu, the / separated name is correct

lazygit/pkg/gui/controllers/diffing_menu_action.go

Lines 38 to 42 in 40d6800

	FindSuggestionsFunc: self.c.Helpers().Suggestions.GetRefsSuggestionsFunc(),
	HandleConfirm: func(response string) error {
	self.c.Modes().Diffing.Ref = strings.TrimSpace(response)
	return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})
	},

Similar point goes with the remote checkout menu, that needs the / separated name when passing it down into the underlying git branch --track <local-name> <remote-name>

lazygit/pkg/gui/controllers/branches_controller.go

Lines 483 to 489 in 40d6800

	FindSuggestionsFunc: self.c.Helpers().Suggestions.GetRefsSuggestionsFunc(),
	HandleConfirm: func(response string) error {
	self.c.LogAction("Checkout branch")
	_, branchName, found := self.c.Helpers().Refs.ParseRemoteBranchName(response)
	if found {
	return self.c.Helpers().Refs.CheckoutRemoteBranch(response, branchName)
	}