help: simplify implementation of help subsystem

Following changes are applied:
- the main help command is implemented in a way similar to the other
  commands (choice between internal and external variation of the
  command is handled with the common handler cmd.RunModuleFunc)
- help for every external command is tied to a command itself over
  cmd.SetHelp function rather than add separate help subcommand to
  the main help command (it allows to obtain help for internal and
  external commands uniformly)
- corresponding Cobra commands are created for all available external
  commands rather than only for the invoked one (being combined with
  the uniform handling mentioned above it allows to make implementation
  of the internal variation of help command trivial)
- external commands are also implemented over the common handler
  cmd.RunModuleFunc to reduce code duplication

Close #1136

Author: elhimov

cli/cmd/external.go

@@ -1,82 +1,69 @@
 package cmd
 
 import (
-	"strings"
-
-	"github.com/apex/log"
 	"github.com/spf13/cobra"
 	"github.com/tarantool/tt/cli/modules"
-	"github.com/tarantool/tt/cli/util"
+	"golang.org/x/exp/slices"
 )
 
 // configureExternalCmd configures external commands.
-func configureExternalCmd(rootCmd *cobra.Command,
-	modulesInfo *modules.ModulesInfo, forceInternal bool, args []string) {
+func configureExternalCmd(rootCmd *cobra.Command, modulesInfo *modules.ModulesInfo,
+	forceInternal bool) {
 	configureExistsCmd(rootCmd, modulesInfo, forceInternal)
-	configureNonExistentCmd(rootCmd, modulesInfo, args)
+	configureNonExistentCmd(rootCmd, modulesInfo)
+}
+
+// externalModuleHelpFunc returns function that displays help for the specified external module.
+func externalModuleHelpFunc(manifest modules.Manifest) func(*cobra.Command, []string) {
+	return func(cmd *cobra.Command, args []string) {
+		help, err := modules.GetExternalModuleHelp(manifest.Main)
+		if err != nil {
+			cmd.PrintErrf("failed to get help for module %q: %s\n", manifest.Name, err)
+			return
+		}
+		cmd.Print(help)
+	}
 }
 
 // configureExistsCmd configures an external commands
 // that have internal implementation.
 func configureExistsCmd(rootCmd *cobra.Command, modulesInfo *modules.ModulesInfo,
 	forceInternal bool) {
 	for _, cmd := range rootCmd.Commands() {
-		if _, found := (*modulesInfo)[cmd.CommandPath()]; found {
-			cmd.DisableFlagParsing = !forceInternal
+		if manifest, found := (*modulesInfo)[cmd.CommandPath()]; found && !forceInternal {
+			cmd.DisableFlagParsing = true
+			cmd.SetHelpFunc(externalModuleHelpFunc(manifest))
 		}
 	}
 }
 
 // configureNonExistentCmd configures an external command that
 // has no internal implementation within the Tarantool CLI.
-func configureNonExistentCmd(rootCmd *cobra.Command,
-	modulesInfo *modules.ModulesInfo, args []string) {
-	// Since the user can pass flags, to determine the name of
-	// an external command we have to take the first non-flag argument.
-	externalCmd := args[0]
-	for _, name := range args {
-		if !strings.HasPrefix(name, "-") && name != "help" {
-			externalCmd = name
-			break
-		}
-	}
-
+func configureNonExistentCmd(rootCmd *cobra.Command, modulesInfo *modules.ModulesInfo) {
 	// We avoid overwriting existing commands - we should add a command only
 	// if it doesn't have an internal implementation in Tarantool CLI.
+	// So first collect list of internal command names.
+	internalCmdNames := []string{"help"}
 	for _, cmd := range rootCmd.Commands() {
-		if cmd.Name() == externalCmd {
-			return
-		}
+		internalCmdNames = append(internalCmdNames, cmd.Name())
 	}
 
-	helpCmd := util.GetHelpCommand(rootCmd)
-	externalCmdPath := rootCmd.Name() + " " + externalCmd
-	if _, found := (*modulesInfo)[externalCmdPath]; found {
-		rootCmd.AddCommand(newExternalCommand(modulesInfo, externalCmd,
-			externalCmdPath, nil))
-		helpCmd.AddCommand(newExternalCommand(modulesInfo, externalCmd, externalCmdPath,
-			[]string{"--help"}))
+	// Add external command only if it doesn't have an internal implementation in Tarantool CLI.
+	for _, manifest := range *modulesInfo {
+		if !slices.Contains(internalCmdNames, manifest.Name) {
+			rootCmd.AddCommand(newExternalCmd(manifest))
+		}
 	}
 }
 
-// newExternalCommand returns a pointer to a new external
+// newExternalCmd returns a pointer to a new external
 // command that will call modules.RunCmd.
-func newExternalCommand(modulesInfo *modules.ModulesInfo,
-	cmdName, cmdPath string, addArgs []string) *cobra.Command {
-	cmd := &cobra.Command{
-		Use: cmdName,
-		Run: func(cmd *cobra.Command, args []string) {
-			if addArgs != nil {
-				args = append(args, addArgs...)
-			}
-
-			cmdCtx.Cli.ForceInternal = false
-			if err := modules.RunCmd(&cmdCtx, cmdPath, modulesInfo, nil, args); err != nil {
-				log.Fatalf(err.Error())
-			}
-		},
+func newExternalCmd(manifest modules.Manifest) *cobra.Command {
+	var cmd = &cobra.Command{
+		Use:                manifest.Name,
+		Run:                RunModuleFunc(nil),
+		DisableFlagParsing: true,
 	}
-
-	cmd.DisableFlagParsing = true
+	cmd.SetHelpFunc(externalModuleHelpFunc(manifest))
 	return cmd
 }

cli/cmd/help.go

@@ -2,80 +2,39 @@ package cmd
 
 import (
 	"fmt"
-	"os"
 	"strings"
 
-	"github.com/apex/log"
 	"github.com/spf13/cobra"
 	"github.com/tarantool/tt/cli/cmdcontext"
 	"github.com/tarantool/tt/cli/modules"
 	"github.com/tarantool/tt/cli/util"
 )
 
-// DefaultHelpFunc is a type of the standard built-in
-// cobra implementation of the help function.
-type DefaultHelpFunc func(*cobra.Command, []string)
-
-// configureHelpCommand configures our own help module
-// so that it can be external as well.
-// If the help is called for an external module
-// (for example `tt help version`), we try to get the help from it.
-func configureHelpCommand(cmdCtx *cmdcontext.CmdCtx, rootCmd *cobra.Command) error {
+func configureHelpCommand(rootCmd *cobra.Command, modulesInfo *modules.ModulesInfo) {
 	// Add information about external modules into help template.
-	rootCmd.SetUsageTemplate(fmt.Sprintf(usageTemplate, getExternalCommandsString(&modulesInfo)))
-	defaultHelp := rootCmd.HelpFunc()
-
-	rootCmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
-		cmdCtx.CommandName = cmd.Name()
-		if len(os.Args) == 1 || util.Find(args, "-h") != -1 || util.Find(args, "--help") != -1 {
-			defaultHelp(cmd, nil)
-			return
-		}
+	rootCmd.SetUsageTemplate(fmt.Sprintf(usageTemplate, getExternalCommandsString(modulesInfo)))
 
-		args = modules.GetDefaultCmdArgs("help")
-		err := modules.RunCmd(cmdCtx, "tt help", &modulesInfo,
-			getInternalHelpFunc(cmd, defaultHelp), args)
+	internalHelpModule := func(cmdCtx *cmdcontext.CmdCtx, args []string) error {
+		cmd, _, err := rootCmd.Find(args)
 		if err != nil {
-			log.Fatalf(err.Error())
+			return err
 		}
-	})
-
-	// Add valid arguments for completion.
-	helpCmd := util.GetHelpCommand(rootCmd)
-	for name := range modulesInfo {
-		helpCmd.ValidArgs = append(helpCmd.ValidArgs, name)
+		cmd.Help()
+		return nil
 	}
 
-	return nil
-}
-
-// getInternalHelpFunc returns a internal implementation of help module.
-func getInternalHelpFunc(cmd *cobra.Command, help DefaultHelpFunc) modules.InternalFunc {
-	return func(cmdCtx *cmdcontext.CmdCtx, args []string) error {
-		switch manifest, found := modulesInfo[cmd.CommandPath()]; {
-		// Cases when we have to run the "default" help:
-		// - `tt help` and no external help module.
-		// 	It looks strange: if we type the command `tt help`,
-		// 	the call to cmd.Name() returns `tt` and I don’t know
-		// 	what is the reason. If there is an external help module,
-		// 	then we also cannot be here (see the code below) and it
-		// 	is enough to check cmd.Name() == "tt".
-		// - `tt help -I`
-		// - `tt --help`, `tt -h` or `tt` (look code above).
-		case cmd.Name() == "tt", !found:
-			help(cmd, nil)
-		// We make a call to the external module (if it exists) with the `--help` flag.
-		default:
-			helpMsg, err := modules.GetExternalModuleHelp(manifest.Main)
-			if err != nil {
-				return err
-			}
-
-			cmd.Print(helpMsg)
-		}
+	helpCmd := &cobra.Command{
+		Use:   "help [command]",
+		Short: "Help about any command",
+		Run:   RunModuleFunc(internalHelpModule),
+	}
 
-		return nil
+	// Add valid arguments for completion.
+	for _, subCmd := range rootCmd.Commands() {
+		helpCmd.ValidArgs = append(helpCmd.ValidArgs, subCmd.Name())
 	}
+
+	rootCmd.SetHelpCommand(helpCmd)
 }
 
 // getExternalCommandsString returns a pretty string

cli/cmd/root.go

@@ -228,7 +228,6 @@ After that tt will be able to manage the application using 'replicaset_example'
 
 	// Setup decoration for Command's error messages.
 	rootCmd.SetErr(&logErrorWriterDecorator{rootCmd.ErrOrStderr(), logHandler})
-	rootCmd.InitDefaultHelpCmd()
 
 	return rootCmd
 }
@@ -314,13 +313,8 @@ func InitRoot() {
 
 	// External commands must be configured in a special way.
 	// This is necessary, for example, so that we can pass arguments to these commands.
-	if len(os.Args) > 1 {
-		configureExternalCmd(rootCmd, &modulesInfo, cmdCtx.Cli.ForceInternal, os.Args[1:])
-	}
+	configureExternalCmd(rootCmd, &modulesInfo, cmdCtx.Cli.ForceInternal)
 
 	// Configure help command.
-	err = configureHelpCommand(&cmdCtx, rootCmd)
-	if err != nil {
-		log.Fatalf("Failed to set up help command: %s", err)
-	}
+	configureHelpCommand(rootCmd, &modulesInfo)
 }

cli/modules/run.go

@@ -30,11 +30,8 @@ func RunCmd(cmdCtx *cmdcontext.CmdCtx, cmdPath string, modulesInfo *ModulesInfo,
 	internal InternalFunc, args []string,
 ) error {
 	manifest, found := (*modulesInfo)[cmdPath]
-	if !found || cmdCtx.Cli.ForceInternal {
-		if internal != nil {
-			return internal(cmdCtx, args)
-		}
-		return fmt.Errorf("no internal command for %q to run", cmdPath)
+	if !found || (cmdCtx.Cli.ForceInternal && internal != nil) {
+		return internal(cmdCtx, args)
 	}
 
 	f, err := cmdCtx.Integrity.Repository.Read(manifest.Main)

cli/util/util.go

@@ -155,17 +155,6 @@ func ParseYAML(path string) (map[string]interface{}, error) {
 	return raw, nil
 }
 
-// GetHelpCommand returns the help command for the passed cmd argument.
-func GetHelpCommand(cmd *cobra.Command) *cobra.Command {
-	for _, subcmd := range cmd.Commands() {
-		if subcmd.Name() == "help" {
-			return subcmd
-		}
-	}
-
-	return nil
-}
-
 // GetHomeDir returns current home directory.
 func GetHomeDir() (string, error) {
 	usr, err := user.Current()