From aae3253799854eb53a11501120a2a78844ec11ac Mon Sep 17 00:00:00 2001 From: ANSHUL SINGH <72524975+ekanshul@users.noreply.github.com> Date: Thu, 20 Aug 2026 02:16:41 +0530 Subject: [PATCH 1/2] cli: explain misplaced flags before the subcommand Main.Run consumed args[0] as the subcommand before any flag parser ran, so "litestream -config c.yml databases" printed the bare usage screen and exited 1 with no explanation. Return a usage error that names the problem and reorders the user's own arguments into a pasteable command, offering LITESTREAM_CONFIG when -config is the misplaced flag. Fixes #1405 Co-Authored-By: Claude Fable 5 --- cmd/litestream/main.go | 48 ++++++++++++++++++++++++++++++++- cmd/litestream/main_cli_test.go | 45 +++++++++++++++++++++++++++++++ cmd/litestream/main_test.go | 43 ++++++++++++++++++++++++++--- 3 files changed, 131 insertions(+), 5 deletions(-) diff --git a/cmd/litestream/main.go b/cmd/litestream/main.go index ecb383e92..796f75ce9 100644 --- a/cmd/litestream/main.go +++ b/cmd/litestream/main.go @@ -14,6 +14,7 @@ import ( "path" "path/filepath" "runtime/debug" + "slices" "strconv" "strings" "time" @@ -230,14 +231,59 @@ func (m *Main) Run(ctx context.Context, args []string) (err error) { if cmd == "help" || cmd == "-h" || cmd == "-help" || cmd == "--help" { m.Usage() return nil - } else if cmd == "" || strings.HasPrefix(cmd, "-") { + } else if cmd == "" { m.Usage() return flag.ErrHelp + } else if strings.HasPrefix(cmd, "-") { + return misplacedFlagsError(append([]string{cmd}, args...)) } return fmt.Errorf("litestream %s: unknown command", cmd) } } +// commandNames lists the subcommands dispatched by Main.Run. +var commandNames = []string{ + "databases", "info", "list", "ltx", "register", "replicate", "reset", + "restore", "start", "status", "stop", "sync", "unregister", "version", +} + +// misplacedFlagsError returns the error for flags passed before the +// subcommand, e.g. "litestream -config c.yml databases". Flags are only +// parsed by subcommands, so the hint reorders the user's own arguments into +// a form that can be pasted back. When -config is the misplaced flag it also +// offers LITESTREAM_CONFIG, which is what works for shell aliases. +func misplacedFlagsError(args []string) error { + var cmd, configPath string + var rest, envRest []string // envRest omits the -config flag. + for i := 0; i < len(args); i++ { + arg := args[i] + if cmd == "" && slices.Contains(commandNames, arg) { + cmd = arg + continue + } + rest = append(rest, arg) + if name := strings.TrimLeft(arg, "-"); name == "config" && i+1 < len(args) { + i++ + configPath = args[i] + rest = append(rest, configPath) + } else if strings.HasPrefix(name, "config=") { + configPath = strings.TrimPrefix(name, "config=") + } else { + envRest = append(envRest, arg) + } + } + if cmd == "" { + cmd = "" + } + + hint := strings.Join(append([]string{"litestream", cmd}, rest...), " ") + if configPath != "" { + // main() prints the hint after "Try: ", so align the alternative under it. + hint += "\n or: " + strings.Join(append([]string{"LITESTREAM_CONFIG=" + configPath, "litestream", cmd}, envRest...), " ") + } + return &usageError{message: "flags must come after the subcommand", hint: hint} +} + // Usage prints the help screen to STDOUT. func (m *Main) Usage() { fmt.Println(` diff --git a/cmd/litestream/main_cli_test.go b/cmd/litestream/main_cli_test.go index 94282548c..620db7340 100644 --- a/cmd/litestream/main_cli_test.go +++ b/cmd/litestream/main_cli_test.go @@ -95,6 +95,51 @@ func TestMainRequiredArgumentErrorsIncludeTryHints(t *testing.T) { } } +func TestMainMisplacedFlagsIncludeTryHints(t *testing.T) { + tests := []struct { + name string + args []string + hint string + }{ + { + name: "ConfigBeforeCommand", + args: []string{"-config", "./c.yml", "databases"}, + hint: "litestream databases -config ./c.yml\n or: LITESTREAM_CONFIG=./c.yml litestream databases", + }, + { + name: "ConfigEqualsBeforeCommand", + args: []string{"--config=./c.yml", "restore", "-o", "/tmp/example.db", "s3://bucket/prefix"}, + hint: "litestream restore --config=./c.yml -o /tmp/example.db s3://bucket/prefix\n or: LITESTREAM_CONFIG=./c.yml litestream restore -o /tmp/example.db s3://bucket/prefix", + }, + { + name: "OtherFlagBeforeCommand", + args: []string{"-replica", "s3://bucket/prefix", "register", "/tmp/example.db"}, + hint: "litestream register -replica s3://bucket/prefix /tmp/example.db", + }, + { + name: "NoCommand", + args: []string{"-config", "./c.yml"}, + hint: "litestream -config ./c.yml\n or: LITESTREAM_CONFIG=./c.yml litestream ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stdout, stderr, exitCode := runLitestreamMain(t, tt.args...) + if exitCode != 1 { + t.Fatalf("exit code=%d, want 1", exitCode) + } + if stdout != "" { + t.Fatalf("expected empty stdout, got:\n%s", stdout) + } + want := "Error: flags must come after the subcommand\nTry: " + tt.hint + "\n" + if stderr != want { + t.Fatalf("unexpected stderr:\n%s\nwant:\n%s", stderr, want) + } + }) + } +} + func TestMainHarness(t *testing.T) { if os.Getenv("LITESTREAM_TEST_MAIN") != "1" { t.Skip("helper process only") diff --git a/cmd/litestream/main_test.go b/cmd/litestream/main_test.go index d90fb0cec..6f850ad88 100644 --- a/cmd/litestream/main_test.go +++ b/cmd/litestream/main_test.go @@ -53,11 +53,46 @@ func TestMain_RunHelp(t *testing.T) { t.Fatalf("Run returned error %v, want %v", err, flag.ErrHelp) } }) +} - t.Run("UnknownFlag", func(t *testing.T) { - err := main.NewMain().Run(context.Background(), []string{"-config", "litestream.yml"}) - if !errors.Is(err, flag.ErrHelp) { - t.Fatalf("Run returned error %v, want %v", err, flag.ErrHelp) +func TestMain_RunMisplacedFlags(t *testing.T) { + const want = "flags must come after the subcommand" + + t.Run("FlagBeforeCommand", func(t *testing.T) { + for _, args := range [][]string{ + {"-config", "litestream.yml", "databases"}, + {"-config", "litestream.yml"}, + } { + var err error + stdout := captureStdout(t, func() { + err = main.NewMain().Run(context.Background(), args) + }) + if err == nil || err.Error() != want { + t.Fatalf("Run(%v) returned error %v, want %q", args, err, want) + } else if stdout != "" { + t.Fatalf("Run(%v) printed usage on stdout:\n%s", args, stdout) + } + } + }) + + t.Run("HelpFlagStillPrintsUsage", func(t *testing.T) { + for _, arg := range []string{"-h", "-help", "--help"} { + var err error + stdout := captureStdout(t, func() { + err = main.NewMain().Run(context.Background(), []string{arg}) + }) + if err != nil { + t.Fatalf("Run(%s) returned error: %v", arg, err) + } else if !strings.Contains(stdout, "litestream [arguments]") { + t.Fatalf("Run(%s) did not print usage, got:\n%s", arg, stdout) + } + } + }) + + t.Run("FlagAfterCommand", func(t *testing.T) { + err := main.NewMain().Run(context.Background(), []string{"databases", "-config", filepath.Join(t.TempDir(), "missing.yml")}) + if err == nil || err.Error() == want { + t.Fatalf("Run returned error %v, want config error", err) } }) } From a3e8342ff5a9691c6e1280df02360ad787f4244d Mon Sep 17 00:00:00 2001 From: ANSHUL SINGH <72524975+ekanshul@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:50:02 +0530 Subject: [PATCH 2/2] cli: report misplaced flags without guessing the command Replace the command-name scanner with a helper that only reports where a flag belongs. The scanner could not tell a command from a flag value, so "litestream -socket status info" was rewritten as "litestream status -socket info", reversing the user's intent. It also needed its own copy of the command list. The hint is now a fixed form rather than a rewrite of the arguments, and -config additionally points at LITESTREAM_CONFIG for shell aliases. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/litestream/main.go | 56 ++++---------- cmd/litestream/main_cli_test.go | 131 ++++++++++++++++++++++---------- cmd/litestream/main_test.go | 43 +---------- 3 files changed, 109 insertions(+), 121 deletions(-) diff --git a/cmd/litestream/main.go b/cmd/litestream/main.go index 796f75ce9..2d44c8ffb 100644 --- a/cmd/litestream/main.go +++ b/cmd/litestream/main.go @@ -14,7 +14,6 @@ import ( "path" "path/filepath" "runtime/debug" - "slices" "strconv" "strings" "time" @@ -235,53 +234,30 @@ func (m *Main) Run(ctx context.Context, args []string) (err error) { m.Usage() return flag.ErrHelp } else if strings.HasPrefix(cmd, "-") { - return misplacedFlagsError(append([]string{cmd}, args...)) + return misplacedFlagError(cmd) } return fmt.Errorf("litestream %s: unknown command", cmd) } } -// commandNames lists the subcommands dispatched by Main.Run. -var commandNames = []string{ - "databases", "info", "list", "ltx", "register", "replicate", "reset", - "restore", "start", "status", "stop", "sync", "unregister", "version", -} - -// misplacedFlagsError returns the error for flags passed before the -// subcommand, e.g. "litestream -config c.yml databases". Flags are only -// parsed by subcommands, so the hint reorders the user's own arguments into -// a form that can be pasted back. When -config is the misplaced flag it also -// offers LITESTREAM_CONFIG, which is what works for shell aliases. -func misplacedFlagsError(args []string) error { - var cmd, configPath string - var rest, envRest []string // envRest omits the -config flag. - for i := 0; i < len(args); i++ { - arg := args[i] - if cmd == "" && slices.Contains(commandNames, arg) { - cmd = arg - continue +// misplacedFlagError returns a usage error for a flag that appears before the +// subcommand, e.g. "litestream -config c.yml databases". Only the subcommands +// parse flags, so the hint shows where the flag belongs rather than guessing +// the intended command. Shell aliases that lead with -config are pointed at +// LITESTREAM_CONFIG, which works in any position. +func misplacedFlagError(arg string) error { + name := strings.TrimLeft(strings.SplitN(arg, "=", 2)[0], "-") + if name == "config" { + return &usageError{ + message: "flags must come after the subcommand", + hint: "litestream -config PATH\n" + + " or: LITESTREAM_CONFIG=PATH litestream ", } - rest = append(rest, arg) - if name := strings.TrimLeft(arg, "-"); name == "config" && i+1 < len(args) { - i++ - configPath = args[i] - rest = append(rest, configPath) - } else if strings.HasPrefix(name, "config=") { - configPath = strings.TrimPrefix(name, "config=") - } else { - envRest = append(envRest, arg) - } - } - if cmd == "" { - cmd = "" } - - hint := strings.Join(append([]string{"litestream", cmd}, rest...), " ") - if configPath != "" { - // main() prints the hint after "Try: ", so align the alternative under it. - hint += "\n or: " + strings.Join(append([]string{"LITESTREAM_CONFIG=" + configPath, "litestream", cmd}, envRest...), " ") + return &usageError{ + message: "flags must come after the subcommand", + hint: "litestream [flags]", } - return &usageError{message: "flags must come after the subcommand", hint: hint} } // Usage prints the help screen to STDOUT. diff --git a/cmd/litestream/main_cli_test.go b/cmd/litestream/main_cli_test.go index 620db7340..48f5ed574 100644 --- a/cmd/litestream/main_cli_test.go +++ b/cmd/litestream/main_cli_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "os" "os/exec" + "path/filepath" "strings" "testing" ) @@ -95,49 +96,95 @@ func TestMainRequiredArgumentErrorsIncludeTryHints(t *testing.T) { } } -func TestMainMisplacedFlagsIncludeTryHints(t *testing.T) { - tests := []struct { - name string - args []string - hint string - }{ - { - name: "ConfigBeforeCommand", - args: []string{"-config", "./c.yml", "databases"}, - hint: "litestream databases -config ./c.yml\n or: LITESTREAM_CONFIG=./c.yml litestream databases", - }, - { - name: "ConfigEqualsBeforeCommand", - args: []string{"--config=./c.yml", "restore", "-o", "/tmp/example.db", "s3://bucket/prefix"}, - hint: "litestream restore --config=./c.yml -o /tmp/example.db s3://bucket/prefix\n or: LITESTREAM_CONFIG=./c.yml litestream restore -o /tmp/example.db s3://bucket/prefix", - }, - { - name: "OtherFlagBeforeCommand", - args: []string{"-replica", "s3://bucket/prefix", "register", "/tmp/example.db"}, - hint: "litestream register -replica s3://bucket/prefix /tmp/example.db", - }, - { - name: "NoCommand", - args: []string{"-config", "./c.yml"}, - hint: "litestream -config ./c.yml\n or: LITESTREAM_CONFIG=./c.yml litestream ", - }, - } +func TestMainFlagPlacement(t *testing.T) { + const message = "Error: flags must come after the subcommand\n" + const configHint = "Try: litestream -config PATH\n or: LITESTREAM_CONFIG=PATH litestream \n" + const genericHint = "Try: litestream [flags]\n" - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - stdout, stderr, exitCode := runLitestreamMain(t, tt.args...) - if exitCode != 1 { - t.Fatalf("exit code=%d, want 1", exitCode) - } - if stdout != "" { - t.Fatalf("expected empty stdout, got:\n%s", stdout) - } - want := "Error: flags must come after the subcommand\nTry: " + tt.hint + "\n" - if stderr != want { - t.Fatalf("unexpected stderr:\n%s\nwant:\n%s", stderr, want) - } - }) - } + t.Run("FlagBeforeCommand", func(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + { + name: "Config", + args: []string{"-config", "/etc/litestream.yml", "databases"}, + want: message + configHint, + }, + { + name: "ConfigWithEquals", + args: []string{"--config=/etc/litestream.yml", "databases"}, + want: message + configHint, + }, + { + // The invocation from the review: "status" may be the value + // of -socket and "info" the intended command, so no hint can + // name the command without guessing. + name: "Socket", + args: []string{"-socket", "status", "info"}, + want: message + genericHint, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stdout, stderr, exitCode := runLitestreamMain(t, tt.args...) + if exitCode != 1 { + t.Fatalf("exit code=%d, want 1", exitCode) + } + if stdout != "" { + t.Fatalf("expected empty stdout, got:\n%s", stdout) + } + if stderr != tt.want { + t.Fatalf("unexpected stderr:\n%s\nwant:\n%s", stderr, tt.want) + } + }) + } + }) + + // Each help flag starts with "-", so each one has to keep reaching the + // help branch rather than falling through to the misplaced-flag branch. + t.Run("ExplicitHelp", func(t *testing.T) { + for _, arg := range []string{"-h", "-help", "--help"} { + t.Run(arg, func(t *testing.T) { + stdout, stderr, exitCode := runLitestreamMain(t, arg) + if exitCode != 0 { + t.Fatalf("exit code=%d, want 0\nstderr:\n%s", exitCode, stderr) + } + if stderr != "" { + t.Fatalf("expected empty stderr, got:\n%s", stderr) + } + if !strings.Contains(stdout, "litestream [arguments]") { + t.Fatalf("expected usage on stdout, got:\n%s", stdout) + } + }) + } + }) + + t.Run("NoArguments", func(t *testing.T) { + stdout, stderr, exitCode := runLitestreamMain(t) + if exitCode != 1 { + t.Fatalf("exit code=%d, want 1", exitCode) + } + if stderr != "" { + t.Fatalf("expected empty stderr, got:\n%s", stderr) + } + if !strings.Contains(stdout, "litestream [arguments]") { + t.Fatalf("expected usage on stdout, got:\n%s", stdout) + } + }) + + t.Run("FlagAfterCommand", func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "missing.yml") + _, stderr, exitCode := runLitestreamMain(t, "databases", "-config", configPath) + if exitCode != 1 { + t.Fatalf("exit code=%d, want 1", exitCode) + } + if strings.Contains(stderr, "flags must come after the subcommand") { + t.Fatalf("correctly positioned flag reported as misplaced:\n%s", stderr) + } + }) } func TestMainHarness(t *testing.T) { diff --git a/cmd/litestream/main_test.go b/cmd/litestream/main_test.go index 6f850ad88..64f73c9b7 100644 --- a/cmd/litestream/main_test.go +++ b/cmd/litestream/main_test.go @@ -53,46 +53,11 @@ func TestMain_RunHelp(t *testing.T) { t.Fatalf("Run returned error %v, want %v", err, flag.ErrHelp) } }) -} - -func TestMain_RunMisplacedFlags(t *testing.T) { - const want = "flags must come after the subcommand" - - t.Run("FlagBeforeCommand", func(t *testing.T) { - for _, args := range [][]string{ - {"-config", "litestream.yml", "databases"}, - {"-config", "litestream.yml"}, - } { - var err error - stdout := captureStdout(t, func() { - err = main.NewMain().Run(context.Background(), args) - }) - if err == nil || err.Error() != want { - t.Fatalf("Run(%v) returned error %v, want %q", args, err, want) - } else if stdout != "" { - t.Fatalf("Run(%v) printed usage on stdout:\n%s", args, stdout) - } - } - }) - - t.Run("HelpFlagStillPrintsUsage", func(t *testing.T) { - for _, arg := range []string{"-h", "-help", "--help"} { - var err error - stdout := captureStdout(t, func() { - err = main.NewMain().Run(context.Background(), []string{arg}) - }) - if err != nil { - t.Fatalf("Run(%s) returned error: %v", arg, err) - } else if !strings.Contains(stdout, "litestream [arguments]") { - t.Fatalf("Run(%s) did not print usage, got:\n%s", arg, stdout) - } - } - }) - t.Run("FlagAfterCommand", func(t *testing.T) { - err := main.NewMain().Run(context.Background(), []string{"databases", "-config", filepath.Join(t.TempDir(), "missing.yml")}) - if err == nil || err.Error() == want { - t.Fatalf("Run returned error %v, want config error", err) + t.Run("MisplacedFlag", func(t *testing.T) { + err := main.NewMain().Run(context.Background(), []string{"-config", "litestream.yml"}) + if err == nil || err.Error() != "flags must come after the subcommand" { + t.Fatalf("Run returned error %v, want misplaced flag error", err) } }) }