Fix: Handling of --help arg for sub commands

cmd/src/cmd.go

@@ -42,86 +42,147 @@ func (c *command) matches(name string) bool {
 // commander represents a top-level command with subcommands.
 type commander []*command
 
-// run runs the command.
+// Run the command
 func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []string) {
-	// Parse flags.
-	flagSet.Usage = func() {
-		_, _ = fmt.Fprint(flag.CommandLine.Output(), usageText)
+
+	// NOTE: This function is quite brittle
+	// Especially with printing helper text at all 3 different levels of depth
+
+	// 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{
+		"help",
+		"-help",
+		"--help",
+		"-h",
+		"--h",
 	}
-	if !flagSet.Parsed() {
-		_ = flagSet.Parse(args)
+
+	for _, arg := range args {
+		if slices.Contains(helpFlags, arg) {
+			helpRequested = true
+		} else {
+			filteredArgs = append(filteredArgs, arg)
+		}
 	}
 
-	// Print usage if the command is "help".
-	if flagSet.Arg(0) == "help" || flagSet.NArg() == 0 {
-		flagSet.SetOutput(os.Stdout)
-		flagSet.Usage()
-		os.Exit(0)
+	// Define the usage function for the commander
+	flagSet.Usage = func() {
+		_, _ = fmt.Fprint(flag.CommandLine.Output(), usageText)
 	}
 
-	// Configure default usage funcs for commands.
-	for _, cmd := range c {
-		cmd := cmd
-		if cmd.usageFunc != nil {
-			cmd.flagSet.Usage = cmd.usageFunc
-			continue
-		}
-		cmd.flagSet.Usage = func() {
-			_, _ = fmt.Fprintf(flag.CommandLine.Output(), "Usage of '%s %s':\n", cmdName, cmd.flagSet.Name())
-			cmd.flagSet.PrintDefaults()
-		}
+	// Parse the commander's flags, if not already parsed
+	if !flagSet.Parsed() {
+		_ = flagSet.Parse(filteredArgs)
 	}
 
-	// Find the subcommand to execute.
+	// Find the subcommand to execute
+	// 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
 	for _, cmd := range c {
+
+		// If the first arg is not this registered commmand in the loop, try the next registered command
 		if !cmd.matches(name) {
 			continue
 		}
+		// If the first arg is this registered commmand in the loop, then try and run it, then exit
 
-		// Read global configuration now.
+		// Set up the usage function for this subcommand
+		if cmd.usageFunc != nil {
+			// If the subcommand has a usageFunc defined, then use it
+			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)
 		}
 
-		// Print help to stdout if requested
-		if slices.IndexFunc(args, func(s string) bool {
-			return s == "--help"
-		}) >= 0 {
-			cmd.flagSet.SetOutput(os.Stdout)
-			flag.CommandLine.SetOutput(os.Stdout)
-			cmd.flagSet.Usage()
-			os.Exit(0)
+		// Get the remainder of the args, excluding the first arg / this command name
+		args := flagSet.Args()[1:]
+
+		// 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)
+
+		// 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 subcommand flags.
-		args := flagSet.Args()[1:]
+		// 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.
-		if err := cmd.handler(flagSet.Args()[1:]); err != nil {
+		// Execute the subcommand
+		// Handle any errors returned
+		if err := cmd.handler(args); err != nil {
+
+			// 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 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 just exit the application cleanly
 		os.Exit(0)
 	}
-	log.Printf("%s: unknown subcommand %q", cmdName, name)
-	log.Fatalf("Run '%s help' for usage.", cmdName)
+
+	// To make it after the big loop, that means name didn't match any registered commands
+	if name != "" {
+		log.Printf("%s: unknown command %q", cmdName, name)
+		flagSet.Usage()
+		os.Exit(2)
+	}
+
+	// Special case to handle --help usage text for src command
+	if helpRequested {
+		// Set output to stdout, for usage / helper text printed for the --help flag (flag package defaults to stderr)
+		flagSet.SetOutput(os.Stdout)
+		flagSet.Usage()
+		os.Exit(0)
+	}
+
+	// Special case to handle src command with no args
+	flagSet.Usage()
+	os.Exit(2)
+
 }