Improve CLI and messages (#26341)

Follow the CLI refactoring 1. Remove the "checkCommandFlags" helper 2. Unify the web startup message, make them have consistent names as `./gitea help` 3. Fine tune some other messages (see the diff)
go-gitea · Aug 5, 2023 · 4f51347 · 4f51347
1 parent ecb04cc
commit 4f51347
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 52 deletions.
diff --git a/cmd/main.go b/cmd/main.go
@@ -6,8 +6,6 @@ package cmd
 import (
 	"fmt"
 	"os"
-	"reflect"
-	"strings"
 
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -58,7 +56,6 @@ func appGlobalFlags() []cli.Flag {
 	return []cli.Flag{
 		// make the builtin flags at the top
 		helpFlag,
-		cli.VersionFlag,
 
 		// shared configuration flags, they are for global and for each sub-command at the same time
 		// eg: such command is valid: "./gitea --config /tmp/app.ini web --config /tmp/app.ini", while it's discouraged indeed
@@ -120,36 +117,6 @@ func prepareWorkPathAndCustomConf(action cli.ActionFunc) func(ctx *cli.Context)
 	}
 }
 
-func reflectGet(v any, fieldName string) any {
-	e := reflect.ValueOf(v).Elem()
-	return e.FieldByName(fieldName).Interface()
-}
-
-// https://cli.urfave.org/migrate-v1-to-v2/#flag-aliases-are-done-differently
-// Sadly v2 doesn't warn you if a comma is in the name. (https://github.com/urfave/cli/issues/1103)
-func checkCommandFlags(c any) bool {
-	var cmds []*cli.Command
-	if app, ok := c.(*cli.App); ok {
-		cmds = app.Commands
-	} else {
-		cmds = c.(*cli.Command).Subcommands
-	}
-	ok := true
-	for _, cmd := range cmds {
-		for _, flag := range cmd.Flags {
-			flagName := reflectGet(flag, "Name").(string)
-			if strings.Contains(flagName, ",") {
-				ok = false
-				log.Error("cli.Flag can't have comma in its Name: %q, use Aliases instead", flagName)
-			}
-		}
-		if !checkCommandFlags(cmd) {
-			ok = false
-		}
-	}
-	return ok
-}
-
 func NewMainApp() *cli.App {
 	app := cli.NewApp()
 	app.EnableBashCompletion = true
@@ -187,6 +154,7 @@ func NewMainApp() *cli.App {
 	app.DefaultCommand = CmdWeb.Name
 
 	globalFlags := appGlobalFlags()
+	app.Flags = append(app.Flags, cli.VersionFlag)
 	app.Flags = append(app.Flags, globalFlags...)
 	app.HideHelp = true // use our own help action to show helps (with more information like default config)
 	app.Before = PrepareConsoleLoggerLevel(log.INFO)
@@ -196,8 +164,5 @@ func NewMainApp() *cli.App {
 	app.Commands = append(app.Commands, subCmdWithConfig...)
 	app.Commands = append(app.Commands, subCmdStandalone...)
 
-	if !checkCommandFlags(app) {
-		panic("some flags are incorrect") // this is a runtime check to help developers
-	}
 	return app
 }
diff --git a/cmd/web.go b/cmd/web.go
@@ -107,13 +107,18 @@ func createPIDFile(pidPath string) {
 	}
 }
 
-func serveInstall(ctx *cli.Context) error {
+func showWebStartupMessage(msg string) {
 	log.Info("Gitea version: %s%s", setting.AppVer, setting.AppBuiltWith)
-	log.Info("App path: %s", setting.AppPath)
-	log.Info("Work path: %s", setting.AppWorkPath)
-	log.Info("Custom path: %s", setting.CustomPath)
-	log.Info("Config file: %s", setting.CustomConf)
-	log.Info("Prepare to run install page")
+	log.Info("* RunMode: %s", setting.RunMode)
+	log.Info("* AppPath: %s", setting.AppPath)
+	log.Info("* WorkPath: %s", setting.AppWorkPath)
+	log.Info("* CustomPath: %s", setting.CustomPath)
+	log.Info("* ConfigFile: %s", setting.CustomConf)
+	log.Info("%s", msg)
+}
+
+func serveInstall(ctx *cli.Context) error {
+	showWebStartupMessage("Prepare to run install page")
 
 	routers.InitWebInstallPage(graceful.GetManager().HammerContext())
 
@@ -150,29 +155,24 @@ func serveInstalled(ctx *cli.Context) error {
 	setting.LoadCommonSettings()
 	setting.MustInstalled()
 
-	log.Info("Gitea version: %s%s", setting.AppVer, setting.AppBuiltWith)
-	log.Info("App path: %s", setting.AppPath)
-	log.Info("Work path: %s", setting.AppWorkPath)
-	log.Info("Custom path: %s", setting.CustomPath)
-	log.Info("Config file: %s", setting.CustomConf)
-	log.Info("Run mode: %s", setting.RunMode)
-	log.Info("Prepare to run web server")
+	showWebStartupMessage("Prepare to run web server")
 
 	if setting.AppWorkPathMismatch {
 		log.Error("WORK_PATH from config %q doesn't match other paths from environment variables or command arguments. "+
-			"Only WORK_PATH in config should be set and used. Please remove the other outdated work paths from environment variables and command arguments", setting.CustomConf)
+			"Only WORK_PATH in config should be set and used. Please make sure the path in config file is correct, "+
+			"remove the other outdated work paths from environment variables and command arguments", setting.CustomConf)
 	}
 
 	rootCfg := setting.CfgProvider
 	if rootCfg.Section("").Key("WORK_PATH").String() == "" {
 		saveCfg, err := rootCfg.PrepareSaving()
 		if err != nil {
-			log.Error("Unable to prepare saving WORK_PATH=%s to config %q: %v\nYou must set it manually, otherwise there might be bugs when accessing the git repositories.", setting.AppWorkPath, setting.CustomConf, err)
+			log.Error("Unable to prepare saving WORK_PATH=%s to config %q: %v\nYou should set it manually, otherwise there might be bugs when accessing the git repositories.", setting.AppWorkPath, setting.CustomConf, err)
 		} else {
 			rootCfg.Section("").Key("WORK_PATH").SetValue(setting.AppWorkPath)
 			saveCfg.Section("").Key("WORK_PATH").SetValue(setting.AppWorkPath)
 			if err = saveCfg.Save(); err != nil {
-				log.Error("Unable to update WORK_PATH=%s to config %q: %v\nYou must set it manually, otherwise there might be bugs when accessing the git repositories.", setting.AppWorkPath, setting.CustomConf, err)
+				log.Error("Unable to update WORK_PATH=%s to config %q: %v\nYou should set it manually, otherwise there might be bugs when accessing the git repositories.", setting.AppWorkPath, setting.CustomConf, err)
 			}
 		}
 	}