STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1453 from cli/pr-commands-isolate

Browse source 
Isolate pr review, diff, checkout commands
This commit is contained in:
Mislav Marohnić 2020-08-04 18:47:06 +02:00 • committed by GitHub
commit b4ffced654
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 1007 additions and 759 deletions
Download patch file Download diff file Expand all files Collapse all files

24
api/queries_pr.go
View file

@ -4,7 +4,7 @@ import (
"context"
"errors"
"fmt"
"io/ioutil"
"io"
"net/http"
"strings"
@ -209,36 +209,28 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) {
return
}
func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNumber int) (string, error) {
func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNumber int) (io.ReadCloser, error) {
url := fmt.Sprintf("https://api.github.com/repos/%s/pulls/%d",
ghrepo.FullName(baseRepo), prNumber)
req, err := http.NewRequest("GET", url, nil)
if err != nil {
return "", err
return nil, 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
return nil, err
}
if resp.StatusCode == 404 {
return "", &NotFoundError{errors.New("pull request not found")}
return nil, &NotFoundError{errors.New("pull request not found")}
} else if resp.StatusCode != 200 {
return nil, handleHTTPError(resp)
}
return "", errors.New("pull request diff lookup failed")
return resp.Body, nil
}
func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) {

9
api/queries_repo.go
View file

@ -27,9 +27,7 @@ type Repository struct {
IsPrivate bool
HasIssuesEnabled bool
ViewerPermission string
DefaultBranchRef struct {
Name string
}
DefaultBranchRef BranchRef
Parent *Repository
@ -42,6 +40,11 @@ type RepositoryOwner struct {
Login string
}
// BranchRef is the branch name in a GitHub repository
type BranchRef struct {
Name string
}
// RepoOwner is the login name of the owner
func (r Repository) RepoOwner() string {
return r.Owner.Login

1
command/pr.go
View file

@ -27,7 +27,6 @@ func init() {
prCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format")
RootCmd.AddCommand(prCmd)
prCmd.AddCommand(prCheckoutCmd)
prCmd.AddCommand(prCreateCmd)
prCmd.AddCommand(prStatusCmd)
prCmd.AddCommand(prCloseCmd)

104
command/pr_diff.go
View file

@ -1,104 +0,0 @@
package command
import (
"fmt"
"os"
"strings"
"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
}
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
if err != nil {
return fmt.Errorf("could not find pull request: %w", err)
}
diff, err := apiClient.PullRequestDiff(baseRepo, pr.Number)
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"
}

104
command/pr_diff_test.go
View file

@ -1,104 +0,0 @@
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(200, bytes.NewBufferString(`
{ "data": { "repository": {
"pullRequest": { "number": 123 }
} } }
`))
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
`

125
command/pr_lookup.go
View file

@ -2,120 +2,33 @@ package command
import (
"fmt"
"net/url"
"regexp"
"strconv"
"strings"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmd/pr/shared"
"github.com/spf13/cobra"
)
func prFromArgs(ctx context.Context, apiClient *api.Client, cmd *cobra.Command, args []string) (*api.PullRequest, ghrepo.Interface, error) {
if len(args) == 1 {
// First check to see if the prString is a url, return repo from url if found. This
// is run first because we don't need to run determineBaseRepo for this path
prString := args[0]
pr, r, err := prFromURL(ctx, apiClient, prString)
if pr != nil || err != nil {
return pr, r, err
}
var arg string
if len(args) > 0 {
arg = args[0]
}
repo, err := determineBaseRepo(apiClient, cmd, ctx)
if err != nil {
return nil, nil, fmt.Errorf("could not determine base repo: %w", err)
}
// If there are no args see if we can guess the PR from the current branch
if len(args) == 0 {
pr, err := prForCurrentBranch(ctx, apiClient, repo)
return pr, repo, err
} else {
prString := args[0]
// Next see if the prString is a number and use that to look up the url
pr, err := prFromNumberString(ctx, apiClient, repo, prString)
if pr != nil || err != nil {
return pr, repo, err
}
// Last see if it is a branch name
pr, err = api.PullRequestForBranch(apiClient, repo, "", prString)
return pr, repo, err
}
}
func prFromNumberString(ctx context.Context, apiClient *api.Client, repo ghrepo.Interface, s string) (*api.PullRequest, error) {
if prNumber, err := strconv.Atoi(strings.TrimPrefix(s, "#")); err == nil {
return api.PullRequestByNumber(apiClient, repo, prNumber)
}
return nil, nil
}
var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)
func prFromURL(ctx context.Context, apiClient *api.Client, s string) (*api.PullRequest, ghrepo.Interface, error) {
u, err := url.Parse(s)
if err != nil {
return nil, nil, nil
}
if u.Scheme != "https" && u.Scheme != "http" {
return nil, nil, nil
}
m := pullURLRE.FindStringSubmatch(u.Path)
if m == nil {
return nil, nil, nil
}
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
prNumberString := m[3]
pr, err := prFromNumberString(ctx, apiClient, repo, prNumberString)
return pr, repo, err
}
func prForCurrentBranch(ctx context.Context, apiClient *api.Client, repo ghrepo.Interface) (*api.PullRequest, error) {
prHeadRef, err := ctx.Branch()
if err != nil {
return nil, err
}
branchConfig := git.ReadBranchConfig(prHeadRef)
// the branch is configured to merge a special PR head ref
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
return prFromNumberString(ctx, apiClient, repo, m[1])
}
var branchOwner string
if branchConfig.RemoteURL != nil {
// the branch merges from a remote specified by URL
if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil {
branchOwner = r.RepoOwner()
}
} else if branchConfig.RemoteName != "" {
// the branch merges from a remote specified by name
rem, _ := ctx.Remotes()
if r, err := rem.FindByName(branchConfig.RemoteName); err == nil {
branchOwner = r.RepoOwner()
}
}
if branchOwner != "" {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork
if !strings.EqualFold(branchOwner, repo.RepoOwner()) {
prHeadRef = fmt.Sprintf("%s:%s", branchOwner, prHeadRef)
}
}
return api.PullRequestForBranch(apiClient, repo, "", prHeadRef)
return shared.PRFromArgs(
apiClient,
func() (ghrepo.Interface, error) {
repo, err := determineBaseRepo(apiClient, cmd, ctx)
if err != nil {
return nil, fmt.Errorf("could not determine base repo: %w", err)
}
return repo, nil
},
func() (string, error) {
return ctx.Branch()
},
func() (context.Remotes, error) {
return ctx.Remotes()
}, arg)
}

16
command/root.go
View file

@ -16,12 +16,16 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghinstance"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/internal/run"
apiCmd "github.com/cli/cli/pkg/cmd/api"
gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create"
prCheckoutCmd "github.com/cli/cli/pkg/cmd/pr/checkout"
prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff"
prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review"
repoCmd "github.com/cli/cli/pkg/cmd/repo"
repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone"
repoCreateCmd "github.com/cli/cli/pkg/cmd/repo/create"
@ -112,6 +116,13 @@ func init() {
}
return cfg, nil
},
Branch: func() (string, error) {
currentBranch, err := git.CurrentBranch()
if err != nil {
return "", fmt.Errorf("could not determine current branch: %w", err)
}
return currentBranch, nil
},
}
RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil))
@ -159,6 +170,10 @@ func init() {
repoCmd.Cmd.AddCommand(repoCreateCmd.NewCmdCreate(cmdFactory, nil))
repoCmd.Cmd.AddCommand(creditsCmd.NewCmdRepoCredits(&repoResolvingCmdFactory, nil))
prCmd.AddCommand(prReviewCmd.NewCmdReview(&repoResolvingCmdFactory, nil))
prCmd.AddCommand(prDiffCmd.NewCmdDiff(&repoResolvingCmdFactory, nil))
prCmd.AddCommand(prCheckoutCmd.NewCmdCheckout(&repoResolvingCmdFactory, nil))
RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil))
}
@ -393,6 +408,7 @@ func formatRemoteURL(cmd *cobra.Command, repo ghrepo.Interface) string {
return fmt.Sprintf("https://%s/%s/%s.git", repo.RepoHost(), repo.RepoOwner(), repo.RepoName())
}
// TODO there is a parallel implementation for isolated commands
func determineEditor(cmd *cobra.Command) (string, error) {
editorCommand := os.Getenv("GH_EDITOR")
if editorCommand == "" {

21
command/root_test.go
View file

@ -2,8 +2,6 @@ package command
import (
"testing"
"github.com/cli/cli/internal/ghrepo"
)
func TestChangelogURL(t *testing.T) {
@ -42,22 +40,3 @@ func TestChangelogURL(t *testing.T) {
t.Errorf("expected %s to create url %s but got %s", tag, url, result)
}
}
func TestRemoteURLFormatting_no_config(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
result := formatRemoteURL(prCheckoutCmd, ghrepo.New("OWNER", "REPO"))
eq(t, result, "https://github.com/OWNER/REPO.git")
}
func TestRemoteURLFormatting_ssh_config(t *testing.T) {
cfg := `---
hosts:
github.com:
user: OWNER
oauth_token: MUSTBEHIGHCUZIMATOKEN
git_protocol: ssh
`
initBlankContext(cfg, "OWNER/REPO", "master")
result := formatRemoteURL(prCheckoutCmd, ghrepo.New("OWNER", "REPO"))
eq(t, result, "git@github.com:OWNER/REPO.git")
}

13
command/testing.go
View file

@ -2,7 +2,6 @@ package command
import (
"bytes"
"errors"
"fmt"
"github.com/cli/cli/api"
@ -102,18 +101,6 @@ func RunCommand(args string) (*cmdOut, error) {
return &cmdOut{&outBuf, &errBuf}, err
}
type errorStub struct {
message string
}
func (s errorStub) Output() ([]byte, error) {
return nil, errors.New(s.message)
}
func (s errorStub) Run() error {
return errors.New(s.message)
}
func stubTerminal(connected bool) func() {
isTerminal := utils.IsTerminal
utils.IsTerminal = func(_ interface{}) bool {

94
command/pr_checkout.go → pkg/cmd/pr/checkout/checkout.go
View file

@ -1,41 +1,101 @@
package command
package checkout
import (
"errors"
"fmt"
"net/http"
"os"
"os/exec"
"strings"
"github.com/spf13/cobra"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/internal/run"
"github.com/cli/cli/pkg/cmd/pr/shared"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/spf13/cobra"
)
func prCheckout(cmd *cobra.Command, args []string) error {
ctx := contextForCommand(cmd)
currentBranch, _ := ctx.Branch()
remotes, err := ctx.Remotes()
type CheckoutOptions struct {
HttpClient func() (*http.Client, error)
Config func() (config.Config, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
Remotes func() (context.Remotes, error)
Branch func() (string, error)
SelectorArg string
}
func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobra.Command {
opts := &CheckoutOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
Config: f.Config,
BaseRepo: f.BaseRepo,
Remotes: f.Remotes,
Branch: f.Branch,
}
cmd := &cobra.Command{
Use: "checkout {<number> | <url> | <branch>}",
Short: "Check out a pull request in git",
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return &cmdutil.FlagError{Err: errors.New("argument required")}
}
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) > 0 {
opts.SelectorArg = args[0]
}
if runF != nil {
return runF(opts)
}
return checkoutRun(opts)
},
}
return cmd
}
func checkoutRun(opts *CheckoutOptions) error {
currentBranch, err := opts.Branch()
if err != nil {
return err
}
apiClient, err := apiClientForContext(ctx)
remotes, err := opts.Remotes()
if err != nil {
return err
}
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)
pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg)
if err != nil {
return err
}
cfg, err := opts.Config()
if err != nil {
return err
}
protocol, _ := cfg.Get(baseRepo.RepoHost(), "git_protocol")
baseRemote, _ := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName())
// baseRemoteSpec is a repository URL or a remote name to be used in git fetch
baseURLOrName := formatRemoteURL(cmd, baseRepo)
baseURLOrName := ghrepo.FormatRemoteURL(baseRepo, protocol)
if baseRemote != nil {
baseURLOrName = baseRemote.Name
}
@ -95,7 +155,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {
mergeRef := ref
if pr.MaintainerCanModify {
headRepo := ghrepo.NewWithHost(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name, baseRepo.RepoHost())
remote = formatRemoteURL(cmd, headRepo)
remote = ghrepo.FormatRemoteURL(headRepo, protocol)
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
}
if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" {
@ -115,15 +175,3 @@ func prCheckout(cmd *cobra.Command, args []string) error {
return nil
}
var prCheckoutCmd = &cobra.Command{
Use: "checkout {<number> | <url> | <branch>}",
Short: "Check out a pull request in Git",
Args: func(cmd *cobra.Command, args []string) error {
if len(args) < 1 {
return errors.New("requires a pull request number as an argument")
}
return nil
},
RunE: prCheckout,
}

246
command/pr_checkout_test.go → pkg/cmd/pr/checkout/checkout_test.go
View file

@ -1,31 +1,105 @@
package command
package checkout
import (
"bytes"
"encoding/json"
"errors"
"io/ioutil"
"net/http"
"os/exec"
"reflect"
"strings"
"testing"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/internal/run"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/test"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
func TestPRCheckout_sameRepo(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
func eq(t *testing.T, got interface{}, expected interface{}) {
t.Helper()
if !reflect.DeepEqual(got, expected) {
t.Errorf("expected: %v, got: %v", expected, got)
}
http := initFakeHTTP()
}
type errorStub struct {
message string
}
func (s errorStub) Output() ([]byte, error) {
return nil, errors.New(s.message)
}
func (s errorStub) Run() error {
return errors.New(s.message)
}
func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cli string) (*test.CmdOut, error) {
io, _, stdout, stderr := iostreams.Test()
factory := &cmdutil.Factory{
IOStreams: io,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: rt}, nil
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return api.InitRepoHostname(&api.Repository{
Name: "REPO",
Owner: api.RepositoryOwner{Login: "OWNER"},
DefaultBranchRef: api.BranchRef{Name: "master"},
}, "github.com"), nil
},
Remotes: func() (context.Remotes, error) {
if remotes == nil {
return context.Remotes{
{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
}
return remotes, nil
},
Branch: func() (string, error) {
return branch, nil
},
}
cmd := NewCmdCheckout(factory, nil)
argv, err := shlex.Split(cli)
if err != nil {
return nil, err
}
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(ioutil.Discard)
cmd.SetErr(ioutil.Discard)
_, err = cmd.ExecuteC()
return &test.CmdOut{
OutBuf: stdout,
ErrBuf: stderr,
}, err
}
func TestPRCheckout_sameRepo(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -54,11 +128,15 @@ func TestPRCheckout_sameRepo(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
eq(t, err, nil)
eq(t, output.String(), "")
output, err := runCommand(http, nil, "master", `123`)
if !assert.NoError(t, err) {
return
}
eq(t, len(ranCommands), 4)
assert.Equal(t, "", output.String())
if !assert.Equal(t, 4, len(ranCommands)) {
return
}
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature")
eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature")
eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote origin")
@ -66,15 +144,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
}
func TestPRCheckout_urlArg(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -103,7 +173,7 @@ func TestPRCheckout_urlArg(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout https://github.com/OWNER/REPO/pull/123/files`)
output, err := runCommand(http, nil, "master", `https://github.com/OWNER/REPO/pull/123/files`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -112,15 +182,7 @@ func TestPRCheckout_urlArg(t *testing.T) {
}
func TestPRCheckout_urlArg_differentBase(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -154,7 +216,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout https://github.com/OTHER/POE/pull/123/files`)
output, err := runCommand(http, nil, "master", `https://github.com/OTHER/POE/pull/123/files`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -176,17 +238,8 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
}
func TestPRCheckout_branchArg(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequests": { "nodes": [
@ -215,7 +268,7 @@ func TestPRCheckout_branchArg(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout hubot:feature`)
output, err := runCommand(http, nil, "master", `hubot:feature`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -224,17 +277,8 @@ func TestPRCheckout_branchArg(t *testing.T) {
}
func TestPRCheckout_existingBranch(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -263,7 +307,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -274,18 +318,19 @@ func TestPRCheckout_existingBranch(t *testing.T) {
}
func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
"robot-fork": "hubot/REPO",
})
initContext = func() context.Context {
return ctx
remotes := context.Remotes{
{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
},
{
Remote: &git.Remote{Name: "robot-fork"},
Repo: ghrepo.New("hubot", "REPO"),
},
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -314,7 +359,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, remotes, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -326,17 +371,8 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
}
func TestPRCheckout_differentRepo(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -365,7 +401,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -377,17 +413,8 @@ func TestPRCheckout_differentRepo(t *testing.T) {
}
func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -416,7 +443,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -426,17 +453,8 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
}
func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("feature")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -465,7 +483,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "feature", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -475,17 +493,8 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
}
func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("feature")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -508,7 +517,7 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
if assert.Errorf(t, err, "expected command to fail") {
assert.Equal(t, `invalid branch name: "-foo"`, err.Error())
}
@ -516,17 +525,8 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
}
func TestPRCheckout_maintainerCanModify(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -555,7 +555,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")

134
pkg/cmd/pr/diff/diff.go Normal file
View file

@ -0,0 +1,134 @@
package diff
import (
"bufio"
"fmt"
"io"
"net/http"
"strings"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmd/pr/shared"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/spf13/cobra"
)
type DiffOptions struct {
HttpClient func() (*http.Client, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
Remotes func() (context.Remotes, error)
Branch func() (string, error)
SelectorArg string
UseColor string
}
func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Command {
opts := &DiffOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
BaseRepo: f.BaseRepo,
Remotes: f.Remotes,
Branch: f.Branch,
}
cmd := &cobra.Command{
Use: "diff [<number> | <url> | <branch>]",
Short: "View changes in a pull request",
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) > 0 {
opts.SelectorArg = args[0]
}
if !validColorFlag(opts.UseColor) {
return &cmdutil.FlagError{Err: fmt.Errorf("did not understand color: %q. Expected one of always, never, or auto", opts.UseColor)}
}
if opts.UseColor == "auto" && !opts.IO.IsStdoutTTY() {
opts.UseColor = "never"
}
if runF != nil {
return runF(opts)
}
return diffRun(opts)
},
}
cmd.Flags().StringVar(&opts.UseColor, "color", "auto", "Use color in diff output: {always|never|auto}")
return cmd
}
func diffRun(opts *DiffOptions) error {
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)
pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg)
if err != nil {
return err
}
diff, err := apiClient.PullRequestDiff(baseRepo, pr.Number)
if err != nil {
return fmt.Errorf("could not find pull request diff: %w", err)
}
defer diff.Close()
if opts.UseColor == "never" {
_, err = io.Copy(opts.IO.Out, diff)
return err
}
diffLines := bufio.NewScanner(diff)
for diffLines.Scan() {
diffLine := diffLines.Text()
switch {
case isHeaderLine(diffLine):
fmt.Fprintf(opts.IO.Out, "\x1b[1;38m%s\x1b[m\n", diffLine)
case isAdditionLine(diffLine):
fmt.Fprintf(opts.IO.Out, "\x1b[32m%s\x1b[m\n", diffLine)
case isRemovalLine(diffLine):
fmt.Fprintf(opts.IO.Out, "\x1b[31m%s\x1b[m\n", diffLine)
default:
fmt.Fprintln(opts.IO.Out, diffLine)
}
}
if err := diffLines.Err(); err != nil {
return fmt.Errorf("error reading pull request diff: %w", err)
}
return nil
}
var diffHeaderPrefixes = []string{"+++", "---", "diff", "index"}
func isHeaderLine(dl string) bool {
for _, p := range diffHeaderPrefixes {
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"
}

183
pkg/cmd/pr/diff/diff_test.go Normal file
View file

@ -0,0 +1,183 @@
package diff
import (
"bytes"
"io/ioutil"
"net/http"
"testing"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/test"
"github.com/google/go-cmp/cmp"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli string) (*test.CmdOut, error) {
io, _, stdout, stderr := iostreams.Test()
io.SetStdoutTTY(isTTY)
io.SetStdinTTY(isTTY)
io.SetStderrTTY(isTTY)
factory := &cmdutil.Factory{
IOStreams: io,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: rt}, nil
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Remotes: func() (context.Remotes, error) {
if remotes == nil {
return context.Remotes{
{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
}
return remotes, nil
},
Branch: func() (string, error) {
return "feature", nil
},
}
cmd := NewCmdDiff(factory, nil)
argv, err := shlex.Split(cli)
if err != nil {
return nil, err
}
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(ioutil.Discard)
cmd.SetErr(ioutil.Discard)
_, err = cmd.ExecuteC()
return &test.CmdOut{
OutBuf: stdout,
ErrBuf: stderr,
}, err
}
func TestPRDiff_validation(t *testing.T) {
_, err := runCommand(nil, nil, false, "--color=doublerainbow")
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, `did not understand color: "doublerainbow". Expected one of always, never, or auto`, err.Error())
}
func TestPRDiff_no_current_pr(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [] } } } }
`))
_, err := runCommand(http, nil, false, "")
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, `no open pull requests found for branch "feature"`, err.Error())
}
func TestPRDiff_argument_not_found(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"pullRequest": { "number": 123 }
} } }
`))
http.StubResponse(404, bytes.NewBufferString(""))
_, err := runCommand(http, nil, false, "123")
if err == nil {
t.Fatal("expected error", err)
}
assert.Equal(t, `could not find pull request diff: pull request not found`, err.Error())
}
func TestPRDiff_notty(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
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(http, nil, false, "")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
if diff := cmp.Diff(testDiff, output.String()); diff != "" {
t.Errorf("command output did not match:\n%s", diff)
}
}
func TestPRDiff_tty(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
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(http, nil, true, "")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
assert.Contains(t, output.String(), "\x1b[32m+site: bin/gh\x1b[m")
}
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
`

