cmd/syncthing: Mostly replace urfave/cli command line parser with alecthomas/kong (#9166)

`syncthing cli` subcommand was using urfave/cli as the command parser.
This PR replace it with kong, which the main command uses.

Some help texts and error message format are changed. Other than that,
all the command usage and logic remains unchanged.

There's only one place which still uses urfave/cli, which is `syncthing
cli config`, because it uses some magic to dynamically build commands
from struct reflects. I used kong's `passthrough:""` tag to pass any
argument following `syncthing cli config` to urfave/cli parser.

This PR also fixes #9041

---------

Co-authored-by: Jakob Borg <[email protected]>

cmd/syncthing/cli/config.go

@@ -13,6 +13,7 @@ import (
 	"reflect"
 
 	"github.com/AudriusButkevicius/recli"
+	"github.com/alecthomas/kong"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/urfave/cli"
 )
@@ -23,9 +24,20 @@ type configHandler struct {
 	err           error
 }
 
-func getConfigCommand(f *apiClientFactory) (cli.Command, error) {
+type configCommand struct {
+	Args []string `arg:"" default:"-h"`
+}
+
+func (c *configCommand) Run(ctx Context, _ *kong.Context) error {
+	app := cli.NewApp()
+	app.Name = "syncthing"
+	app.Author = "The Syncthing Authors"
+	app.Metadata = map[string]interface{}{
+		"clientFactory": ctx.clientFactory,
+	}
+
 	h := new(configHandler)
-	h.client, h.err = f.getClient()
+	h.client, h.err = ctx.clientFactory.getClient()
 	if h.err == nil {
 		h.cfg, h.err = getConfig(h.client)
 	}
@@ -38,17 +50,15 @@ func getConfigCommand(f *apiClientFactory) (cli.Command, error) {
 
 	commands, err := recli.New(recliCfg).Construct(&h.cfg)
 	if err != nil {
-		return cli.Command{}, fmt.Errorf("config reflect: %w", err)
+		return fmt.Errorf("config reflect: %w", err)
 	}
 
-	return cli.Command{
-		Name:        "config",
-		HideHelp:    true,
-		Usage:       "Configuration modification command group",
-		Subcommands: commands,
-		Before:      h.configBefore,
-		After:       h.configAfter,
-	}, nil
+	app.Commands = commands
+	app.HideHelp = true
+	app.Before = h.configBefore
+	app.After = h.configAfter
+
+	return app.Run(append([]string{app.Name}, c.Args...))
 }
 
 func (h *configHandler) configBefore(c *cli.Context) error {

cmd/syncthing/cli/debug.go

@@ -9,47 +9,37 @@ package cli
 import (
 	"fmt"
 	"net/url"
-
-	"github.com/urfave/cli"
 )
 
-var debugCommand = cli.Command{
-	Name:     "debug",
-	HideHelp: true,
-	Usage:    "Debug command group",
-	Subcommands: []cli.Command{
-		{
-			Name:      "file",
-			Usage:     "Show information about a file (or directory/symlink)",
-			ArgsUsage: "FOLDER-ID PATH",
-			Action:    expects(2, debugFile()),
-		},
-		indexCommand,
-		{
-			Name:      "profile",
-			Usage:     "Save a profile to help figuring out what Syncthing does.",
-			ArgsUsage: "cpu | heap",
-			Action:    expects(1, profile()),
-		},
-	},
+type fileCommand struct {
+	FolderID string `arg:""`
+	Path     string `arg:""`
 }
 
-func debugFile() cli.ActionFunc {
-	return func(c *cli.Context) error {
-		query := make(url.Values)
-		query.Set("folder", c.Args()[0])
-		query.Set("file", normalizePath(c.Args()[1]))
-		return indexDumpOutput("debug/file?" + query.Encode())(c)
-	}
+func (f *fileCommand) Run(ctx Context) error {
+	indexDumpOutput := indexDumpOutputWrapper(ctx.clientFactory)
+
+	query := make(url.Values)
+	query.Set("folder", f.FolderID)
+	query.Set("file", normalizePath(f.Path))
+	return indexDumpOutput("debug/file?" + query.Encode())
 }
 
-func profile() cli.ActionFunc {
-	return func(c *cli.Context) error {
-		switch t := c.Args()[0]; t {
-		case "cpu", "heap":
-			return saveToFile(fmt.Sprintf("debug/%vprof", c.Args()[0]))(c)
-		default:
-			return fmt.Errorf("expected cpu or heap as argument, got %v", t)
-		}
+type profileCommand struct {
+	Type string `arg:"" help:"cpu | heap"`
+}
+
+func (p *profileCommand) Run(ctx Context) error {
+	switch t := p.Type; t {
+	case "cpu", "heap":
+		return saveToFile(fmt.Sprintf("debug/%vprof", p.Type), ctx.clientFactory)
+	default:
+		return fmt.Errorf("expected cpu or heap as argument, got %v", t)
 	}
 }
+
+type debugCommand struct {
+	File    fileCommand    `cmd:"" help:"Show information about a file (or directory/symlink)"`
+	Profile profileCommand `cmd:"" help:"Save a profile to help figuring out what Syncthing does"`
+	Index   indexCommand   `cmd:"" help:"Show information about the index (database)"`
+}

cmd/syncthing/cli/errors.go

@@ -11,36 +11,25 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/urfave/cli"
+	"github.com/alecthomas/kong"
 )
 
-var errorsCommand = cli.Command{
-	Name:     "errors",
-	HideHelp: true,
-	Usage:    "Error command group",
-	Subcommands: []cli.Command{
-		{
-			Name:   "show",
-			Usage:  "Show pending errors",
-			Action: expects(0, indexDumpOutput("system/error")),
-		},
-		{
-			Name:      "push",
-			Usage:     "Push an error to active clients",
-			ArgsUsage: "ERROR-MESSAGE",
-			Action:    expects(1, errorsPush),
-		},
-		{
-			Name:   "clear",
-			Usage:  "Clear pending errors",
-			Action: expects(0, emptyPost("system/error/clear")),
-		},
-	},
+type errorsCommand struct {
+	Show  struct{}          `cmd:"" help:"Show pending errors"`
+	Push  errorsPushCommand `cmd:"" help:"Push an error to active clients"`
+	Clear struct{}          `cmd:"" help:"Clear pending errors"`
 }
 
-func errorsPush(c *cli.Context) error {
-	client := c.App.Metadata["client"].(APIClient)
-	errStr := strings.Join(c.Args(), " ")
+type errorsPushCommand struct {
+	ErrorMessage string `arg:""`
+}
+
+func (e *errorsPushCommand) Run(ctx Context) error {
+	client, err := ctx.clientFactory.getClient()
+	if err != nil {
+		return err
+	}
+	errStr := e.ErrorMessage
 	response, err := client.Post("system/error", strings.TrimSpace(errStr))
 	if err != nil {
 		return err
@@ -59,3 +48,13 @@ func errorsPush(c *cli.Context) error {
 	}
 	return nil
 }
+
+func (*errorsCommand) Run(ctx Context, kongCtx *kong.Context) error {
+	switch kongCtx.Selected().Name {
+	case "show":
+		return indexDumpOutput("system/error", ctx.clientFactory)
+	case "clear":
+		return emptyPost("system/error/clear", ctx.clientFactory)
+	}
+	return nil
+}

cmd/syncthing/cli/index.go

@@ -7,32 +7,26 @@
 package cli
 
 import (
-	"github.com/urfave/cli"
+	"github.com/alecthomas/kong"
 )
 
-var indexCommand = cli.Command{
-	Name:  "index",
-	Usage: "Show information about the index (database)",
-	Subcommands: []cli.Command{
-		{
-			Name:   "dump",
-			Usage:  "Print the entire db",
-			Action: expects(0, indexDump),
-		},
-		{
-			Name:   "dump-size",
-			Usage:  "Print the db size of different categories of information",
-			Action: expects(0, indexDumpSize),
-		},
-		{
-			Name:   "check",
-			Usage:  "Check the database for inconsistencies",
-			Action: expects(0, indexCheck),
-		},
-		{
-			Name:   "account",
-			Usage:  "Print key and value size statistics per key type",
-			Action: expects(0, indexAccount),
-		},
-	},
+type indexCommand struct {
+	Dump     struct{} `cmd:"" help:"Print the entire db"`
+	DumpSize struct{} `cmd:"" help:"Print the db size of different categories of information"`
+	Check    struct{} `cmd:"" help:"Check the database for inconsistencies"`
+	Account  struct{} `cmd:"" help:"Print key and value size statistics per key type"`
+}
+
+func (*indexCommand) Run(kongCtx *kong.Context) error {
+	switch kongCtx.Selected().Name {
+	case "dump":
+		return indexDump()
+	case "dump-size":
+		return indexDumpSize()
+	case "check":
+		return indexCheck()
+	case "account":
+		return indexAccount()
+	}
+	return nil
 }

cmd/syncthing/cli/index_accounting.go

@@ -10,12 +10,10 @@ import (
 	"fmt"
 	"os"
 	"text/tabwriter"
-
-	"github.com/urfave/cli"
 )
 
 // indexAccount prints key and data size statistics per class
-func indexAccount(*cli.Context) error {
+func indexAccount() error {
 	ldb, err := getDB()
 	if err != nil {
 		return err

cmd/syncthing/cli/index_dump.go

@@ -11,13 +11,11 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/urfave/cli"
-
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/protocol"
 )
 
-func indexDump(*cli.Context) error {
+func indexDump() error {
 	ldb, err := getDB()
 	if err != nil {
 		return err

cmd/syncthing/cli/index_dumpsize.go

@@ -11,12 +11,10 @@ import (
 	"fmt"
 	"sort"
 
-	"github.com/urfave/cli"
-
 	"github.com/syncthing/syncthing/lib/db"
 )
 
-func indexDumpSize(*cli.Context) error {
+func indexDumpSize() error {
 	type sizedElement struct {
 		key  string
 		size int

cmd/syncthing/cli/index_idxck.go

@@ -13,8 +13,6 @@ import (
 	"fmt"
 	"sort"
 
-	"github.com/urfave/cli"
-
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/protocol"
 )
@@ -35,7 +33,7 @@ type sequenceKey struct {
 	sequence uint64
 }
 
-func indexCheck(*cli.Context) (err error) {
+func indexCheck() (err error) {
 	ldb, err := getDB()
 	if err != nil {
 		return err

cmd/syncthing/cli/main.go

@@ -8,165 +8,83 @@ package cli
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
-	"io"
 	"os"
-	"strings"
+	"os/exec"
 
 	"github.com/alecthomas/kong"
 	"github.com/flynn-archive/go-shlex"
-	"github.com/urfave/cli"
 
 	"github.com/syncthing/syncthing/cmd/syncthing/cmdutil"
 	"github.com/syncthing/syncthing/lib/config"
 )
 
-type preCli struct {
+type CLI struct {
+	cmdutil.CommonOptions
+	DataDir    string `name:"data" placeholder:"PATH" env:"STDATADIR" help:"Set data directory (database and logs)"`
 	GUIAddress string `name:"gui-address"`
 	GUIAPIKey  string `name:"gui-apikey"`
-	HomeDir    string `name:"home"`
-	ConfDir    string `name:"config"`
-	DataDir    string `name:"data"`
-}
 
-func Run() error {
-	// This is somewhat a hack around a chicken and egg problem. We need to set
-	// the home directory and potentially other flags to know where the
-	// syncthing instance is running in order to get it's config ... which we
-	// then use to construct the actual CLI ... at which point it's too late to
-	// add flags there...
-	c := preCli{}
-	parseFlags(&c)
-	return runInternal(c, os.Args)
+	Show       showCommand      `cmd:"" help:"Show command group"`
+	Debug      debugCommand     `cmd:"" help:"Debug command group"`
+	Operations operationCommand `cmd:"" help:"Operation command group"`
+	Errors     errorsCommand    `cmd:"" help:"Error command group"`
+	Config     configCommand    `cmd:"" help:"Configuration modification command group" passthrough:""`
+	Stdin      stdinCommand     `cmd:"" name:"-" help:"Read commands from stdin"`
 }
 
-func RunWithArgs(cliArgs []string) error {
-	c := preCli{}
-	parseFlagsWithArgs(cliArgs, &c)
-	return runInternal(c, cliArgs)
+type Context struct {
+	clientFactory *apiClientFactory
 }
 
-func runInternal(c preCli, cliArgs []string) error {
-	// Not set as default above because the strings can be really long.
-	err := cmdutil.SetConfigDataLocationsFromFlags(c.HomeDir, c.ConfDir, c.DataDir)
+func (cli CLI) AfterApply(kongCtx *kong.Context) error {
+	err := cmdutil.SetConfigDataLocationsFromFlags(cli.HomeDir, cli.ConfDir, cli.DataDir)
 	if err != nil {
-		return fmt.Errorf("Command line options: %w", err)
+		return fmt.Errorf("command line options: %w", err)
 	}
+
 	clientFactory := &apiClientFactory{
 		cfg: config.GUIConfiguration{
-			RawAddress: c.GUIAddress,
-			APIKey:     c.GUIAPIKey,
+			RawAddress: cli.GUIAddress,
+			APIKey:     cli.GUIAPIKey,
 		},
 	}
 
-	configCommand, err := getConfigCommand(clientFactory)
-	if err != nil {
-		return err
+	context := Context{
+		clientFactory: clientFactory,
 	}
 
-	// Implement the same flags at the upper CLI, but do nothing with them.
-	// This is so that the usage text is the same
-	fakeFlags := []cli.Flag{
-		cli.StringFlag{
-			Name:  "gui-address",
-			Usage: "Override GUI address to `URL` (e.g. \"192.0.2.42:8443\")",
-		},
-		cli.StringFlag{
-			Name:  "gui-apikey",
-			Usage: "Override GUI API key to `API-KEY`",
-		},
-		cli.StringFlag{
-			Name:  "home",
-			Usage: "Set configuration and data directory to `PATH`",
-		},
-		cli.StringFlag{
-			Name:  "config",
-			Usage: "Set configuration directory (config and keys) to `PATH`",
-		},
-		cli.StringFlag{
-			Name:  "data",
-			Usage: "Set data directory (database and logs) to `PATH`",
-		},
-	}
-
-	// Construct the actual CLI
-	app := cli.NewApp()
-	app.Author = "The Syncthing Authors"
-	app.Metadata = map[string]interface{}{
-		"clientFactory": clientFactory,
-	}
-	app.Commands = []cli.Command{{
-		Name:  "cli",
-		Usage: "Syncthing command line interface",
-		Flags: fakeFlags,
-		Subcommands: []cli.Command{
-			configCommand,
-			showCommand,
-			operationCommand,
-			errorsCommand,
-			debugCommand,
-			{
-				Name:     "-",
-				HideHelp: true,
-				Usage:    "Read commands from stdin",
-				Action: func(ctx *cli.Context) error {
-					if ctx.NArg() > 0 {
-						return errors.New("command does not expect any arguments")
-					}
-
-					// Drop the `-` not to recurse into self.
-					args := make([]string, len(cliArgs)-1)
-					copy(args, cliArgs)
-
-					fmt.Println("Reading commands from stdin...", args)
-					scanner := bufio.NewScanner(os.Stdin)
-					for scanner.Scan() {
-						input, err := shlex.Split(scanner.Text())
-						if err != nil {
-							return fmt.Errorf("parsing input: %w", err)
-						}
-						if len(input) == 0 {
-							continue
-						}
-						err = app.Run(append(args, input...))
-						if err != nil {
-							return err
-						}
-					}
-					return scanner.Err()
-				},
-			},
-		},
-	}}
-
-	return app.Run(cliArgs)
+	kongCtx.Bind(context)
+	return nil
 }
 
-func parseFlags(c *preCli) error {
-	// kong only needs to parse the global arguments after "cli" and before the
-	// subcommand (if any).
-	if len(os.Args) <= 2 {
-		return nil
-	}
-	return parseFlagsWithArgs(os.Args[2:], c)
-}
+type stdinCommand struct{}
+
+func (*stdinCommand) Run() error {
+	// Drop the `-` not to recurse into self.
+	args := make([]string, len(os.Args)-1)
+	copy(args, os.Args)
 
-func parseFlagsWithArgs(args []string, c *preCli) error {
-	for i := 0; i < len(args); i++ {
-		if !strings.HasPrefix(args[i], "--") {
-			args = args[:i]
-			break
+	fmt.Println("Reading commands from stdin...", args)
+	scanner := bufio.NewScanner(os.Stdin)
+	for scanner.Scan() {
+		input, err := shlex.Split(scanner.Text())
+		if err != nil {
+			return fmt.Errorf("parsing input: %w", err)
 		}
-		if !strings.Contains(args[i], "=") {
-			i++
+		if len(input) == 0 {
+			continue
+		}
+		cmd := exec.Command(os.Args[0], append(args[1:], input...)...)
+		out, err := cmd.CombinedOutput()
+		fmt.Print(string(out))
+		if err != nil {
+			if _, ok := err.(*exec.ExitError); ok {
+				// we will continue loop no matter the command succeeds or not
+				continue
+			}
+			return err
 		}
 	}
-	// We don't want kong to print anything nor os.Exit (e.g. on -h)
-	parser, err := kong.New(c, kong.Writers(io.Discard, io.Discard), kong.Exit(func(int) {}))
-	if err != nil {
-		return err
-	}
-	_, err = parser.Parse(args)
-	return err
+	return scanner.Err()
 }

cmd/syncthing/cli/operations.go

@@ -12,48 +12,43 @@ import (
 	"fmt"
 	"path/filepath"
 
+	"github.com/alecthomas/kong"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/fs"
-	"github.com/urfave/cli"
 )
 
-var operationCommand = cli.Command{
-	Name:     "operations",
-	HideHelp: true,
-	Usage:    "Operation command group",
-	Subcommands: []cli.Command{
-		{
-			Name:   "restart",
-			Usage:  "Restart syncthing",
-			Action: expects(0, emptyPost("system/restart")),
-		},
-		{
-			Name:   "shutdown",
-			Usage:  "Shutdown syncthing",
-			Action: expects(0, emptyPost("system/shutdown")),
-		},
-		{
-			Name:   "upgrade",
-			Usage:  "Upgrade syncthing (if a newer version is available)",
-			Action: expects(0, emptyPost("system/upgrade")),
-		},
-		{
-			Name:      "folder-override",
-			Usage:     "Override changes on folder (remote for sendonly, local for receiveonly). WARNING: Destructive - deletes/changes your data.",
-			ArgsUsage: "FOLDER-ID",
-			Action:    expects(1, foldersOverride),
-		},
-		{
-			Name:      "default-ignores",
-			Usage:     "Set the default ignores (config) from a file",
-			ArgsUsage: "PATH",
-			Action:    expects(1, setDefaultIgnores),
-		},
-	},
+type folderOverrideCommand struct {
+	FolderID string `arg:""`
 }
 
-func foldersOverride(c *cli.Context) error {
-	client, err := getClientFactory(c).getClient()
+type defaultIgnoresCommand struct {
+	Path string `arg:""`
+}
+
+type operationCommand struct {
+	Restart        struct{}              `cmd:"" help:"Restart syncthing"`
+	Shutdown       struct{}              `cmd:"" help:"Shutdown syncthing"`
+	Upgrade        struct{}              `cmd:"" help:"Upgrade syncthing (if a newer version is available)"`
+	FolderOverride folderOverrideCommand `cmd:"" help:"Override changes on folder (remote for sendonly, local for receiveonly). WARNING: Destructive - deletes/changes your data"`
+	DefaultIgnores defaultIgnoresCommand `cmd:"" help:"Set the default ignores (config) from a file"`
+}
+
+func (*operationCommand) Run(ctx Context, kongCtx *kong.Context) error {
+	f := ctx.clientFactory
+
+	switch kongCtx.Selected().Name {
+	case "restart":
+		return emptyPost("system/restart", f)
+	case "shutdown":
+		return emptyPost("system/shutdown", f)
+	case "upgrade":
+		return emptyPost("system/upgrade", f)
+	}
+	return nil
+}
+
+func (f *folderOverrideCommand) Run(ctx Context) error {
+	client, err := ctx.clientFactory.getClient()
 	if err != nil {
 		return err
 	}
@@ -61,7 +56,7 @@ func foldersOverride(c *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	rid := c.Args()[0]
+	rid := f.FolderID
 	for _, folder := range cfg.Folders {
 		if folder.ID == rid {
 			response, err := client.Post("db/override", "")
@@ -86,12 +81,12 @@ func foldersOverride(c *cli.Context) error {
 	return fmt.Errorf("Folder %q not found", rid)
 }
 
-func setDefaultIgnores(c *cli.Context) error {
-	client, err := getClientFactory(c).getClient()
+func (d *defaultIgnoresCommand) Run(ctx Context) error {
+	client, err := ctx.clientFactory.getClient()
 	if err != nil {
 		return err
 	}
-	dir, file := filepath.Split(c.Args()[0])
+	dir, file := filepath.Split(d.Path)
 	filesystem := fs.NewFilesystem(fs.FilesystemTypeBasic, dir)
 
 	fd, err := filesystem.Open(file)

cmd/syncthing/cli/pending.go

@@ -9,37 +9,30 @@ package cli
 import (
 	"net/url"
 
-	"github.com/urfave/cli"
+	"github.com/alecthomas/kong"
 )
 
-var pendingCommand = cli.Command{
-	Name:     "pending",
-	HideHelp: true,
-	Usage:    "Pending subcommand group",
-	Subcommands: []cli.Command{
-		{
-			Name:   "devices",
-			Usage:  "Show pending devices",
-			Action: expects(0, indexDumpOutput("cluster/pending/devices")),
-		},
-		{
-			Name:  "folders",
-			Usage: "Show pending folders",
-			Flags: []cli.Flag{
-				cli.StringFlag{Name: "device", Usage: "Show pending folders offered by given device"},
-			},
-			Action: expects(0, folders()),
-		},
-	},
+type pendingCommand struct {
+	Devices struct{} `cmd:"" help:"Show pending devices"`
+	Folders struct {
+		Device string `help:"Show pending folders offered by given device"`
+	} `cmd:"" help:"Show pending folders"`
 }
 
-func folders() cli.ActionFunc {
-	return func(c *cli.Context) error {
-		if c.String("device") != "" {
+func (p *pendingCommand) Run(ctx Context, kongCtx *kong.Context) error {
+	indexDumpOutput := indexDumpOutputWrapper(ctx.clientFactory)
+
+	switch kongCtx.Selected().Name {
+	case "devices":
+		return indexDumpOutput("cluster/pending/devices")
+	case "folders":
+		if p.Folders.Device != "" {
 			query := make(url.Values)
-			query.Set("device", c.String("device"))
-			return indexDumpOutput("cluster/pending/folders?" + query.Encode())(c)
+			query.Set("device", p.Folders.Device)
+			return indexDumpOutput("cluster/pending/folders?" + query.Encode())
 		}
-		return indexDumpOutput("cluster/pending/folders")(c)
+		return indexDumpOutput("cluster/pending/folders")
 	}
+
+	return nil
 }

cmd/syncthing/cli/show.go

@@ -7,44 +7,36 @@
 package cli
 
 import (
-	"github.com/urfave/cli"
+	"github.com/alecthomas/kong"
 )
 
-var showCommand = cli.Command{
-	Name:     "show",
-	HideHelp: true,
-	Usage:    "Show command group",
-	Subcommands: []cli.Command{
-		{
-			Name:   "version",
-			Usage:  "Show syncthing client version",
-			Action: expects(0, indexDumpOutput("system/version")),
-		},
-		{
-			Name:   "config-status",
-			Usage:  "Show configuration status, whether or not a restart is required for changes to take effect",
-			Action: expects(0, indexDumpOutput("config/restart-required")),
-		},
-		{
-			Name:   "system",
-			Usage:  "Show system status",
-			Action: expects(0, indexDumpOutput("system/status")),
-		},
-		{
-			Name:   "connections",
-			Usage:  "Report about connections to other devices",
-			Action: expects(0, indexDumpOutput("system/connections")),
-		},
-		{
-			Name:   "discovery",
-			Usage:  "Show the discovered addresses of remote devices (from cache of the running syncthing instance)",
-			Action: expects(0, indexDumpOutput("system/discovery")),
-		},
-		pendingCommand,
-		{
-			Name:   "usage",
-			Usage:  "Show usage report",
-			Action: expects(0, indexDumpOutput("svc/report")),
-		},
-	},
+type showCommand struct {
+	Version      struct{}       `cmd:"" help:"Show syncthing client version"`
+	ConfigStatus struct{}       `cmd:"" help:"Show configuration status, whether or not a restart is required for changes to take effect"`
+	System       struct{}       `cmd:"" help:"Show system status"`
+	Connections  struct{}       `cmd:"" help:"Report about connections to other devices"`
+	Discovery    struct{}       `cmd:"" help:"Show the discovered addresses of remote devices (from cache of the running syncthing instance)"`
+	Usage        struct{}       `cmd:"" help:"Show usage report"`
+	Pending      pendingCommand `cmd:"" help:"Pending subcommand group"`
+}
+
+func (*showCommand) Run(ctx Context, kongCtx *kong.Context) error {
+	indexDumpOutput := indexDumpOutputWrapper(ctx.clientFactory)
+
+	switch kongCtx.Selected().Name {
+	case "version":
+		return indexDumpOutput("system/version")
+	case "config-status":
+		return indexDumpOutput("config/restart-required")
+	case "system":
+		return indexDumpOutput("system/status")
+	case "connections":
+		return indexDumpOutput("system/connections")
+	case "discovery":
+		return indexDumpOutput("system/discovery")
+	case "usage":
+		return indexDumpOutput("svc/report")
+	}
+
+	return nil
 }

cmd/syncthing/cli/utils.go

@@ -19,7 +19,6 @@ import (
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/db/backend"
 	"github.com/syncthing/syncthing/lib/locations"
-	"github.com/urfave/cli"
 )
 
 func responseToBArray(response *http.Response) ([]byte, error) {
@@ -30,68 +29,72 @@ func responseToBArray(response *http.Response) ([]byte, error) {
 	return bytes, response.Body.Close()
 }
 
-func emptyPost(url string) cli.ActionFunc {
-	return func(c *cli.Context) error {
-		client, err := getClientFactory(c).getClient()
-		if err != nil {
-			return err
-		}
-		_, err = client.Post(url, "")
+func emptyPost(url string, apiClientFactory *apiClientFactory) error {
+	client, err := apiClientFactory.getClient()
+	if err != nil {
 		return err
 	}
+	_, err = client.Post(url, "")
+	return err
 }
 
-func indexDumpOutput(url string) cli.ActionFunc {
-	return func(c *cli.Context) error {
-		client, err := getClientFactory(c).getClient()
-		if err != nil {
-			return err
-		}
-		response, err := client.Get(url)
-		if errors.Is(err, errNotFound) {
-			return errors.New("not found (folder/file not in database)")
-		}
-		if err != nil {
-			return err
-		}
-		return prettyPrintResponse(response)
+func indexDumpOutputWrapper(apiClientFactory *apiClientFactory) func(url string) error {
+	return func(url string) error {
+		return indexDumpOutput(url, apiClientFactory)
 	}
 }
 
-func saveToFile(url string) cli.ActionFunc {
-	return func(c *cli.Context) error {
-		client, err := getClientFactory(c).getClient()
-		if err != nil {
-			return err
-		}
-		response, err := client.Get(url)
-		if err != nil {
-			return err
-		}
-		_, params, err := mime.ParseMediaType(response.Header.Get("Content-Disposition"))
-		if err != nil {
-			return err
-		}
-		filename := params["filename"]
-		if filename == "" {
-			return errors.New("Missing filename in response")
-		}
-		bs, err := responseToBArray(response)
-		if err != nil {
-			return err
-		}
-		f, err := os.Create(filename)
-		if err != nil {
-			return err
-		}
-		defer f.Close()
-		_, err = f.Write(bs)
-		if err != nil {
-			return err
-		}
-		fmt.Println("Wrote results to", filename)
+func indexDumpOutput(url string, apiClientFactory *apiClientFactory) error {
+	client, err := apiClientFactory.getClient()
+	if err != nil {
+		return err
+	}
+	response, err := client.Get(url)
+	if errors.Is(err, errNotFound) {
+		return errors.New("not found (folder/file not in database)")
+	}
+	if err != nil {
 		return err
 	}
+	return prettyPrintResponse(response)
+}
+
+func saveToFile(url string, apiClientFactory *apiClientFactory) error {
+	client, err := apiClientFactory.getClient()
+	if err != nil {
+		return err
+	}
+	response, err := client.Get(url)
+	if err != nil {
+		return err
+	}
+	_, params, err := mime.ParseMediaType(response.Header.Get("Content-Disposition"))
+	if err != nil {
+		return err
+	}
+	filename := params["filename"]
+	if filename == "" {
+		return errors.New("Missing filename in response")
+	}
+	bs, err := responseToBArray(response)
+	if err != nil {
+		return err
+	}
+	f, err := os.Create(filename)
+	if err != nil {
+		return err
+	}
+	_, err = f.Write(bs)
+	if err != nil {
+		_ = f.Close()
+		return err
+	}
+	err = f.Close()
+	if err != nil {
+		return err
+	}
+	fmt.Println("Wrote results to", filename)
+	return nil
 }
 
 func getConfig(c APIClient) (config.Configuration, error) {
@@ -111,19 +114,6 @@ func getConfig(c APIClient) (config.Configuration, error) {
 	return cfg, nil
 }
 
-func expects(n int, actionFunc cli.ActionFunc) cli.ActionFunc {
-	return func(ctx *cli.Context) error {
-		if ctx.NArg() != n {
-			plural := ""
-			if n != 1 {
-				plural = "s"
-			}
-			return fmt.Errorf("expected %d argument%s, got %d", n, plural, ctx.NArg())
-		}
-		return actionFunc(ctx)
-	}
-}
-
 func prettyPrintJSON(data interface{}) error {
 	enc := json.NewEncoder(os.Stdout)
 	enc.SetIndent("", "  ")
@@ -159,7 +149,3 @@ func nulString(bs []byte) string {
 func normalizePath(path string) string {
 	return filepath.ToSlash(filepath.Clean(path))
 }
-
-func getClientFactory(c *cli.Context) *apiClientFactory {
-	return c.App.Metadata["clientFactory"].(*apiClientFactory)
-}

cmd/syncthing/main.go

@@ -139,7 +139,7 @@ var entrypoint struct {
 	Serve    serveOptions `cmd:"" help:"Run Syncthing"`
 	Generate generate.CLI `cmd:"" help:"Generate key and config, then exit"`
 	Decrypt  decrypt.CLI  `cmd:"" help:"Decrypt or verify an encrypted folder"`
-	Cli      struct{}     `cmd:"" help:"Command line interface for Syncthing"`
+	Cli      cli.CLI      `cmd:"" help:"Command line interface for Syncthing"`
 }
 
 // serveOptions are the options for the `syncthing serve` command.
@@ -213,17 +213,6 @@ func defaultVars() kong.Vars {
 }
 
 func main() {
-	// The "cli" subcommand uses a different command line parser, and e.g. help
-	// gets mangled when integrating it as a subcommand -> detect it here at the
-	// beginning.
-	if len(os.Args) > 1 && os.Args[1] == "cli" {
-		if err := cli.Run(); err != nil {
-			fmt.Println(err)
-			os.Exit(1)
-		}
-		return
-	}
-
 	// First some massaging of the raw command line to fit the new model.
 	// Basically this means adding the default command at the front, and
 	// converting -options to --options.
@@ -249,7 +238,15 @@ func main() {
 
 	// Create a parser with an overridden help function to print our extra
 	// help info.
-	parser, err := kong.New(&entrypoint, kong.Help(helpHandler), defaultVars())
+	parser, err := kong.New(
+		&entrypoint,
+		kong.ConfigureHelp(kong.HelpOptions{
+			NoExpandSubcommands: true,
+			Compact:             true,
+		}),
+		kong.Help(helpHandler),
+		defaultVars(),
+	)
 	if err != nil {
 		log.Fatal(err)
 	}