Fixed parsefileargs to throw errors with incorrect line numbers

jlsestak · jlsestak · commit 1657cf46c192 · 2021-06-08T21:23:50.000-07:00
diff --git 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,19 +146,22 @@ 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 {
 			url += "/tree/" + opts.Branch + "/" + arr[0]
 		}
 	}
 	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,29 +176,34 @@ 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 {
 			url += "/tree/" + branchName + "/" + arr[0]
 		}
 		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 {
