Skip to content

Commit 45dec1b

Browse files
author
Nate Smith
authored
Merge pull request cli#962 from cli/pr-diff
pr diff
2 parents e0dbf37 + 983a1d9 commit 45dec1b

File tree

3 files changed

+273
-0
lines changed

3 files changed

+273
-0
lines changed

api/queries_pr.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ package api
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
7+
"io/ioutil"
8+
"net/http"
69
"strings"
710

811
"github.com/shurcooL/githubv4"
@@ -201,6 +204,39 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) {
201204
return
202205
}
203206

207+
func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNum int) (string, error) {
208+
url := fmt.Sprintf("https://api.github.com/repos/%s/pulls/%d",
209+
ghrepo.FullName(baseRepo), prNum)
210+
req, err := http.NewRequest("GET", url, nil)
211+
if err != nil {
212+
return "", err
213+
}
214+
215+
req.Header.Set("Accept", "application/vnd.github.v3.diff; charset=utf-8")
216+
217+
resp, err := c.http.Do(req)
218+
if err != nil {
219+
return "", err
220+
}
221+
defer resp.Body.Close()
222+
223+
b, err := ioutil.ReadAll(resp.Body)
224+
if err != nil {
225+
return "", err
226+
}
227+
228+
if resp.StatusCode == 200 {
229+
return string(b), nil
230+
}
231+
232+
if resp.StatusCode == 404 {
233+
return "", &NotFoundError{errors.New("pull request not found")}
234+
}
235+
236+
return "", errors.New("pull request diff lookup failed")
237+
238+
}
239+
204240
func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) {
205241
type edges struct {
206242
TotalCount int

command/pr_diff.go

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package command
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"os"
7+
"strconv"
8+
"strings"
9+
10+
"github.com/cli/cli/api"
11+
"github.com/cli/cli/utils"
12+
"github.com/spf13/cobra"
13+
)
14+
15+
var prDiffCmd = &cobra.Command{
16+
Use: "diff {<number> | <url>}",
17+
Short: "View a pull request's changes.",
18+
RunE: prDiff,
19+
}
20+
21+
func init() {
22+
prDiffCmd.Flags().StringP("color", "c", "auto", "Whether or not to output color: {always|never|auto}")
23+
24+
prCmd.AddCommand(prDiffCmd)
25+
}
26+
27+
func prDiff(cmd *cobra.Command, args []string) error {
28+
color, err := cmd.Flags().GetString("color")
29+
if err != nil {
30+
return err
31+
}
32+
if !validColorFlag(color) {
33+
return fmt.Errorf("did not understand color: %q. Expected one of always, never, or auto", color)
34+
}
35+
36+
ctx := contextForCommand(cmd)
37+
apiClient, err := apiClientForContext(ctx)
38+
if err != nil {
39+
return err
40+
}
41+
42+
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
43+
if err != nil {
44+
return fmt.Errorf("could not determine base repo: %w", err)
45+
}
46+
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)
79+
if err != nil {
80+
return fmt.Errorf("could not find pull request diff: %w", err)
81+
}
82+
83+
out := cmd.OutOrStdout()
84+
if color == "auto" {
85+
color = "never"
86+
isTTY := false
87+
if outFile, isFile := out.(*os.File); isFile {
88+
isTTY = utils.IsTerminal(outFile)
89+
if isTTY {
90+
color = "always"
91+
}
92+
}
93+
}
94+
95+
if color == "never" {
96+
fmt.Fprint(out, diff)
97+
return nil
98+
}
99+
100+
out = colorableOut(cmd)
101+
for _, diffLine := range strings.Split(diff, "\n") {
102+
output := diffLine
103+
switch {
104+
case isHeaderLine(diffLine):
105+
output = utils.Bold(diffLine)
106+
case isAdditionLine(diffLine):
107+
output = utils.Green(diffLine)
108+
case isRemovalLine(diffLine):
109+
output = utils.Red(diffLine)
110+
}
111+
112+
fmt.Fprintln(out, output)
113+
}
114+
115+
return nil
116+
}
117+
118+
func isHeaderLine(dl string) bool {
119+
prefixes := []string{"+++", "---", "diff", "index"}
120+
for _, p := range prefixes {
121+
if strings.HasPrefix(dl, p) {
122+
return true
123+
}
124+
}
125+
return false
126+
}
127+
128+
func isAdditionLine(dl string) bool {
129+
return strings.HasPrefix(dl, "+")
130+
}
131+
132+
func isRemovalLine(dl string) bool {
133+
return strings.HasPrefix(dl, "-")
134+
}
135+
136+
func validColorFlag(c string) bool {
137+
return c == "auto" || c == "always" || c == "never"
138+
}

