From 7088857692ca042414b7f37d28739eb5bc24ded0 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 11:48:52 +0200 Subject: [PATCH] Count -v once and document -v -v for debug output (closes #125) urfave/cli before v2.25.5 counted a flag given by its alias twice, so one -v or --verbose already gave debug output. Bump it to v2.27.7, which counts it once: one -v gives verbose output, two give debug. The -v help text now says -v -v instead of -vv, which stays refused: urfave/cli's option for combined short flags would let a flag that takes a value read the next letter as its value. -v and --verbose together stay refused: urfave/cli v2 refuses a flag given under two of its names, and one flag with an alias keeps help and parsing simple. The bump changes some help output; generate and fetch now name their arguments. Tests start each run at the default log level. Model: opus-5-5 --- go.mod | 6 +- go.sum | 12 ++-- internal/cli/entry_test.go | 109 +++++++++++++++++++++++++++++++++++-- internal/cli/mfer.go | 17 +++--- 4 files changed, 123 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index 6101001..0c4e529 100644 --- a/go.mod +++ b/go.mod @@ -12,13 +12,13 @@ require ( github.com/pterm/pterm v0.12.35 github.com/spf13/afero v1.8.0 github.com/stretchr/testify v1.8.1 - github.com/urfave/cli/v2 v2.23.6 + github.com/urfave/cli/v2 v2.27.7 google.golang.org/protobuf v1.28.1 ) require ( github.com/atomicgo/cursor v0.0.1 // indirect - github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect + github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect github.com/fatih/color v1.7.0 // indirect github.com/gookit/color v1.4.2 // indirect github.com/klauspost/cpuid/v2 v2.0.9 // indirect @@ -34,7 +34,7 @@ require ( github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/spaolacci/murmur3 v1.1.0 // indirect github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778 // indirect - github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect + github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 // indirect golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e // indirect golang.org/x/sys v0.1.0 // indirect golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect diff --git a/go.sum b/go.sum index 534670d..33429c5 100644 --- a/go.sum +++ b/go.sum @@ -61,8 +61,8 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk= github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk= -github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= -github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo= +github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -231,12 +231,12 @@ github.com/tj/go-buffer v1.1.0/go.mod h1:iyiJpfFcR2B9sXu7KvjbT9fpM4mOelRSDTbntVj github.com/tj/go-elastic v0.0.0-20171221160941-36157cbbebc2/go.mod h1:WjeM0Oo1eNAjXGDx2yma7uG2XoyRZTq1uv3M/o7imD0= github.com/tj/go-kinesis v0.0.0-20171128231115-08b17f58cb1b/go.mod h1:/yhzCV0xPfx6jb1bBgRFjl5lytqVqZXEaeqWP8lTEao= github.com/tj/go-spin v1.1.0/go.mod h1:Mg1mzmePZm4dva8Qz60H2lHwmJ2loum4VIrLgVnKwh4= -github.com/urfave/cli/v2 v2.23.6 h1:iWmtKD+prGo1nKUtLO0Wg4z9esfBM4rAV4QRLQiEmJ4= -github.com/urfave/cli/v2 v2.23.6/go.mod h1:GHupkWPMM0M/sj1a2b4wUrWBPzazNrIjouW6fmdJLxc= +github.com/urfave/cli/v2 v2.27.7 h1:bH59vdhbjLv3LAvIu6gd0usJHgoTTPhCFib8qqOwXYU= +github.com/urfave/cli/v2 v2.27.7/go.mod h1:CyNAG/xg+iAOg0N4MPGZqVmv2rCoP267496AOXUZjA4= github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778 h1:QldyIu/L63oPpyvQmHgvgickp1Yw510KJOqX7H24mg8= github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778/go.mod h1:2MuV+tbUrU1zIOPMxZ5EncGwgmMJsa+9ucAQZXxsObs= -github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU= -github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8= +github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 h1:gEOO8jv9F4OT7lGCjxCBTO/36wtF6j2nSip77qHd4x4= +github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1/go.mod h1:Ohn+xnUBiLI6FVj/9LpzZWtj1/D6lUovWYBkxHVV3aM= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index 8169b8d..1489606 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -8,6 +8,8 @@ import ( "io" "math/rand" "os" + "slices" + "strings" "sync" "testing" @@ -30,6 +32,7 @@ const ( testFlagBase = "--base" testFlagNoExtra = "--no-extra-files" testFlagVersion = "--version" + testFlagVerbose = "--verbose" ) var errSimulatedWrite = errors.New("simulated write failure") @@ -42,20 +45,32 @@ var errSimulatedWrite = errors.New("simulated write failure") var runMu sync.Mutex // runCLI invokes RunWithOptions while holding runMu so parallel tests -// capture their own output. Before releasing the lock it points the -// process-global logger at io.Discard: other tests log outside the lock -// (manifest loads, scans), and those lines must not land in this run's -// buffers once it has returned and its test is reading them. +// capture their own output, and returns its exit code. func runCLI(opts *RunOptions) int { + exitCode, _ := runCLIWithLevel(opts) + + return exitCode +} + +// runCLIWithLevel is runCLI that also returns the log level the run left +// set, read while runMu still keeps other runs from changing it. Each run +// starts at the default level, as a new process does. Before releasing the +// lock it points the process-global logger at io.Discard: other tests log +// outside the lock (manifest loads, scans), and those lines must not land in +// this run's buffers once it has returned and its test is reading them. +func runCLIWithLevel(opts *RunOptions) (int, log.Level) { runMu.Lock() defer runMu.Unlock() + log.SetLevel(log.InfoLevel) + exitCode := RunWithOptions(opts) + level := log.GetLevel() log.SetOutput(io.Discard, io.Discard) log.Init() - return exitCode + return exitCode, level } func TestMain(m *testing.M) { @@ -235,6 +250,90 @@ func TestRootVerbosityFlags(t *testing.T) { }) } +// commandsTakingVerbose returns the command lines -v can follow: the root and +// the generate, check, freshen and fetch subcommands. +func commandsTakingVerbose() [][]string { + return [][]string{ + {testApp}, + {testApp, cmdGenerate}, + {testApp, cmdCheck}, + {testApp, cmdFreshen}, + {testApp, cmdFetch}, + } +} + +// TestVerboseCount asserts that one -v or --verbose gives verbose output and +// two -v give debug output (issue #125). urfave/cli before v2.25.5 counted a +// flag given by its alias twice, so one -v gave debug output. +func TestVerboseCount(t *testing.T) { + t.Parallel() + + cases := []struct { + flags []string + want log.Level + }{ + {[]string{"-v"}, log.VerboseLevel}, + {[]string{testFlagVerbose}, log.VerboseLevel}, + {[]string{"-v", "-v"}, log.DebugLevel}, + } + + for _, command := range commandsTakingVerbose() { + for _, tc := range cases { + args := slices.Concat(command, tc.flags) + + t.Run(strings.Join(args, " "), func(t *testing.T) { + t.Parallel() + + _, level := runCLIWithLevel(testOpts(args, afero.NewMemMapFs())) + assert.Equal(t, tc.want, level) + }) + } + } +} + +// TestCombinedShortVerboseRefused asserts that -vv is refused (issue #125): +// single-letter flags do not combine, so the -v help text says -v -v. +func TestCombinedShortVerboseRefused(t *testing.T) { + t.Parallel() + + for _, command := range commandsTakingVerbose() { + args := slices.Concat(command, []string{"-vv"}) + + t.Run(strings.Join(args, " "), func(t *testing.T) { + t.Parallel() + + opts := testOpts(args, afero.NewMemMapFs()) + exitCode, level := runCLIWithLevel(opts) + + assert.Equal(t, 1, exitCode) + assert.Contains(t, testStderr(t, opts), "flag provided but not defined: -vv") + assert.Equal(t, log.InfoLevel, level) + }) + } +} + +// TestShortAndLongVerboseRefused asserts that -v and --verbose given together +// are refused (issue #125): urfave/cli v2 refuses a flag given under two of its +// names, and one flag with an alias keeps help and parsing simple. +func TestShortAndLongVerboseRefused(t *testing.T) { + t.Parallel() + + for _, command := range commandsTakingVerbose() { + args := slices.Concat(command, []string{"-v", testFlagVerbose}) + + t.Run(strings.Join(args, " "), func(t *testing.T) { + t.Parallel() + + opts := testOpts(args, afero.NewMemMapFs()) + exitCode, level := runCLIWithLevel(opts) + + assert.Equal(t, 1, exitCode) + assert.Contains(t, testStderr(t, opts), "Cannot use two forms of the same flag") + assert.Equal(t, log.InfoLevel, level) + }) + } +} + func TestHelpCommand(t *testing.T) { t.Parallel() diff --git a/internal/cli/mfer.go b/internal/cli/mfer.go index ccd878a..a2f8739 100644 --- a/internal/cli/mfer.go +++ b/internal/cli/mfer.go @@ -17,6 +17,7 @@ import ( const ( cmdGenerate = "generate" cmdCheck = "check" + cmdFreshen = "freshen" cmdExport = "export" cmdFetch = "fetch" cmdVersion = "version" @@ -118,7 +119,7 @@ func commonFlags() []cli.Flag { &cli.BoolFlag{ Name: "verbose", Aliases: []string{"v"}, - Usage: "Increase verbosity (-v for verbose, -vv for debug)", + Usage: "Increase verbosity (-v for verbose, -v -v for debug)", Count: new(int), }, &cli.BoolFlag{ @@ -131,9 +132,10 @@ func commonFlags() []cli.Flag { func (mfa *CLIApp) generateCommand() *cli.Command { return &cli.Command{ - Name: cmdGenerate, - Aliases: []string{"gen"}, - Usage: "Generate manifest file", + Name: cmdGenerate, + Aliases: []string{"gen"}, + Usage: "Generate manifest file", + ArgsUsage: "[path ...]", Action: func(c *cli.Context) error { mfa.setVerbosity(c) mfa.printBanner() @@ -227,7 +229,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command { func (mfa *CLIApp) freshenCommand() *cli.Command { return &cli.Command{ - Name: "freshen", + Name: cmdFreshen, Usage: "Update manifest with changed, new, and removed files", ArgsUsage: manifestArgsUsage, Action: func(c *cli.Context) error { @@ -324,8 +326,9 @@ func (mfa *CLIApp) listCommand() *cli.Command { func (mfa *CLIApp) fetchCommand() *cli.Command { return &cli.Command{ - Name: cmdFetch, - Usage: "fetch manifest and referenced files", + Name: cmdFetch, + Usage: "fetch manifest and referenced files", + ArgsUsage: "URL", Action: func(c *cli.Context) error { mfa.setVerbosity(c) mfa.printBanner()