cli: explain misplaced flags before the subcommand - #1453
Conversation
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 benbjohnson#1405 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
corylanou
left a comment
There was a problem hiding this comment.
Thanks for addressing the bare usage output here. I agree with the behavior goal, but I don't think we should reconstruct the invocation by scanning for a hard-coded command name.
The helper is not the parser, so it cannot reliably distinguish a command from a flag value. For example:
$ litestream -socket status info
Error: flags must come after the subcommand
Try: litestream status -socket infoHere, status can be the value of -socket and info the intended command, but the suggestion reverses that interpretation. There are related correctness issues:
- A config path containing spaces is joined without shell quoting, so the result is not pasteable.
litestream -config c.yml versionsuggestslitestream version -config c.yml, althoughversiondoes not accept-config.- The command names already exist in the dispatcher and usage text. This adds another list that must remain synchronized; #1380 is currently adding the
mcpcommand.
Could we keep this PR focused on producing a targeted, correct diagnostic without trying to infer and rewrite the full command? Something like:
} else if strings.HasPrefix(cmd, "-") {
return misplacedFlagError(cmd)
}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 <command> -config PATH\n" +
" or: LITESTREAM_CONFIG=PATH litestream <command>",
}
}
return &usageError{
message: "flags must come after the subcommand",
hint: "litestream <command> [flags]",
}
}That removes commandNames, slices, and the token scanner while still fixing #1405 and pointing alias users to LITESTREAM_CONFIG. It also cannot emit a confidently incorrect command.
The CLI tests can cover -config PATH, --config=PATH, another leading flag, explicit help, no arguments, and a correctly positioned flag. I would avoid asserting reconstructed user arguments.
If we want litestream -config ./c.yml databases to actually work, I would prefer a separate change that makes -config a real root flag using a top-level flag.FlagSet. Go's parser stops at the first non-flag argument, so it can parse root flags, select the subcommand, and leave the remaining arguments for the subcommand parser: https://pkg.go.dev/flag#hdr-Command_line_flag_syntax
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) <noreply@anthropic.com>
|
Rewritten to your design in a3e8342. The second one is your example, and it is now a test case. Since the hint no longer names a command, the ambiguity you pointed out cannot be resolved wrongly, and the quoting and
I have updated the PR description, which still described the scanner. One input this PR changes that we have not discussed, in case you want it handled differently:
Not addressed here, per your review: making
|
Summary
Replace the bare usage screen printed for
litestream -config c.yml databaseswith a targeted error that says flags go after the subcommand, and points-configusers atLITESTREAM_CONFIG.Problem
Main.Run()takesargs[0]as the subcommand before any flag parser runs. A leading flag falls into them.Usage(); return flag.ErrHelpbranch, so the user gets the full command list on stdout, exit 1, and nothing on stderr. Users read that as "the flag does not exist" (#1405, #1400).Before (origin/main 63225f1):
After:
Any other leading flag gets the neutral form:
Solution
In
Run(), a first token that starts with-and is not-h/-help/--helpnow returns ausageError(the existingError:/Try:mechanism from #1297) instead of printing usage.misplacedFlagErrorlooks only at that token: if it namesconfigit adds anor:line usingLITESTREAM_CONFIG, which is what solves the shell-alias case in #1400; otherwise the hint is a fixed form.The hint deliberately does not try to reconstruct the user's command.
Run()is not the flag parser, so it cannot tell a command from a flag value: inlitestream -socket status info,statusmay be the value of-socketandinfothe intended command. An earlier revision of this PR scanned for a known command name and reordered the arguments, which reversed that case and needed its own copy of the command list. Empty args,-h/-help/--help,help, unknown commands, and flags after the subcommand behave exactly as before.Scope
In scope:
cmd/litestream/main.go: split the existing branch inRun(), plusmisplacedFlagErrormain_test.goand subprocess CLI tests inmain_cli_test.goNot in scope:
-configa real root flag with a top-levelflag.FlagSetis a separate change.LITESTREAM_CONFIGmore prominently on the website (separate repo)Test Plan
TestMainFlagPlacementcovers-config PATH,--config=PATH, another leading flag,-h/-help/--help, no arguments, and a correctly positioned flag. All pass locally (macOS, Go 1.26;staticcheckandgoimportsclean,uvx pre-commit run --all-filesclean).Related
This change was made with AI assistance (Claude Code) and reviewed by the author.