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
This commit is contained in:
Mislav Marohnić 2019-11-18 23:47:22 +01:00
parent 508f6787f0
commit eff8847513
4 changed files with 124 additions and 15 deletions

View file

@ -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)

View file

@ -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)
}

View file

@ -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())

View file

@ -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")