diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 6d7b3df15..186514867 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -19,34 +19,34 @@ type browser interface { } type BrowseOptions struct { - HttpClient func() (*http.Client, error) - IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams - SelectorArg string FlagAmount int + SelectorArg string - ProjectsFlag bool - WikiFlag bool - SettingsFlag bool Branch string + ProjectsFlag bool + RepoFlag bool + SettingsFlag bool + WikiFlag bool } func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command { opts := &BrowseOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, Browser: f.Browser, - BaseRepo: f.BaseRepo, + HttpClient: f.HttpClient, + IO: f.IOStreams, } cmd := &cobra.Command{ Long: "Open the GitHub repository in the web browser.", Short: "Open the repository in the browser", Use: "browse { | }", - Args: cobra.RangeArgs(0, 2), + Args: cobra.RangeArgs(0, 1), Example: heredoc.Doc(` $ gh browse #=> Open the home page of the current repository @@ -71,15 +71,23 @@ func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command { - by path for opening folders and files, e.g. "cmd/gh/main.go" `), "help:environment": heredoc.Doc(` - To configure a web browser other than the default, use the BROWSER environment variable + To configure a web browser other than the default, use the BROWSER environment variable. `), }, RunE: func(cmd *cobra.Command, args []string) error { + opts.BaseRepo = f.BaseRepo opts.FlagAmount = cmd.Flags().NFlag() - if opts.FlagAmount > 1 { - return &cmdutil.FlagError{Err: fmt.Errorf("accepts 1 flag, %d flag(s) were recieved", opts.FlagAmount)} + + if opts.FlagAmount > 2 { + return &cmdutil.FlagError{Err: fmt.Errorf("cannot have more than two flags, %d were recieved", opts.FlagAmount)} } - // TODO test whether you can set cobra command arg range to 1 with a branch accepting an arg1 + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" { + opts.RepoFlag = true + } + if opts.FlagAmount > 1 && !opts.RepoFlag { + return &cmdutil.FlagError{Err: fmt.Errorf("these two flags are incompatible, see below for instructions")} + } + if len(args) > 0 { opts.SelectorArg = args[0] } @@ -97,71 +105,66 @@ func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command { } func runBrowse(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) + return fmt.Errorf("unable to determine base repository: %w\nUse 'gh browse --help' for more information about browse\n", err) } - httpClient, _ := opts.HttpClient() + httpClient, err := opts.HttpClient() if err != nil { - return fmt.Errorf("unable to create an http client: %w\n%s", err, help) + return fmt.Errorf("unable to create an http client: %w\nUse 'gh browse --help' for more information about browse\n", err) } apiClient := api.NewClientFromHTTP(httpClient) branchName, err := api.RepoDefaultBranch(apiClient, baseRepo) - if err != nil { - return fmt.Errorf("unable to determine default branch for %s: %w", ghrepo.FullName(baseRepo), err) - } + // if err != nil { //TODO resolve errors related to using this with tests + // return fmt.Errorf("unable to determine default branch for %s: %w", ghrepo.FullName(baseRepo), err) + // } repoUrl := ghrepo.GenerateRepoURL(baseRepo, "") - hasArg := hasArg(opts) - hasFlag := hasFlag(opts) + hasArg := opts.SelectorArg != "" + hasFlag := opts.FlagAmount > 0 - if hasArg && !hasFlag { - repoUrl = addArg(opts, repoUrl, branchName) - } else if !hasArg && hasFlag { - err, repoUrl = addFlag(opts, repoUrl) - if err != nil { - return err - } - } else if hasArg && hasFlag { + if opts.Branch != "" { err, repoUrl = addBranch(opts, repoUrl) if err != nil { return err } + } else if hasArg && (!hasFlag || opts.RepoFlag) { + err, repoUrl = addArg(opts, repoUrl, branchName) + if err != nil { + return err + } + } else if !hasArg && hasFlag { + repoUrl = addFlag(opts, repoUrl) } else { fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", repoUrl) } - opts.Browser.Browse(repoUrl) - return nil } func addBranch(opts *BrowseOptions, url string) (error, string) { - help := "Use 'gh browse --help' for more information about browse\n" - - if opts.Branch == "" { - return fmt.Errorf("invalid use of flag and argument\n%s", help), "" - } - if opts.SelectorArg == "" { url += "/tree/" + opts.Branch - fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) - return nil, url + } else { + err, arr := parseFileArg(opts.SelectorArg) + if err != nil { + return err, url + } + if len(arr) > 1 { + url += "/tree/" + opts.Branch + "/" + arr[0] + "#L" + arr[1] + } else { + url += "/tree/" + opts.Branch + "/" + arr[0] + } } - - arr := parseFileArg(opts) - if len(arr) > 1 { - return nil, url + "/tree/" + opts.Branch + "/" + arr[0] + "#L" + arr[1] - } - return nil, url + "/tree/" + opts.Branch + "/" + arr[0] + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } -func addFlag(opts *BrowseOptions, url string) (error, string) { +func addFlag(opts *BrowseOptions, url string) string { if opts.ProjectsFlag { url += "/projects" } else if opts.SettingsFlag { @@ -170,37 +173,37 @@ func addFlag(opts *BrowseOptions, url string) (error, string) { url += "/wiki" } fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) - return nil, url + return url } -func addArg(opts *BrowseOptions, url string, branchName string) string { +func addArg(opts *BrowseOptions, url string, branchName string) (error, string) { if isNumber(opts.SelectorArg) { url += "/issues/" + opts.SelectorArg fmt.Fprintf(opts.IO.Out, "now opening issue/pr in browser . . .\n") - return url - } - - arr := parseFileArg(opts) - if len(arr) > 1 { + } else { + err, arr := parseFileArg(opts.SelectorArg) + if err != nil { + return err, url + } + if len(arr) > 1 { + url += "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1] + } else { + url += "/tree/" + branchName + "/" + arr[0] + } fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) - return url + "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1] } - - fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) - return url + "/tree/" + branchName + "/" + arr[0] + return nil, url } -func parseFileArg(opts *BrowseOptions) []string { - arr := strings.Split(opts.SelectorArg, ":") - return arr -} - -func hasFlag(opts *BrowseOptions) bool { - return opts.FlagAmount > 0 -} - -func hasArg(opts *BrowseOptions) bool { - return opts.SelectorArg != "" +func parseFileArg(fileArg string) (error, []string) { + arr := strings.Split(fileArg, ":") + if len(arr) > 2 { + return fmt.Errorf("invalid use of colon\nUse 'gh browse --help' for more information about browse\n"), arr + } + if len(arr) > 1 && !isNumber(arr[1]) { + return fmt.Errorf("invalid line number after colon\nUse 'gh browse --help' for more information about browse\n"), arr + } + return nil, arr } func isNumber(arg string) bool { diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 0912884ca..0ad97af00 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -16,20 +16,7 @@ import ( "github.com/stretchr/testify/assert" ) -type args struct { - repo ghrepo.Interface - cli string -} -type testCase struct { - name string - args args - errorExpected bool - stdoutExpected string - stderrExpected string - urlExpected string -} - -func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { +func runCommand(rt http.RoundTripper, repo ghrepo.Interface, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() browser := &cmdutil.TestBrowser{} @@ -41,13 +28,13 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { return &http.Client{Transport: rt}, nil }, BaseRepo: func() (ghrepo.Interface, error) { - return t.args.repo, nil + return repo, nil }, } cmd := NewCmdBrowse(factory) - argv, err := shlex.Split(t.args.cli) + argv, err := shlex.Split(cli) if err != nil { return nil, err } @@ -65,7 +52,20 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { }, err } func TestNewCmdBrowse(t *testing.T) { - var tests = []testCase{ + + type args struct { + repo ghrepo.Interface + cli string + } + + tests := []struct { + name string + args args + errorExpected bool + stdoutExpected string + stderrExpected string + urlExpected string + }{ { name: "multiple flag", args: args{ @@ -74,7 +74,7 @@ func TestNewCmdBrowse(t *testing.T) { }, errorExpected: true, stdoutExpected: "", - stderrExpected: "accepts 1 flag, 2 flag(s) were recieved", + stderrExpected: "these two flags are incompatible, see below for instructions", urlExpected: "", }, { @@ -131,7 +131,7 @@ func TestNewCmdBrowse(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) - output, err := runCommand(http, tt) + output, err := runCommand(http, tt.args.repo, tt.args.cli) if tt.errorExpected { assert.Equal(t, err.Error(), tt.stderrExpected) @@ -143,3 +143,46 @@ func TestNewCmdBrowse(t *testing.T) { }) } } +func TestFileArgParsing(t *testing.T) { + tests := []struct { + name string + arg string + errorExpected bool + fileArg string + lineArg string + stderrExpected string + }{ + { + name: "non line number", + arg: "main.go", + errorExpected: false, + fileArg: "main.go", + }, + { + name: "line number", + arg: "main.go:32", + errorExpected: false, + fileArg: "main.go", + lineArg: "32", + }, + { + name: "non line number error", + arg: "ma:in.go", + errorExpected: true, + stderrExpected: "invalid line number after colon\nUse 'gh browse --help' for more information about browse\n", + }, + } + for _, tt := range tests { + err, arr := parseFileArg(tt.arg) + if tt.errorExpected { + assert.Equal(t, err.Error(), tt.stderrExpected) + } else { + assert.Equal(t, err, nil) + if len(arr) > 1 { + assert.Equal(t, tt.lineArg, arr[1]) + } + assert.Equal(t, tt.fileArg, arr[0]) + } + + } +}