Merge pull request #962 from cli/pr-diff

pr diff

api/queries_pr.go

@@ -2,7 +2,10 @@ package api
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"io/ioutil"
+	"net/http"
 	"strings"
 
 	"github.com/shurcooL/githubv4"
@@ -201,6 +204,39 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) {
 	return
 }
 
+func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNum int) (string, error) {
+	url := fmt.Sprintf("https://api.github.com/repos/%s/pulls/%d",
+		ghrepo.FullName(baseRepo), prNum)
+	req, err := http.NewRequest("GET", url, nil)
+	if err != nil {
+		return "", err
+	}
+
+	req.Header.Set("Accept", "application/vnd.github.v3.diff; charset=utf-8")
+
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return "", err
+	}
+	defer resp.Body.Close()
+
+	b, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return "", err
+	}
+
+	if resp.StatusCode == 200 {
+		return string(b), nil
+	}
+
+	if resp.StatusCode == 404 {
+		return "", &NotFoundError{errors.New("pull request not found")}
+	}
+
+	return "", errors.New("pull request diff lookup failed")
+
+}
+
 func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) {
 	type edges struct {
 		TotalCount int

command/pr_diff.go

@@ -0,0 +1,138 @@
+package command
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"strconv"
+	"strings"
+
+	"github.com/cli/cli/api"
+	"github.com/cli/cli/utils"
+	"github.com/spf13/cobra"
+)
+
+var prDiffCmd = &cobra.Command{
+	Use:   "diff {<number> | <url>}",
+	Short: "View a pull request's changes.",
+	RunE:  prDiff,
+}
+
+func init() {
+	prDiffCmd.Flags().StringP("color", "c", "auto", "Whether or not to output color: {always|never|auto}")
+
+	prCmd.AddCommand(prDiffCmd)
+}
+
+func prDiff(cmd *cobra.Command, args []string) error {
+	color, err := cmd.Flags().GetString("color")
+	if err != nil {
+		return err
+	}
+	if !validColorFlag(color) {
+		return fmt.Errorf("did not understand color: %q. Expected one of always, never, or auto", color)
+	}
+
+	ctx := contextForCommand(cmd)
+	apiClient, err := apiClientForContext(ctx)
+	if err != nil {
+		return err
+	}
+
+	baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
+	if err != nil {
+		return fmt.Errorf("could not determine base repo: %w", err)
+	}
+
+	// begin pr resolution boilerplate
+	var prNum int
+	branchWithOwner := ""
+
+	if len(args) == 0 {
+		prNum, branchWithOwner, err = prSelectorForCurrentBranch(ctx, baseRepo)
+		if err != nil {
+			return fmt.Errorf("could not query for pull request for current branch: %w", err)
+		}
+	} else {
+		prArg, repo := prFromURL(args[0])
+		if repo != nil {
+			baseRepo = repo
+		} else {
+			prArg = strings.TrimPrefix(args[0], "#")
+		}
+		prNum, err = strconv.Atoi(prArg)
+		if err != nil {
+			return errors.New("could not parse pull request argument")
+		}
+	}
+
+	if prNum < 1 {
+		pr, err := api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner)
+		if err != nil {
+			return fmt.Errorf("could not find pull request: %w", err)
+		}
+		prNum = pr.Number
+	}
+	// end pr resolution boilerplate
+
+	diff, err := apiClient.PullRequestDiff(baseRepo, prNum)
+	if err != nil {
+		return fmt.Errorf("could not find pull request diff: %w", err)
+	}
+
+	out := cmd.OutOrStdout()
+	if color == "auto" {
+		color = "never"
+		isTTY := false
+		if outFile, isFile := out.(*os.File); isFile {
+			isTTY = utils.IsTerminal(outFile)
+			if isTTY {
+				color = "always"
+			}
+		}
+	}
+
+	if color == "never" {
+		fmt.Fprint(out, diff)
+		return nil
+	}
+
+	out = colorableOut(cmd)
+	for _, diffLine := range strings.Split(diff, "\n") {
+		output := diffLine
+		switch {
+		case isHeaderLine(diffLine):
+			output = utils.Bold(diffLine)
+		case isAdditionLine(diffLine):
+			output = utils.Green(diffLine)
+		case isRemovalLine(diffLine):
+			output = utils.Red(diffLine)
+		}
+
+		fmt.Fprintln(out, output)
+	}
+
+	return nil
+}
+
+func isHeaderLine(dl string) bool {
+	prefixes := []string{"+++", "---", "diff", "index"}
+	for _, p := range prefixes {
+		if strings.HasPrefix(dl, p) {
+			return true
+		}
+	}
+	return false
+}
+
+func isAdditionLine(dl string) bool {
+	return strings.HasPrefix(dl, "+")
+}
+
+func isRemovalLine(dl string) bool {
+	return strings.HasPrefix(dl, "-")
+}
+
+func validColorFlag(c string) bool {
+	return c == "auto" || c == "always" || c == "never"
+}

