Skip to content

Commit ee2b38d

Browse files
probablycoreyprobablycorey
authored
Merge pull request cli#1020 from cli/pr-lookup-v2
Consistent PR lookup interface
2 parents 1036666 + f490a49 commit ee2b38d

File tree

10 files changed

+159
-244
lines changed

10 files changed

+159
-244
lines changed

‎api/queries_pr.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,9 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) {
205205
return
206206
}
207207

208-
func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNum int) (string, error) {
208+
func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNumber int) (string, error) {
209209
url := fmt.Sprintf("https://api.github.com/repos/%s/pulls/%d",
210-
ghrepo.FullName(baseRepo), prNum)
210+
ghrepo.FullName(baseRepo), prNumber)
211211
req, err := http.NewRequest("GET", url, nil)
212212
if err != nil {
213213
return "", err
@@ -235,7 +235,6 @@ func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNum int) (string, e
235235
}
236236

237237
return "", errors.New("pull request diff lookup failed")
238-
239238
}
240239

241240
func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) {

‎command/pr.go‎

Lines changed: 8 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -302,59 +302,16 @@ func prView(cmd *cobra.Command, args []string) error {
302302
return err
303303
}
304304

305-
var baseRepo ghrepo.Interface
306-
var prArg string
307-
if len(args) > 0 {
308-
prArg = args[0]
309-
if prNum, repo := prFromURL(prArg); repo != nil {
310-
prArg = prNum
311-
baseRepo = repo
312-
}
313-
}
314-
315-
if baseRepo == nil {
316-
baseRepo, err = determineBaseRepo(apiClient, cmd, ctx)
317-
if err != nil {
318-
return err
319-
}
320-
}
321-
322305
web, err := cmd.Flags().GetBool("web")
323306
if err != nil {
324307
return err
325308
}
326309

327-
var openURL string
328-
var pr *api.PullRequest
329-
if len(args) > 0 {
330-
pr, err = prFromArg(apiClient, baseRepo, prArg)
331-
if err != nil {
332-
return err
333-
}
334-
openURL = pr.URL
335-
} else {
336-
prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo)
337-
if err != nil {
338-
return err
339-
}
340-
341-
if prNumber > 0 {
342-
openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(baseRepo), prNumber)
343-
if !web {
344-
pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber)
345-
if err != nil {
346-
return err
347-
}
348-
}
349-
} else {
350-
pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner)
351-
if err != nil {
352-
return err
353-
}
354-
355-
openURL = pr.URL
356-
}
310+
pr, _, err := prFromArgs(ctx, apiClient, cmd, args)
311+
if err != nil {
312+
return err
357313
}
314+
openURL := pr.URL
358315

