diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index cc2f866be..ea133d3c9 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -105,16 +105,15 @@ 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, 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) @@ -129,9 +128,15 @@ func runBrowse(opts *BrowseOptions) error { hasFlag := opts.FlagAmount > 0 if opts.Branch != "" { - repoUrl = addBranch(opts, repoUrl) - } else if hasArg && !hasFlag || opts.RepoFlag { - repoUrl = addArg(opts, repoUrl, branchName) + 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 { @@ -141,11 +146,14 @@ func runBrowse(opts *BrowseOptions) error { return nil } -func addBranch(opts *BrowseOptions, url string) string { +func addBranch(opts *BrowseOptions, url string) (error, string) { if opts.SelectorArg == "" { url += "/tree/" + opts.Branch } else { - arr := parseFileArg(opts) + err, arr := parseFileArg(opts) + if err != nil { + return err, url + } if len(arr) > 1 { url += "/tree/" + opts.Branch + "/" + arr[0] + "#L" + arr[1] } else { @@ -153,7 +161,7 @@ func addBranch(opts *BrowseOptions, url string) string { } } fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) - return url + return nil, url } func addFlag(opts *BrowseOptions, url string) string { @@ -168,15 +176,15 @@ func addFlag(opts *BrowseOptions, url string) string { 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") } else { - arr := parseFileArg(opts) - // if err != nil { - - // } + err, arr := parseFileArg(opts) + if err != nil { + return err, url + } if len(arr) > 1 { url += "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1] } else { @@ -184,13 +192,18 @@ func addArg(opts *BrowseOptions, url string, branchName string) string { } fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) } - return url + return nil, url } -func parseFileArg(opts *BrowseOptions) []string { - // TODO make sure the line number is the second arg, and that there are only two elements in arr +func parseFileArg(opts *BrowseOptions) (error, []string) { arr := strings.Split(opts.SelectorArg, ":") - return arr + 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("nvalid line number after colon\nUse 'gh browse --help' for more information about browse\n"), arr + } + return nil, arr } func isNumber(arg string) bool {