diff --git a/.vscode/settings.json b/.vscode/settings.json index 99a52d6d6..6caa6e9f6 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,4 +2,4 @@ "search.exclude": { "vendor/**": true } -} \ No newline at end of file +} diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index fabf40c76..d679abd4b 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -33,18 +33,6 @@ type BrowseOptions struct { BranchFlag bool } -type exitCode int - -const ( - exitUrlSuccess exitCode = 0 - exitNonUrlSuccess exitCode = 1 - exitNotInRepo exitCode = 2 - exitTooManyFlags exitCode = 3 - exitTooManyArgs exitCode = 4 - exitExpectedArg exitCode = 5 - exitInvalidCombo exitCode = 6 -) - func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command { opts := &BrowseOptions{ @@ -108,84 +96,105 @@ func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command { } func openInBrowser(cmd *cobra.Command, opts *BrowseOptions) error { - + help := "Use 'gh browse --help' for more information about browse\n" baseRepo, err := opts.BaseRepo() + if err != nil { + return fmt.Errorf("unable to determine base repository: %w\n%s", err, help) + } + httpClient, _ := opts.HttpClient() apiClient := api.NewClientFromHTTP(httpClient) branchName, _ := api.RepoDefaultBranch(apiClient, baseRepo) - if !inRepo(err) { - return printExit(exitNotInRepo, cmd, opts, "") - } - if getFlagAmount(cmd) > 1 { - return printExit(exitTooManyFlags, cmd, opts, "") + return fmt.Errorf("accepts 1 flag, %d flag(s) were recieved\n%s", getFlagAmount(cmd), help) } repoUrl := ghrepo.GenerateRepoURL(baseRepo, "") - response := exitUrlSuccess if !hasArg(opts) && hasFlag(cmd) { - response, repoUrl = addFlag(opts, repoUrl) + err, repoUrl = addFlag(opts, repoUrl) + if err != nil { + return err + } } else if hasArg(opts) && !hasFlag(cmd) { - response, repoUrl = addArg(opts, repoUrl, branchName) + err, repoUrl = addArg(opts, repoUrl, branchName) + if err != nil { + return err + } } else if hasArg(opts) && hasFlag(cmd) { - response, repoUrl = addCombined(opts, repoUrl, branchName) + err, repoUrl = addCombined(opts, repoUrl, branchName) + if err != nil { + return err + } } - if response == exitUrlSuccess || response == exitNonUrlSuccess { - opts.Browser.Browse(repoUrl) + if repoUrl == ghrepo.GenerateRepoURL(baseRepo, "") { + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", repoUrl) } + opts.Browser.Browse(repoUrl) - return printExit(response, cmd, opts, repoUrl) + return nil } -func addCombined(opts *BrowseOptions, url string, branchName string) (exitCode, string) { +func addCombined(opts *BrowseOptions, url string, branchName string) (error, string) { + help := "Use 'gh browse --help' for more information about browse\n" if !opts.BranchFlag { - return exitInvalidCombo, "" + return fmt.Errorf("invalid use of flag and argument\n%s", help), "" } if opts.AdditionalArg == "" { - return exitUrlSuccess, url + "/tree/" + opts.SelectorArg + url += "/tree/" + opts.SelectorArg + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } arr := parseFileArg(opts) if len(arr) > 1 { - return exitUrlSuccess, url + "/tree/" + opts.AdditionalArg + "/" + arr[0] + "#L" + arr[1] + url += "/tree/" + opts.SelectorArg + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } - return exitUrlSuccess, url + "/tree/" + opts.AdditionalArg + "/" + arr[0] + url += "/tree/" + opts.SelectorArg + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } -func addFlag(opts *BrowseOptions, url string) (exitCode, string) { +func addFlag(opts *BrowseOptions, url string) (error, string) { if opts.ProjectsFlag { - return exitUrlSuccess, url + "/projects" + url += "/projects" } else if opts.SettingsFlag { - return exitUrlSuccess, url + "/settings" + url += "/settings" } else if opts.WikiFlag { - return exitUrlSuccess, url + "/wiki" + url += "/wiki" } - return exitExpectedArg, "" + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } -func addArg(opts *BrowseOptions, url string, branchName string) (exitCode, string) { +func addArg(opts *BrowseOptions, url string, branchName string) (error, string) { + help := "Use 'gh browse --help' for more information about browse\n" if opts.AdditionalArg != "" { - return exitTooManyArgs, "" + return fmt.Errorf("accepts 1 arg, 2 arg(s) were recieved\n%s", help), "" } if isNumber(opts.SelectorArg) { url += "/issues/" + opts.SelectorArg - return exitNonUrlSuccess, url + fmt.Fprintf(opts.IO.Out, "now opening issue/pr in browser . . .\n") + return nil, url } arr := parseFileArg(opts) if len(arr) > 1 { - return exitUrlSuccess, url + "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1] + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url + "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1] } - return exitUrlSuccess, url + "/tree/" + branchName + "/" + arr[0] + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url + "/tree/" + branchName + "/" + arr[0] } func parseFileArg(opts *BrowseOptions) []string { @@ -193,32 +202,6 @@ func parseFileArg(opts *BrowseOptions) []string { return arr } -func printExit(exit exitCode, cmd *cobra.Command, opts *BrowseOptions, url string) error { - w := opts.IO.ErrOut - cs := opts.IO.ColorScheme() - help := "Use 'gh browse --help' for more information about browse\n" - - switch exit { - case exitUrlSuccess: - //fmt.Fprintf(w, "now opening %s in browser . . .\n", cs.Bold(url)) - fmt.Fprintln(w, "Hello World One") - case exitNonUrlSuccess: - //fmt.Fprintf(w, "now opening issue/pr in browser . . .\n") - fmt.Fprintln(w, "Hello World Two") - case exitNotInRepo: - return fmt.Errorf("change directory to a repository to open in browser\n%s", help) - case exitTooManyFlags: - return fmt.Errorf("accepts 1 flag, %d flag(s) were recieved\n%s", getFlagAmount(cmd), help) - case exitTooManyArgs: - return fmt.Errorf("accepts 1 arg, 2 arg(s) were received \n%s", help) - case exitExpectedArg: - return fmt.Errorf("expected argument with this flag %s\n%s", cs.Bold(url), help) - case exitInvalidCombo: - return fmt.Errorf("invalid use of flag and argument\n%s", help) - } - return nil -} - func hasFlag(cmd *cobra.Command) bool { return getFlagAmount(cmd) > 0 } @@ -231,10 +214,6 @@ func getFlagAmount(cmd *cobra.Command) int { return cmd.Flags().NFlag() } -func inRepo(err error) bool { - return err == nil -} - func isNumber(arg string) bool { _, err := strconv.Atoi(arg) if err != nil { diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index abb9975bd..d1c9442c3 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -2,10 +2,11 @@ package browse import ( "bytes" - //"io/ioutil" + "io/ioutil" "net/http" "testing" + "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" @@ -26,6 +27,7 @@ type testCase struct { errorExpected bool stdoutExpected string stderrExpected string + urlExpected string } func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { @@ -39,8 +41,11 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { 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 + return t.args.repo, nil }, } @@ -53,8 +58,8 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { cmd.SetArgs(argv) cmd.SetIn(&bytes.Buffer{}) - cmd.SetOut(stdout) - cmd.SetErr(stderr) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) _, err = cmd.ExecuteC() return &test.CmdOut{ @@ -64,33 +69,66 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { }, err } func TestNewCmdBrowse(t *testing.T) { - var tests = []testCase{ { - name: "test1", + name: "multiple flag", args: args{ - repo: ghrepo.New("bchadwic", "cli"), + repo: ghrepo.New("jessica", "cli"), cli: "--settings --projects", }, errorExpected: true, stdoutExpected: "", - stderrExpected: "Error: accepts 1 flag, 2 flag(s) were recieved\nUse 'gh browse --help' for more information about browse\n\n", + stderrExpected: "accepts 1 flag, 2 flag(s) were recieved\nUse 'gh browse --help' for more information about browse\n", + urlExpected: "", + }, + { + name: "settings flag", + args: args{ + repo: ghrepo.New("husrav", "cli"), + cli: "--settings", + }, + errorExpected: false, + stdoutExpected: "now opening https://github.com/husrav/cli/settings in browser . . .\n", + stderrExpected: "", + urlExpected: "https://github.com/husrav/cli/settings", + }, + { + name: "projects flag", + args: args{ + repo: ghrepo.New("ben", "cli"), + cli: "--projects", + }, + errorExpected: false, + stdoutExpected: "now opening https://github.com/ben/cli/projects in browser . . .\n", + stderrExpected: "", + urlExpected: "https://github.com/ben/cli/projects", + }, + { + name: "wiki flag", + args: args{ + repo: ghrepo.New("thanh", "cli"), + cli: "--wiki", + }, + errorExpected: false, + stdoutExpected: "now opening https://github.com/thanh/cli/wiki in browser . . .\n", + stderrExpected: "", + urlExpected: "https://github.com/thanh/cli/wiki", + }, + { + name: "branch flag", + args: args{ + repo: ghrepo.New("ken", "cli"), + cli: "--branch", + }, + errorExpected: false, + stdoutExpected: "", + stderrExpected: "Aa", + urlExpected: "", }, - // { - // name: "test2", - // args: args{ - // repo: ghrepo.New("bchadwic", "cli"), - // cli: "--settings", - // }, - // errorExpected: false, - // stdoutExpected: "hello world", - // stderrExpected: "hello world", - // }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - http := &httpmock.Registry{} defer http.Verify(t) @@ -100,28 +138,12 @@ func TestNewCmdBrowse(t *testing.T) { output, err := runCommand(http, tt) if tt.errorExpected { - assert.Error(t, err) + assert.Equal(t, err.Error(), tt.stderrExpected) + } else { + assert.Equal(t, err, nil) } - - assert.Contains(t, output.OutBuf.String(), tt.stdoutExpected) // success outputs - - assert.Contains(t, output.ErrBuf.String(), tt.stderrExpected) // error outputs - + assert.Equal(t, output.OutBuf.String(), tt.stdoutExpected) + assert.Equal(t, tt.urlExpected, output.BrowsedURL) }) } } - -// http := initFakeHTTP() -// defer http.Verify(t) - -// _, cmdTeardown := run.Stub() -// defer cmdTeardown(t) - -// output, err := runCommand(http, true, "--web -a peter -l bug -l docs -L 10 -s merged -B trunk") -// if err != nil { -// t.Errorf("error running command `pr list` with `--web` flag: %v", err) -// } - -// assert.Equal(t, "", output.String()) -// assert.Equal(t, "Opening github.com/OWNER/REPO/pulls in your browser.\n", output.Stderr()) -// assert.Equal(t, "https://github.com/OWNER/REPO/pulls?q=is%3Apr+is%3Amerged+assignee%3Apeter+label%3Abug+label%3Adocs+base%3Atrunk", output.BrowsedURL) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 17b91ea81..ab3880e55 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -96,7 +96,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { repoResolvingCmdFactory := *f repoResolvingCmdFactory.BaseRepo = resolvedBaseRepo(f) - cmd.AddCommand(browseCmd.NewCmdBrowse(&repoResolvingCmdFactory)) // adds to the commands Commands() + cmd.AddCommand(browseCmd.NewCmdBrowse(&repoResolvingCmdFactory)) cmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) cmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) cmd.AddCommand(releaseCmd.NewCmdRelease(&repoResolvingCmdFactory))