Give -v to verbose, move --version to -V (closes #64)
check / check (push) Successful in 1m18s
check / check (push) Successful in 1m18s
urfave/cli's built-in version flag claimed -v, colliding with the -v verbose alias on every subcommand, so "mfer -v --version" failed with a parser error. Verbose is the more common meaning of -v in tools that offer both, so verbose keeps -v at the root and on every subcommand, and the version flag takes -V (--version still works). User-visible change: -v alone now prints help with verbose logging, not the version; use -V or --version. The --version flag and the version subcommand share one printer, so their output is identical. Tests cover -v, --verbose, --version, -V, -v --version and --verbose --version. The README roadmap item for --version is checked off. Model: opus-4-8 (implementation); opus-5-5 (rebase)
This commit is contained in:
@@ -417,8 +417,8 @@ proto `go_package` option. Which is canonical?
|
|||||||
matching Scanner
|
matching Scanner
|
||||||
- [ ] Add `--deterministic` flag or make it default — omit `createdAt`, sort
|
- [ ] Add `--deterministic` flag or make it default — omit `createdAt`, sort
|
||||||
files (pending design question answer)
|
files (pending design question answer)
|
||||||
- [ ] Wire `--version` flag properly (currently only a `version` subcommand
|
- [x] Wire `--version` flag properly: `--version` and `-V` print the same line
|
||||||
exists; top-level `--version` shows urfave/cli generic output)
|
as the `version` subcommand, and `-v` means verbose
|
||||||
- [ ] Add retry logic to `fetch` — currently no retries on transient HTTP
|
- [ ] Add retry logic to `fetch` — currently no retries on transient HTTP
|
||||||
errors; needs exponential backoff
|
errors; needs exponential backoff
|
||||||
- [ ] `fetch` command uses bare `http.Get` with no timeout — needs `http.Client`
|
- [ ] `fetch` command uses bare `http.Get` with no timeout — needs `http.Client`
|
||||||
|
|||||||
@@ -24,6 +24,8 @@ only thing left of the `chore/align-repo-policies` branch is the list below.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-09-21: fixed the `-v` collision between `--verbose` and `--version`;
|
||||||
|
verbose owns `-v`, and version answers to `--version` and `-V` (#64)
|
||||||
- 2026-10-03: pinned the CLI error messages by driving the functions that emit
|
- 2026-10-03: pinned the CLI error messages by driving the functions that emit
|
||||||
them in `internal/cli/errmsg_test.go`, and made the freshen mtime-presence
|
them in `internal/cli/errmsg_test.go`, and made the freshen mtime-presence
|
||||||
test distinguish an absent mtime from the epoch (#87)
|
test distinguish an absent mtime from the epoch (#87)
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ const (
|
|||||||
testManifest = "/manifest.mf"
|
testManifest = "/manifest.mf"
|
||||||
testFlagBase = "--base"
|
testFlagBase = "--base"
|
||||||
testFlagNoExtra = "--no-extra-files"
|
testFlagNoExtra = "--no-extra-files"
|
||||||
|
testFlagVersion = "--version"
|
||||||
)
|
)
|
||||||
|
|
||||||
var errSimulatedWrite = errors.New("simulated write failure")
|
var errSimulatedWrite = errors.New("simulated write failure")
|
||||||
@@ -102,7 +103,7 @@ func TestVersionCommand(t *testing.T) {
|
|||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
fs := afero.NewMemMapFs()
|
fs := afero.NewMemMapFs()
|
||||||
opts := testOpts([]string{testApp, "version"}, fs)
|
opts := testOpts([]string{testApp, cmdVersion}, fs)
|
||||||
|
|
||||||
exitCode := runCLI(opts)
|
exitCode := runCLI(opts)
|
||||||
|
|
||||||
@@ -113,6 +114,69 @@ func TestVersionCommand(t *testing.T) {
|
|||||||
assert.Contains(t, stdout, "abc123")
|
assert.Contains(t, stdout, "abc123")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestVFlagCollision covers the -v/--verbose vs --version flag interaction
|
||||||
|
// (issue #64). Verbose owns -v; version answers to --version and -V. None of
|
||||||
|
// these invocations may produce a parser error, and the two ways of asking
|
||||||
|
// for the version must print the same thing.
|
||||||
|
func TestVFlagCollision(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// Invocations that must print the version and exit 0.
|
||||||
|
versionCases := map[string][]string{
|
||||||
|
"long version flag": {testApp, testFlagVersion},
|
||||||
|
"short version flag": {testApp, "-V"},
|
||||||
|
"verbose then version": {testApp, "-v", testFlagVersion},
|
||||||
|
"long verbose and version": {testApp, "--verbose", testFlagVersion},
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, args := range versionCases {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
opts := testOpts(args, afero.NewMemMapFs())
|
||||||
|
exitCode := runCLI(opts)
|
||||||
|
|
||||||
|
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
|
||||||
|
assert.Contains(t, testStdout(t, opts), mfer.Version)
|
||||||
|
assert.NotContains(t, testStderr(t, opts), "two forms of the same flag")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// Invocations that must enable verbose and exit 0 without a parser error.
|
||||||
|
verboseCases := map[string][]string{
|
||||||
|
"short verbose flag": {testApp, "-v"},
|
||||||
|
"long verbose flag": {testApp, "--verbose"},
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, args := range verboseCases {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
opts := testOpts(args, afero.NewMemMapFs())
|
||||||
|
exitCode := runCLI(opts)
|
||||||
|
|
||||||
|
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
|
||||||
|
assert.Contains(t, testStdout(t, opts), cmdGenerate,
|
||||||
|
"root should show help listing subcommands")
|
||||||
|
assert.Empty(t, testStderr(t, opts))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestVersionFlagAndCommandMatch asserts that "mfer --version" and
|
||||||
|
// "mfer version" produce identical output (issue #64).
|
||||||
|
func TestVersionFlagAndCommandMatch(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
flagOpts := testOpts([]string{testApp, testFlagVersion}, afero.NewMemMapFs())
|
||||||
|
require.Equal(t, 0, runCLI(flagOpts))
|
||||||
|
|
||||||
|
cmdOpts := testOpts([]string{testApp, cmdVersion}, afero.NewMemMapFs())
|
||||||
|
require.Equal(t, 0, runCLI(cmdOpts))
|
||||||
|
|
||||||
|
assert.Equal(t, testStdout(t, flagOpts), testStdout(t, cmdOpts))
|
||||||
|
}
|
||||||
|
|
||||||
func TestHelpCommand(t *testing.T) {
|
func TestHelpCommand(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
+25
-2
@@ -19,6 +19,7 @@ const (
|
|||||||
cmdCheck = "check"
|
cmdCheck = "check"
|
||||||
cmdExport = "export"
|
cmdExport = "export"
|
||||||
cmdFetch = "fetch"
|
cmdFetch = "fetch"
|
||||||
|
cmdVersion = "version"
|
||||||
|
|
||||||
flagProgress = "progress"
|
flagProgress = "progress"
|
||||||
|
|
||||||
@@ -68,6 +69,12 @@ func (mfa *CLIApp) VersionString() string {
|
|||||||
return mfer.Version
|
return mfer.Version
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// printVersion writes the version line shared by the --version flag and the
|
||||||
|
// version subcommand, so both produce identical output.
|
||||||
|
func (mfa *CLIApp) printVersion() {
|
||||||
|
_, _ = fmt.Fprintf(mfa.Stdout, "%s version %s\n", mfa.appname, mfa.VersionString())
|
||||||
|
}
|
||||||
|
|
||||||
func (mfa *CLIApp) printBanner() {
|
func (mfa *CLIApp) printBanner() {
|
||||||
if log.GetLevel() <= log.InfoLevel {
|
if log.GetLevel() <= log.InfoLevel {
|
||||||
_, _ = fmt.Fprintln(mfa.Stdout, banner)
|
_, _ = fmt.Fprintln(mfa.Stdout, banner)
|
||||||
@@ -266,10 +273,10 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
|
|||||||
|
|
||||||
func (mfa *CLIApp) versionCommand() *cli.Command {
|
func (mfa *CLIApp) versionCommand() *cli.Command {
|
||||||
return &cli.Command{
|
return &cli.Command{
|
||||||
Name: "version",
|
Name: cmdVersion,
|
||||||
Usage: "Show version",
|
Usage: "Show version",
|
||||||
Action: func(_ *cli.Context) error {
|
Action: func(_ *cli.Context) error {
|
||||||
_, _ = fmt.Fprintln(mfa.Stdout, mfa.VersionString())
|
mfa.printVersion()
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
@@ -325,6 +332,20 @@ func (mfa *CLIApp) run(args []string) {
|
|||||||
log.SetOutput(mfa.Stdout, mfa.Stderr)
|
log.SetOutput(mfa.Stdout, mfa.Stderr)
|
||||||
log.Init()
|
log.Init()
|
||||||
|
|
||||||
|
// -v means verbose, not version. urfave/cli's built-in version flag
|
||||||
|
// claims -v by default, colliding with the -v verbose alias used here and
|
||||||
|
// on every subcommand; that collision makes "mfer -v --version" fail to
|
||||||
|
// parse. Verbose is the more common meaning of -v in tools that offer
|
||||||
|
// both, so verbose keeps -v and the version flag takes the capital -V.
|
||||||
|
// VersionFlag and VersionPrinter are urfave/cli package globals; run() is
|
||||||
|
// serialized in tests, so assigning them here is safe.
|
||||||
|
cli.VersionFlag = &cli.BoolFlag{
|
||||||
|
Name: cmdVersion,
|
||||||
|
Aliases: []string{"V"},
|
||||||
|
Usage: "print the version",
|
||||||
|
}
|
||||||
|
cli.VersionPrinter = func(_ *cli.Context) { mfa.printVersion() }
|
||||||
|
|
||||||
mfa.app = &cli.App{
|
mfa.app = &cli.App{
|
||||||
Name: mfa.appname,
|
Name: mfa.appname,
|
||||||
Usage: "Manifest generator",
|
Usage: "Manifest generator",
|
||||||
@@ -332,11 +353,13 @@ func (mfa *CLIApp) run(args []string) {
|
|||||||
EnableBashCompletion: true,
|
EnableBashCompletion: true,
|
||||||
Writer: mfa.Stdout,
|
Writer: mfa.Stdout,
|
||||||
ErrWriter: mfa.Stderr,
|
ErrWriter: mfa.Stderr,
|
||||||
|
Flags: commonFlags(),
|
||||||
Action: func(c *cli.Context) error {
|
Action: func(c *cli.Context) error {
|
||||||
if c.Args().Len() > 0 {
|
if c.Args().Len() > 0 {
|
||||||
return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First())
|
return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
mfa.setVerbosity(c)
|
||||||
mfa.printBanner()
|
mfa.printBanner()
|
||||||
|
|
||||||
return cli.ShowAppHelp(c)
|
return cli.ShowAppHelp(c)
|
||||||
|
|||||||
Reference in New Issue
Block a user