Skip to content

Don't assume remote name is origin #4775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 63 additions & 9 deletions pkg/commands/git_commands/remote.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package git_commands

import (
"errors"
"fmt"
"path"
"strings"

"github.com/jesseduffield/gocui"
Expand Down Expand Up @@ -66,24 +68,76 @@ func (self *RemoteCommands) DeleteRemoteTag(task gocui.Task, remoteName string,
return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run()
}

// CheckRemoteBranchExists Returns remote branch
// CheckRemoteBranchExists returns a boolean indicating whether or not
// the given branch has an upstream.
func (self *RemoteCommands) CheckRemoteBranchExists(branchName string) bool {
cmdArgs := NewGitCmd("show-ref").
Arg("--verify", "--", fmt.Sprintf("refs/remotes/origin/%s", branchName)).
ToArgv()

_, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()

_, err := self.getRemoteRef(branchName)
return err == nil
}

// Resolve what might be a aliased URL into a full URL
// SEE: `man -P 'less +/--get-url +n' git-ls-remote`
func (self *RemoteCommands) GetRemoteURL(remoteName string) (string, error) {
func (self *RemoteCommands) GetRemoteURL() (string, error) {
remoteName := self.getRemoteName()
if remoteName == "" {
return "", errors.New("could not find upstream remote")
}

cmdArgs := NewGitCmd("ls-remote").
Arg("--get-url", remoteName).
ToArgv()

url, err := self.cmd.New(cmdArgs).RunWithOutput()
url, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()
return strings.TrimSpace(url), err
}

func (self *RemoteCommands) CommitExistsOnRemote(commitHash string) bool {
return self.findRemoteBranchForCommit(commitHash) != ""
}

func (self *RemoteCommands) getRemoteRef(branchName string) (string, error) {
cmdArgs := NewGitCmd("rev-parse").
Arg("--symbolic-full-name", fmt.Sprintf("%s@{upstream}", branchName)).
ToArgv()

remote, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()
if err != nil && branchName == "" {
// if we couldn't find an upstream and the caller isn't asking about a specific
// branch we'll return the first valid remote we find (if any)
cmdArgs := NewGitCmd("rev-parse").
Arg("--symbolic-full-name", "--remotes").
ToArgv()
remote, err = self.cmd.New(cmdArgs).DontLog().RunWithOutput()
remote, _, _ = strings.Cut(remote, "\n")
}

return remote, err
}

func (self *RemoteCommands) getRemoteName() string {
ref, err := self.getRemoteRef("")
if err != nil {
return ""
}
// refs/remotes/remote-name/branch
// ^^^^^^^^^^^
return path.Base(path.Dir(ref))
}

func (self *RemoteCommands) findRemoteBranchForCommit(commitHash string) string {
cmdArgs := NewGitCmd("branch").
Arg("--remotes", "--contains", commitHash).
ToArgv()

remotes, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()
if err != nil {
return ""
}

// grab the first returned remote
remote, _, _ := strings.Cut(remotes, "\n")
if remote != "" {
return path.Base(remote)
}
return ""
}
25 changes: 17 additions & 8 deletions pkg/commands/hosting_service/hosting_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,23 @@ type HostingServiceMgr struct {

// see https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#custom-pull-request-urls
configServiceDomains map[string]string

commitExistsOnRemote func(string) bool
}

// NewHostingServiceMgr creates new instance of PullRequest
func NewHostingServiceMgr(log logrus.FieldLogger, tr *i18n.TranslationSet, remoteURL string, configServiceDomains map[string]string) *HostingServiceMgr {
func NewHostingServiceMgr(
log logrus.FieldLogger,
tr *i18n.TranslationSet, remoteURL string,
configServiceDomains map[string]string,
commitExistsOnRemote func(string) bool,
) *HostingServiceMgr {
return &HostingServiceMgr{
log: log,
tr: tr,
remoteURL: remoteURL,
configServiceDomains: configServiceDomains,
commitExistsOnRemote: commitExistsOnRemote,
}
}

Expand All @@ -56,9 +64,10 @@ func (self *HostingServiceMgr) GetCommitURL(commitHash string) (string, error) {
return "", err
}

pullRequestURL := gitService.getCommitURL(commitHash)

return pullRequestURL, nil
if !self.commitExistsOnRemote(commitHash) {
return "", errors.New("no remote URL available")
}
return gitService.getCommitURL(commitHash), nil
}

func (self *HostingServiceMgr) getService() (*Service, error) {
Expand Down Expand Up @@ -166,17 +175,17 @@ type Service struct {
}

func (self *Service) getPullRequestURLIntoDefaultBranch(from string) string {
return self.resolveUrl(self.pullRequestURLIntoDefaultBranch, map[string]string{"From": from})
return self.resolveURL(self.pullRequestURLIntoDefaultBranch, map[string]string{"From": from})
}

func (self *Service) getPullRequestURLIntoTargetBranch(from string, to string) string {
return self.resolveUrl(self.pullRequestURLIntoTargetBranch, map[string]string{"From": from, "To": to})
return self.resolveURL(self.pullRequestURLIntoTargetBranch, map[string]string{"From": from, "To": to})
}

func (self *Service) getCommitURL(commitHash string) string {
return self.resolveUrl(self.commitURL, map[string]string{"CommitHash": commitHash})
return self.resolveURL(self.commitURL, map[string]string{"CommitHash": commitHash})
}

func (self *Service) resolveUrl(templateString string, args map[string]string) string {
func (self *Service) resolveURL(templateString string, args map[string]string) string {
return self.repoURL + utils.ResolvePlaceholderString(templateString, args)
}
2 changes: 1 addition & 1 deletion pkg/commands/hosting_service/hosting_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ func TestGetPullRequestURL(t *testing.T) {
t.Run(s.testName, func(t *testing.T) {
tr := i18n.EnglishTranslationSet()
log := &fakes.FakeFieldLogger{}
hostingServiceMgr := NewHostingServiceMgr(log, tr, s.remoteUrl, s.configServiceDomains)
hostingServiceMgr := NewHostingServiceMgr(log, tr, s.remoteUrl, s.configServiceDomains, nil)
s.test(hostingServiceMgr.GetPullRequestURL(s.from, s.to))
log.AssertErrors(t, s.expectedLoggedErrors)
})
Expand Down
40 changes: 26 additions & 14 deletions pkg/gui/controllers/basic_commits_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,13 @@ func (self *BasicCommitsController) copyCommitAttribute(commit *models.Commit) e
}
}

var commitTagsDisabled *types.DisabledReason
if len(commit.Tags) == 0 {
commitTagsDisabled = &types.DisabledReason{
Text: self.c.Tr.CommitHasNoTags,
}
}

items := []*types.MenuItem{
{
Label: self.c.Tr.CommitHash,
Expand Down Expand Up @@ -191,7 +198,8 @@ func (self *BasicCommitsController) copyCommitAttribute(commit *models.Commit) e
Key: 'b',
},
{
Label: self.c.Tr.CommitURL,
Label: self.c.Tr.CommitURL,
DisabledReason: self.canCopyCommitURL(commit),
OnPress: func() error {
return self.copyCommitURLToClipboard(commit)
},
Expand All @@ -211,22 +219,16 @@ func (self *BasicCommitsController) copyCommitAttribute(commit *models.Commit) e
},
Key: 'a',
},
}

commitTagsItem := types.MenuItem{
Label: self.c.Tr.CommitTags,
OnPress: func() error {
return self.copyCommitTagsToClipboard(commit)
{
Label: self.c.Tr.CommitTags,
DisabledReason: commitTagsDisabled,
OnPress: func() error {
return self.copyCommitTagsToClipboard(commit)
},
Key: 't',
},
Key: 't',
}

if len(commit.Tags) == 0 {
commitTagsItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.CommitHasNoTags}
}

items = append(items, &commitTagsItem)

return self.c.Menu(types.CreateMenuOptions{
Title: self.c.Tr.Actions.CopyCommitAttributeToClipboard,
Items: items,
Expand Down Expand Up @@ -382,6 +384,16 @@ func (self *BasicCommitsController) canCopyCommits(selectedCommits []*models.Com
return nil
}

func (self *BasicCommitsController) canCopyCommitURL(commit *models.Commit) *types.DisabledReason {
_, err := self.c.Helpers().Host.GetCommitURL(commit.Hash())
if err != nil {
return &types.DisabledReason{
Text: self.c.Tr.CommitHasNoURL,
}
}
return nil
}

func (self *BasicCommitsController) handleOldCherryPickKey() error {
msg := utils.ResolvePlaceholderString(self.c.Tr.OldCherryPickKeyWarning,
map[string]string{
Expand Down
11 changes: 9 additions & 2 deletions pkg/gui/controllers/helpers/host_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,17 @@ func (self *HostHelper) GetCommitURL(commitHash string) (string, error) {
// getting this on every request rather than storing it in state in case our remoteURL changes
// from one invocation to the next.
func (self *HostHelper) getHostingServiceMgr() (*hosting_service.HostingServiceMgr, error) {
remoteUrl, err := self.c.Git().Remote.GetRemoteURL("origin")
remoteUrl, err := self.c.Git().Remote.GetRemoteURL()
if err != nil {
return nil, err
}
configServices := self.c.UserConfig().Services
return hosting_service.NewHostingServiceMgr(self.c.Log, self.c.Tr, remoteUrl, configServices), nil
hostingServiceMgr := hosting_service.NewHostingServiceMgr(
self.c.Log,
self.c.Tr,
remoteUrl,
configServices,
self.c.Git().Remote.CommitExistsOnRemote,
)
return hostingServiceMgr, nil
}
2 changes: 2 additions & 0 deletions pkg/i18n/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,7 @@ type TranslationSet struct {
CommitTagsCopiedToClipboard string
CommitHasNoTags string
CommitHasNoMessageBody string
CommitHasNoURL string
PatchCopiedToClipboard string
CopiedToClipboard string
ErrCannotEditDirectory string
Expand Down Expand Up @@ -1766,6 +1767,7 @@ func EnglishTranslationSet() *TranslationSet {
CommitTagsCopiedToClipboard: "Commit tags copied to clipboard",
CommitHasNoTags: "Commit has no tags",
CommitHasNoMessageBody: "Commit has no message body",
CommitHasNoURL: "Commit does not exist on remote",
PatchCopiedToClipboard: "Patch copied to clipboard",
CopiedToClipboard: "copied to clipboard",
ErrCannotEditDirectory: "Cannot edit directories: you can only edit individual files",
Expand Down