Skip to content

Commit 111e8db

Browse files
committed
Pass web browser to each individual command
This removes sensitivity to the BROWSER environment variable in tests and makes it easier to verify the URL that the browser was invoked with without having to stub sub-processes.
1 parent 2ab073d commit 111e8db

File tree

34 files changed

+445
-495
lines changed

34 files changed

+445
-495
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/MakeNowJust/heredoc v1.0.0
88
github.com/briandowns/spinner v1.11.1
99
github.com/charmbracelet/glamour v0.2.1-0.20200724174618-1246d13c1684
10+
github.com/cli/browser v1.1.0
1011
github.com/cli/oauth v0.8.0
1112
github.com/cli/safeexec v1.0.0
1213
github.com/cpuguy83/go-md2man/v2 v2.0.0

go.sum

Lines changed: 139 additions & 1 deletion
Large diffs are not rendered by default.

internal/authflow/flow.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010

1111
"github.com/cli/cli/api"
1212
"github.com/cli/cli/internal/ghinstance"
13-
"github.com/cli/cli/pkg/browser"
13+
"github.com/cli/cli/pkg/cmdutil"
1414
"github.com/cli/cli/pkg/iostreams"
1515
"github.com/cli/oauth"
1616
)
@@ -93,11 +93,9 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
9393
fmt.Fprintf(w, "- %s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost)
9494
_ = waitForEnter(IO.In)
9595

96-
browseCmd, err := browser.Command(url)
97-
if err != nil {
98-
return err
99-
}
100-
if err := browseCmd.Run(); err != nil {
96+
// FIXME: read the browser from cmd Factory rather than recreating it
97+
browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut)
98+
if err := browser.Browse(url); err != nil {
10199
fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url)
102100
fmt.Fprintf(w, " %s\n", err)
103101
fmt.Fprint(w, " Please try entering the URL in your browser manually\n")

pkg/browser/browser.go

Lines changed: 0 additions & 80 deletions
This file was deleted.

pkg/browser/browser_test.go

Lines changed: 0 additions & 75 deletions
This file was deleted.

pkg/cmd/factory/default.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,6 @@ func New(appVersion string) *cmdutil.Factory {
7676
return currentBranch, nil
7777
},
7878
Executable: ghExecutable,
79+
Browser: cmdutil.NewBrowser(os.Getenv("BROWSER"), io.Out, io.ErrOut),
7980
}
8081
}

pkg/cmd/gist/create/create.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import (
2323
"github.com/spf13/cobra"
2424
)
2525

26+
type browser interface {
27+
Browse(string) error
28+
}
29+
2630
type CreateOptions struct {
2731
IO *iostreams.IOStreams
2832

@@ -33,12 +37,14 @@ type CreateOptions struct {
3337
WebMode bool
3438

3539
HttpClient func() (*http.Client, error)
40+
Browser browser
3641
}
3742

3843
func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command {
3944
opts := CreateOptions{
4045
IO: f.IOStreams,
4146
HttpClient: f.HttpClient,
47+
Browser: f.Browser,
4248
}
4349

4450
cmd := &cobra.Command{
@@ -144,7 +150,7 @@ func createRun(opts *CreateOptions) error {
144150
if opts.WebMode {
145151
fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", utils.DisplayURL(gist.HTMLURL))
146152

147-
return utils.OpenInBrowser(gist.HTMLURL)
153+
return opts.Browser.Browse(gist.HTMLURL)
148154
}
149155

150156
fmt.Fprintln(opts.IO.Out, gist.HTMLURL)

pkg/cmd/gist/create/create_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ func Test_createRun(t *testing.T) {
171171
wantStderr string
172172
wantParams map[string]interface{}
173173
wantErr bool
174+
wantBrowse string
174175
}{
175176
{
176177
name: "public",
@@ -264,6 +265,7 @@ func Test_createRun(t *testing.T) {
264265
wantOut: "Opening gist.github.com/aa5a315d61ae9438b18d in your browser.\n",
265266
wantStderr: "- Creating gist fixture.txt\n✓ Created gist fixture.txt\n",
266267
wantErr: false,
268+
wantBrowse: "https://gist.github.com/aa5a315d61ae9438b18d",
267269
wantParams: map[string]interface{}{
268270
"description": "",
269271
"updated_at": "0001-01-01T00:00:00Z",
@@ -291,11 +293,11 @@ func Test_createRun(t *testing.T) {
291293
io, stdin, stdout, stderr := iostreams.Test()
292294
tt.opts.IO = io
293295

294-
cs, teardown := run.Stub()
296+
browser := &cmdutil.TestBrowser{}
297+
tt.opts.Browser = browser
298+
299+
_, teardown := run.Stub()
295300
defer teardown(t)
296-
if tt.opts.WebMode {
297-
cs.Register(`https://gist\.github\.com/aa5a315d61ae9438b18d$`, 0, "")
298-
}
299301

300302
t.Run(tt.name, func(t *testing.T) {
301303
stdin.WriteString(tt.stdin)
@@ -313,6 +315,7 @@ func Test_createRun(t *testing.T) {
313315
assert.Equal(t, tt.wantStderr, stderr.String())
314316
assert.Equal(t, tt.wantParams, reqBody)
315317
reg.Verify(t)
318+
browser.Verify(t, tt.wantBrowse)
316319
})
317320
}
318321
}

pkg/cmd/gist/view/view.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@ import (
1919
"github.com/spf13/cobra"
2020
)
2121

22+
type browser interface {
23+
Browse(string) error
24+
}
25+
2226
type ViewOptions struct {
2327
IO *iostreams.IOStreams
2428
HttpClient func() (*http.Client, error)
29+
Browser browser
2530

2631
Selector string
2732
Filename string
@@ -34,6 +39,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
3439
opts := &ViewOptions{
3540
IO: f.IOStreams,
3641
HttpClient: f.HttpClient,
42+
Browser: f.Browser,
3743
}
3844

3945
cmd := &cobra.Command{
@@ -94,7 +100,7 @@ func viewRun(opts *ViewOptions) error {
94100
if opts.IO.IsStderrTTY() {
95101
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(gistURL))
96102
}
97-
return utils.OpenInBrowser(gistURL)
103+
return opts.Browser.Browse(gistURL)
98104
}
99105

100106
if strings.Contains(gistID, "/") {

pkg/cmd/issue/comment/comment.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
issueShared "github.com/cli/cli/pkg/cmd/issue/shared"
1010
prShared "github.com/cli/cli/pkg/cmd/pr/shared"
1111
"github.com/cli/cli/pkg/cmdutil"
12-
"github.com/cli/cli/utils"
1312
"github.com/spf13/cobra"
1413
)
1514

@@ -20,7 +19,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
2019
EditSurvey: prShared.CommentableEditSurvey(f.Config, f.IOStreams),
2120
InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams),
2221
ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey,
23-
OpenInBrowser: utils.OpenInBrowser,
22+
OpenInBrowser: f.Browser.Browse,
2423
}
2524

2625
var bodyFile string

0 commit comments

Comments
 (0)