From eff8847513256d0730026e4a14a370838cc77bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Nov 2019 23:47:22 +0100 Subject: [PATCH] Improve detecting PR for the current branch Now reads git branch configuration and handles these cases: branch ["foo"] remote origin merge refs/heads/bar branch ["foo"] remote other-remote merge refs/heads/foo branch ["foo"] remote https://github.com/OWNER/REPO.git merge refs/heads/bar branch ["foo"] remote origin merge refs/pull/123/head --- command/pr.go | 40 +++++++++++++++++++++++++++++++++---- command/pr_test.go | 50 ++++++++++++++++++++++++++++++++++++---------- context/remote.go | 9 +++++++++ git/git.go | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 15 deletions(-) diff --git a/command/pr.go b/command/pr.go index 64cd6b871..f42411429 100644 --- a/command/pr.go +++ b/command/pr.go @@ -4,9 +4,12 @@ import ( "fmt" "os" "os/exec" + "regexp" "strconv" + "strings" "github.com/github/gh-cli/api" + "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" @@ -243,11 +246,40 @@ func prView(cmd *cobra.Command, args []string) error { return err } - pr, err := api.PullRequestForBranch(apiClient, baseRepo, currentBranch) - if err != nil { - return err + branchConfig := git.ReadBranchConfig(currentBranch) + prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`) + if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil { + openURL = fmt.Sprintf("https://github.com/%s/%s/pull/%s", baseRepo.RepoOwner(), baseRepo.RepoName(), m[1]) + } else { + branchWithOwner := currentBranch + var owner string + + if branchConfig.RemoteURL != nil { + if r, err := context.RepoFromURL(branchConfig.RemoteURL); err == nil { + owner = r.RepoOwner() + } + } else if branchConfig.RemoteName != "" { + rem, _ := ctx.Remotes() + if r, err := rem.FindByName(branchConfig.RemoteName); err == nil { + owner = r.RepoOwner() + } + } + + if owner != "" { + if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { + branchWithOwner = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") + } + if !strings.EqualFold(owner, baseRepo.RepoOwner()) { + branchWithOwner = fmt.Sprintf("%s:%s", owner, branchWithOwner) + } + } + + pr, err := api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner) + if err != nil { + return err + } + openURL = pr.URL } - openURL = pr.URL } fmt.Printf("Opening %s in your browser.\n", openURL) diff --git a/command/pr_test.go b/command/pr_test.go index 24cc14735..84d91ce07 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -8,6 +8,7 @@ import ( "os/exec" "reflect" "regexp" + "strings" "testing" "github.com/github/gh-cli/test" @@ -99,7 +100,7 @@ func TestPRList_filtering(t *testing.T) { eq(t, reqBody.Variables.Labels, []string{"one", "two"}) } -func TestPRView(t *testing.T) { +func TestPRView_currentBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -109,8 +110,13 @@ func TestPRView(t *testing.T) { var seenCmd *exec.Cmd restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { - seenCmd = cmd - return &outputStub{} + switch strings.Join(cmd.Args, " ") { + case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: + return &outputStub{} + default: + seenCmd = cmd + return &outputStub{} + } }) defer restoreCmd() @@ -132,8 +138,8 @@ func TestPRView(t *testing.T) { } } -func TestPRView_NoActiveBranch(t *testing.T) { - initBlankContext("OWNER/REPO", "master") +func TestPRView_noResultsForBranch(t *testing.T) { + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() jsonFile, _ := os.Open("../test/fixtures/prView_NoActiveBranch.json") @@ -142,22 +148,44 @@ func TestPRView_NoActiveBranch(t *testing.T) { var seenCmd *exec.Cmd restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { - seenCmd = cmd - return &outputStub{} + switch strings.Join(cmd.Args, " ") { + case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: + return &outputStub{} + default: + seenCmd = cmd + return &outputStub{} + } }) defer restoreCmd() - output, err := test.RunCommand(RootCmd, "pr view") - if err == nil || err.Error() != `no open pull requests found for branch "master"` { + _, err := test.RunCommand(RootCmd, "pr view") + if err == nil || err.Error() != `no open pull requests found for branch "blueberries"` { t.Errorf("error running command `pr view`: %v", err) } if seenCmd != nil { t.Fatalf("unexpected command: %v", seenCmd.Args) } +} - // Now run again but provide a PR number - output, err = test.RunCommand(RootCmd, "pr view 23") +func TestPRView_numberArg(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "url": "https://github.com/OWNER/REPO/pull/23" + } } } } + `)) + + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() + + output, err := test.RunCommand(RootCmd, "pr view 23") if err != nil { t.Errorf("error running command `pr view`: %v", err) } diff --git a/context/remote.go b/context/remote.go index 6c41eeaf1..a660a85cb 100644 --- a/context/remote.go +++ b/context/remote.go @@ -72,6 +72,15 @@ func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url return } +// RepoFromURL maps a URL to a GitHubRepository +func RepoFromURL(u *url.URL) (GitHubRepository, error) { + owner, repo, err := repoFromURL(u) + if err != nil { + return nil, err + } + return ghRepo{owner, repo}, nil +} + func repoFromURL(u *url.URL) (string, string, error) { if !strings.EqualFold(u.Hostname(), defaultHostname) { return "", "", fmt.Errorf("unsupported hostname: %s", u.Hostname()) diff --git a/git/git.go b/git/git.go index 8824872a4..b5eba2d59 100644 --- a/git/git.go +++ b/git/git.go @@ -4,9 +4,11 @@ import ( "bytes" "fmt" "io/ioutil" + "net/url" "os" "os/exec" "path/filepath" + "regexp" "strings" "github.com/github/gh-cli/utils" @@ -212,6 +214,44 @@ func Push(remote string, ref string) error { return cmd.Run() } +type BranchConfig struct { + RemoteName string + RemoteURL *url.URL + MergeRef string +} + +// ReadBranchConfig parses the `branch.BRANCH.(remote|merge)` part of git config +func ReadBranchConfig(branch string) (cfg BranchConfig) { + prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch)) + configCmd := GitCommand("config", "--get-regexp", fmt.Sprintf("^%s(remote|merge)$", prefix)) + output, err := utils.PrepareCmd(configCmd).Output() + if err != nil { + return + } + for _, line := range outputLines(output) { + parts := strings.SplitN(line, " ", 2) + if len(parts) < 2 { + continue + } + keys := strings.Split(parts[0], ".") + switch keys[len(keys)-1] { + case "remote": + if strings.Contains(parts[1], ":") { + u, err := ParseURL(parts[1]) + if err != nil { + continue + } + cfg.RemoteURL = u + } else { + cfg.RemoteName = parts[1] + } + case "merge": + cfg.MergeRef = parts[1] + } + } + return +} + func outputLines(output []byte) []string { lines := strings.TrimSuffix(string(output), "\n") return strings.Split(lines, "\n")