231
command/pr_review.go → pkg/cmd/pr/review/review.go
View file

@ -1,79 +1,171 @@
package command
package review
import (
"errors"
"fmt"
"net/http"
"github.com/AlecAivazis/survey/v2"
"github.com/MakeNowJust/heredoc"
"github.com/spf13/cobra"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmd/pr/shared"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/pkg/prompt"
"github.com/cli/cli/pkg/surveyext"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
func init() {
prCmd.AddCommand(prReviewCmd)
type ReviewOptions struct {
HttpClient func() (*http.Client, error)
Config func() (config.Config, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
Remotes func() (context.Remotes, error)
Branch func() (string, error)
prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request")
prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request")
prReviewCmd.Flags().BoolP("comment", "c", false, "Comment on a pull request")
prReviewCmd.Flags().StringP("body", "b", "", "Specify the body of a review")
SelectorArg string
Approve bool
RequestChanges bool
Comment bool
Body string
}
var prReviewCmd = &cobra.Command{
Use: "review [<number> | <url> | <branch>]",
Short: "Add a review to a pull request",
Long: `Add a review to a pull request.
func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Command {
opts := &ReviewOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
Config: f.Config,
BaseRepo: f.BaseRepo,
Remotes: f.Remotes,
Branch: f.Branch,
}
Without an argument, the pull request that belongs to the current branch is reviewed.`,
Example: heredoc.Doc(`
# approve the pull request of the current branch
$ gh pr review --approve
# leave a review comment for the current branch
$ gh pr review --comment -b "interesting"
# add a review for a specific pull request
$ gh pr review 123
# request changes on a specific pull request
$ gh pr review 123 -r -b "needs more ASCII art"
`),
Args: cobra.MaximumNArgs(1),
RunE: prReview,
cmd := &cobra.Command{
Use: "review [<number> | <url> | <branch>]",
Short: "Add a review to a pull request",
Long: heredoc.Doc(`
Add a review to a pull request.
Without an argument, the pull request that belongs to the current branch is reviewed.
`),
Example: heredoc.Doc(`
# approve the pull request of the current branch
$ gh pr review --approve
# leave a review comment for the current branch
$ gh pr review --comment -b "interesting"
# add a review for a specific pull request
$ gh pr review 123
# request changes on a specific pull request
$ gh pr review 123 -r -b "needs more ASCII art"
`),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) > 0 {
opts.SelectorArg = args[0]
}
if runF != nil {
return runF(opts)
}
return reviewRun(opts)
},
}
cmd.Flags().BoolVarP(&opts.Approve, "approve", "a", false, "Approve pull request")
cmd.Flags().BoolVarP(&opts.RequestChanges, "request-changes", "r", false, "Request changes on a pull request")
cmd.Flags().BoolVarP(&opts.Comment, "comment", "c", false, "Comment on a pull request")
cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Specify the body of a review")
return cmd
}
func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
func reviewRun(opts *ReviewOptions) error {
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)
reviewData, err := processReviewOpt(opts)
if err != nil {
return fmt.Errorf("did not understand desired review action: %w", err)
}
pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg)
if err != nil {
return err
}
if reviewData == nil {
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
if err != nil {
return err
}
reviewData, err = reviewSurvey(opts.IO, editorCommand)
if err != nil {
return err
}
if reviewData == nil && err == nil {
fmt.Fprint(opts.IO.ErrOut, "Discarding.\n")
return nil
}
}
err = api.AddReview(apiClient, baseRepo, pr, reviewData)
if err != nil {
return fmt.Errorf("failed to create review: %w", err)
}
if !opts.IO.IsStdoutTTY() || !opts.IO.IsStderrTTY() {
return nil
}
switch reviewData.State {
case api.ReviewComment:
fmt.Fprintf(opts.IO.ErrOut, "%s Reviewed pull request #%d\n", utils.Gray("-"), pr.Number)
case api.ReviewApprove:
fmt.Fprintf(opts.IO.ErrOut, "%s Approved pull request #%d\n", utils.Green("✓"), pr.Number)
case api.ReviewRequestChanges:
fmt.Fprintf(opts.IO.ErrOut, "%s Requested changes to pull request #%d\n", utils.Red("+"), pr.Number)
}
return nil
}
// TODO: move to Command.Args, raise FlagError
func processReviewOpt(opts *ReviewOptions) (*api.PullRequestReviewInput, error) {
found := 0
flag := ""
var state api.PullRequestReviewState
if cmd.Flags().Changed("approve") {
if opts.Approve {
found++
flag = "approve"
state = api.ReviewApprove
}
if cmd.Flags().Changed("request-changes") {
if opts.RequestChanges {
found++
flag = "request-changes"
state = api.ReviewRequestChanges
}
if cmd.Flags().Changed("comment") {
if opts.Comment {
found++
flag = "comment"
state = api.ReviewComment
}
body, err := cmd.Flags().GetString("body")
if err != nil {
return nil, err
}
body := opts.Body
if found == 0 && body == "" {
if connectedToTerminal(cmd) {
if opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() {
return nil, nil // signal interactive mode
}
return nil, errors.New("--approve, --request-changes, or --comment required when not attached to a tty")
@ -93,63 +185,7 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
}, nil
}
func prReview(cmd *cobra.Command, args []string) error {
ctx := contextForCommand(cmd)
apiClient, err := apiClientForContext(ctx)
if err != nil {
return err
}
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
if err != nil {
return err
}
reviewData, err := processReviewOpt(cmd)
if err != nil {
return fmt.Errorf("did not understand desired review action: %w", err)
}
stderr := colorableErr(cmd)
if reviewData == nil {
reviewData, err = reviewSurvey(cmd)
if err != nil {
return err
}
if reviewData == nil && err == nil {
fmt.Fprint(stderr, "Discarding.\n")
return nil
}
}
err = api.AddReview(apiClient, baseRepo, pr, reviewData)
if err != nil {
return fmt.Errorf("failed to create review: %w", err)
}
if !connectedToTerminal(cmd) {
return nil
}
switch reviewData.State {
case api.ReviewComment:
fmt.Fprintf(stderr, "%s Reviewed pull request #%d\n", utils.Gray("-"), pr.Number)
case api.ReviewApprove:
fmt.Fprintf(stderr, "%s Approved pull request #%d\n", utils.Green("✓"), pr.Number)
case api.ReviewRequestChanges:
fmt.Fprintf(stderr, "%s Requested changes to pull request #%d\n", utils.Red("+"), pr.Number)
}
return nil
}
func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
editorCommand, err := determineEditor(cmd)
if err != nil {
return nil, err
}
func reviewSurvey(io *iostreams.IOStreams, editorCommand string) (*api.PullRequestReviewInput, error) {
typeAnswers := struct {
ReviewType string
}{}
@ -167,7 +203,7 @@ func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
},
}
err = prompt.SurveyAsk(typeQs, &typeAnswers)
err := prompt.SurveyAsk(typeQs, &typeAnswers)
if err != nil {
return nil, err
}
@ -218,13 +254,12 @@ func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
}
if len(bodyAnswers.Body) > 0 {
out := colorableOut(cmd)
renderedBody, err := utils.RenderMarkdown(bodyAnswers.Body)
if err != nil {
return nil, err
}
fmt.Fprintf(out, "Got:\n%s", renderedBody)
fmt.Fprintf(io.Out, "Got:\n%s", renderedBody)
}
confirm := false

313
command/pr_review_test.go → pkg/cmd/pr/review/review_test.go
View file

@ -1,58 +1,112 @@
package command
package review
import (
"bytes"
"encoding/json"
"io/ioutil"
"net/http"
"regexp"
"testing"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/pkg/prompt"
"github.com/cli/cli/test"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli string) (*test.CmdOut, error) {
io, _, stdout, stderr := iostreams.Test()
io.SetStdoutTTY(isTTY)
io.SetStdinTTY(isTTY)
io.SetStderrTTY(isTTY)
factory := &cmdutil.Factory{
IOStreams: io,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: rt}, nil
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Remotes: func() (context.Remotes, error) {
if remotes == nil {
return context.Remotes{
{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
}
return remotes, nil
},
Branch: func() (string, error) {
return "feature", nil
},
}
cmd := NewCmdReview(factory, nil)
argv, err := shlex.Split(cli)
if err != nil {
return nil, err
}
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(ioutil.Discard)
cmd.SetErr(ioutil.Discard)
_, err = cmd.ExecuteC()
return &test.CmdOut{
OutBuf: stdout,
ErrBuf: stderr,
}, err
}
func TestPRReview_validation(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
for _, cmd := range []string{
`pr review --approve --comment 123`,
`pr review --approve --comment -b"hey" 123`,
`--approve --comment 123`,
`--approve --comment -b"hey" 123`,
} {
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"pullRequest": { "number": 123 }
} } }
`))
_, err := RunCommand(cmd)
if err == nil {
t.Fatal("expected error")
}
eq(t, err.Error(), "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment")
t.Run(cmd, func(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
_, err := runCommand(http, nil, false, cmd)
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment", err.Error())
})
}
}
func TestPRReview_bad_body(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"pullRequest": { "number": 123 }
} } }
`))
_, err := RunCommand(`pr review 123 -b "radical"`)
http := &httpmock.Registry{}
defer http.Verify(t)
_, err := runCommand(http, nil, false, `123 -b "radical"`)
if err == nil {
t.Fatal("expected error")
}
eq(t, err.Error(), "did not understand desired review action: --body unsupported without --approve, --request-changes, or --comment")
assert.Equal(t, "did not understand desired review action: --body unsupported without --approve, --request-changes, or --comment", err.Error())
}
func TestPRReview_url_arg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequest": {
"id": "foobar123",
@ -72,7 +126,7 @@ func TestPRReview_url_arg(t *testing.T) {
} } } } `))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
output, err := RunCommand("pr review --approve https://github.com/OWNER/REPO/pull/123")
output, err := runCommand(http, nil, true, "--approve https://github.com/OWNER/REPO/pull/123")
if err != nil {
t.Fatalf("error running pr review: %s", err)
}
@ -91,16 +145,14 @@ func TestPRReview_url_arg(t *testing.T) {
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
eq(t, reqBody.Variables.Input.PullRequestID, "foobar123")
eq(t, reqBody.Variables.Input.Event, "APPROVE")
eq(t, reqBody.Variables.Input.Body, "")
assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
assert.Equal(t, "", reqBody.Variables.Input.Body)
}
func TestPRReview_number_arg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequest": {
"id": "foobar123",
@ -120,14 +172,14 @@ func TestPRReview_number_arg(t *testing.T) {
} } } } `))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
output, err := RunCommand("pr review --approve 123")
output, err := runCommand(http, nil, true, "--approve 123")
if err != nil {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
Variables struct {
Input struct {
@ -139,16 +191,14 @@ func TestPRReview_number_arg(t *testing.T) {
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
eq(t, reqBody.Variables.Input.PullRequestID, "foobar123")
eq(t, reqBody.Variables.Input.Event, "APPROVE")
eq(t, reqBody.Variables.Input.Body, "")
assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
assert.Equal(t, "", reqBody.Variables.Input.Body)
}
func TestPRReview_no_arg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
@ -159,14 +209,14 @@ func TestPRReview_no_arg(t *testing.T) {
] } } } }`))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
output, err := RunCommand(`pr review --comment -b "cool story"`)
output, err := runCommand(http, nil, true, `--comment -b "cool story"`)
if err != nil {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.Stderr(), "Reviewed pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
Variables struct {
Input struct {
@ -178,37 +228,25 @@ func TestPRReview_no_arg(t *testing.T) {
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
eq(t, reqBody.Variables.Input.PullRequestID, "foobar123")
eq(t, reqBody.Variables.Input.Event, "COMMENT")
eq(t, reqBody.Variables.Input.Body, "cool story")
assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
assert.Equal(t, "COMMENT", reqBody.Variables.Input.Event)
assert.Equal(t, "cool story", reqBody.Variables.Input.Body)
}
func TestPRReview_blank_comment(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"pullRequest": { "number": 123 }
} } }
`))
http := &httpmock.Registry{}
defer http.Verify(t)
_, err := RunCommand(`pr review --comment 123`)
eq(t, err.Error(), "did not understand desired review action: body cannot be blank for comment review")
_, err := runCommand(http, nil, false, `--comment 123`)
assert.Equal(t, "did not understand desired review action: body cannot be blank for comment review", err.Error())
}
func TestPRReview_blank_request_changes(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"pullRequest": { "number": 123 }
} } }
`))
http := &httpmock.Registry{}
defer http.Verify(t)
_, err := RunCommand(`pr review -r 123`)
eq(t, err.Error(), "did not understand desired review action: body cannot be blank for request-changes review")
_, err := runCommand(http, nil, false, `-r 123`)
assert.Equal(t, "did not understand desired review action: body cannot be blank for request-changes review", err.Error())
}
func TestPRReview(t *testing.T) {
@ -218,63 +256,53 @@ func TestPRReview(t *testing.T) {
ExpectedBody string
}
cases := []c{
{`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"},
{`pr review --approve`, "APPROVE", ""},
{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"},
{`pr review --comment --body "i donno"`, "COMMENT", "i donno"},
{`--request-changes -b"bad"`, "REQUEST_CHANGES", "bad"},
{`--approve`, "APPROVE", ""},
{`--approve -b"hot damn"`, "APPROVE", "hot damn"},
{`--comment --body "i donno"`, "COMMENT", "i donno"},
}
for _, kase := range cases {
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",
"id": "foobar123",
"headRefName": "feature",
"baseRefName": "master" }
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
t.Run(kase.Cmd, func(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
"id": "foobar123",
"headRefName": "feature",
"baseRefName": "master" }
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
_, err := RunCommand(kase.Cmd)
if err != nil {
t.Fatalf("got unexpected error running %s: %s", kase.Cmd, err)
}
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
Variables struct {
Input struct {
Event string
Body string
}
_, err := runCommand(http, nil, false, kase.Cmd)
if err != nil {
t.Fatalf("got unexpected error running %s: %s", kase.Cmd, err)
}
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
eq(t, reqBody.Variables.Input.Event, kase.ExpectedEvent)
eq(t, reqBody.Variables.Input.Body, kase.ExpectedBody)
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
Variables struct {
Input struct {
Event string
Body string
}
}
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
assert.Equal(t, kase.ExpectedEvent, reqBody.Variables.Input.Event)
assert.Equal(t, kase.ExpectedBody, reqBody.Variables.Input.Body)
})
}
}
func TestPRReview_nontty_insufficient_flags(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(false)()
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 := &httpmock.Registry{}
defer http.Verify(t)
output, err := RunCommand("pr review")
output, err := runCommand(http, nil, false, "")
if err == nil {
t.Fatal("expected error")
}
@ -285,10 +313,8 @@ func TestPRReview_nontty_insufficient_flags(t *testing.T) {
}
func TestPRReview_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
@ -300,7 +326,7 @@ func TestPRReview_nontty(t *testing.T) {
`))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
output, err := RunCommand("pr review -c -bcool")
output, err := runCommand(http, nil, false, "-c -bcool")
if err != nil {
t.Fatalf("unexpected error running command: %s", err)
}
@ -308,7 +334,7 @@ func TestPRReview_nontty(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
Variables struct {
Input struct {
@ -324,10 +350,8 @@ func TestPRReview_nontty(t *testing.T) {
}
func TestPRReview_interactive(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
@ -360,7 +384,7 @@ func TestPRReview_interactive(t *testing.T) {
},
})
output, err := RunCommand(`pr review`)
output, err := runCommand(http, nil, true, "")
if err != nil {
t.Fatalf("got unexpected error running pr review: %s", err)
}
@ -371,7 +395,7 @@ func TestPRReview_interactive(t *testing.T) {
"Got:",
"cool.*story")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
Variables struct {
Input struct {
@ -382,15 +406,13 @@ func TestPRReview_interactive(t *testing.T) {
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
eq(t, reqBody.Variables.Input.Event, "APPROVE")
eq(t, reqBody.Variables.Input.Body, "cool story")
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
assert.Equal(t, "cool story", reqBody.Variables.Input.Body)
}
func TestPRReview_interactive_no_body(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
@ -399,7 +421,7 @@ func TestPRReview_interactive_no_body(t *testing.T) {
"baseRefName": "master" }
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
as, teardown := prompt.InitAskStubber()
defer teardown()
@ -422,18 +444,16 @@ func TestPRReview_interactive_no_body(t *testing.T) {
},
})
_, err := RunCommand(`pr review`)
_, err := runCommand(http, nil, true, "")
if err == nil {
t.Fatal("expected error")
}
eq(t, err.Error(), "this type of review cannot be blank")
assert.Equal(t, "this type of review cannot be blank", err.Error())
}
func TestPRReview_interactive_blank_approve(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
@ -466,7 +486,7 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
},
})
output, err := RunCommand(`pr review`)
output, err := runCommand(http, nil, true, "")
if err != nil {
t.Fatalf("got unexpected error running pr review: %s", err)
}
@ -478,7 +498,7 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
Variables struct {
Input struct {
@ -489,7 +509,6 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
eq(t, reqBody.Variables.Input.Event, "APPROVE")
eq(t, reqBody.Variables.Input.Body, "")
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
assert.Equal(t, "", reqBody.Variables.Input.Body)
}

121
pkg/cmd/pr/shared/lookup.go Normal file
View file

@ -0,0 +1,121 @@
package shared
import (
"fmt"
"net/url"
"regexp"
"strconv"
"strings"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
)
// PRFromArgs looks up the pull request from either the number/branch/URL argument or one belonging to the current branch
//
// NOTE: this API isn't great, but is here as a compatibility layer between old-style and new-style commands
func PRFromArgs(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, error), branchFn func() (string, error), remotesFn func() (context.Remotes, error), arg string) (*api.PullRequest, ghrepo.Interface, error) {
if arg != "" {
// First check to see if the prString is a url, return repo from url if found. This
// is run first because we don't need to run determineBaseRepo for this path
pr, r, err := prFromURL(apiClient, arg)
if pr != nil || err != nil {
return pr, r, err
}
}
repo, err := baseRepoFn()
if err != nil {
return nil, nil, fmt.Errorf("could not determine base repo: %w", err)
}
// If there are no args see if we can guess the PR from the current branch
if arg == "" {
pr, err := prForCurrentBranch(apiClient, repo, branchFn, remotesFn)
return pr, repo, err
} else {
// Next see if the prString is a number and use that to look up the url
pr, err := prFromNumberString(apiClient, repo, arg)
if pr != nil || err != nil {
return pr, repo, err
}
// Last see if it is a branch name
pr, err = api.PullRequestForBranch(apiClient, repo, "", arg)
return pr, repo, err
}
}
func prFromNumberString(apiClient *api.Client, repo ghrepo.Interface, s string) (*api.PullRequest, error) {
if prNumber, err := strconv.Atoi(strings.TrimPrefix(s, "#")); err == nil {
return api.PullRequestByNumber(apiClient, repo, prNumber)
}
return nil, nil
}
var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)
func prFromURL(apiClient *api.Client, s string) (*api.PullRequest, ghrepo.Interface, error) {
u, err := url.Parse(s)
if err != nil {
return nil, nil, nil
}
if u.Scheme != "https" && u.Scheme != "http" {
return nil, nil, nil
}
m := pullURLRE.FindStringSubmatch(u.Path)
if m == nil {
return nil, nil, nil
}
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
prNumberString := m[3]
pr, err := prFromNumberString(apiClient, repo, prNumberString)
return pr, repo, err
}
func prForCurrentBranch(apiClient *api.Client, repo ghrepo.Interface, branchFn func() (string, error), remotesFn func() (context.Remotes, error)) (*api.PullRequest, error) {
prHeadRef, err := branchFn()
if err != nil {
return nil, err
}
branchConfig := git.ReadBranchConfig(prHeadRef)
// the branch is configured to merge a special PR head ref
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
return prFromNumberString(apiClient, repo, m[1])
}
var branchOwner string
if branchConfig.RemoteURL != nil {
// the branch merges from a remote specified by URL
if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil {
branchOwner = r.RepoOwner()
}
} else if branchConfig.RemoteName != "" {
// the branch merges from a remote specified by name
rem, _ := remotesFn()
if r, err := rem.FindByName(branchConfig.RemoteName); err == nil {
branchOwner = r.RepoOwner()
}
}
if branchOwner != "" {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork
if !strings.EqualFold(branchOwner, repo.RepoOwner()) {
prHeadRef = fmt.Sprintf("%s:%s", branchOwner, prHeadRef)
}
}
return api.PullRequestForBranch(apiClient, repo, "", prHeadRef)
}

1
pkg/cmd/repo/clone/clone.go
View file

@ -70,6 +70,7 @@ func cloneRun(opts *CloneOptions) error {
if err != nil {
return err
}
// TODO: GHE support
protocol, err := cfg.Get("", "git_protocol")
if err != nil {
return err

1
pkg/cmd/repo/create/create.go
View file

@ -147,6 +147,7 @@ func createRun(opts *CreateOptions) error {
if err != nil {
return err
}
// TODO: GHE support
protocol, err := cfg.Get("", "git_protocol")
if err != nil {
return err

1
pkg/cmd/repo/fork/fork.go
View file

@ -186,6 +186,7 @@ func forkRun(opts *ForkOptions) error {
if err != nil {
return err
}
// TODO: GHE support
protocol, err := cfg.Get("", "git_protocol")
if err != nil {
return err

1
pkg/cmdutil/factory.go
View file

@ -15,4 +15,5 @@ type Factory struct {
BaseRepo func() (ghrepo.Interface, error)
Remotes func() (context.Remotes, error)
Config func() (config.Config, error)
Branch func() (string, error)
}

23
pkg/cmdutil/legacy.go Normal file
View file

@ -0,0 +1,23 @@
package cmdutil
import (
"fmt"
"os"
"github.com/cli/cli/internal/config"
)
// TODO: consider passing via Factory
// TODO: support per-hostname settings
func DetermineEditor(cf func() (config.Config, error)) (string, error) {
editorCommand := os.Getenv("GH_EDITOR")
if editorCommand == "" {
cfg, err := cf()
if err != nil {
return "", fmt.Errorf("could not read config: %w", err)
}
editorCommand, _ = cfg.Get("", "editor")
}
return editorCommand, nil
}