diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index c28ad2d5d..87ab559a8 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -107,12 +107,12 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co func runBrowse(opts *BrowseOptions) error { baseRepo, err := opts.BaseRepo() if err != nil { - return fmt.Errorf("unable to determine base repository: %w\nUse 'gh browse --help' for more information about browse\n", err) + return fmt.Errorf("unable to determine base repository: %w\n", err) } httpClient, err := opts.HttpClient() if err != nil { - return fmt.Errorf("unable to create an http client: %w\nUse 'gh browse --help' for more information about browse\n", err) + return fmt.Errorf("unable to create an http client: %w\n", err) } url := ghrepo.GenerateRepoURL(baseRepo, "") @@ -133,7 +133,7 @@ func runBrowse(opts *BrowseOptions) error { if isNumber(opts.SelectorArg) { url += "/issues/" + opts.SelectorArg } else { - arr, err := parseFileArg(opts.SelectorArg) + fileArg, err := parseFileArg(opts.SelectorArg) if err != nil { return err } @@ -147,33 +147,28 @@ func runBrowse(opts *BrowseOptions) error { } url += "/tree/" + branchName + "/" } - if opts.SelectorArg != "" { - if len(arr) > 1 { - url += arr[0] + "#L" + arr[1] - } else { - url += arr[0] - } - } + url += fileArg } } - err = opts.Browser.Browse(url) - if opts.IO.IsStdoutTTY() && err == nil { + if opts.IO.IsStdoutTTY() { fmt.Fprintf(opts.IO.Out, "now opening %s in browser\n", url) } - - return err + return opts.Browser.Browse(url) } -func parseFileArg(fileArg string) ([]string, error) { +func parseFileArg(fileArg string) (string, error) { arr := strings.Split(fileArg, ":") if len(arr) > 2 { - return arr, fmt.Errorf("invalid use of colon\nUse 'gh browse --help' for more information about browse\n") + return "", fmt.Errorf("invalid use of colon\nUse 'gh browse --help' for more information about browse\n") } - if len(arr) > 1 && !isNumber(arr[1]) { - return arr, fmt.Errorf("invalid line number after colon\nUse 'gh browse --help' for more information about browse\n") + if len(arr) > 1 { + if !isNumber(arr[1]) { + return "", fmt.Errorf("invalid line number after colon\nUse 'gh browse --help' for more information about browse\n") + } + return arr[0] + "#L" + arr[1], nil } - return arr, nil + return arr[0], nil } func isNumber(arg string) bool { diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index e9bea0dfc..9faf8de10 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -271,27 +271,25 @@ func Test_runBrowse(t *testing.T) { } } -func TestFileArgParsing(t *testing.T) { +func Test_parseFileArg(t *testing.T) { tests := []struct { - name string - arg string - errorExpected bool - fileArg string - lineArg string - stderrExpected string + name string + arg string + errorExpected bool + expectedFileArg string + stderrExpected string }{ { - name: "non line number", - arg: "main.go", - errorExpected: false, - fileArg: "main.go", + name: "non line number", + arg: "main.go", + errorExpected: false, + expectedFileArg: "main.go", }, { - name: "line number", - arg: "main.go:32", - errorExpected: false, - fileArg: "main.go", - lineArg: "32", + name: "line number", + arg: "main.go:32", + errorExpected: false, + expectedFileArg: "main.go#L32", }, { name: "non line number error", @@ -301,15 +299,12 @@ func TestFileArgParsing(t *testing.T) { }, } for _, tt := range tests { - arr, err := parseFileArg(tt.arg) + fileArg, err := 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]) + assert.Equal(t, tt.expectedFileArg, fileArg) } } }