add --web flag to pr diff command (#6439)
This commit is contained in:
ffalor
GitHub
parent
a756ffb1e8
commit
e7270e401d
2 changed files with 108 additions and 48 deletions
|
|
@ -11,8 +11,10 @@ import (
|
|||
|
||||
"github.com/MakeNowJust/heredoc"
|
||||
"github.com/cli/cli/v2/api"
|
||||
"github.com/cli/cli/v2/internal/browser"
|
||||
"github.com/cli/cli/v2/internal/ghinstance"
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
"github.com/cli/cli/v2/internal/text"
|
||||
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
|
||||
"github.com/cli/cli/v2/pkg/cmdutil"
|
||||
"github.com/cli/cli/v2/pkg/iostreams"
|
||||
|
|
@ -22,6 +24,7 @@ import (
|
|||
type DiffOptions struct {
|
||||
HttpClient func() (*http.Client, error)
|
||||
IO *iostreams.IOStreams
|
||||
Browser browser.Browser
|
||||
|
||||
Finder shared.PRFinder
|
||||
|
||||
|
|
@ -29,12 +32,14 @@ type DiffOptions struct {
|
|||
UseColor bool
|
||||
Patch bool
|
||||
NameOnly bool
|
||||
BrowserMode bool
|
||||
}
|
||||
|
||||
func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Command {
|
||||
opts := &DiffOptions{
|
||||
IO: f.IOStreams,
|
||||
HttpClient: f.HttpClient,
|
||||
Browser: f.Browser,
|
||||
}
|
||||
|
||||
var colorFlag string
|
||||
|
|
@ -46,7 +51,9 @@ func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Comman
|
|||
View changes in a pull request.
|
||||
|
||||
Without an argument, the pull request that belongs to the current branch
|
||||
is selected.
|
||||
is selected.
|
||||
|
||||
With '--web', open the pull request diff in a web browser instead.
|
||||
`),
|
||||
Args: cobra.MaximumNArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
|
|
@ -81,6 +88,7 @@ func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Comman
|
|||
cmdutil.StringEnumFlag(cmd, &colorFlag, "color", "", "auto", []string{"always", "never", "auto"}, "Use color in diff output")
|
||||
cmd.Flags().BoolVar(&opts.Patch, "patch", false, "Display diff in patch format")
|
||||
cmd.Flags().BoolVar(&opts.NameOnly, "name-only", false, "Display only names of changed files")
|
||||
cmd.Flags().BoolVarP(&opts.BrowserMode, "web", "w", false, "Open the pull request diff in the browser")
|
||||
|
||||
return cmd
|
||||
}
|
||||
|
|
@ -90,11 +98,24 @@ func diffRun(opts *DiffOptions) error {
|
|||
Selector: opts.SelectorArg,
|
||||
Fields: []string{"number"},
|
||||
}
|
||||
|
||||
if opts.BrowserMode {
|
||||
findOptions.Fields = []string{"url"}
|
||||
}
|
||||
|
||||
pr, baseRepo, err := opts.Finder.Find(findOptions)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if opts.BrowserMode {
|
||||
openUrl := fmt.Sprintf("%s/files", pr.URL)
|
||||
if opts.IO.IsStdoutTTY() {
|
||||
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(openUrl))
|
||||
}
|
||||
return opts.Browser.Browse(openUrl)
|
||||
}
|
||||
|
||||
httpClient, err := opts.HttpClient()
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -9,12 +9,12 @@ import (
|
|||
"testing"
|
||||
|
||||
"github.com/cli/cli/v2/api"
|
||||
"github.com/cli/cli/v2/internal/browser"
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
|
||||
"github.com/cli/cli/v2/pkg/cmdutil"
|
||||
"github.com/cli/cli/v2/pkg/httpmock"
|
||||
"github.com/cli/cli/v2/pkg/iostreams"
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/google/shlex"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
|
@ -92,6 +92,16 @@ func Test_NewCmdDiff(t *testing.T) {
|
|||
isTTY: true,
|
||||
wantErr: "invalid argument \"doublerainbow\" for \"--color\" flag: valid values are {always|never|auto}",
|
||||
},
|
||||
{
|
||||
name: "web mode",
|
||||
args: "123 --web",
|
||||
isTTY: true,
|
||||
want: DiffOptions{
|
||||
SelectorArg: "123",
|
||||
UseColor: true,
|
||||
BrowserMode: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
@ -130,19 +140,22 @@ func Test_NewCmdDiff(t *testing.T) {
|
|||
|
||||
assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg)
|
||||
assert.Equal(t, tt.want.UseColor, opts.UseColor)
|
||||
assert.Equal(t, tt.want.BrowserMode, opts.BrowserMode)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_diffRun(t *testing.T) {
|
||||
pr := &api.PullRequest{Number: 123}
|
||||
pr := &api.PullRequest{Number: 123, URL: "https://github.com/OWNER/REPO/pull/123"}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
opts DiffOptions
|
||||
rawDiff string
|
||||
wantAccept string
|
||||
wantStdout string
|
||||
name string
|
||||
opts DiffOptions
|
||||
wantFields []string
|
||||
wantStdout string
|
||||
wantStderr string
|
||||
wantBrowsedURL string
|
||||
httpStubs func(*httpmock.Registry)
|
||||
}{
|
||||
{
|
||||
name: "no color",
|
||||
|
|
@ -151,9 +164,11 @@ func Test_diffRun(t *testing.T) {
|
|||
UseColor: false,
|
||||
Patch: false,
|
||||
},
|
||||
rawDiff: fmt.Sprintf(testDiff, "", "", "", ""),
|
||||
wantAccept: "application/vnd.github.v3.diff",
|
||||
wantFields: []string{"number"},
|
||||
wantStdout: fmt.Sprintf(testDiff, "", "", "", ""),
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
stubDiffRequest(reg, "application/vnd.github.v3.diff", fmt.Sprintf(testDiff, "", "", "", ""))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with color",
|
||||
|
|
@ -162,9 +177,11 @@ func Test_diffRun(t *testing.T) {
|
|||
UseColor: true,
|
||||
Patch: false,
|
||||
},
|
||||
rawDiff: fmt.Sprintf(testDiff, "", "", "", ""),
|
||||
wantAccept: "application/vnd.github.v3.diff",
|
||||
wantFields: []string{"number"},
|
||||
wantStdout: fmt.Sprintf(testDiff, "\x1b[m", "\x1b[1;38m", "\x1b[32m", "\x1b[31m"),
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
stubDiffRequest(reg, "application/vnd.github.v3.diff", fmt.Sprintf(testDiff, "", "", "", ""))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "patch format",
|
||||
|
|
@ -173,9 +190,11 @@ func Test_diffRun(t *testing.T) {
|
|||
UseColor: false,
|
||||
Patch: true,
|
||||
},
|
||||
rawDiff: fmt.Sprintf(testDiff, "", "", "", ""),
|
||||
wantAccept: "application/vnd.github.v3.patch",
|
||||
wantFields: []string{"number"},
|
||||
wantStdout: fmt.Sprintf(testDiff, "", "", "", ""),
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
stubDiffRequest(reg, "application/vnd.github.v3.patch", fmt.Sprintf(testDiff, "", "", "", ""))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "name only",
|
||||
|
|
@ -185,51 +204,51 @@ func Test_diffRun(t *testing.T) {
|
|||
Patch: false,
|
||||
NameOnly: true,
|
||||
},
|
||||
rawDiff: fmt.Sprintf(testDiff, "", "", "", ""),
|
||||
wantAccept: "application/vnd.github.v3.diff",
|
||||
wantFields: []string{"number"},
|
||||
wantStdout: ".github/workflows/releases.yml\nMakefile\n",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
stubDiffRequest(reg, "application/vnd.github.v3.diff", fmt.Sprintf(testDiff, "", "", "", ""))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "web mode",
|
||||
opts: DiffOptions{
|
||||
SelectorArg: "123",
|
||||
BrowserMode: true,
|
||||
},
|
||||
wantFields: []string{"url"},
|
||||
wantStderr: "Opening github.com/OWNER/REPO/pull/123/files in your browser.\n",
|
||||
wantBrowsedURL: "https://github.com/OWNER/REPO/pull/123/files",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
httpReg := &httpmock.Registry{}
|
||||
defer httpReg.Verify(t)
|
||||
|
||||
var gotAccept string
|
||||
httpReg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/pulls/123"),
|
||||
func(req *http.Request) (*http.Response, error) {
|
||||
gotAccept = req.Header.Get("Accept")
|
||||
return &http.Response{
|
||||
StatusCode: 200,
|
||||
Request: req,
|
||||
Body: io.NopCloser(strings.NewReader(tt.rawDiff)),
|
||||
}, nil
|
||||
})
|
||||
|
||||
opts := tt.opts
|
||||
opts.HttpClient = func() (*http.Client, error) {
|
||||
if tt.httpStubs != nil {
|
||||
tt.httpStubs(httpReg)
|
||||
}
|
||||
tt.opts.HttpClient = func() (*http.Client, error) {
|
||||
return &http.Client{Transport: httpReg}, nil
|
||||
}
|
||||
|
||||
ios, _, stdout, stderr := iostreams.Test()
|
||||
opts.IO = ios
|
||||
finder := shared.NewMockFinder("123", pr, ghrepo.New("OWNER", "REPO"))
|
||||
finder.ExpectFields([]string{"number"})
|
||||
opts.Finder = finder
|
||||
browser := &browser.Stub{}
|
||||
tt.opts.Browser = browser
|
||||
|
||||
if err := diffRun(&opts); err != nil {
|
||||
t.Fatalf("unexpected error: %s", err)
|
||||
}
|
||||
if diff := cmp.Diff(tt.wantStdout, stdout.String()); diff != "" {
|
||||
t.Errorf("command output did not match:\n%s", diff)
|
||||
}
|
||||
if stderr.String() != "" {
|
||||
t.Errorf("unexpected stderr output: %s", stderr.String())
|
||||
}
|
||||
if gotAccept != tt.wantAccept {
|
||||
t.Errorf("unexpected Accept header: %s", gotAccept)
|
||||
}
|
||||
ios, _, stdout, stderr := iostreams.Test()
|
||||
ios.SetStdoutTTY(true)
|
||||
tt.opts.IO = ios
|
||||
|
||||
finder := shared.NewMockFinder("123", pr, ghrepo.New("OWNER", "REPO"))
|
||||
finder.ExpectFields(tt.wantFields)
|
||||
tt.opts.Finder = finder
|
||||
|
||||
err := diffRun(&tt.opts)
|
||||
assert.NoError(t, err)
|
||||
|
||||
assert.Equal(t, tt.wantStdout, stdout.String())
|
||||
assert.Equal(t, tt.wantStderr, stderr.String())
|
||||
assert.Equal(t, tt.wantBrowsedURL, browser.BrowsedURL())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -349,3 +368,23 @@ func Test_changedFileNames(t *testing.T) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
func stubDiffRequest(reg *httpmock.Registry, accept, diff string) {
|
||||
reg.Register(
|
||||
func(req *http.Request) bool {
|
||||
if !strings.EqualFold(req.Method, "GET") {
|
||||
return false
|
||||
}
|
||||
if req.URL.EscapedPath() != "/repos/OWNER/REPO/pulls/123" {
|
||||
return false
|
||||
}
|
||||
return req.Header.Get("Accept") == accept
|
||||
},
|
||||
func(req *http.Request) (*http.Response, error) {
|
||||
return &http.Response{
|
||||
StatusCode: 200,
|
||||
Request: req,
|
||||
Body: io.NopCloser(strings.NewReader(diff)),
|
||||
}, nil
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue