feat: simplify flag parsing in config plugin

Commands should be written such that they take arguments as is.

docs/appendices/0.21.0-migration-guide.md

@@ -7,6 +7,8 @@ The `tls` name is no longer a reserved app name, and can be used by applications
 ## Deprecations
 
 - `git#git_deploy_branch()` is deprecated in favor of `plugn trigger git-deploy-branch`.
+- The `config` command is deprecated in favor of `config:show`.
+  - Usage of this command in conjunction with either the `--export` or `--shell` flag is deprecated in favor of `config:export --format` with the correct format value (`exports` or `shell`, respectively).
 
 ## Removals
 

plugins/config/Makefile

@@ -1,4 +1,4 @@
-SUBCOMMANDS = subcommands/export subcommands/get subcommands/set subcommands/unset subcommands/keys subcommands/bundle
+SUBCOMMANDS = subcommands/bundle subcommands/export subcommands/get subcommands/keys subcommands/show subcommands/set subcommands/unset
 TRIGGERS = triggers/config-get triggers/config-get-global
 BUILD = commands subcommands triggers
 PLUGIN_NAME = config

plugins/config/config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"regexp"
@@ -129,6 +130,27 @@ func triggerUpdate(appName string, operation string, args []string) {
 	}
 }
 
+//getEnvironment for the given app (global config if appName is empty). Merge with global environment if merged is true.
+func getEnvironment(appName string, merged bool) (env *Env) {
+	var err error
+	if appName != "" && merged {
+		env, err = LoadMergedAppEnv(appName)
+	} else {
+		env, err = loadAppOrGlobalEnv(appName)
+	}
+	if err != nil {
+		common.LogFail(err.Error())
+	}
+	return env
+}
+
+func getAppNameOrGlobal(appName string, global bool) (string, error) {
+	if appName == "" && !global {
+		return appName, errors.New("Please specify an app to run the command on or --global")
+	}
+	return appName, nil
+}
+
 func loadAppOrGlobalEnv(appName string) (env *Env, err error) {
 	if appName == "" || appName == "--global" {
 		return LoadGlobalEnv()

plugins/config/src/commands/commands.go

@@ -21,11 +21,12 @@ Additional commands:`
 
 	helpContent = `
     config (<app>|--global), Pretty-print an app or global environment
-    config:bundle (<app>|--global) [--merged], Bundle environment into tarfile
+    config:bundle [--merged] (<app>|--global), Bundle environment into tarfile
-    config:clear (<app>|--global), Clears environment variables
+    config:clear [--no-restart] (<app>|--global), Clears environment variables
-    config:export (<app>|--global) [--envfile], Export a global or app environment
+    config:export [--format=FORMAT] [--merged] (<app>|--global), Export a global or app environment
-    config:get (<app>|--global) KEY, Display a global or app-specific config value
+    config:get [--quoted] (<app>|--global) KEY, Display a global or app-specific config value
-    config:keys (<app>|--global) [--merged], Show keys set in environment
+    config:keys [--merged] (<app>|--global), Show keys set in environment
+    config:show [--merged] (<app>|--global), Show keys set in environment
     config:set [--encoded] [--no-restart] (<app>|--global) KEY1=VALUE1 [KEY2=VALUE2 ...], Set one or more config vars
     config:unset [--no-restart] (<app>|--global) KEY1 [KEY2 ...], Unset one or more config vars
 `
@@ -37,14 +38,19 @@ func main() {
 
 	cmd := flag.Arg(0)
 	switch cmd {
-	case "config", "config:show":
+	case "config":
+		common.LogWarn("Deprecated: Use the 'config:show' command instead")
 		args := flag.NewFlagSet("config:show", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		shell := args.Bool("shell", false, "--shell: in a single-line for usage in command-line utilities [deprecated]")
 		export := args.Bool("export", false, "--export: print the env as eval-compatible exports [deprecated]")
 		merged := args.Bool("merged", false, "--merged: display the app's environment merged with the global environment")
 		args.Parse(os.Args[2:])
-		config.CommandShow(args.Args(), *global, *shell, *export, *merged)
+		appName := args.Arg(0)
+		err := config.CommandShow(appName, *global, *merged, *shell, *export)
+		if err != nil {
+			common.LogFail(err.Error())
+		}
 	case "config:help":
 		usage()
 	case "help":

plugins/config/src/subcommands/subcommands.go

@@ -10,58 +10,87 @@ import (
 	"github.com/dokku/dokku/plugins/config"
 )
 
+func getKeys(args []string, global bool) []string {
+	keys := args
+	if !global && len(keys) > 1 {
+		keys = keys[1:]
+	}
+	return keys
+}
+
 // main entrypoint to all subcommands
 func main() {
 	parts := strings.Split(os.Args[0], "/")
 	subcommand := parts[len(parts)-1]
 
+	var err error
 	switch subcommand {
 	case "bundle":
 		args := flag.NewFlagSet("config:bundle", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		merged := args.Bool("merged", false, "--merged: merge app environment and global environment")
 		args.Parse(os.Args[2:])
-		config.CommandBundle(args.Args(), *global, *merged)
+		appName := args.Arg(0)
+		err = config.CommandBundle(appName, *global, *merged)
 	case "clear":
 		args := flag.NewFlagSet("config:clear", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		noRestart := args.Bool("no-restart", false, "--no-restart: no restart")
 		args.Parse(os.Args[2:])
-		config.CommandClear(args.Args(), *global, *noRestart)
+		appName := args.Arg(0)
+		err = config.CommandClear(appName, *global, *noRestart)
 	case "export":
 		args := flag.NewFlagSet("config:export", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		merged := args.Bool("merged", false, "--merged: merge app environment and global environment")
 		format := args.String("format", "exports", "--format: [ exports | envfile | docker-args | shell | pretty | json | json-list ] which format to export as)")
 		args.Parse(os.Args[2:])
-		config.CommandExport(args.Args(), *global, *merged, *format)
+		appName := args.Arg(0)
+		err = config.CommandExport(appName, *global, *merged, *format)
 	case "get":
 		args := flag.NewFlagSet("config:get", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		quoted := args.Bool("quoted", false, "--quoted: get the value quoted")
 		args.Parse(os.Args[2:])
-		config.CommandGet(args.Args(), *global, *quoted)
+		appName := args.Arg(0)
+		keys := getKeys(args.Args(), *global)
+		err = config.CommandGet(appName, keys, *global, *quoted)
 	case "keys":
 		args := flag.NewFlagSet("config:keys", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		merged := args.Bool("merged", false, "--merged: merge app environment and global environment")
 		args.Parse(os.Args[2:])
-		config.CommandKeys(args.Args(), *global, *merged)
+		appName := args.Arg(0)
+		err = config.CommandKeys(appName, *global, *merged)
+	case "show":
+		args := flag.NewFlagSet("config:show", flag.ExitOnError)
+		global := args.Bool("global", false, "--global: use the global environment")
+		merged := args.Bool("merged", false, "--merged: display the app's environment merged with the global environment")
+		args.Parse(os.Args[2:])
+		appName := args.Arg(0)
+		err = config.CommandShow(appName, *global, *merged, false, false)
 	case "set":
 		args := flag.NewFlagSet("config:set", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		encoded := args.Bool("encoded", false, "--encoded: interpret VALUEs as base64")
 		noRestart := args.Bool("no-restart", false, "--no-restart: no restart")
 		args.Parse(os.Args[2:])
-		config.CommandSet(args.Args(), *global, *noRestart, *encoded)
+		appName := args.Arg(0)
+		pairs := getKeys(args.Args(), *global)
+		err = config.CommandSet(appName, pairs, *global, *noRestart, *encoded)
 	case "unset":
 		args := flag.NewFlagSet("config:unset", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: use the global environment")
 		noRestart := args.Bool("no-restart", false, "--no-restart: no restart")
 		args.Parse(os.Args[2:])
-		config.CommandUnset(args.Args(), *global, *noRestart)
+		appName := args.Arg(0)
+		keys := getKeys(args.Args(), *global)
+		err = config.CommandUnset(appName, keys, *global, *noRestart)
 	default:
 		common.LogFail(fmt.Sprintf("Invalid plugin subcommand call: %s", subcommand))
 	}
 
+	if err != nil {
+		common.LogFail(err.Error())
+	}
 }

plugins/config/subcommands.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -9,18 +10,154 @@ import (
 	"github.com/dokku/dokku/plugins/common"
 )
 
-//CommandShow implements config:show
+// CommandBundle implements config:bundle
-func CommandShow(args []string, global bool, shell bool, export bool, merged bool) {
+func CommandBundle(appName string, global bool, merged bool) error {
-	appName, _ := getCommonArgs(global, args)
+	appName, err := getAppNameOrGlobal(appName, global)
+	if err != nil {
+		return err
+	}
+
+	env := getEnvironment(appName, merged)
+	return env.ExportBundle(os.Stdout)
+}
+
+// CommandClear implements config:clear
+func CommandClear(appName string, global bool, noRestart bool) error {
+	appName, err := getAppNameOrGlobal(appName, global)
+	if err != nil {
+		return err
+	}
+
+	return UnsetAll(appName, !noRestart)
+}
+
+// CommandExport implements config:export
+func CommandExport(appName string, global bool, merged bool, format string) error {
+	appName, err := getAppNameOrGlobal(appName, global)
+	if err != nil {
+		return err
+	}
+
+	env := getEnvironment(appName, merged)
+	exportType := ExportFormatExports
+	suffix := "\n"
+
+	exportTypes := map[string]ExportFormat{
+		"exports":     ExportFormatExports,
+		"envfile":     ExportFormatEnvfile,
+		"docker-args": ExportFormatDockerArgs,
+		"shell":       ExportFormatShell,
+		"pretty":      ExportFormatPretty,
+		"json":        ExportFormatJSON,
+		"json-list":   ExportFormatJSONList,
+	}
+
+	exportType, ok := exportTypes[format]
+	if !ok {
+		return fmt.Errorf("Unknown export format: %v", format)
+	}
+
+	if exportType == ExportFormatShell {
+		suffix = " "
+	}
+
+	exported := env.Export(exportType)
+	fmt.Print(exported + suffix)
+	return nil
+}
+
+// CommandGet implements config:get
+func CommandGet(appName string, keys []string, global bool, quoted bool) error {
+	appName, err := getAppNameOrGlobal(appName, global)
+	if err != nil {
+		return err
+	}
+
+	if len(keys) != 1 {
+		return fmt.Errorf("Unexpected argument(s): %v", keys[1:])
+	}
+	if len(keys) == 0 {
+		return errors.New("Expected: key")
+	}
+
+	value, ok := Get(appName, keys[0])
+	if !ok {
+		return fmt.Errorf("No value for key %v", keys[0])
+	}
+
+	if quoted {
+		fmt.Printf("'%s'\n", singleQuoteEscape(value))
+	} else {
+		fmt.Printf("%s\n", value)
+	}
+
+	return nil
+}
+
+// CommandKeys implements config:keys
+func CommandKeys(appName string, global bool, merged bool) error {
+	appName, err := getAppNameOrGlobal(appName, global)
+	if err != nil {
+		return err
+	}
+
+	env := getEnvironment(appName, merged)
+	for _, k := range env.Keys() {
+		fmt.Println(k)
+	}
+	return nil
+}
+
+// CommandSet implements config:set
+func CommandSet(appName string, pairs []string, global bool, noRestart bool, encoded bool) error {
+	appName, err := getAppNameOrGlobal(appName, global)
+	if err != nil {
+		return err
+	}
+
+	if len(pairs) == 0 {
+		return errors.New("At least one env pair must be given")
+	}
+	updated := make(map[string]string)
+	for _, e := range pairs {
+		parts := strings.SplitN(e, "=", 2)
+		if len(parts) == 1 {
+			return fmt.Errorf("Invalid env pair: %v", e)
+		}
+
+		key, value := parts[0], parts[1]
+		if encoded {
+			decoded, err := base64.StdEncoding.DecodeString(value)
+			if err != nil {
+				return fmt.Errorf("%s for key '%s'", err.Error(), key)
+			}
+			value = string(decoded)
+		}
+		updated[key] = value
+	}
+
+	return SetMany(appName, updated, !noRestart)
+}
+
+// CommandShow implements config:show
+func CommandShow(appName string, global bool, merged bool, shell bool, export bool) error {
+	appName, err := getAppNameOrGlobal(appName, global)
+	if err != nil {
+		return err
+	}
+
 	env := getEnvironment(appName, merged)
 	if shell && export {
-		common.LogFail("Only one of --shell and --export can be given")
+		return errors.New("Only one of --shell and --export can be given")
 	}
 	if shell {
+		common.LogWarn("Deprecated: Use 'config:export --format shell' instead")
 		fmt.Print(env.Export(ExportFormatShell))
 	} else if export {
+		common.LogWarn("Deprecated: Use 'config:export --format exports' instead")
 		fmt.Println(env.Export(ExportFormatExports))
 	} else {
+		common.LogWarn("Deprecated: Use 'config:export --format pretty' instead")
 		contextName := "global"
 		if appName != "" {
 			contextName = appName
@@ -28,157 +165,20 @@ func CommandShow(args []string, global bool, shell bool, export bool, merged boo
 		common.LogInfo2Quiet(contextName + " env vars")
 		fmt.Println(env.Export(ExportFormatPretty))
 	}
+
+	return nil
 }
 
-//CommandGet implements config:get
+// CommandUnset implements config:unset
-func CommandGet(args []string, global bool, quoted bool) {
+func CommandUnset(appName string, keys []string, global bool, noRestart bool) error {
-	appName, keys := getCommonArgs(global, args)
+	appName, err := getAppNameOrGlobal(appName, global)
-	if len(keys) > 1 {
+	if err != nil {
-		common.LogFail(fmt.Sprintf("Unexpected argument(s): %v", keys[1:]))
+		return err
 	}
+
 	if len(keys) == 0 {
-		common.LogFail("Expected: key")
+		return fmt.Errorf("At least one key must be given")
 	}
-	if value, ok := Get(appName, keys[0]); !ok {
-		os.Exit(1)
-	} else {
-		if quoted {
-			fmt.Printf("'%s'\n", singleQuoteEscape(value))
-		} else {
-			fmt.Printf("%s\n", value)
-		}
-	}
-}
 
-//CommandClear implements config:clear
+	return UnsetMany(appName, keys, !noRestart)
-func CommandClear(args []string, global bool, noRestart bool) {
-	appName, _ := getCommonArgs(global, args)
-	err := UnsetAll(appName, !noRestart)
-	if err != nil {
-		common.LogFail(err.Error())
-	}
-}
-
-//CommandUnset implements config:unset
-func CommandUnset(args []string, global bool, noRestart bool) {
-	appName, keys := getCommonArgs(global, args)
-	if len(keys) == 0 {
-		common.LogFail("At least one key must be given")
-	}
-	err := UnsetMany(appName, keys, !noRestart)
-	if err != nil {
-		common.LogFail(err.Error())
-	}
-}
-
-//CommandSet implements config:set
-func CommandSet(args []string, global bool, noRestart bool, encoded bool) {
-	appName, pairs := getCommonArgs(global, args)
-	if len(pairs) == 0 {
-		common.LogFail("At least one env pair must be given")
-	}
-	updated := make(map[string]string)
-	for _, e := range pairs {
-		parts := strings.SplitN(e, "=", 2)
-		if len(parts) == 1 {
-			common.LogFail("Invalid env pair: " + e)
-		}
-		key, value := parts[0], parts[1]
-		if encoded {
-			decoded, err := base64.StdEncoding.DecodeString(value)
-			if err != nil {
-				common.LogFail(fmt.Sprintf("%s for key '%s'", err.Error(), key))
-			}
-			value = string(decoded)
-		}
-		updated[key] = value
-	}
-	err := SetMany(appName, updated, !noRestart)
-	if err != nil {
-		common.LogFail(err.Error())
-	}
-}
-
-//CommandKeys implements config:keys
-func CommandKeys(args []string, global bool, merged bool) {
-	appName, trailingArgs := getCommonArgs(global, args)
-	if len(trailingArgs) > 0 {
-		common.LogFail(fmt.Sprintf("Trailing argument(s): %v", trailingArgs))
-	}
-	env := getEnvironment(appName, merged)
-	for _, k := range env.Keys() {
-		fmt.Println(k)
-	}
-}
-
-//CommandExport implements config:export
-func CommandExport(args []string, global bool, merged bool, format string) {
-	appName, trailingArgs := getCommonArgs(global, args)
-	if len(trailingArgs) > 0 {
-		common.LogFail(fmt.Sprintf("Trailing argument(s): %v", trailingArgs))
-	}
-	env := getEnvironment(appName, merged)
-	exportType := ExportFormatExports
-	suffix := "\n"
-	switch format {
-	case "exports":
-		exportType = ExportFormatExports
-	case "envfile":
-		exportType = ExportFormatEnvfile
-	case "docker-args":
-		exportType = ExportFormatDockerArgs
-	case "shell":
-		exportType = ExportFormatShell
-		suffix = " "
-	case "pretty":
-		exportType = ExportFormatPretty
-	case "json":
-		exportType = ExportFormatJSON
-	case "json-list":
-		exportType = ExportFormatJSONList
-	default:
-		common.LogFail(fmt.Sprintf("Unknown export format: %v", format))
-	}
-	exported := env.Export(exportType)
-	fmt.Print(exported + suffix)
-}
-
-//CommandBundle implements config:bundle
-func CommandBundle(args []string, global bool, merged bool) {
-	appName, trailingArgs := getCommonArgs(global, args)
-	if len(trailingArgs) > 0 {
-		common.LogFail(fmt.Sprintf("Trailing argument(s): %v", trailingArgs))
-	}
-	env := getEnvironment(appName, merged)
-	env.ExportBundle(os.Stdout)
-}
-
-//getEnvironment for the given app (global config if appName is empty). Merge with global environment if merged is true.
-func getEnvironment(appName string, merged bool) (env *Env) {
-	var err error
-	if appName != "" && merged {
-		env, err = LoadMergedAppEnv(appName)
-	} else {
-		env, err = loadAppOrGlobalEnv(appName)
-	}
-	if err != nil {
-		common.LogFail(err.Error())
-	}
-	return env
-}
-
-//getCommonArgs extracts common positional args (appName and keys)
-func getCommonArgs(global bool, args []string) (appName string, keys []string) {
-	keys = args
-	if !global {
-		if len(args) > 0 {
-			appName = args[0]
-		}
-		if appName == "" {
-			common.LogFail("Please specify an app or --global")
-		} else {
-			keys = args[1:]
-		}
-	}
-	return appName, keys
 }