Skip to content

Commit eff8847

Browse files
mislavmislav
committed
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
1 parent 508f678 commit eff8847

File tree

4 files changed

+124
-15
lines changed

4 files changed

+124
-15
lines changed

‎command/pr.go‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import (
44
"fmt"
55
"os"
66
"os/exec"
7+
"regexp"
78
"strconv"
9+
"strings"
810

911
"github.com/github/gh-cli/api"
12+
"github.com/github/gh-cli/context"
1013
"github.com/github/gh-cli/git"
1114
"github.com/github/gh-cli/utils"
1215
"github.com/spf13/cobra"
@@ -243,11 +246,40 @@ func prView(cmd *cobra.Command, args []string) error {
243246
return err
244247
}
245248

246-
pr, err := api.PullRequestForBranch(apiClient, baseRepo, currentBranch)
247-
if err != nil {
248-
return err
249+
branchConfig := git.ReadBranchConfig(currentBranch)
250+
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
251+
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
252+
openURL = fmt.Sprintf("https://github.com/%s/%s/pull/%s", baseRepo.RepoOwner(), baseRepo.RepoName(), m[1])
253+
} else {
254+
branchWithOwner := currentBranch
255+
var owner string
256+
257+
if branchConfig.RemoteURL != nil {
258+
if r, err := context.RepoFromURL(branchConfig.RemoteURL); err == nil {
259+
owner = r.RepoOwner()
260+
}
261+
} else if branchConfig.RemoteName != "" {
262+
rem, _ := ctx.Remotes()
263+
if r, err := rem.FindByName(branchConfig.RemoteName); err == nil {
264+
owner = r.RepoOwner()
265+
}
266+
}
267+
268+
if owner != "" {
269+
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
270+
branchWithOwner = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
271+
}
272+
if !strings.EqualFold(owner, baseRepo.RepoOwner()) {
273+
branchWithOwner = fmt.Sprintf("%s:%s", owner, branchWithOwner)
274+
}
275+
}
276+
277+
pr, err := api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner)
278+
if err != nil {
279+
return err
280+
}
281+
openURL = pr.URL
249282
}
250-
openURL = pr.URL
251283
}
252284

253285
fmt.Printf("Opening %s in your browser.\n", openURL)

‎command/pr_test.go‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"os/exec"
99
"reflect"
1010
"regexp"
11+
"strings"
1112
"testing"
1213

1314
"github.com/github/gh-cli/test"
@@ -99,7 +100,7 @@ func TestPRList_filtering(t *testing.T) {
99100
eq(t, reqBody.Variables.Labels, []string{"one", "two"})
100101
}
101102

102-
func TestPRView(t *testing.T) {
103+
func TestPRView_currentBranch(t *testing.T) {
103104
initBlankContext("OWNER/REPO", "blueberries")
104105
http := initFakeHTTP()
105106

@@ -109,8 +110,13 @@ func TestPRView(t *testing.T) {
109110

110111
var seenCmd *exec.Cmd
111112
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
112-
seenCmd = cmd
113-
return &outputStub{}
113+
switch strings.Join(cmd.Args, " ") {
114+
case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`:
115+
return &outputStub{}
116+
default:
117+
seenCmd = cmd
118+
return &outputStub{}
119+
}
114120
})
115121
defer restoreCmd()
116122

@@ -132,8 +138,8 @@ func TestPRView(t *testing.T) {
132138
}
133139
}
134140

135-
func TestPRView_NoActiveBranch(t *testing.T) {
136-
initBlankContext("OWNER/REPO", "master")
141+
func TestPRView_noResultsForBranch(t *testing.T) {
142+
initBlankContext("OWNER/REPO", "blueberries")
137143
http := initFakeHTTP()
138144

139145
jsonFile, _ := os.Open("../test/fixtures/prView_NoActiveBranch.json")
@@ -142,22 +148,44 @@ func TestPRView_NoActiveBranch(t *testing.T) {
142148

143149
var seenCmd *exec.Cmd
144150
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
145-
seenCmd = cmd
146-
return &outputStub{}
151+
switch strings.Join(cmd.Args, " ") {
152+
case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`:
153+
return &outputStub{}
154+
default:
155+
seenCmd = cmd
156+
return &outputStub{}
157+
}
147158
})
148159
defer restoreCmd()
149160

150-
output, err := test.RunCommand(RootCmd, "pr view")
151-
if err == nil || err.Error() != `no open pull requests found for branch "master"` {
161+
_, err := test.RunCommand(RootCmd, "pr view")
162+
if err == nil || err.Error() != `no open pull requests found for branch "blueberries"` {
152163
t.Errorf("error running command `pr view`: %v", err)
153164
}
154165

155166
if seenCmd != nil {
156167
t.Fatalf("unexpected command: %v", seenCmd.Args)
157168
}
169+
}
170+
171+
func TestPRView_numberArg(t *testing.T) {
172+
initBlankContext("OWNER/REPO", "master")
173+
http := initFakeHTTP()
174+
175+
http.StubResponse(200, bytes.NewBufferString(`
176+
{ "data": { "repository": { "pullRequest": {
177+
"url": "https://github.com/OWNER/REPO/pull/23"
178+
} } } }
179+
`))
180+
181+
var seenCmd *exec.Cmd
182+
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
183+
seenCmd = cmd
184+
return &outputStub{}
185+
})
186+
defer restoreCmd()
158187