command/pr_diff_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package command
2+
3+
import (
4+
"bytes"
5+
"testing"
6+
)
7+
8+
func TestPRDiff_validation(t *testing.T) {
9+
_, err := RunCommand("pr diff --color=doublerainbow")
10+
if err == nil {
11+
t.Fatal("expected error")
12+
}
13+
eq(t, err.Error(), `did not understand color: "doublerainbow". Expected one of always, never, or auto`)
14+
}
15+
16+
func TestPRDiff_no_current_pr(t *testing.T) {
17+
initBlankContext("", "OWNER/REPO", "master")
18+
http := initFakeHTTP()
19+
http.StubRepoResponse("OWNER", "REPO")
20+
http.StubResponse(200, bytes.NewBufferString(`
21+
{ "data": { "repository": { "pullRequests": { "nodes": [
22+
{ "url": "https://github.com/OWNER/REPO/pull/123",
23+
"number": 123,
24+
"id": "foobar123",
25+
"headRefName": "feature",
26+
"baseRefName": "master" }
27+
] } } } }`))
28+
http.StubResponse(200, bytes.NewBufferString(testDiff))
29+
_, err := RunCommand("pr diff")
30+
if err == nil {
31+
t.Fatal("expected error", err)
32+
}
33+
eq(t, err.Error(), `could not find pull request: no open pull requests found for branch "master"`)
34+
}
35+
36+
func TestPRDiff_argument_not_found(t *testing.T) {
37+
initBlankContext("", "OWNER/REPO", "master")
38+
http := initFakeHTTP()
39+
http.StubRepoResponse("OWNER", "REPO")
40+
http.StubResponse(404, bytes.NewBufferString(""))
41+
_, err := RunCommand("pr diff 123")
42+
if err == nil {
43+
t.Fatal("expected error", err)
44+
}
45+
eq(t, err.Error(), `could not find pull request diff: pull request not found`)
46+
}
47+
48+
func TestPRDiff(t *testing.T) {
49+
initBlankContext("", "OWNER/REPO", "feature")
50+
http := initFakeHTTP()
51+
http.StubRepoResponse("OWNER", "REPO")
52+
http.StubResponse(200, bytes.NewBufferString(`
53+
{ "data": { "repository": { "pullRequests": { "nodes": [
54+
{ "url": "https://github.com/OWNER/REPO/pull/123",
55+
"number": 123,
56+
"id": "foobar123",
57+
"headRefName": "feature",
58+
"baseRefName": "master" }
59+
] } } } }`))
60+
http.StubResponse(200, bytes.NewBufferString(testDiff))
61+
output, err := RunCommand("pr diff")
62+
if err != nil {
63+
t.Fatalf("unexpected error: %s", err)
64+
}
65+
eq(t, output.String(), testDiff)
66+
}
67+
68+
const testDiff = `diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml
69+
index 73974448..b7fc0154 100644
70+
--- a/.github/workflows/releases.yml
71+
+++ b/.github/workflows/releases.yml
72+
@@ -44,6 +44,11 @@ jobs:
73+
token: ${{secrets.SITE_GITHUB_TOKEN}}
74+
- name: Publish documentation site
75+
if: "!contains(github.ref, '-')" # skip prereleases
76+
+ env:
77+
+ GIT_COMMITTER_NAME: cli automation
78+
+ GIT_AUTHOR_NAME: cli automation
79+
+ GIT_COMMITTER_EMAIL: noreply@github.com
80+
+ GIT_AUTHOR_EMAIL: noreply@github.com
81+
run: make site-publish
82+
- name: Move project cards
83+
if: "!contains(github.ref, '-')" # skip prereleases
84+
diff --git a/Makefile b/Makefile
85+
index f2b4805c..3d7bd0f9 100644
86+
--- a/Makefile
87+
+++ b/Makefile
88+
@@ -22,8 +22,8 @@ test:
89+
go test ./...
90+
.PHONY: test
91+
92+
-site:
93+
- git clone https://github.com/github/cli.github.com.git "$@"
94+
+site: bin/gh
95+
+ bin/gh repo clone github/cli.github.com "$@"
96+
97+
site-docs: site
98+
git -C site pull
99+
`

0 commit comments

Comments
 (0)