359316
if web {
360317
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
@@ -372,12 +329,7 @@ func prClose(cmd *cobra.Command, args []string) error {
372329
return err
373330
}
374331

375-
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
376-
if err != nil {
377-
return err
378-
}
379-
380-
pr, err := prFromArg(apiClient, baseRepo, args[0])
332+
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
381333
if err != nil {
382334
return err
383335
}
@@ -407,12 +359,7 @@ func prReopen(cmd *cobra.Command, args []string) error {
407359
return err
408360
}
409361

410-
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
411-
if err != nil {
412-
return err
413-
}
414-
415-
pr, err := prFromArg(apiClient, baseRepo, args[0])
362+
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
416363
if err != nil {
417364
return err
418365
}
@@ -444,41 +391,11 @@ func prMerge(cmd *cobra.Command, args []string) error {
444391
return err
445392
}
446393

447-
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
394+
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
448395
if err != nil {
449396
return err
450397
}
451398

452-
var pr *api.PullRequest
453-
if len(args) > 0 {
454-
var prNumber string
455-
n, _ := prFromURL(args[0])
456-
if n != "" {
457-
prNumber = n
458-
} else {
459-
prNumber = args[0]
460-
}
461-
462-
pr, err = prFromArg(apiClient, baseRepo, prNumber)
463-
if err != nil {
464-
return err
465-
}
466-
} else {
467-
prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo)
468-
if err != nil {
469-
return err
470-
}
471-
472-
if prNumber != 0 {
473-
pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber)
474-
} else {
475-
pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner)
476-
}
477-
if err != nil {
478-
return err
479-
}
480-
}
481-
482399
if pr.Mergeable == "CONFLICTING" {
483400
err := fmt.Errorf("%s Pull request #%d has conflicts and isn't mergeable ", utils.Red("!"), pr.Number)
484401
return err
@@ -712,41 +629,11 @@ func prReady(cmd *cobra.Command, args []string) error {
712629
return err
713630
}
714631

715-
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
632+
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
716633
if err != nil {
717634
return err
718635
}
719636

720-
var pr *api.PullRequest
721-
if len(args) > 0 {
722-
var prNumber string
723-
n, _ := prFromURL(args[0])
724-
if n != "" {
725-
prNumber = n
726-
} else {
727-
prNumber = args[0]
728-
}
729-
730-
pr, err = prFromArg(apiClient, baseRepo, prNumber)
731-
if err != nil {
732-
return err
733-
}
734-
} else {
735-
prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo)
736-
if err != nil {
737-
return err
738-
}
739-
740-
if prNumber != 0 {
741-
pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber)
742-
} else {
743-
pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner)
744-
}
745-
if err != nil {
746-
return err
747-
}
748-
}
749-
750637
if pr.Closed {
751638
err := fmt.Errorf("%s Pull request #%d is closed. Only draft pull requests can be marked as \"ready for review\"", utils.Red("!"), pr.Number)
752639
return err
@@ -941,23 +828,6 @@ func prProjectList(pr api.PullRequest) string {
941828
return list
942829
}
943830

944-
var prURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`)
945-
946-
func prFromURL(arg string) (string, ghrepo.Interface) {
947-
if m := prURLRE.FindStringSubmatch(arg); m != nil {
948-
return m[3], ghrepo.New(m[1], m[2])
949-
}
950-
return "", nil
951-
}
952-
953-
func prFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.PullRequest, error) {
954-
if prNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#")); err == nil {
955-
return api.PullRequestByNumber(apiClient, baseRepo, prNumber)
956-
}
957-
958-
return api.PullRequestForBranch(apiClient, baseRepo, "", arg)
959-
}
960-
961831
func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) {
962832
prHeadRef, err = ctx.Branch()
963833
if err != nil {

‎command/pr_checkout.go‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {
2626
return err
2727
}
2828

29-
var baseRepo ghrepo.Interface
30-
prArg := args[0]
31-
if prNum, repo := prFromURL(prArg); repo != nil {
32-
prArg = prNum
33-
baseRepo = repo
34-
}
35-
36-
if baseRepo == nil {
37-
baseRepo, err = determineBaseRepo(apiClient, cmd, ctx)
38-
if err != nil {
39-
return err
40-
}
41-
}
42-
43-
pr, err := prFromArg(apiClient, baseRepo, prArg)
29+
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
4430
if err != nil {
4531
return err
4632
}
@@ -59,6 +45,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {
5945

6046
var cmdQueue [][]string
6147
newBranchName := pr.HeadRefName
48+
6249
if headRemote != nil {
6350
// there is an existing git remote for PR head
6451
remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName)

‎command/pr_checkout_test.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ func TestPRCheckout_urlArg(t *testing.T) {
7676
return ctx
7777
}
7878
http := initFakeHTTP()
79-
8079
http.StubResponse(200, bytes.NewBufferString(`
8180
{ "data": { "repository": { "pullRequest": {
8281
"number": 123,
@@ -125,7 +124,6 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
125124
return ctx
126125
}
127126
http := initFakeHTTP()
128-
129127
http.StubResponse(200, bytes.NewBufferString(`
130128
{ "data": { "repository": { "pullRequest": {
131129
"number": 123,

‎command/pr_diff.go‎

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
package command
22

33
import (
4-
"errors"
54
"fmt"
65
"os"
7-
"strconv"
86
"strings"
97

10-
"github.com/cli/cli/api"
118
"github.com/cli/cli/utils"
129
"github.com/spf13/cobra"
1310
)
@@ -39,43 +36,12 @@ func prDiff(cmd *cobra.Command, args []string) error {
3936
return err
4037
}
4138

42-
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
39+
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
4340
if err != nil {
44-
return fmt.Errorf("could not determine base repo: %w", err)
41+
return fmt.Errorf("could not find pull request: %w", err)
4542
}
4643

47-
// begin pr resolution boilerplate
48-
var prNum int
49-
branchWithOwner := ""
50-
51-
if len(args) == 0 {
52-
prNum, branchWithOwner, err = prSelectorForCurrentBranch(ctx, baseRepo)
53-
if err != nil {
54-
return fmt.Errorf("could not query for pull request for current branch: %w", err)
55-
}
56-
} else {
57-
prArg, repo := prFromURL(args[0])
58-
if repo != nil {
59-
baseRepo = repo
60-
} else {
61-
prArg = strings.TrimPrefix(args[0], "#")
62-
}
63-
prNum, err = strconv.Atoi(prArg)
64-
if err != nil {
65-
return errors.New("could not parse pull request argument")
66-
}
67-
}
68-
69-
if prNum < 1 {
70-
pr, err := api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner)
71-
if err != nil {
72-
return fmt.Errorf("could not find pull request: %w", err)
73-
}
74-
prNum = pr.Number
75-
}
76-
// end pr resolution boilerplate
77-
78-
diff, err := apiClient.PullRequestDiff(baseRepo, prNum)
44+
diff, err := apiClient.PullRequestDiff(baseRepo, pr.Number)
7945
if err != nil {
8046
return fmt.Errorf("could not find pull request diff: %w", err)
8147
}

‎command/pr_diff_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ func TestPRDiff_argument_not_found(t *testing.T) {
3737
initBlankContext("", "OWNER/REPO", "master")
3838
http := initFakeHTTP()
3939
http.StubRepoResponse("OWNER", "REPO")
40+
http.StubResponse(200, bytes.NewBufferString(`
41+
{ "data": { "repository": {
42+
"pullRequest": { "number": 123 }
43+
} } }
44+
`))
4045
http.StubResponse(404, bytes.NewBufferString(""))
4146
_, err := RunCommand("pr diff 123")
4247
if err == nil {

0 commit comments

Comments
 (0)