From ed40bdb0f5c6f78be104db079863b386a8a850a0 Mon Sep 17 00:00:00 2001 From: Bob Badour Date: Fri, 21 Nov 2025 18:23:57 +0000 Subject: [PATCH 1/4] Make externally-defined flags more robust. Use delegation to implement the Value+Flag paradigm used for other flag types. Implement all of the Flag interfaces except Count. Borrowed heavily from flag_bool.go and flag_generic.go Add context to the error message when setting an external flag's value to its default value to avoid cryptic error messages like: `"syntax error: expected file.go:234"` And suppress the error for odd-ball external flags that report the string representation of the zero-value of some structure as the default, but do not accept that string as input. Detect such odd-balls by their `Get()` method returning `nil`. --- command_setup.go | 2 +- flag_ext.go | 159 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 157 insertions(+), 4 deletions(-) diff --git a/command_setup.go b/command_setup.go index cac4a30314..7c7776c2d2 100644 --- a/command_setup.go +++ b/command_setup.go @@ -66,7 +66,7 @@ func (cmd *Command) setupDefaults(osArgs []string) { flag.VisitAll(func(f *flag.Flag) { // skip test flags if !strings.HasPrefix(f.Name, ignoreFlagPrefix) { - cmd.Flags = append(cmd.Flags, &extFlag{f}) + cmd.Flags = append(cmd.Flags, &extFlag{f, ""}) } }) } diff --git a/flag_ext.go b/flag_ext.go index 9972af7c56..16e8966df8 100644 --- a/flag_ext.go +++ b/flag_ext.go @@ -1,14 +1,72 @@ package cli -import "flag" +import ( + "context" + "flag" + "fmt" + "reflect" + "strings" +) + +var _ Value = (*externalValue)(nil) + +// -- Value Value +type externalValue struct { + e *extFlag +} + +// Below functions are to satisfy the flag.Value interface + +func (ev *externalValue) Set(s string) error { + if ev != nil && ev.e.f != nil { + return ev.e.f.Value.Set(s) + } + return nil +} + +func (ev *externalValue) Get() any { + if ev != nil && ev.e.f != nil { + return ev.e.f.Value.(flag.Getter).Get() + } + return nil +} + +func (ev *externalValue) String() string { + if ev != nil && ev.e.f != nil { + return ev.e.String() + } + return "" +} + +func (ev *externalValue) IsBoolFlag() bool { + if ev == nil || ev.e.f == nil { + return false + } + bf, ok := ev.e.f.Value.(boolFlag) + return ok && bf.IsBoolFlag() +} + +var _ Flag = (*extFlag)(nil) +var _ ActionableFlag = (*extFlag)(nil) +var _ CategorizableFlag = (*extFlag)(nil) +var _ DocGenerationFlag = (*extFlag)(nil) +var _ DocGenerationMultiValueFlag = (*extFlag)(nil) +var _ LocalFlag = (*extFlag)(nil) +var _ RequiredFlag = (*extFlag)(nil) +var _ VisibleFlag = (*extFlag)(nil) type extFlag struct { - f *flag.Flag + f *flag.Flag + category string } func (e *extFlag) PreParse() error { if e.f.DefValue != "" { - return e.Set("", e.f.DefValue) + // suppress errors for write-only external flags that always return nil + if err := e.Set("", e.f.DefValue); err != nil && e.f.Value.(flag.Getter).Get() != nil { + // wrap error with some context for the user + return fmt.Errorf("external flag --%s default %q: %w", e.f.Name, e.f.DefValue, err) + } } return nil @@ -30,6 +88,43 @@ func (e *extFlag) Names() []string { return []string{e.f.Name} } +// IsBoolFlag returns whether the flag doesn't need to accept args +func (e *extFlag) IsBoolFlag() bool { + if e == nil || e.f == nil { + return false + } + return (&externalValue{e}).IsBoolFlag() +} + +// IsDefaultVisible returns true if the flag is not hidden, otherwise false +func (e *extFlag) IsDefaultVisible() bool { + return true +} + +// IsLocal returns false if flag needs to be persistent across subcommands +func (e *extFlag) IsLocal() bool { + return false +} + +// IsMultiValueFlag returns true if the value type T can take multiple +// values from cmd line. This is true for slice and map type flags +func (e *extFlag) IsMultiValueFlag() bool { + if e == nil || e.f == nil { + return false + } + // TBD how to specify + if reflect.TypeOf(e.f.Value) == nil { + return false + } + kind := reflect.TypeOf(e.f.Value).Kind() + return kind == reflect.Slice || kind == reflect.Map +} + +// IsRequired returns whether or not the flag is required +func (e *extFlag) IsRequired() bool { + return false +} + func (e *extFlag) IsSet() bool { return false } @@ -61,3 +156,61 @@ func (e *extFlag) GetDefaultText() string { func (e *extFlag) GetEnvVars() []string { return nil } + +// RunAction executes flag action if set +func (e *extFlag) RunAction(ctx context.Context, cmd *Command) error { + return nil +} + +// TypeName returns the type of the flag. +func (e *extFlag) TypeName() string { + if e == nil || e.f == nil { + return "" + } + ty := reflect.TypeOf(e.f.Value) + if ty == nil { + return "" + } + // convert the typename to generic type + convertToGenericType := func(name string) string { + prefixMap := map[string]string{ + "float": "float", + "int": "int", + "uint": "uint", + } + for prefix, genericType := range prefixMap { + if strings.HasPrefix(name, prefix) { + return genericType + } + } + return strings.ToLower(name) + } + + switch ty.Kind() { + // if it is a Slice, then return the slice's inner type. Will nested slices be used in the future? + case reflect.Slice: + elemType := ty.Elem() + return convertToGenericType(elemType.Name()) + // if it is a Map, then return the map's key and value types. + case reflect.Map: + keyType := ty.Key() + valueType := ty.Elem() + return fmt.Sprintf("%s=%s", convertToGenericType(keyType.Name()), convertToGenericType(valueType.Name())) + default: + return convertToGenericType(ty.Name()) + } +} + +// GetCategory returns the category of the flag +func (e *extFlag) GetCategory() string { + if e == nil { + return "" + } + return e.category +} + +func (e *extFlag) SetCategory(c string) { + if e != nil { + e.category = c + } +} From c0f321bbc416bb3ec68f0563ec78fb6d98e01c51 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:10:18 +0000 Subject: [PATCH 2/4] chore(deps): bump actions/checkout from 5 to 6 Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/lint.yml | 2 +- .github/workflows/publish-docs.yml | 4 ++-- .github/workflows/test.yml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index ee856959a6..3f7044e5d2 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -19,7 +19,7 @@ jobs: steps: - name: Clone repository - uses: actions/checkout@v5 + uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index 4033d9caf9..9894cec67a 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -15,7 +15,7 @@ jobs: name: test-docs runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 @@ -41,7 +41,7 @@ jobs: needs: [test-docs] runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 with: fetch-depth: 0 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ed47879cbb..f45a1a19aa 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,7 +24,7 @@ jobs: runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 From f9ae5da5dc78c5db2793a19bbbd538f2527f17f3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Nov 2025 17:28:24 +0000 Subject: [PATCH 3/4] chore(deps): bump mkdocs-material in the python-packages group Bumps the python-packages group with 1 update: [mkdocs-material](https://github.com/squidfunk/mkdocs-material). Updates `mkdocs-material` from 9.6.23 to 9.7.0 - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](https://github.com/squidfunk/mkdocs-material/compare/9.6.23...9.7.0) --- updated-dependencies: - dependency-name: mkdocs-material dependency-version: 9.7.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: python-packages ... Signed-off-by: dependabot[bot] --- mkdocs-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mkdocs-requirements.txt b/mkdocs-requirements.txt index 5f3a0c3643..68ec3cada0 100644 --- a/mkdocs-requirements.txt +++ b/mkdocs-requirements.txt @@ -1,5 +1,5 @@ mkdocs-git-revision-date-localized-plugin==1.5.0 -mkdocs-material==9.6.23 +mkdocs-material==9.7.0 mkdocs==1.6.1 mkdocs-redirects==1.2.2 pygments==2.19.2 From 33f33f43e7d8820a08db6470964bfb3f5c5fa8c0 Mon Sep 17 00:00:00 2001 From: Bob Badour Date: Wed, 26 Nov 2025 21:22:52 +0000 Subject: [PATCH 4/4] Fix:(issue_2234): Include empty argument Include an empty argument and all subsequent arguments. --- command_parse.go | 1 + command_test.go | 14 ++++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/command_parse.go b/command_parse.go index aa95ae1677..63d859c42c 100644 --- a/command_parse.go +++ b/command_parse.go @@ -80,6 +80,7 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { firstArg := strings.TrimSpace(rargs[0]) if len(firstArg) == 0 { + posArgs = append(posArgs, rargs[0:]...) break } diff --git a/command_test.go b/command_test.go index 424e8c0eda..ede1bda84f 100644 --- a/command_test.go +++ b/command_test.go @@ -867,12 +867,12 @@ var defaultCommandSubCommandTests = []struct { {"", "jimbob", "foobar", true}, {"", "j", "foobar", true}, {"", "carly", "foobar", true}, - {"", "jimmers", "foobar", true}, + {"", "jimmers", "foobar", false}, {"", "jimmers", "", true}, - {" ", "jimmers", "foobar", true}, - /*{"", "", "", true}, + {" ", "jimmers", "foobar", false}, + {"", "", "", true}, {" ", "", "", false}, - {" ", "j", "", false},*/ + {" ", "j", "", false}, {"bat", "", "batbaz", true}, {"nothing", "", "batbaz", true}, {"nothing", "", "", false}, @@ -917,6 +917,7 @@ var defaultCommandFlagTests = []struct { }{ {"foobar", "", "foobar", true}, {"foobar", "-c derp", "foobar", true}, + {"foobar", "-c=", "foobar", true}, {"batbaz", "", "foobar", true}, {"b", "", "", true}, {"f", "", "", true}, @@ -930,13 +931,14 @@ var defaultCommandFlagTests = []struct { {"", "-j", "", true}, {" ", "-j", "foobar", true}, {"", "", "", true}, - {" ", "", "", true}, - {" ", "-j", "", true}, + {" ", "", "", false}, + {" ", "-j", "", false}, {"bat", "", "batbaz", true}, {"nothing", "", "batbaz", true}, {"nothing", "", "", false}, {"nothing", "--jimbob", "batbaz", true}, {"nothing", "--carly", "", false}, + {"nothing", "--carly=", "", false}, } func TestCommand_RunDefaultCommandWithFlags(t *testing.T) {