command/pr_diff_test.go

@@ -0,0 +1,99 @@
+package command
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestPRDiff_validation(t *testing.T) {
+	_, err := RunCommand("pr diff --color=doublerainbow")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	eq(t, err.Error(), `did not understand color: "doublerainbow". Expected one of always, never, or auto`)
+}
+
+func TestPRDiff_no_current_pr(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "master")
+	http := initFakeHTTP()
+	http.StubRepoResponse("OWNER", "REPO")
+	http.StubResponse(200, bytes.NewBufferString(`
+		{ "data": { "repository": { "pullRequests": { "nodes": [
+			{ "url": "https://github.com/OWNER/REPO/pull/123",
+			  "number": 123,
+			  "id": "foobar123",
+			  "headRefName": "feature",
+				"baseRefName": "master" }
+		] } } } }`))
+	http.StubResponse(200, bytes.NewBufferString(testDiff))
+	_, err := RunCommand("pr diff")
+	if err == nil {
+		t.Fatal("expected error", err)
+	}
+	eq(t, err.Error(), `could not find pull request: no open pull requests found for branch "master"`)
+}
+
+func TestPRDiff_argument_not_found(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "master")
+	http := initFakeHTTP()
+	http.StubRepoResponse("OWNER", "REPO")
+	http.StubResponse(404, bytes.NewBufferString(""))
+	_, err := RunCommand("pr diff 123")
+	if err == nil {
+		t.Fatal("expected error", err)
+	}
+	eq(t, err.Error(), `could not find pull request diff: pull request not found`)
+}
+
+func TestPRDiff(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "feature")
+	http := initFakeHTTP()
+	http.StubRepoResponse("OWNER", "REPO")
+	http.StubResponse(200, bytes.NewBufferString(`
+		{ "data": { "repository": { "pullRequests": { "nodes": [
+			{ "url": "https://github.com/OWNER/REPO/pull/123",
+			  "number": 123,
+			  "id": "foobar123",
+			  "headRefName": "feature",
+				"baseRefName": "master" }
+		] } } } }`))
+	http.StubResponse(200, bytes.NewBufferString(testDiff))
+	output, err := RunCommand("pr diff")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	eq(t, output.String(), testDiff)
+}
+
+const testDiff = `diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml
+index 73974448..b7fc0154 100644
+--- a/.github/workflows/releases.yml
++++ b/.github/workflows/releases.yml
+@@ -44,6 +44,11 @@ jobs:
+           token: ${{secrets.SITE_GITHUB_TOKEN}}
+       - name: Publish documentation site
+         if: "!contains(github.ref, '-')" # skip prereleases
++        env:
++          GIT_COMMITTER_NAME: cli automation
++          GIT_AUTHOR_NAME: cli automation
++          GIT_COMMITTER_EMAIL: noreply@github.com
++          GIT_AUTHOR_EMAIL: noreply@github.com
+         run: make site-publish
+       - name: Move project cards
+         if: "!contains(github.ref, '-')" # skip prereleases
+diff --git a/Makefile b/Makefile
+index f2b4805c..3d7bd0f9 100644
+--- a/Makefile
++++ b/Makefile
+@@ -22,8 +22,8 @@ test:
+ 	go test ./...
+ .PHONY: test
+ 
+-site:
+-	git clone https://github.com/github/cli.github.com.git "$@"
++site: bin/gh
++	bin/gh repo clone github/cli.github.com "$@"
+ 
+ site-docs: site
+ 	git -C site pull
+`