159-
// Now run again but provide a PR number
160-
output, err = test.RunCommand(RootCmd, "pr view 23")
188+
output, err := test.RunCommand(RootCmd, "pr view 23")
161189
if err != nil {
162190
t.Errorf("error running command `pr view`: %v", err)
163191
}

‎context/remote.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url
7272
return
7373
}
7474

75+
// RepoFromURL maps a URL to a GitHubRepository
76+
func RepoFromURL(u *url.URL) (GitHubRepository, error) {
77+
owner, repo, err := repoFromURL(u)
78+
if err != nil {
79+
return nil, err
80+
}
81+
return ghRepo{owner, repo}, nil
82+
}
83+
7584
func repoFromURL(u *url.URL) (string, string, error) {
7685
if !strings.EqualFold(u.Hostname(), defaultHostname) {
7786
return "", "", fmt.Errorf("unsupported hostname: %s", u.Hostname())

‎git/git.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"bytes"
55
"fmt"
66
"io/ioutil"
7+
"net/url"
78
"os"
89
"os/exec"
910
"path/filepath"
11+
"regexp"
1012
"strings"
1113

1214
"github.com/github/gh-cli/utils"
@@ -212,6 +214,44 @@ func Push(remote string, ref string) error {
212214
return cmd.Run()
213215
}
214216

217+
type BranchConfig struct {
218+
RemoteName string
219+
RemoteURL *url.URL
220+
MergeRef string
221+
}
222+
223+
// ReadBranchConfig parses the `branch.BRANCH.(remote|merge)` part of git config
224+
func ReadBranchConfig(branch string) (cfg BranchConfig) {
225+
prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch))
226+
configCmd := GitCommand("config", "--get-regexp", fmt.Sprintf("^%s(remote|merge)$", prefix))
227+
output, err := utils.PrepareCmd(configCmd).Output()
228+
if err != nil {
229+
return
230+
}
231+
for _, line := range outputLines(output) {
232+
parts := strings.SplitN(line, " ", 2)
233+
if len(parts) < 2 {
234+
continue
235+
}
236+
keys := strings.Split(parts[0], ".")
237+
switch keys[len(keys)-1] {
238+
case "remote":
239+
if strings.Contains(parts[1], ":") {
240+
u, err := ParseURL(parts[1])
241+
if err != nil {
242+
continue
243+
}
244+
cfg.RemoteURL = u
245+
} else {
246+
cfg.RemoteName = parts[1]
247+
}
248+
case "merge":
249+
cfg.MergeRef = parts[1]
250+
}
251+
}
252+
return
253+
}
254+
215255
func outputLines(output []byte) []string {
216256
lines := strings.TrimSuffix(string(output), "\n")
217257
return strings.Split(lines, "\n")

0 commit comments

Comments
 (0)