Deduplicated usageFunc loop, re-added flag parse check, clarified comments

Author: marcleblanc2

cmd/src/cmd.go

@@ -45,18 +45,19 @@ type commander []*command
 // Run the command
 func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []string) {
 
-	// Check if --help args are anywhere in the command; if yes, then
-	// remove it from the list of args at this point to avoid interrupting recursive function calls,
-	// and append it to the deepest command / subcommand
+	// Check if --help args are anywhere in the command
+	// If yes, then remove it from the list of args at this point,
+	// then append it to the deepest command / subcommand, later,
+	// to avoid outputting usage text for a commander when a subcommand is specified
 	filteredArgs := make([]string, 0, len(args))
 	helpRequested := false
 
 	helpFlags := []string{
-		"--h",
+		"help",
+		"-help",
 		"--help",
 		"-h",
-		"-help",
-		"help",
+		"--h",
 	}
 
 	for _, arg := range args {
@@ -67,44 +68,19 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []
 		}
 	}
 
-	// Define the usage function from usageText
+	// Define the usage function for the commander
 	flagSet.Usage = func() {
 		_, _ = fmt.Fprint(flag.CommandLine.Output(), usageText)
 	}
 
-	// Parse the command's flags, if not already parsed
+	// Parse the commander's flags, if not already parsed
 	if !flagSet.Parsed() {
 		_ = flagSet.Parse(filteredArgs)
 	}
 
-	// If no subcommands remain (or help requested with no subcommands), print usage
-	if flagSet.NArg() == 0 {
-		flagSet.SetOutput(os.Stdout)
-		flagSet.Usage()
-		os.Exit(0)
-	}
-
-	// Configure default usage funcs for all commands.
-	for _, cmd := range c {
-		cmd := cmd
-
-		// If the command / subcommand has defined its own usageFunc, then use it
-		if cmd.usageFunc != nil {
-			cmd.flagSet.Usage = cmd.usageFunc
-			continue
-		}
-
-		// If the command / subcommand has not defined its own usageFunc,
-		// then generate a basic default usageFunc,
-		// using the command's defined flagSet and their defaults
-		cmd.flagSet.Usage = func() {
-			_, _ = fmt.Fprintf(flag.CommandLine.Output(), "Usage of '%s %s':\n", cmdName, cmd.flagSet.Name())
-			cmd.flagSet.PrintDefaults()
-		}
-	}
-
 	// Find the subcommand to execute
-	// Assume the subcommand is the first arg in the flagSet
+	// This assumes the subcommand is the first arg in the flagSet,
+	// i.e. any global args have been removed from the flagSet
 	name := flagSet.Arg(0)
 
 	// Loop through the list of all registered subcommands
@@ -116,57 +92,74 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []
 		}
 		// If the first arg is this registered commmand in the loop, then try and run it, then exit
 
+		// Set up the usage function for this subcommand
+		// If the subcommand has a usageFunc defined, then use it
+		if cmd.usageFunc != nil {
+			cmd.flagSet.Usage = cmd.usageFunc
+			} else {
+			// If the subcommand does not have a usageFunc defined,
+			// then define a simple default one,
+			// using the list of flags defined in the subcommand, and their description strings
+			cmd.flagSet.Usage = func() {
+				_, _ = fmt.Fprintf(flag.CommandLine.Output(), "Usage of '%s %s':\n", cmdName, cmd.flagSet.Name())
+				cmd.flagSet.PrintDefaults()
+			}
+		}
+
 		// Read global configuration
 		var err error
 		cfg, err = readConfig()
 		if err != nil {
 			log.Fatal("reading config: ", err)
 		}
 
-		// Get the remaining args, to pass to the subcommand, as an unparsed array of previously parsed args
+		// Get the remainder of the args, excluding the first arg / this command name
 		args := flagSet.Args()[1:]
 
-		// Set output to stdout for help (flag package defaults to stderr)
+		// Set output to stdout, for usage / helper text printed for the --help flag (flag package defaults to stderr)
 		cmd.flagSet.SetOutput(os.Stdout)
 		flag.CommandLine.SetOutput(os.Stdout)
 
-		// Note: We can't parse flags here because commanders need to pass unparsed args to subcommand handlers
-		// Each handler is responsible for parsing its own flags
-		// All commands must use `flagSet := flag.NewFlagSet("<name>", flag.ExitOnError)` to ensure usage helper text is printed automatically on arg parse errors
-		// Parse the subcommand's args, on its behalf, to test if flag.ExitOnError is not set
-		// if err := cmd.flagSet.Parse(args); err != nil {
-		// 	fmt.Printf("Error parsing subcommand flags: %s\n", err)
-		// 	panic(fmt.Sprintf("all registered commands should use flag.ExitOnError: error: %s", err))
-		// }
-
 		// If the --help arg was provided, re-add it here for the lowest command to parse and action
 		if helpRequested {
 			args = append(args, "-h")
 		}
 
+		// Parse the subcommand's args, on its behalf, to test and ensure flag.ExitOnError is set
+		// just in case any future authors of subcommands forget to set flag.ExitOnError
+		if err := cmd.flagSet.Parse(args); err != nil {
+			fmt.Printf("Error parsing subcommand flags: %s\n", err)
+			panic(fmt.Sprintf("all registered commands should use flag.ExitOnError: error: %s", err))
+		}
+
 		// Execute the subcommand
+		// Handle any errors returned
 		if err := cmd.handler(args); err != nil {
 
-			// If the subcommand returns a UsageError, then print the error and usage helper text
+			// If the returned error is of type UsageError
 			if _, ok := err.(*cmderrors.UsageError); ok {
+				// then print the error and usage helper text, both to stderr
 				log.Printf("error: %s\n\n", err)
 				cmd.flagSet.SetOutput(os.Stderr)
 				flag.CommandLine.SetOutput(os.Stderr)
 				cmd.flagSet.Usage()
 				os.Exit(2)
 			}
 
-			// If the subcommand returns any other error, then print the error
+			// If the returned error is of type ExitCodeError
 			if e, ok := err.(*cmderrors.ExitCodeError); ok {
+				// Then log the error and exit with the exit code
 				if e.HasError() {
 					log.Println(e)
 				}
 				os.Exit(e.Code())
 			}
+
+			// For all other types of errors, log them as fatal, and exit
 			log.Fatal(err)
 		}
 
-		// If no error was returned, then exit the application
+		// If no error was returned, then just exit the application cleanly
 		os.Exit(0)
 	}