From 23dc6dd49db2080fe52b5f3feef6f496682560a1 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 07:48:12 +0000 Subject: [PATCH] Give -v to verbose, move --version to -V (closes #64) urfave/cli's built-in version flag claimed -v by default, colliding with the -v verbose alias used on every subcommand; "mfer -v --version" failed to parse with an internal-sounding parser error. Verbose is the more common meaning of -v in tools that offer both, so verbose now keeps -v at the root and on every subcommand, and the version flag takes the capital -V (--version still works). User-visible change: -v alone now prints help with verbose logging rather than the version; use -V or --version for the version. The --version flag and the version subcommand now share one printer, so they emit identical output. Adds tests covering -v, --verbose, --version, -v --version, and --verbose --version (exit code and output). Model: opus-4-8 --- TODO.md | 2 ++ internal/cli/entry_test.go | 66 +++++++++++++++++++++++++++++++++++++- internal/cli/mfer.go | 27 ++++++++++++++-- 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/TODO.md b/TODO.md index 06f9dba..dcea4fd 100644 --- a/TODO.md +++ b/TODO.md @@ -24,6 +24,8 @@ only thing left of the `chore/align-repo-policies` branch is the list below. # 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-08-09: added `.prettierrc`/`.prettierignore`, gave `script/fmt` and `script/fmt-check` one shared prettier file set via `script/prettier`, dropped the `|| true` that hid prettier failures, and added a node-based Dockerfile diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index 61868ac..376cbd8 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -27,6 +27,7 @@ const ( testManifest = "/manifest.mf" testFlagBase = "--base" testFlagNoExtra = "--no-extra-files" + testFlagVersion = "--version" ) var errSimulatedWrite = errors.New("simulated write failure") @@ -102,7 +103,7 @@ func TestVersionCommand(t *testing.T) { t.Parallel() fs := afero.NewMemMapFs() - opts := testOpts([]string{testApp, "version"}, fs) + opts := testOpts([]string{testApp, cmdVersion}, fs) exitCode := runCLI(opts) @@ -113,6 +114,69 @@ func TestVersionCommand(t *testing.T) { 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) { t.Parallel() diff --git a/internal/cli/mfer.go b/internal/cli/mfer.go index e9f2e26..55ec59f 100644 --- a/internal/cli/mfer.go +++ b/internal/cli/mfer.go @@ -18,6 +18,7 @@ const ( cmdGenerate = "generate" cmdCheck = "check" cmdExport = "export" + cmdVersion = "version" flagProgress = "progress" @@ -67,6 +68,12 @@ func (mfa *CLIApp) VersionString() string { 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() { if log.GetLevel() <= log.InfoLevel { _, _ = fmt.Fprintln(mfa.Stdout, banner) @@ -265,10 +272,10 @@ func (mfa *CLIApp) exportCommand() *cli.Command { func (mfa *CLIApp) versionCommand() *cli.Command { return &cli.Command{ - Name: "version", + Name: cmdVersion, Usage: "Show version", Action: func(_ *cli.Context) error { - _, _ = fmt.Fprintln(mfa.Stdout, mfa.VersionString()) + mfa.printVersion() return nil }, @@ -324,6 +331,20 @@ func (mfa *CLIApp) run(args []string) { log.SetOutput(mfa.Stdout, mfa.Stderr) 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{ Name: mfa.appname, Usage: "Manifest generator", @@ -331,11 +352,13 @@ func (mfa *CLIApp) run(args []string) { EnableBashCompletion: true, Writer: mfa.Stdout, ErrWriter: mfa.Stderr, + Flags: commonFlags(), Action: func(c *cli.Context) error { if c.Args().Len() > 0 { return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First()) } + mfa.setVerbosity(c) mfa.printBanner() return cli.ShowAppHelp(c)