From b41e8b71c79e468a4b241960c1ce469a1d762f73 Mon Sep 17 00:00:00 2001 From: Bill Johnson <436997+dubrie@users.noreply.github.com> Date: Tue, 2 Jun 2020 13:02:28 -0700 Subject: Switching from whitelist to Allowlist terminology --- flag.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/flag.go b/flag.go index 7c058de..a685e42 100644 --- a/flag.go +++ b/flag.go @@ -124,8 +124,8 @@ const ( PanicOnError ) -// ParseErrorsWhitelist defines the parsing errors that can be ignored -type ParseErrorsWhitelist struct { +// ParseErrorsAllowlist defines the parsing errors that can be ignored +type ParseErrorsAllowlist struct { // UnknownFlags will ignore unknown flags errors and continue parsing rest of the flags UnknownFlags bool } @@ -145,8 +145,8 @@ type FlagSet struct { // help/usage messages. SortFlags bool - // ParseErrorsWhitelist is used to configure a whitelist of errors - ParseErrorsWhitelist ParseErrorsWhitelist + // ParseErrorsAllowlist is used to configure an allowlist of errors + ParseErrorsAllowlist ParseErrorsAllowlist name string parsed bool @@ -973,7 +973,7 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin case name == "help": f.usage() return a, ErrHelp - case f.ParseErrorsWhitelist.UnknownFlags: + case f.ParseErrorsAllowlist.UnknownFlags: // --unknown=unknownval arg ... // we do not want to lose arg in this case if len(split) >= 2 { @@ -1028,7 +1028,7 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse f.usage() err = ErrHelp return - case f.ParseErrorsWhitelist.UnknownFlags: + case f.ParseErrorsAllowlist.UnknownFlags: // '-f=arg arg ...' // we do not want to lose arg in this case if len(shorthands) > 2 && shorthands[1] == '=' { -- cgit v1.2.3 From 0aa4171c4229535ef4247b9283732fb21867b08e Mon Sep 17 00:00:00 2001 From: Bill Johnson <436997+dubrie@users.noreply.github.com> Date: Wed, 10 Jun 2020 08:14:08 -0700 Subject: Update flag_test.go --- flag_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flag_test.go b/flag_test.go index 58a5d25..76535f3 100644 --- a/flag_test.go +++ b/flag_test.go @@ -408,7 +408,7 @@ func testParseWithUnknownFlags(f *FlagSet, t *testing.T) { if f.Parsed() { t.Error("f.Parse() = true before Parse") } - f.ParseErrorsWhitelist.UnknownFlags = true + f.ParseErrorsAllowlist.UnknownFlags = true f.BoolP("boola", "a", false, "bool value") f.BoolP("boolb", "b", false, "bool2 value") -- cgit v1.2.3 From 1a9275f338e607971cca74c95ac1bc01e4904f7a Mon Sep 17 00:00:00 2001 From: Bhargav Ravuri Date: Sat, 19 Nov 2022 21:24:37 +0530 Subject: Remove Redundant "Unknown-Flag" Error Signed-off-by: Bhargav Ravuri --- flag.go | 1 - 1 file changed, 1 deletion(-) diff --git a/flag.go b/flag.go index 7c058de..2723762 100644 --- a/flag.go +++ b/flag.go @@ -916,7 +916,6 @@ func VarP(value Value, name, shorthand, usage string) { func (f *FlagSet) failf(format string, a ...interface{}) error { err := fmt.Errorf(format, a...) if f.errorHandling != ContinueOnError { - fmt.Fprintln(f.Output(), err) f.usage() } return err -- cgit v1.2.3 From 7104d9034645340261ce7b590f8c7a9be484521e Mon Sep 17 00:00:00 2001 From: shawn Date: Tue, 23 Jul 2024 15:19:16 +0800 Subject: fix: correct argument length check in FlagSet.Parse Signed-off-by: shawn --- flag.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/flag.go b/flag.go index 7c058de..71773a8 100644 --- a/flag.go +++ b/flag.go @@ -27,23 +27,32 @@ unaffected. Define flags using flag.String(), Bool(), Int(), etc. This declares an integer flag, -flagname, stored in the pointer ip, with type *int. + var ip = flag.Int("flagname", 1234, "help message for flagname") + If you like, you can bind the flag to a variable using the Var() functions. + var flagvar int func init() { flag.IntVar(&flagvar, "flagname", 1234, "help message for flagname") } + Or you can create custom flags that satisfy the Value interface (with pointer receivers) and couple them to flag parsing by + flag.Var(&flagVal, "name", "help message for flagname") + For such flags, the default value is just the initial value of the variable. After all flags are defined, call + flag.Parse() + to parse the command line into the defined flags. Flags may then be used directly. If you're using the flags themselves, they are all pointers; if you bind to variables, they're values. + fmt.Println("ip has value ", *ip) fmt.Println("flagvar has value ", flagvar) @@ -54,22 +63,26 @@ The arguments are indexed from 0 through flag.NArg()-1. The pflag package also defines some new functions that are not in flag, that give one-letter shorthands for flags. You can use these by appending 'P' to the name of any function that defines a flag. + var ip = flag.IntP("flagname", "f", 1234, "help message") var flagvar bool func init() { flag.BoolVarP(&flagvar, "boolname", "b", true, "help message") } flag.VarP(&flagval, "varname", "v", "help message") + Shorthand letters can be used with single dashes on the command line. Boolean shorthand flags can be combined with other shorthand flags. Command line flag syntax: + --flag // boolean flags only --flag=x Unlike the flag package, a single dash before an option means something different than a double dash. Single dashes signify a series of shorthand letters for flags. All but the last shorthand letter must be boolean flags. + // boolean flags -f -abc @@ -934,9 +947,9 @@ func (f *FlagSet) usage() { } } -//--unknown (args will be empty) -//--unknown --next-flag ... (args will be --next-flag ...) -//--unknown arg ... (args will be arg ...) +// --unknown (args will be empty) +// --unknown --next-flag ... (args will be --next-flag ...) +// --unknown arg ... (args will be arg ...) func stripUnknownFlagValue(args []string) []string { if len(args) == 0 { //--unknown @@ -1135,7 +1148,7 @@ func (f *FlagSet) Parse(arguments []string) error { } f.parsed = true - if len(arguments) < 0 { + if len(arguments) == 0 { return nil } -- cgit v1.2.3 From f9b6619ed0e0d771f7812fea23cd8b514faf2b2d Mon Sep 17 00:00:00 2001 From: shawn Date: Thu, 5 Dec 2024 23:02:18 +0800 Subject: fix: add test for argument length check --- flag_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/flag_test.go b/flag_test.go index 58a5d25..0502618 100644 --- a/flag_test.go +++ b/flag_test.go @@ -100,6 +100,12 @@ func TestEverything(t *testing.T) { } } +func TestNoArgument(t *testing.T) { + if GetCommandLine().Parse([]string{}) != nil { + t.Error("parse failed for empty argument list") + } +} + func TestUsage(t *testing.T) { called := false ResetForTesting(func() { called = true }) @@ -1134,7 +1140,6 @@ func TestMultipleNormalizeFlagNameInvocations(t *testing.T) { } } -// func TestHiddenFlagInUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") @@ -1149,7 +1154,6 @@ func TestHiddenFlagInUsage(t *testing.T) { } } -// func TestHiddenFlagUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") -- cgit v1.2.3 From 1118d46dfe34c2d77aee04d819315a21942aa444 Mon Sep 17 00:00:00 2001 From: Hu Jun Date: Fri, 27 Dec 2024 12:06:14 -0800 Subject: add support equivalent to golang flag.TextVar(), also fixes the test failure as described in #368 --- flag_test.go | 6 ++--- text.go | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ text_test.go | 53 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 text.go create mode 100644 text_test.go diff --git a/flag_test.go b/flag_test.go index 58a5d25..9faaba4 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1134,7 +1134,6 @@ func TestMultipleNormalizeFlagNameInvocations(t *testing.T) { } } -// func TestHiddenFlagInUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") @@ -1149,7 +1148,6 @@ func TestHiddenFlagInUsage(t *testing.T) { } } -// func TestHiddenFlagUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") @@ -1238,8 +1236,8 @@ func TestPrintDefaults(t *testing.T) { fs.PrintDefaults() got := buf.String() if got != defaultOutput { - fmt.Println("\n" + got) - fmt.Println("\n" + defaultOutput) + fmt.Print("\n" + got + "\n") + fmt.Print("\n" + defaultOutput + "\n") t.Errorf("got %q want %q\n", got, defaultOutput) } } diff --git a/text.go b/text.go new file mode 100644 index 0000000..3726606 --- /dev/null +++ b/text.go @@ -0,0 +1,81 @@ +package pflag + +import ( + "encoding" + "fmt" + "reflect" +) + +// following is copied from go 1.23.4 flag.go +type textValue struct{ p encoding.TextUnmarshaler } + +func newTextValue(val encoding.TextMarshaler, p encoding.TextUnmarshaler) textValue { + ptrVal := reflect.ValueOf(p) + if ptrVal.Kind() != reflect.Ptr { + panic("variable value type must be a pointer") + } + defVal := reflect.ValueOf(val) + if defVal.Kind() == reflect.Ptr { + defVal = defVal.Elem() + } + if defVal.Type() != ptrVal.Type().Elem() { + panic(fmt.Sprintf("default type does not match variable type: %v != %v", defVal.Type(), ptrVal.Type().Elem())) + } + ptrVal.Elem().Set(defVal) + return textValue{p} +} + +func (v textValue) Set(s string) error { + return v.p.UnmarshalText([]byte(s)) +} + +func (v textValue) Get() interface{} { + return v.p +} + +func (v textValue) String() string { + if m, ok := v.p.(encoding.TextMarshaler); ok { + if b, err := m.MarshalText(); err == nil { + return string(b) + } + } + return "" +} + +//end of copy + +func (v textValue) Type() string { + return reflect.ValueOf(v.p).Type().Name() +} + +// GetText set out, which implements encoding.UnmarshalText, to the value of a flag with given name +func (f *FlagSet) GetText(name string, out encoding.TextUnmarshaler) error { + flag := f.Lookup(name) + if flag == nil { + return fmt.Errorf("flag accessed but not defined: %s", name) + } + if flag.Value.Type() != reflect.TypeOf(out).Name() { + fmt.Errorf("trying to get %s value of flag of type %s", reflect.TypeOf(out).Name(), flag.Value.Type()) + } + return out.UnmarshalText([]byte(flag.Value.String())) +} + +// TextVar defines a flag with a specified name, default value, and usage string. The argument p must be a pointer to a variable that will hold the value of the flag, and p must implement encoding.TextUnmarshaler. If the flag is used, the flag value will be passed to p's UnmarshalText method. The type of the default value must be the same as the type of p. +func (f *FlagSet) TextVar(p encoding.TextUnmarshaler, name string, value encoding.TextMarshaler, usage string) { + f.VarP(newTextValue(value, p), name, "", usage) +} + +// TextVarP is like TextVar, but accepts a shorthand letter that can be used after a single dash. +func (f *FlagSet) TextVarP(p encoding.TextUnmarshaler, name, shorthand string, value encoding.TextMarshaler, usage string) { + f.VarP(newTextValue(value, p), name, shorthand, usage) +} + +// TextVar defines a flag with a specified name, default value, and usage string. The argument p must be a pointer to a variable that will hold the value of the flag, and p must implement encoding.TextUnmarshaler. If the flag is used, the flag value will be passed to p's UnmarshalText method. The type of the default value must be the same as the type of p. +func TextVar(p encoding.TextUnmarshaler, name string, value encoding.TextMarshaler, usage string) { + CommandLine.VarP(newTextValue(value, p), name, "", usage) +} + +// TextVarP is like TextVar, but accepts a shorthand letter that can be used after a single dash. +func TextVarP(p encoding.TextUnmarshaler, name, shorthand string, value encoding.TextMarshaler, usage string) { + CommandLine.VarP(newTextValue(value, p), name, shorthand, usage) +} diff --git a/text_test.go b/text_test.go new file mode 100644 index 0000000..2a667ab --- /dev/null +++ b/text_test.go @@ -0,0 +1,53 @@ +package pflag + +import ( + "fmt" + "os" + "testing" + "time" +) + +func setUpTime(t *time.Time) *FlagSet { + f := NewFlagSet("test", ContinueOnError) + f.TextVar(t, "time", time.Now(), "time stamp") + return f +} + +func TestText(t *testing.T) { + testCases := []struct { + input string + success bool + expected time.Time + }{ + {"2003-01-02T15:04:05Z", true, time.Date(2003, 1, 2, 15, 04, 05, 0, time.UTC)}, + {"2003-01-02 15:05:01", false, time.Date(2002, 1, 2, 15, 05, 05, 07, time.UTC)}, + {"2024-11-22T03:01:02Z", true, time.Date(2024, 11, 22, 3, 1, 02, 0, time.UTC)}, + {"2006-01-02T15:04:05+07:00", true, time.Date(2006, 1, 2, 15, 4, 5, 0, time.FixedZone("UTC+7", 7*60*60))}, + } + + devnull, _ := os.Open(os.DevNull) + os.Stderr = devnull + for i := range testCases { + var ts time.Time + f := setUpTime(&ts) + tc := &testCases[i] + arg := fmt.Sprintf("--time=%s", tc.input) + err := f.Parse([]string{arg}) + if err != nil && tc.success == true { + t.Errorf("expected success, got %q", err) + continue + } else if err == nil && tc.success == false { + t.Errorf("expected failure, but succeeded") + continue + } else if tc.success { + parsedT := new(time.Time) + err := f.GetText("time", parsedT) + if err != nil { + t.Errorf("Got error trying to fetch the time flag: %v", err) + } + if !parsedT.Equal(tc.expected) { + t.Errorf("expected %q, got %q", tc.expected, parsedT) + } + } + } +} -- cgit v1.2.3 From f48cbd1964b57ff7c17e2f233feb49c03efe6417 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Thu, 9 Jan 2025 01:25:58 +0100 Subject: add github actions Signed-off-by: Mark Sagi-Kazar --- .editorconfig | 12 ++++++++++ .github/.editorconfig | 2 ++ .github/dependabot.yaml | 12 ++++++++++ .github/workflows/ci.yaml | 60 +++++++++++++++++++++++++++++++++++++++++++++++ .golangci.yaml | 4 ++++ 5 files changed, 90 insertions(+) create mode 100644 .editorconfig create mode 100644 .github/.editorconfig create mode 100644 .github/dependabot.yaml create mode 100644 .github/workflows/ci.yaml create mode 100644 .golangci.yaml diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..4492e9f --- /dev/null +++ b/.editorconfig @@ -0,0 +1,12 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 4 +indent_style = space +insert_final_newline = true +trim_trailing_whitespace = true + +[*.go] +indent_style = tab diff --git a/.github/.editorconfig b/.github/.editorconfig new file mode 100644 index 0000000..0902c6a --- /dev/null +++ b/.github/.editorconfig @@ -0,0 +1,2 @@ +[{*.yml,*.yaml}] +indent_size = 2 diff --git a/.github/dependabot.yaml b/.github/dependabot.yaml new file mode 100644 index 0000000..73aa36f --- /dev/null +++ b/.github/dependabot.yaml @@ -0,0 +1,12 @@ +version: 2 + +updates: + - package-ecosystem: gomod + directory: / + schedule: + interval: daily + + - package-ecosystem: github-actions + directory: / + schedule: + interval: daily diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..292c29e --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,60 @@ +name: CI + +on: + push: + branches: [master] + pull_request: + +jobs: + test: + name: Test + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + go: ["1.21", "1.22", "1.23"] + + steps: + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Set up Go + uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 + with: + go-version: ${{ matrix.go }} + + - name: Test + # Cannot enable shuffle for now because some tests rely on global state and order + # run: go test -race -v -shuffle=on ./... + run: go test -race -v ./... + + lint: + name: Lint + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Set up Go + uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 + with: + go-version: "1.23" + + - name: Lint + uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 + with: + version: v1.63.4 + + dependency-review: + name: Dependency review + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + + steps: + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Dependency Review + uses: actions/dependency-review-action@3b139cfc5fae8b618d3eae3675e383bb1769c019 # v4.5.0 diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..b274f24 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,4 @@ +linters: + disable-all: true + enable: + - nolintlint -- cgit v1.2.3 From a0f4ddd9fe01ac8fece07be6e3f3ae5dcd7ceec0 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Thu, 9 Jan 2025 01:34:01 +0100 Subject: fix govet Signed-off-by: Mark Sagi-Kazar --- flag_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/flag_test.go b/flag_test.go index 58a5d25..643f099 100644 --- a/flag_test.go +++ b/flag_test.go @@ -433,7 +433,7 @@ func testParseWithUnknownFlags(f *FlagSet, t *testing.T) { "-u=unknown3Value", "-p", "unknown4Value", - "-q", //another unknown with bool value + "-q", // another unknown with bool value "-y", "ee", "--unknown7=unknown7value", @@ -899,7 +899,7 @@ func TestChangingArgs(t *testing.T) { // Test that -help invokes the usage message and returns ErrHelp. func TestHelp(t *testing.T) { - var helpCalled = false + helpCalled := false fs := NewFlagSet("help test", ContinueOnError) fs.Usage = func() { helpCalled = true } var flag bool @@ -998,6 +998,7 @@ func getDeprecatedFlagSet() *FlagSet { f.MarkDeprecated("badflag", "use --good-flag instead") return f } + func TestDeprecatedFlagInDocs(t *testing.T) { f := getDeprecatedFlagSet() @@ -1134,7 +1135,6 @@ func TestMultipleNormalizeFlagNameInvocations(t *testing.T) { } } -// func TestHiddenFlagInUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") @@ -1149,7 +1149,6 @@ func TestHiddenFlagInUsage(t *testing.T) { } } -// func TestHiddenFlagUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") @@ -1239,7 +1238,7 @@ func TestPrintDefaults(t *testing.T) { got := buf.String() if got != defaultOutput { fmt.Println("\n" + got) - fmt.Println("\n" + defaultOutput) + fmt.Printf("\n" + defaultOutput) t.Errorf("got %q want %q\n", got, defaultOutput) } } -- cgit v1.2.3 From 100ab0eb250792014fc1594d94f7fb5c5f0dee37 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Thu, 9 Jan 2025 01:35:44 +0100 Subject: disable unsupported dependency graph for now Signed-off-by: Mark Sagi-Kazar --- .github/workflows/ci.yaml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 292c29e..42f7614 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -46,15 +46,3 @@ jobs: uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 with: version: v1.63.4 - - dependency-review: - name: Dependency review - runs-on: ubuntu-latest - if: github.event_name == 'pull_request' - - steps: - - name: Checkout repository - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - - name: Dependency Review - uses: actions/dependency-review-action@3b139cfc5fae8b618d3eae3675e383bb1769c019 # v4.5.0 -- cgit v1.2.3 From 9edfc8d416bea8aecfbdf0790a838f95ae133078 Mon Sep 17 00:00:00 2001 From: MidnightRocket Date: Tue, 18 Mar 2025 14:07:18 +0100 Subject: Fix defaultIsZeroValue check for generic Value type --- flag.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flag.go b/flag.go index 7c058de..4bdbd0c 100644 --- a/flag.go +++ b/flag.go @@ -551,7 +551,7 @@ func (f *Flag) defaultIsZeroValue() bool { case *intSliceValue, *stringSliceValue, *stringArrayValue: return f.DefValue == "[]" default: - switch f.Value.String() { + switch f.DefValue { case "false": return true case "": -- cgit v1.2.3 From edb16648484c5752d70fd84f08b3aecc46e61339 Mon Sep 17 00:00:00 2001 From: MidnightRocket Date: Fri, 21 Mar 2025 18:02:55 +0100 Subject: Add better test for defaultIsZeroValue for generic Value type --- flag_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/flag_test.go b/flag_test.go index 643f099..344eb07 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1183,6 +1183,7 @@ const defaultOutput = ` --A for bootstrapping, allo --StringSlice strings string slice with zero default --Z int an int that defaults to zero --custom custom custom Value implementation + --custom-with-val custom custom value which has been set from command line while help is shown --customP custom a VarP with default (default 10) --maxT timeout set timeout for dial -v, --verbose count verbosity @@ -1234,6 +1235,14 @@ func TestPrintDefaults(t *testing.T) { cv2 := customValue(10) fs.VarP(&cv2, "customP", "", "a VarP with default") + // Simulate case where a value has been provided and the help screen is shown + var cv3 customValue + fs.Var(&cv3, "custom-with-val", "custom value which has been set from command line while help is shown") + err := fs.Parse([]string{"--custom-with-val", "3"}) + if err != nil { + t.Error("Parsing flags failed:", err) + } + fs.PrintDefaults() got := buf.String() if got != defaultOutput { -- cgit v1.2.3 From c96309303407244840d6f2bc00ae32dc95390c25 Mon Sep 17 00:00:00 2001 From: MidnightRocket Date: Fri, 21 Mar 2025 18:03:46 +0100 Subject: Improve readability for error in TestPrintDefaults --- flag_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/flag_test.go b/flag_test.go index 344eb07..0dbe874 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1246,9 +1246,7 @@ func TestPrintDefaults(t *testing.T) { fs.PrintDefaults() got := buf.String() if got != defaultOutput { - fmt.Println("\n" + got) - fmt.Printf("\n" + defaultOutput) - t.Errorf("got %q want %q\n", got, defaultOutput) + t.Errorf("\n--- Got:\n%s--- Wanted:\n%s\n", got, defaultOutput) } } -- cgit v1.2.3 From e9268909c269ea54836435ff420ce0d5d59324ac Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Mon, 21 Apr 2025 21:28:13 -0700 Subject: test: Commonly-returned error messages Add tests to ensure invalid arguments and unknown flag error messages don't change. --- flag_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/flag_test.go b/flag_test.go index 0dbe874..aa2f434 100644 --- a/flag_test.go +++ b/flag_test.go @@ -103,9 +103,14 @@ func TestEverything(t *testing.T) { func TestUsage(t *testing.T) { called := false ResetForTesting(func() { called = true }) - if GetCommandLine().Parse([]string{"--x"}) == nil { + err := GetCommandLine().Parse([]string{"--x"}) + expectedErr := "unknown flag: --x" + if err == nil { t.Error("parse did not fail for unknown flag") } + if err.Error() != expectedErr { + t.Errorf("expected error %q, got %q", expectedErr, err.Error()) + } if called { t.Error("did call Usage while using ContinueOnError") } @@ -131,9 +136,14 @@ func TestAddFlagSet(t *testing.T) { func TestAnnotation(t *testing.T) { f := NewFlagSet("shorthand", ContinueOnError) - if err := f.SetAnnotation("missing-flag", "key", nil); err == nil { + err := f.SetAnnotation("missing-flag", "key", nil) + expectedErr := "no such flag -missing-flag" + if err == nil { t.Errorf("Expected error setting annotation on non-existent flag") } + if err.Error() != expectedErr { + t.Errorf("expected error %q, got %q", expectedErr, err.Error()) + } f.StringP("stringa", "a", "", "string value") if err := f.SetAnnotation("stringa", "key", nil); err != nil { @@ -349,6 +359,33 @@ func testParse(f *FlagSet, t *testing.T) { } else if f.Args()[0] != extra { t.Errorf("expected argument %q got %q", extra, f.Args()[0]) } + // Test unknown + err := f.Parse([]string{"--unknown"}) + expectedErr := "unknown flag: --unknown" + if err == nil { + t.Error("parse did not fail for unknown flag") + } + if err.Error() != expectedErr { + t.Errorf("expected error %q, got %q", expectedErr, err.Error()) + } + // Test invalid + err = f.Parse([]string{"--bool=abcdefg"}) + expectedErr = `invalid argument "abcdefg" for "--bool" flag: strconv.ParseBool: parsing "abcdefg": invalid syntax` + if err == nil { + t.Error("parse did not fail for invalid argument") + } + if err.Error() != expectedErr { + t.Errorf("expected error %q, got %q", expectedErr, err.Error()) + } + // Test required + err = f.Parse([]string{"--int"}) + expectedErr = `flag needs an argument: --int` + if err == nil { + t.Error("parse did not fail for missing argument") + } + if err.Error() != expectedErr { + t.Errorf("expected error %q, got %q", expectedErr, err.Error()) + } } func testParseAll(f *FlagSet, t *testing.T) { @@ -538,6 +575,24 @@ func TestShorthand(t *testing.T) { if f.ArgsLenAtDash() != 1 { t.Errorf("expected argsLenAtDash %d got %d", f.ArgsLenAtDash(), 1) } + // Test unknown + err := f.Parse([]string{"-ukn"}) + expectedErr := "unknown shorthand flag: 'u' in -ukn" + if err == nil { + t.Error("parse did not fail for unknown shorthand flag") + } + if err.Error() != expectedErr { + t.Errorf("expected error %q, got %q", expectedErr, err.Error()) + } + // Test required + err = f.Parse([]string{"-as"}) + expectedErr = `flag needs an argument: 's' in -s` + if err == nil { + t.Error("parse did not fail for missing argument") + } + if err.Error() != expectedErr { + t.Errorf("expected error %q, got %q", expectedErr, err.Error()) + } } func TestShorthandLookup(t *testing.T) { -- cgit v1.2.3 From 8d771585bd8aed7c04b9d409f30599f3c7ed39c7 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Mon, 21 Apr 2025 21:29:30 -0700 Subject: feat: Use error structs for most returned errors This allows callers to differentiate between error types without having to parse the error message string. --- errors.go | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ flag.go | 52 +++++++++++++++++++--------------- 2 files changed, 127 insertions(+), 22 deletions(-) create mode 100644 errors.go diff --git a/errors.go b/errors.go new file mode 100644 index 0000000..4d72a93 --- /dev/null +++ b/errors.go @@ -0,0 +1,97 @@ +package pflag + +import "fmt" + +// notExistErrorMessageType specifies which flavor of "flag does not exist" +// is printed by NotExistError. This allows the related errors to be grouped +// under a single NotExistError struct without making a breaking change to +// the error message text. +type notExistErrorMessageType int + +const ( + flagNotExistMessage notExistErrorMessageType = iota + flagNotDefinedMessage + flagNoSuchFlagMessage + flagUnknownFlagMessage + flagUnknownShorthandFlagMessage +) + +// NotExistError is the error returned when trying to access a flag that +// does not exist in the FlagSet. +type NotExistError struct { + name string + specifiedShorthands string + messageType notExistErrorMessageType +} + +// Error implements error. +func (e *NotExistError) Error() string { + switch e.messageType { + case flagNotExistMessage: + return fmt.Sprintf("flag %q does not exist", e.name) + + case flagNotDefinedMessage: + return fmt.Sprintf("flag accessed but not defined: %s", e.name) + + case flagNoSuchFlagMessage: + return fmt.Sprintf("no such flag -%v", e.name) + + case flagUnknownFlagMessage: + return fmt.Sprintf("unknown flag: --%s", e.name) + + case flagUnknownShorthandFlagMessage: + c := rune(e.name[0]) + return fmt.Sprintf("unknown shorthand flag: %q in -%s", c, e.specifiedShorthands) + } + + panic(fmt.Errorf("unknown flagNotExistErrorMessageType: %v", e.messageType)) +} + +// ValueRequiredError is the error returned when a flag needs an argument but +// no argument was provided. +type ValueRequiredError struct { + flag *Flag + specifiedName string + specifiedShorthands string +} + +// Error implements error. +func (e *ValueRequiredError) Error() string { + if len(e.specifiedShorthands) > 0 { + c := rune(e.specifiedName[0]) + return fmt.Sprintf("flag needs an argument: %q in -%s", c, e.specifiedShorthands) + } + + return fmt.Sprintf("flag needs an argument: --%s", e.specifiedName) +} + +// InvalidValueError is the error returned when an invalid value is used +// for a flag. +type InvalidValueError struct { + flag *Flag + value string + cause error +} + +// Error implements error. +func (e *InvalidValueError) Error() string { + flag := e.flag + var flagName string + if flag.Shorthand != "" && flag.ShorthandDeprecated == "" { + flagName = fmt.Sprintf("-%s, --%s", flag.Shorthand, flag.Name) + } else { + flagName = fmt.Sprintf("--%s", flag.Name) + } + return fmt.Sprintf("invalid argument %q for %q flag: %v", e.value, flagName, e.cause) +} + +// InvalidSyntaxError is the error returned when a bad flag name is passed on +// the command line. +type InvalidSyntaxError struct { + specifiedFlag string +} + +// Error implements error. +func (e *InvalidSyntaxError) Error() string { + return fmt.Sprintf("bad flag syntax: %s", e.specifiedFlag) +} diff --git a/flag.go b/flag.go index 4bdbd0c..80bd580 100644 --- a/flag.go +++ b/flag.go @@ -381,7 +381,7 @@ func (f *FlagSet) lookup(name NormalizedName) *Flag { func (f *FlagSet) getFlagType(name string, ftype string, convFunc func(sval string) (interface{}, error)) (interface{}, error) { flag := f.Lookup(name) if flag == nil { - err := fmt.Errorf("flag accessed but not defined: %s", name) + err := &NotExistError{name: name, messageType: flagNotDefinedMessage} return nil, err } @@ -411,7 +411,7 @@ func (f *FlagSet) ArgsLenAtDash() int { func (f *FlagSet) MarkDeprecated(name string, usageMessage string) error { flag := f.Lookup(name) if flag == nil { - return fmt.Errorf("flag %q does not exist", name) + return &NotExistError{name: name, messageType: flagNotExistMessage} } if usageMessage == "" { return fmt.Errorf("deprecated message for flag %q must be set", name) @@ -427,7 +427,7 @@ func (f *FlagSet) MarkDeprecated(name string, usageMessage string) error { func (f *FlagSet) MarkShorthandDeprecated(name string, usageMessage string) error { flag := f.Lookup(name) if flag == nil { - return fmt.Errorf("flag %q does not exist", name) + return &NotExistError{name: name, messageType: flagNotExistMessage} } if usageMessage == "" { return fmt.Errorf("deprecated message for flag %q must be set", name) @@ -441,7 +441,7 @@ func (f *FlagSet) MarkShorthandDeprecated(name string, usageMessage string) erro func (f *FlagSet) MarkHidden(name string) error { flag := f.Lookup(name) if flag == nil { - return fmt.Errorf("flag %q does not exist", name) + return &NotExistError{name: name, messageType: flagNotExistMessage} } flag.Hidden = true return nil @@ -464,18 +464,16 @@ func (f *FlagSet) Set(name, value string) error { normalName := f.normalizeFlagName(name) flag, ok := f.formal[normalName] if !ok { - return fmt.Errorf("no such flag -%v", name) + return &NotExistError{name: name, messageType: flagNoSuchFlagMessage} } err := flag.Value.Set(value) if err != nil { - var flagName string - if flag.Shorthand != "" && flag.ShorthandDeprecated == "" { - flagName = fmt.Sprintf("-%s, --%s", flag.Shorthand, flag.Name) - } else { - flagName = fmt.Sprintf("--%s", flag.Name) + return &InvalidValueError{ + flag: flag, + value: value, + cause: err, } - return fmt.Errorf("invalid argument %q for %q flag: %v", value, flagName, err) } if !flag.Changed { @@ -501,7 +499,7 @@ func (f *FlagSet) SetAnnotation(name, key string, values []string) error { normalName := f.normalizeFlagName(name) flag, ok := f.formal[normalName] if !ok { - return fmt.Errorf("no such flag -%v", name) + return &NotExistError{name: name, messageType: flagNoSuchFlagMessage} } if flag.Annotations == nil { flag.Annotations = map[string][]string{} @@ -911,10 +909,9 @@ func VarP(value Value, name, shorthand, usage string) { CommandLine.VarP(value, name, shorthand, usage) } -// failf prints to standard error a formatted error and usage message and +// fail prints an error message and usage message to standard error and // returns the error. -func (f *FlagSet) failf(format string, a ...interface{}) error { - err := fmt.Errorf(format, a...) +func (f *FlagSet) fail(err error) error { if f.errorHandling != ContinueOnError { fmt.Fprintln(f.Output(), err) f.usage() @@ -960,7 +957,7 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin a = args name := s[2:] if len(name) == 0 || name[0] == '-' || name[0] == '=' { - err = f.failf("bad flag syntax: %s", s) + err = f.fail(&InvalidSyntaxError{specifiedFlag: s}) return } @@ -982,7 +979,7 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin return stripUnknownFlagValue(a), nil default: - err = f.failf("unknown flag: --%s", name) + err = f.fail(&NotExistError{name: name, messageType: flagUnknownFlagMessage}) return } } @@ -1000,13 +997,16 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin a = a[1:] } else { // '--flag' (arg was required) - err = f.failf("flag needs an argument: %s", s) + err = f.fail(&ValueRequiredError{ + flag: flag, + specifiedName: name, + }) return } err = fn(flag, value) if err != nil { - f.failf(err.Error()) + f.fail(err) } return } @@ -1039,7 +1039,11 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse outArgs = stripUnknownFlagValue(outArgs) return default: - err = f.failf("unknown shorthand flag: %q in -%s", c, shorthands) + err = f.fail(&NotExistError{ + name: string(c), + specifiedShorthands: shorthands, + messageType: flagUnknownShorthandFlagMessage, + }) return } } @@ -1062,7 +1066,11 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse outArgs = args[1:] } else { // '-f' (arg was required) - err = f.failf("flag needs an argument: %q in -%s", c, shorthands) + err = f.fail(&ValueRequiredError{ + flag: flag, + specifiedName: string(c), + specifiedShorthands: shorthands, + }) return } @@ -1072,7 +1080,7 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse err = fn(flag, value) if err != nil { - f.failf(err.Error()) + f.fail(err) } return } -- cgit v1.2.3 From ca5cf963c30751583422d6264a5427e9682f1a1a Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Mon, 21 Apr 2025 21:47:13 -0700 Subject: feat: Add getters to error structs --- errors.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/errors.go b/errors.go index 4d72a93..ff11b66 100644 --- a/errors.go +++ b/errors.go @@ -47,6 +47,19 @@ func (e *NotExistError) Error() string { panic(fmt.Errorf("unknown flagNotExistErrorMessageType: %v", e.messageType)) } +// GetSpecifiedName returns the name of the flag (without dashes) as it +// appeared in the parsed arguments. +func (e *NotExistError) GetSpecifiedName() string { + return e.name +} + +// GetSpecifiedShortnames returns the group of shorthand arguments +// (without dashes) that the flag appeared within. If the flag was not in a +// shorthand group, this will return an empty string. +func (e *NotExistError) GetSpecifiedShortnames() string { + return e.specifiedShorthands +} + // ValueRequiredError is the error returned when a flag needs an argument but // no argument was provided. type ValueRequiredError struct { @@ -65,6 +78,24 @@ func (e *ValueRequiredError) Error() string { return fmt.Sprintf("flag needs an argument: --%s", e.specifiedName) } +// GetFlag returns the flag for which the error occurred. +func (e *ValueRequiredError) GetFlag() *Flag { + return e.flag +} + +// GetSpecifiedName returns the name of the flag (without dashes) as it +// appeared in the parsed arguments. +func (e *ValueRequiredError) GetSpecifiedName() string { + return e.specifiedName +} + +// GetSpecifiedShortnames returns the group of shorthand arguments +// (without dashes) that the flag appeared within. If the flag was not in a +// shorthand group, this will return an empty string. +func (e *ValueRequiredError) GetSpecifiedShortnames() string { + return e.specifiedShorthands +} + // InvalidValueError is the error returned when an invalid value is used // for a flag. type InvalidValueError struct { @@ -85,6 +116,21 @@ func (e *InvalidValueError) Error() string { return fmt.Sprintf("invalid argument %q for %q flag: %v", e.value, flagName, e.cause) } +// Unwrap implements errors.Unwrap. +func (e *InvalidValueError) Unwrap() error { + return e.cause +} + +// GetFlag returns the flag for which the error occurred. +func (e *InvalidValueError) GetFlag() *Flag { + return e.flag +} + +// GetValue returns the invalid value that was provided. +func (e *InvalidValueError) GetValue() string { + return e.value +} + // InvalidSyntaxError is the error returned when a bad flag name is passed on // the command line. type InvalidSyntaxError struct { @@ -95,3 +141,9 @@ type InvalidSyntaxError struct { func (e *InvalidSyntaxError) Error() string { return fmt.Sprintf("bad flag syntax: %s", e.specifiedFlag) } + +// GetSpecifiedName returns the exact flag (with dashes) as it +// appeared in the parsed arguments. +func (e *InvalidSyntaxError) GetSpecifiedFlag() string { + return e.specifiedFlag +} -- cgit v1.2.3 From 6ca66b16cbe1b365ce9a6c56faf9b04acb8d8035 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Mon, 21 Apr 2025 21:54:00 -0700 Subject: test: Add tests for error structs --- errors_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 errors_test.go diff --git a/errors_test.go b/errors_test.go new file mode 100644 index 0000000..7b4c7a4 --- /dev/null +++ b/errors_test.go @@ -0,0 +1,67 @@ +package pflag + +import ( + "errors" + "testing" +) + +func TestNotExistError(t *testing.T) { + err := &NotExistError{ + name: "foo", + specifiedShorthands: "bar", + } + + if err.GetSpecifiedName() != "foo" { + t.Errorf("Expected GetSpecifiedName to return %q, got %q", "foo", err.GetSpecifiedName()) + } + if err.GetSpecifiedShortnames() != "bar" { + t.Errorf("Expected GetSpecifiedShortnames to return %q, got %q", "bar", err.GetSpecifiedShortnames()) + } +} + +func TestValueRequiredError(t *testing.T) { + err := &ValueRequiredError{ + flag: &Flag{}, + specifiedName: "foo", + specifiedShorthands: "bar", + } + + if err.GetFlag() == nil { + t.Error("Expected GetSpecifiedName to return its flag field, but got nil") + } + if err.GetSpecifiedName() != "foo" { + t.Errorf("Expected GetSpecifiedName to return %q, got %q", "foo", err.GetSpecifiedName()) + } + if err.GetSpecifiedShortnames() != "bar" { + t.Errorf("Expected GetSpecifiedShortnames to return %q, got %q", "bar", err.GetSpecifiedShortnames()) + } +} + +func TestInvalidValueError(t *testing.T) { + expectedCause := errors.New("error") + err := &InvalidValueError{ + flag: &Flag{}, + value: "foo", + cause: expectedCause, + } + + if err.GetFlag() == nil { + t.Error("Expected GetSpecifiedName to return its flag field, but got nil") + } + if err.GetValue() != "foo" { + t.Errorf("Expected GetValue to return %q, got %q", "foo", err.GetValue()) + } + if err.Unwrap() != expectedCause { + t.Errorf("Expected Unwrwap to return %q, got %q", expectedCause, err.Unwrap()) + } +} + +func TestInvalidSyntaxError(t *testing.T) { + err := &InvalidSyntaxError{ + specifiedFlag: "--=", + } + + if err.GetSpecifiedFlag() != "--=" { + t.Errorf("Expected GetSpecifiedFlag to return %q, got %q", "--=", err.GetSpecifiedFlag()) + } +} -- cgit v1.2.3 From 155e7f39ce86bdc58e103be0e6c786df26f04c73 Mon Sep 17 00:00:00 2001 From: co63oc Date: Tue, 6 May 2025 14:47:37 +0800 Subject: Fix typos --- count.go | 2 +- ipnet_slice.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/count.go b/count.go index a0b2679..d49c014 100644 --- a/count.go +++ b/count.go @@ -85,7 +85,7 @@ func (f *FlagSet) CountP(name, shorthand string, usage string) *int { // Count defines a count flag with specified name, default value, and usage string. // The return value is the address of an int variable that stores the value of the flag. -// A count flag will add 1 to its value evey time it is found on the command line +// A count flag will add 1 to its value every time it is found on the command line func Count(name string, usage string) *int { return CommandLine.CountP(name, "", usage) } diff --git a/ipnet_slice.go b/ipnet_slice.go index 6b541aa..c6e89da 100644 --- a/ipnet_slice.go +++ b/ipnet_slice.go @@ -73,7 +73,7 @@ func (s *ipNetSliceValue) String() string { func ipNetSliceConv(val string) (interface{}, error) { val = strings.Trim(val, "[]") - // Emtpy string would cause a slice with one (empty) entry + // Empty string would cause a slice with one (empty) entry if len(val) == 0 { return []net.IPNet{}, nil } -- cgit v1.2.3 From 011db0c1163722ac2fa4452be8be8123768dc912 Mon Sep 17 00:00:00 2001 From: Hu Jun Date: Sat, 10 May 2025 16:26:11 -0700 Subject: - update text_test.go based on PR review comments - return is missing in one error path of GetText(), fixed --- text.go | 2 +- text_test.go | 31 +++++++++++++++++-------------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/text.go b/text.go index 3726606..886d5a3 100644 --- a/text.go +++ b/text.go @@ -55,7 +55,7 @@ func (f *FlagSet) GetText(name string, out encoding.TextUnmarshaler) error { return fmt.Errorf("flag accessed but not defined: %s", name) } if flag.Value.Type() != reflect.TypeOf(out).Name() { - fmt.Errorf("trying to get %s value of flag of type %s", reflect.TypeOf(out).Name(), flag.Value.Type()) + return fmt.Errorf("trying to get %s value of flag of type %s", reflect.TypeOf(out).Name(), flag.Value.Type()) } return out.UnmarshalText([]byte(flag.Value.String())) } diff --git a/text_test.go b/text_test.go index 2a667ab..e60c136 100644 --- a/text_test.go +++ b/text_test.go @@ -20,7 +20,7 @@ func TestText(t *testing.T) { expected time.Time }{ {"2003-01-02T15:04:05Z", true, time.Date(2003, 1, 2, 15, 04, 05, 0, time.UTC)}, - {"2003-01-02 15:05:01", false, time.Date(2002, 1, 2, 15, 05, 05, 07, time.UTC)}, + {"2003-01-02 15:05:01", false, time.Time{}}, //negative case, invalid layout {"2024-11-22T03:01:02Z", true, time.Date(2024, 11, 22, 3, 1, 02, 0, time.UTC)}, {"2006-01-02T15:04:05+07:00", true, time.Date(2006, 1, 2, 15, 4, 5, 0, time.FixedZone("UTC+7", 7*60*60))}, } @@ -33,21 +33,24 @@ func TestText(t *testing.T) { tc := &testCases[i] arg := fmt.Sprintf("--time=%s", tc.input) err := f.Parse([]string{arg}) - if err != nil && tc.success == true { - t.Errorf("expected success, got %q", err) + if err != nil { + if tc.success { + t.Errorf("expected parsing to succeed, but got %q", err) + } continue - } else if err == nil && tc.success == false { - t.Errorf("expected failure, but succeeded") + } + if !tc.success { + t.Errorf("expected parsing failure, but parsing succeeded") continue - } else if tc.success { - parsedT := new(time.Time) - err := f.GetText("time", parsedT) - if err != nil { - t.Errorf("Got error trying to fetch the time flag: %v", err) - } - if !parsedT.Equal(tc.expected) { - t.Errorf("expected %q, got %q", tc.expected, parsedT) - } } + parsedT := new(time.Time) + err = f.GetText("time", parsedT) + if err != nil { + t.Errorf("Got error trying to fetch the time flag: %v", err) + } + if !parsedT.Equal(tc.expected) { + t.Errorf("expected %q, got %q", tc.expected, parsedT) + } + } } -- cgit v1.2.3 From 9c97fadb5c53411b3c4f0c1021f605b967632efd Mon Sep 17 00:00:00 2001 From: Andrea Tarocchi Date: Sun, 13 Apr 2025 11:25:48 +0200 Subject: fix #423 : Add helper function and some documentation to parse shorthand go test flags. --- README.md | 27 +++++++++++++++++++++++++++ flag.go | 21 +++++++++++++++++---- golangflag.go | 22 ++++++++++++++++++++++ golangflag_test.go | 16 +++++++++++++++- 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 7eacc5b..388c4e5 100644 --- a/README.md +++ b/README.md @@ -284,6 +284,33 @@ func main() { } ``` +### Using pflag with go test +`pflag` does not parse the shorthand versions of go test's built-in flags (i.e., those starting with `-test.`). +For more context, see issues [#63](https://github.com/spf13/pflag/issues/63) and [#238](https://github.com/spf13/pflag/issues/238) for more details. + +For example, if you use pflag in your `TestMain` function and call `pflag.Parse()` after defining your custom flags, running a test like this: +```bash +go test /your/tests -run ^YourTest -v --your-test-pflags +``` +will result in the `-v` flag being ignored. This happens because of the way pflag handles flag parsing, skipping over go test's built-in shorthand flags. +To work around this, you can use the `ParseSkippedFlags` function, which ensures that go test's flags are parsed separately using the standard flag package. + +**Example**: You want to parse go test flags that are otherwise ignore by `pflag.Parse()` +```go +import ( + goflag "flag" + flag "github.com/spf13/pflag" +) + +var ip *int = flag.Int("flagname", 1234, "help message for flagname") + +func main() { + flag.CommandLine.AddGoFlagSet(goflag.CommandLine) + flag.ParseSkippedFlags(os.Args[1:], goflag.CommandLine) + flag.Parse() +} +``` + ## More info You can see the full reference documentation of the pflag package diff --git a/flag.go b/flag.go index 4bdbd0c..4efef70 100644 --- a/flag.go +++ b/flag.go @@ -27,23 +27,32 @@ unaffected. Define flags using flag.String(), Bool(), Int(), etc. This declares an integer flag, -flagname, stored in the pointer ip, with type *int. + var ip = flag.Int("flagname", 1234, "help message for flagname") + If you like, you can bind the flag to a variable using the Var() functions. + var flagvar int func init() { flag.IntVar(&flagvar, "flagname", 1234, "help message for flagname") } + Or you can create custom flags that satisfy the Value interface (with pointer receivers) and couple them to flag parsing by + flag.Var(&flagVal, "name", "help message for flagname") + For such flags, the default value is just the initial value of the variable. After all flags are defined, call + flag.Parse() + to parse the command line into the defined flags. Flags may then be used directly. If you're using the flags themselves, they are all pointers; if you bind to variables, they're values. + fmt.Println("ip has value ", *ip) fmt.Println("flagvar has value ", flagvar) @@ -54,22 +63,26 @@ The arguments are indexed from 0 through flag.NArg()-1. The pflag package also defines some new functions that are not in flag, that give one-letter shorthands for flags. You can use these by appending 'P' to the name of any function that defines a flag. + var ip = flag.IntP("flagname", "f", 1234, "help message") var flagvar bool func init() { flag.BoolVarP(&flagvar, "boolname", "b", true, "help message") } flag.VarP(&flagval, "varname", "v", "help message") + Shorthand letters can be used with single dashes on the command line. Boolean shorthand flags can be combined with other shorthand flags. Command line flag syntax: + --flag // boolean flags only --flag=x Unlike the flag package, a single dash before an option means something different than a double dash. Single dashes signify a series of shorthand letters for flags. All but the last shorthand letter must be boolean flags. + // boolean flags -f -abc @@ -934,9 +947,9 @@ func (f *FlagSet) usage() { } } -//--unknown (args will be empty) -//--unknown --next-flag ... (args will be --next-flag ...) -//--unknown arg ... (args will be arg ...) +// --unknown (args will be empty) +// --unknown --next-flag ... (args will be --next-flag ...) +// --unknown arg ... (args will be arg ...) func stripUnknownFlagValue(args []string) []string { if len(args) == 0 { //--unknown @@ -1014,7 +1027,7 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parseFunc) (outShorts string, outArgs []string, err error) { outArgs = args - if strings.HasPrefix(shorthands, "test.") { + if isGotestShorthandFlag(shorthands) { return } diff --git a/golangflag.go b/golangflag.go index d3dd72b..f563907 100644 --- a/golangflag.go +++ b/golangflag.go @@ -10,6 +10,15 @@ import ( "strings" ) +// go test flags prefixes +func isGotestFlag(flag string) bool { + return strings.HasPrefix(flag, "-test.") +} + +func isGotestShorthandFlag(flag string) bool { + return strings.HasPrefix(flag, "test.") +} + // flagValueWrapper implements pflag.Value around a flag.Value. The main // difference here is the addition of the Type method that returns a string // name of the type. As this is generally unknown, we approximate that with @@ -103,3 +112,16 @@ func (f *FlagSet) AddGoFlagSet(newSet *goflag.FlagSet) { } f.addedGoFlagSets = append(f.addedGoFlagSets, newSet) } + +// ParseSkippedFlags explicitly Parses go test flags (i.e. the one starting with '-test.') with goflag.Parse(), +// since by default those are skipped by pflag.Parse(). +// Typical usage example: `ParseGoTestFlags(os.Args[1:], goflag.CommandLine)` +func ParseSkippedFlags(osArgs []string, goFlagSet *goflag.FlagSet) error { + var skippedFlags []string + for _, f := range osArgs { + if isGotestFlag(f) { + skippedFlags = append(skippedFlags, f) + } + } + return goFlagSet.Parse(skippedFlags) +} diff --git a/golangflag_test.go b/golangflag_test.go index 5bd831b..2ecefef 100644 --- a/golangflag_test.go +++ b/golangflag_test.go @@ -12,11 +12,14 @@ import ( func TestGoflags(t *testing.T) { goflag.String("stringFlag", "stringFlag", "stringFlag") goflag.Bool("boolFlag", false, "boolFlag") + var testxxxValue string + goflag.StringVar(&testxxxValue, "test.xxx", "test.xxx", "it is a test flag") f := NewFlagSet("test", ContinueOnError) f.AddGoFlagSet(goflag.CommandLine) - err := f.Parse([]string{"--stringFlag=bob", "--boolFlag"}) + args := []string{"--stringFlag=bob", "--boolFlag", "-test.xxx=testvalue"} + err := f.Parse(args) if err != nil { t.Fatal("expected no error; get", err) } @@ -40,6 +43,17 @@ func TestGoflags(t *testing.T) { t.Fatal("f.Parsed() return false after f.Parse() called") } + if testxxxValue != "test.xxx" { + t.Fatalf("expected testxxxValue to be test.xxx but got %v", testxxxValue) + } + err = ParseSkippedFlags(args, goflag.CommandLine) + if err != nil { + t.Fatal("expected no error; ParseSkippedFlags", err) + } + if testxxxValue != "testvalue" { + t.Fatalf("expected testxxxValue to be testvalue but got %v", testxxxValue) + } + // in fact it is useless. because `go test` called flag.Parse() if !goflag.CommandLine.Parsed() { t.Fatal("goflag.CommandLine.Parsed() return false after f.Parse() called") -- cgit v1.2.3 From 69bc3bd5b37fa90e994be9acecf7430269591713 Mon Sep 17 00:00:00 2001 From: Georges Varouchas Date: Thu, 22 May 2025 22:56:18 +0400 Subject: add support for Func() and BoolFunc() #426 Add support for two features which landed in the 'flag' package from the standard library: Func() and BoolFunc() and their two pflag specific versions: FuncP() and BoolFuncP() fixes #426 --- bool_func.go | 40 ++++++++++++++ bool_func_test.go | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++ func.go | 37 +++++++++++++ func_test.go | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 377 insertions(+) create mode 100644 bool_func.go create mode 100644 bool_func_test.go create mode 100644 func.go create mode 100644 func_test.go diff --git a/bool_func.go b/bool_func.go new file mode 100644 index 0000000..05783a9 --- /dev/null +++ b/bool_func.go @@ -0,0 +1,40 @@ +package pflag + +// -- func Value +type boolfuncValue func(string) error + +func (f boolfuncValue) Set(s string) error { return f(s) } + +func (f boolfuncValue) Type() string { return "func" } + +func (f boolfuncValue) String() string { return "" } // same behavior as stdlib 'flag' package + +func (f boolfuncValue) IsBoolFlag() bool { return true } + +// BoolFunc defines a func flag with specified name, callback function and usage string. +// +// The callback function will be called every time "--{name}" (or any form that matches the flag) is parsed +// on the command line. +func (f *FlagSet) BoolFunc(name string, usage string, fn func(string) error) { + f.BoolFuncP(name, "", usage, fn) +} + +// BoolFuncP is like BoolFunc, but accepts a shorthand letter that can be used after a single dash. +func (f *FlagSet) BoolFuncP(name, shorthand string, usage string, fn func(string) error) { + var val Value = boolfuncValue(fn) + flag := f.VarPF(val, name, shorthand, usage) + flag.NoOptDefVal = "true" +} + +// BoolFunc defines a func flag with specified name, callback function and usage string. +// +// The callback function will be called every time "--{name}" (or any form that matches the flag) is parsed +// on the command line. +func BoolFunc(name string, usage string, fn func(string) error) { + CommandLine.BoolFuncP(name, "", usage, fn) +} + +// BoolFuncP is like BoolFunc, but accepts a shorthand letter that can be used after a single dash. +func BoolFuncP(name, shorthand string, fn func(string) error, usage string) { + CommandLine.BoolFuncP(name, shorthand, usage, fn) +} diff --git a/bool_func_test.go b/bool_func_test.go new file mode 100644 index 0000000..c81970a --- /dev/null +++ b/bool_func_test.go @@ -0,0 +1,147 @@ +package pflag + +import ( + "errors" + "flag" + "io" + "strings" + "testing" +) + +func TestBoolFunc(t *testing.T) { + var count int + fn := func(_ string) error { + count++ + return nil + } + + fset := NewFlagSet("test", ContinueOnError) + fset.BoolFunc("func", "Callback function", fn) + + err := fset.Parse([]string{"--func", "--func=1", "--func=false"}) + if err != nil { + t.Fatal("expected no error; got", err) + } + + if count != 3 { + t.Fatalf("expected 3 calls to the callback, got %d calls", count) + } +} + +func TestBoolFuncP(t *testing.T) { + var count int + fn := func(_ string) error { + count++ + return nil + } + + fset := NewFlagSet("test", ContinueOnError) + fset.BoolFuncP("bfunc", "b", "Callback function", fn) + + err := fset.Parse([]string{"--bfunc", "--bfunc=0", "--bfunc=false", "-b", "-b=0"}) + if err != nil { + t.Fatal("expected no error; got", err) + } + + if count != 5 { + t.Fatalf("expected 5 calls to the callback, got %d calls", count) + } +} + +func TestBoolFuncCompat(t *testing.T) { + // compare behavior with the stdlib 'flag' package + type BoolFuncFlagSet interface { + BoolFunc(name string, usage string, fn func(string) error) + Parse([]string) error + } + + unitTestErr := errors.New("unit test error") + runCase := func(f BoolFuncFlagSet, name string, args []string) (values []string, err error) { + fn := func(s string) error { + values = append(values, s) + if s == "err" { + return unitTestErr + } + return nil + } + f.BoolFunc(name, "Callback function", fn) + + err = f.Parse(args) + return values, err + } + + t.Run("regular parsing", func(t *testing.T) { + flagName := "bflag" + args := []string{"--bflag", "--bflag=false", "--bflag=1", "--bflag=bar", "--bflag="} + + // It turns out that, even though the function is called "BoolFunc", + // the stanard flag package does not try to parse the value assigned to + // that cli flag as a boolean. The string provided on the command line is + // passed as is to the callback. + // e.g: with "--bflag=not_a_bool" on the command line, the FlagSet does not + // generate an error stating "invalid boolean value", and `fn` will be called + // with "not_a_bool" as an argument. + + stdFSet := flag.NewFlagSet("std test", flag.ContinueOnError) + stdValues, err := runCase(stdFSet, flagName, args) + if err != nil { + t.Fatalf("std flag: expected no error, got %v", err) + } + expected := []string{"true", "false", "1", "bar", ""} + if !cmpLists(expected, stdValues) { + t.Fatalf("std flag: expected %v, got %v", expected, stdValues) + } + + fset := NewFlagSet("pflag test", ContinueOnError) + pflagValues, err := runCase(fset, flagName, args) + if err != nil { + t.Fatalf("pflag: expected no error, got %v", err) + } + if !cmpLists(stdValues, pflagValues) { + t.Fatalf("pflag: expected %v, got %v", stdValues, pflagValues) + } + }) + + t.Run("error triggered by callback", func(t *testing.T) { + flagName := "bflag" + args := []string{"--bflag", "--bflag=err", "--bflag=after"} + + // test behavior of standard flag.Fset with an error triggere by the callback: + // (note: as can be seen in 'runCase()', if the callback sees "err" as a value + // for the bool flag, it will return an error) + stdFSet := flag.NewFlagSet("std test", flag.ContinueOnError) + stdFSet.SetOutput(io.Discard) // suppress output + + // run test case with standard flag.Fset + stdValues, err := runCase(stdFSet, flagName, args) + + // double check the standard behavior: + // - .Parse() should return an error, which contains the error message + if err == nil { + t.Fatalf("std flag: expected an error triggered by callback, got no error instead") + } + if !strings.HasSuffix(err.Error(), unitTestErr.Error()) { + t.Fatalf("std flag: expected unittest error, got unexpected error value: %T %v", err, err) + } + // - the function should have been called twice, with the first two values, + // the final "=after" should not be recorded + expected := []string{"true", "err"} + if !cmpLists(expected, stdValues) { + t.Fatalf("std flag: expected %v, got %v", expected, stdValues) + } + + // now run the test case on a pflag FlagSet: + fset := NewFlagSet("pflag test", ContinueOnError) + pflagValues, err := runCase(fset, flagName, args) + + // check that there is a similar error (note: pflag will _wrap_ the error, while the stdlib + // currently keeps the original message but creates a flat errors.Error) + if !errors.Is(err, unitTestErr) { + t.Fatalf("pflag: got unexpected error value: %T %v", err, err) + } + // the callback should be called the same number of times, with the same values: + if !cmpLists(stdValues, pflagValues) { + t.Fatalf("pflag: expected %v, got %v", stdValues, pflagValues) + } + }) +} diff --git a/func.go b/func.go new file mode 100644 index 0000000..e5f5436 --- /dev/null +++ b/func.go @@ -0,0 +1,37 @@ +package pflag + +// -- func Value +type funcValue func(string) error + +func (f funcValue) Set(s string) error { return f(s) } + +func (f funcValue) Type() string { return "func" } + +func (f funcValue) String() string { return "" } // same behavior as stdlib 'flag' package + +// Func defines a func flag with specified name, callback function and usage string. +// +// The callback function will be called every time "--{name}={value}" (or equivalent) is +// parsed on the command line, with "{value}" as an argument. +func (f *FlagSet) Func(name string, usage string, fn func(string) error) { + f.FuncP(name, "", usage, fn) +} + +// FuncP is like Func, but accepts a shorthand letter that can be used after a single dash. +func (f *FlagSet) FuncP(name string, shorthand string, usage string, fn func(string) error) { + var val Value = funcValue(fn) + f.VarP(val, name, shorthand, usage) +} + +// Func defines a func flag with specified name, callback function and usage string. +// +// The callback function will be called every time "--{name}={value}" (or equivalent) is +// parsed on the command line, with "{value}" as an argument. +func Func(name string, fn func(string) error, usage string) { + CommandLine.FuncP(name, "", usage, fn) +} + +// FuncP is like Func, but accepts a shorthand letter that can be used after a single dash. +func FuncP(name, shorthand string, fn func(string) error, usage string) { + CommandLine.FuncP(name, shorthand, usage, fn) +} diff --git a/func_test.go b/func_test.go new file mode 100644 index 0000000..4ec1cd6 --- /dev/null +++ b/func_test.go @@ -0,0 +1,153 @@ +package pflag + +import ( + "errors" + "flag" + "io" + "strings" + "testing" +) + +func cmpLists(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func TestFunc(t *testing.T) { + var values []string + fn := func(s string) error { + values = append(values, s) + return nil + } + + fset := NewFlagSet("test", ContinueOnError) + fset.Func("fnflag", "Callback function", fn) + + err := fset.Parse([]string{"--fnflag=aa", "--fnflag", "bb"}) + if err != nil { + t.Fatal("expected no error; got", err) + } + + expected := []string{"aa", "bb"} + if !cmpLists(expected, values) { + t.Fatalf("expected %v, got %v", expected, values) + } +} + +func TestFuncP(t *testing.T) { + var values []string + fn := func(s string) error { + values = append(values, s) + return nil + } + + fset := NewFlagSet("test", ContinueOnError) + fset.FuncP("fnflag", "f", "Callback function", fn) + + err := fset.Parse([]string{"--fnflag=a", "--fnflag", "b", "-fc", "-f=d", "-f", "e"}) + if err != nil { + t.Fatal("expected no error; got", err) + } + + expected := []string{"a", "b", "c", "d", "e"} + if !cmpLists(expected, values) { + t.Fatalf("expected %v, got %v", expected, values) + } +} + +func TestFuncCompat(t *testing.T) { + // compare behavior with the stdlib 'flag' package + type FuncFlagSet interface { + Func(name string, usage string, fn func(string) error) + Parse([]string) error + } + + unitTestErr := errors.New("unit test error") + runCase := func(f FuncFlagSet, name string, args []string) (values []string, err error) { + fn := func(s string) error { + values = append(values, s) + if s == "err" { + return unitTestErr + } + return nil + } + f.Func(name, "Callback function", fn) + + err = f.Parse(args) + return values, err + } + + t.Run("regular parsing", func(t *testing.T) { + flagName := "fnflag" + args := []string{"--fnflag=xx", "--fnflag", "yy", "--fnflag=zz"} + + stdFSet := flag.NewFlagSet("std test", flag.ContinueOnError) + stdValues, err := runCase(stdFSet, flagName, args) + if err != nil { + t.Fatalf("std flag: expected no error, got %v", err) + } + expected := []string{"xx", "yy", "zz"} + if !cmpLists(expected, stdValues) { + t.Fatalf("std flag: expected %v, got %v", expected, stdValues) + } + + fset := NewFlagSet("pflag test", ContinueOnError) + pflagValues, err := runCase(fset, flagName, args) + if err != nil { + t.Fatalf("pflag: expected no error, got %v", err) + } + if !cmpLists(stdValues, pflagValues) { + t.Fatalf("pflag: expected %v, got %v", stdValues, pflagValues) + } + }) + + t.Run("error triggered by callback", func(t *testing.T) { + flagName := "fnflag" + args := []string{"--fnflag", "before", "--fnflag", "err", "--fnflag", "after"} + + // test behavior of standard flag.Fset with an error triggere by the callback: + // (note: as can be seen in 'runCase()', if the callback sees "err" as a value + // for the bool flag, it will return an error) + stdFSet := flag.NewFlagSet("std test", flag.ContinueOnError) + stdFSet.SetOutput(io.Discard) // suppress output + + // run test case with standard flag.Fset + stdValues, err := runCase(stdFSet, flagName, args) + + // double check the standard behavior: + // - .Parse() should return an error, which contains the error message + if err == nil { + t.Fatalf("std flag: expected an error triggered by callback, got no error instead") + } + if !strings.HasSuffix(err.Error(), unitTestErr.Error()) { + t.Fatalf("std flag: expected unittest error, got unexpected error value: %T %v", err, err) + } + // - the function should have been called twice, with the first two values, + // the final "=after" should not be recorded + expected := []string{"before", "err"} + if !cmpLists(expected, stdValues) { + t.Fatalf("std flag: expected %v, got %v", expected, stdValues) + } + + // now run the test case on a pflag FlagSet: + fset := NewFlagSet("pflag test", ContinueOnError) + pflagValues, err := runCase(fset, flagName, args) + + // check that there is a similar error (note: pflag will _wrap_ the error, while the stdlib + // currently keeps the original message but creates a flat errors.Error) + if !errors.Is(err, unitTestErr) { + t.Fatalf("pflag: got unexpected error value: %T %v", err, err) + } + // the callback should be called the same number of times, with the same values: + if !cmpLists(stdValues, pflagValues) { + t.Fatalf("pflag: expected %v, got %v", stdValues, pflagValues) + } + }) +} -- cgit v1.2.3 From f4c97c2487b06cff392d2958534e7195f79847fb Mon Sep 17 00:00:00 2001 From: Georges Varouchas Date: Mon, 9 Jun 2025 23:03:34 +0400 Subject: minor: fix typos in comments --- bool_func_test.go | 4 ++-- func_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bool_func_test.go b/bool_func_test.go index c81970a..ffb4fa9 100644 --- a/bool_func_test.go +++ b/bool_func_test.go @@ -75,7 +75,7 @@ func TestBoolFuncCompat(t *testing.T) { args := []string{"--bflag", "--bflag=false", "--bflag=1", "--bflag=bar", "--bflag="} // It turns out that, even though the function is called "BoolFunc", - // the stanard flag package does not try to parse the value assigned to + // the standard flag package does not try to parse the value assigned to // that cli flag as a boolean. The string provided on the command line is // passed as is to the callback. // e.g: with "--bflag=not_a_bool" on the command line, the FlagSet does not @@ -106,7 +106,7 @@ func TestBoolFuncCompat(t *testing.T) { flagName := "bflag" args := []string{"--bflag", "--bflag=err", "--bflag=after"} - // test behavior of standard flag.Fset with an error triggere by the callback: + // test behavior of standard flag.Fset with an error triggered by the callback: // (note: as can be seen in 'runCase()', if the callback sees "err" as a value // for the bool flag, it will return an error) stdFSet := flag.NewFlagSet("std test", flag.ContinueOnError) diff --git a/func_test.go b/func_test.go index 4ec1cd6..9be74fa 100644 --- a/func_test.go +++ b/func_test.go @@ -112,9 +112,9 @@ func TestFuncCompat(t *testing.T) { flagName := "fnflag" args := []string{"--fnflag", "before", "--fnflag", "err", "--fnflag", "after"} - // test behavior of standard flag.Fset with an error triggere by the callback: + // test behavior of standard flag.Fset with an error triggered by the callback: // (note: as can be seen in 'runCase()', if the callback sees "err" as a value - // for the bool flag, it will return an error) + // for the flag, it will return an error) stdFSet := flag.NewFlagSet("std test", flag.ContinueOnError) stdFSet.SetOutput(io.Discard) // suppress output -- cgit v1.2.3 From 4730aa0d979f34d4f7427d524b84043557ba72ef Mon Sep 17 00:00:00 2001 From: Georges Varouchas Date: Mon, 9 Jun 2025 22:38:23 +0400 Subject: fix help message for Func and BoolFunc flags #430 * have '.Type()' for boolfuncValue return "boolfunc" (dedicated value, which now makes it distinct from funcValue) * hide extra "(default )" by stating that "" should be treated as a zero value for a boolFlag note: - a boolfuncValue matches 'case boolFlag:', as it implements the boolFlag interface, - treating "" as a value which shouldn't trigger a "(default )" for a regular Bool flag does not look like a breaking change * hide extra "[=something]" for boolfuncValue * set default placeholder name for "boolfunc" and "func" flag types --- bool_func.go | 2 +- bool_func_test.go | 30 ++++++++++++++++++++++++++++++ flag.go | 8 +++++--- func_test.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/bool_func.go b/bool_func.go index 05783a9..76422bf 100644 --- a/bool_func.go +++ b/bool_func.go @@ -5,7 +5,7 @@ type boolfuncValue func(string) error func (f boolfuncValue) Set(s string) error { return f(s) } -func (f boolfuncValue) Type() string { return "func" } +func (f boolfuncValue) Type() string { return "boolfunc" } func (f boolfuncValue) String() string { return "" } // same behavior as stdlib 'flag' package diff --git a/bool_func_test.go b/bool_func_test.go index ffb4fa9..c16be83 100644 --- a/bool_func_test.go +++ b/bool_func_test.go @@ -145,3 +145,33 @@ func TestBoolFuncCompat(t *testing.T) { } }) } + +func TestBoolFuncUsage(t *testing.T) { + t.Run("regular func flag", func(t *testing.T) { + // regular boolfunc flag: + // expect to see '--flag1' followed by the usageMessage, and no mention of a default value + fset := NewFlagSet("unittest", ContinueOnError) + fset.BoolFunc("flag1", "usage message", func(s string) error { return nil }) + usage := fset.FlagUsagesWrapped(80) + + usage = strings.TrimSpace(usage) + expected := "--flag1 usage message" + if usage != expected { + t.Fatalf("unexpected generated usage message\n expected: %s\n got: %s", expected, usage) + } + }) + + t.Run("func flag with placeholder name", func(t *testing.T) { + // func flag, with a placeholder name: + // if usageMesage contains a placeholder, expect '--flag2 {placeholder}'; still expect no mention of a default value + fset := NewFlagSet("unittest", ContinueOnError) + fset.BoolFunc("flag2", "usage message with `name` placeholder", func(s string) error { return nil }) + usage := fset.FlagUsagesWrapped(80) + + usage = strings.TrimSpace(usage) + expected := "--flag2 name usage message with name placeholder" + if usage != expected { + t.Fatalf("unexpected generated usage message\n expected: %s\n got: %s", expected, usage) + } + }) +} diff --git a/flag.go b/flag.go index 0b5be7a..d4dfbc5 100644 --- a/flag.go +++ b/flag.go @@ -549,7 +549,7 @@ func (f *FlagSet) PrintDefaults() { func (f *Flag) defaultIsZeroValue() bool { switch f.Value.(type) { case boolFlag: - return f.DefValue == "false" + return f.DefValue == "false" || f.DefValue == "" case *durationValue: // Beginning in Go 1.7, duration zero values are "0s" return f.DefValue == "0" || f.DefValue == "0s" @@ -599,8 +599,10 @@ func UnquoteUsage(flag *Flag) (name string, usage string) { name = flag.Value.Type() switch name { - case "bool": + case "bool", "boolfunc": name = "" + case "func": + name = "value" case "float64": name = "float" case "int64": @@ -718,7 +720,7 @@ func (f *FlagSet) FlagUsagesWrapped(cols int) string { switch flag.Value.Type() { case "string": line += fmt.Sprintf("[=\"%s\"]", flag.NoOptDefVal) - case "bool": + case "bool", "boolfunc": if flag.NoOptDefVal != "true" { line += fmt.Sprintf("[=%s]", flag.NoOptDefVal) } diff --git a/func_test.go b/func_test.go index 9be74fa..d492b48 100644 --- a/func_test.go +++ b/func_test.go @@ -151,3 +151,33 @@ func TestFuncCompat(t *testing.T) { } }) } + +func TestFuncUsage(t *testing.T) { + t.Run("regular func flag", func(t *testing.T) { + // regular func flag: + // expect to see '--flag1 value' followed by the usageMessage, and no mention of a default value + fset := NewFlagSet("unittest", ContinueOnError) + fset.Func("flag1", "usage message", func(s string) error { return nil }) + usage := fset.FlagUsagesWrapped(80) + + usage = strings.TrimSpace(usage) + expected := "--flag1 value usage message" + if usage != expected { + t.Fatalf("unexpected generated usage message\n expected: %s\n got: %s", expected, usage) + } + }) + + t.Run("func flag with placeholder name", func(t *testing.T) { + // func flag, with a placeholder name: + // if usageMesage contains a placeholder, expect that name; still expect no mention of a default value + fset := NewFlagSet("unittest", ContinueOnError) + fset.Func("flag2", "usage message with `name` placeholder", func(s string) error { return nil }) + usage := fset.FlagUsagesWrapped(80) + + usage = strings.TrimSpace(usage) + expected := "--flag2 name usage message with name placeholder" + if usage != expected { + t.Fatalf("unexpected generated usage message\n expected: %s\n got: %s", expected, usage) + } + }) +} -- cgit v1.2.3 From 1a4b5b2e5c7ee4a194cebc579bb34198187df73d Mon Sep 17 00:00:00 2001 From: Georges Varouchas Date: Fri, 27 Jun 2025 18:02:03 +0200 Subject: fix discrepancy in order of arguments for Func() and BoolFunc() #433 for no good reason: the order of arguments would differ when calling pflag.BoolFuncP(...) and (*FlagSet).BoolFuncP(...) (same goes for Func() and FuncP()) in this commit: align all functions on stdlib's order fixes #433 --- bool_func.go | 2 +- func.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bool_func.go b/bool_func.go index 76422bf..83d77af 100644 --- a/bool_func.go +++ b/bool_func.go @@ -35,6 +35,6 @@ func BoolFunc(name string, usage string, fn func(string) error) { } // BoolFuncP is like BoolFunc, but accepts a shorthand letter that can be used after a single dash. -func BoolFuncP(name, shorthand string, fn func(string) error, usage string) { +func BoolFuncP(name, shorthand string, usage string, fn func(string) error) { CommandLine.BoolFuncP(name, shorthand, usage, fn) } diff --git a/func.go b/func.go index e5f5436..9f4d88f 100644 --- a/func.go +++ b/func.go @@ -27,11 +27,11 @@ func (f *FlagSet) FuncP(name string, shorthand string, usage string, fn func(str // // The callback function will be called every time "--{name}={value}" (or equivalent) is // parsed on the command line, with "{value}" as an argument. -func Func(name string, fn func(string) error, usage string) { +func Func(name string, usage string, fn func(string) error) { CommandLine.FuncP(name, "", usage, fn) } // FuncP is like Func, but accepts a shorthand letter that can be used after a single dash. -func FuncP(name, shorthand string, fn func(string) error, usage string) { +func FuncP(name, shorthand string, usage string, fn func(string) error) { CommandLine.FuncP(name, shorthand, usage, fn) } -- cgit v1.2.3 From 1992c5a7b88da3490ad7ac088c72dd20b466ee8d Mon Sep 17 00:00:00 2001 From: Maximilian Frank Date: Wed, 23 Mar 2022 12:42:55 +0100 Subject: Add support for time.Time flags --- time.go | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ time_test.go | 65 ++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 time.go create mode 100644 time_test.go diff --git a/time.go b/time.go new file mode 100644 index 0000000..7964346 --- /dev/null +++ b/time.go @@ -0,0 +1,120 @@ +package pflag + +import ( + "fmt" + "strings" + "time" +) + +// TimeValue adapts time.Time for use as a flag. +type TimeValue struct { + *time.Time + formats []string +} + +func newTimeValue(val time.Time, p *time.Time, formats []string) *TimeValue { + *p = val + return &TimeValue{ + Time: p, + formats: formats, + } +} + +// Set time.Time value from string based on accepted formats. +func (d *TimeValue) Set(s string) error { + s = strings.TrimSpace(s) + for _, f := range d.formats { + v, err := time.Parse(f, s) + if err != nil { + continue + } + *d.Time = v + return nil + } + + formatsString := "" + for i, f := range d.formats { + if i > 0 { + formatsString += ", " + } + formatsString += fmt.Sprintf("`%s`", f) + } + + return fmt.Errorf("invalid time format `%s` must be one of: %s", s, formatsString) +} + +// Type name for time.Time flags. +func (d *TimeValue) Type() string { + return "time" +} + +func (d *TimeValue) String() string { return d.Time.Format(time.RFC3339Nano) } + +// GetTime return the time value of a flag with the given name +func (f *FlagSet) GetTime(name string) (time.Time, error) { + flag := f.Lookup(name) + if flag == nil { + err := fmt.Errorf("flag accessed but not defined: %s", name) + return time.Time{}, err + } + + if flag.Value.Type() != "time" { + err := fmt.Errorf("trying to get %s value of flag of type %s", "time", flag.Value.Type()) + return time.Time{}, err + } + + val, ok := flag.Value.(*TimeValue) + if !ok { + return time.Time{}, fmt.Errorf("value %s is not a time", flag.Value) + } + + return *val.Time, nil +} + +// TimeVar defines a time.Time flag with specified name, default value, and usage string. +// The argument p points to a time.Time variable in which to store the value of the flag. +func (f *FlagSet) TimeVar(p *time.Time, name string, value time.Time, formats []string, usage string) { + f.VarP(newTimeValue(value, p, formats), name, "", usage) +} + +// TimeVarP is like TimeVar, but accepts a shorthand letter that can be used after a single dash. +func (f *FlagSet) TimeVarP(p *time.Time, name, shorthand string, value time.Time, formats []string, usage string) { + f.VarP(newTimeValue(value, p, formats), name, shorthand, usage) +} + +// TimeVar defines a time.Time flag with specified name, default value, and usage string. +// The argument p points to a time.Time variable in which to store the value of the flag. +func TimeVar(p *time.Time, name string, value time.Time, formats []string, usage string) { + CommandLine.VarP(newTimeValue(value, p, formats), name, "", usage) +} + +// TimeVarP is like TimeVar, but accepts a shorthand letter that can be used after a single dash. +func TimeVarP(p *time.Time, name, shorthand string, value time.Time, formats []string, usage string) { + CommandLine.VarP(newTimeValue(value, p, formats), name, shorthand, usage) +} + +// Time defines a time.Time flag with specified name, default value, and usage string. +// The return value is the address of a time.Time variable that stores the value of the flag. +func (f *FlagSet) Time(name string, value time.Time, formats []string, usage string) *time.Time { + p := new(time.Time) + f.TimeVarP(p, name, "", value, formats, usage) + return p +} + +// TimeP is like Time, but accepts a shorthand letter that can be used after a single dash. +func (f *FlagSet) TimeP(name, shorthand string, value time.Time, formats []string, usage string) *time.Time { + p := new(time.Time) + f.TimeVarP(p, name, shorthand, value, formats, usage) + return p +} + +// Time defines a time.Time flag with specified name, default value, and usage string. +// The return value is the address of a time.Time variable that stores the value of the flag. +func Time(name string, value time.Time, formats []string, usage string) *time.Time { + return CommandLine.TimeP(name, "", value, formats, usage) +} + +// TimeP is like Time, but accepts a shorthand letter that can be used after a single dash. +func TimeP(name, shorthand string, value time.Time, formats []string, usage string) *time.Time { + return CommandLine.TimeP(name, shorthand, value, formats, usage) +} diff --git a/time_test.go b/time_test.go new file mode 100644 index 0000000..3cb1009 --- /dev/null +++ b/time_test.go @@ -0,0 +1,65 @@ +package pflag + +import ( + "fmt" + "os" + "testing" + "time" +) + +func setUpTimeVar(t *time.Time, formats []string) *FlagSet { + f := NewFlagSet("test", ContinueOnError) + f.TimeVar(t, "time", time.Time{}, formats, "Time") + return f +} + +func TestTime(t *testing.T) { + testCases := []struct { + input string + success bool + expected string + }{ + {"2022-01-01T01:01:01+00:00", true, "2022-01-01T01:01:01Z"}, + {" 2022-01-01T01:01:01+00:00", true, "2022-01-01T01:01:01Z"}, + {"2022-01-01T01:01:01+00:00 ", true, "2022-01-01T01:01:01Z"}, + {"2022-01-01T01:01:01+02:00", true, "2022-01-01T01:01:01+02:00"}, + {"2022-01-01T01:01:01.01+02:00", true, "2022-01-01T01:01:01.01+02:00"}, + {"Sat, 01 Jan 2022 01:01:01 +0000", true, "2022-01-01T01:01:01Z"}, + {"Sat, 01 Jan 2022 01:01:01 +0200", true, "2022-01-01T01:01:01+02:00"}, + {"Sat, 01 Jan 2022 01:01:01 +0000", true, "2022-01-01T01:01:01Z"}, + {"", false, ""}, + {"not a date", false, ""}, + {"2022-01-01 01:01:01", false, ""}, + {"2022-01-01T01:01:01", false, ""}, + {"01 Jan 2022 01:01:01 +0000", false, ""}, + {"Sat, 01 Jan 2022 01:01:01", false, ""}, + } + + devnull, _ := os.Open(os.DevNull) + os.Stderr = devnull + for i := range testCases { + var timeVar time.Time + formats := []string{time.RFC3339Nano, time.RFC1123Z} + f := setUpTimeVar(&timeVar, formats) + + tc := &testCases[i] + + arg := fmt.Sprintf("--time=%s", tc.input) + err := f.Parse([]string{arg}) + if err != nil && tc.success == true { + t.Errorf("expected success, got %q", err) + continue + } else if err == nil && tc.success == false { + t.Errorf("expected failure") + continue + } else if tc.success { + timeResult, err := f.GetTime("time") + if err != nil { + t.Errorf("Got error trying to fetch the Time flag: %v", err) + } + if timeResult.Format(time.RFC3339Nano) != tc.expected { + t.Errorf("expected %q, got %q", tc.expected, timeVar.Format(time.RFC3339Nano)) + } + } + } +} -- cgit v1.2.3 From c5ce22e836c6268eb270e8f28ae5e3729a27c82d Mon Sep 17 00:00:00 2001 From: Maximilian Frank Date: Mon, 14 Jul 2025 08:44:29 +0900 Subject: Use time.Time for expectations in time flag tests --- time_test.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/time_test.go b/time_test.go index 3cb1009..c002910 100644 --- a/time_test.go +++ b/time_test.go @@ -17,22 +17,22 @@ func TestTime(t *testing.T) { testCases := []struct { input string success bool - expected string + expected time.Time }{ - {"2022-01-01T01:01:01+00:00", true, "2022-01-01T01:01:01Z"}, - {" 2022-01-01T01:01:01+00:00", true, "2022-01-01T01:01:01Z"}, - {"2022-01-01T01:01:01+00:00 ", true, "2022-01-01T01:01:01Z"}, - {"2022-01-01T01:01:01+02:00", true, "2022-01-01T01:01:01+02:00"}, - {"2022-01-01T01:01:01.01+02:00", true, "2022-01-01T01:01:01.01+02:00"}, - {"Sat, 01 Jan 2022 01:01:01 +0000", true, "2022-01-01T01:01:01Z"}, - {"Sat, 01 Jan 2022 01:01:01 +0200", true, "2022-01-01T01:01:01+02:00"}, - {"Sat, 01 Jan 2022 01:01:01 +0000", true, "2022-01-01T01:01:01Z"}, - {"", false, ""}, - {"not a date", false, ""}, - {"2022-01-01 01:01:01", false, ""}, - {"2022-01-01T01:01:01", false, ""}, - {"01 Jan 2022 01:01:01 +0000", false, ""}, - {"Sat, 01 Jan 2022 01:01:01", false, ""}, + {"2022-01-01T01:01:01+00:00", true, time.Date(2022, 1, 1, 1, 1, 1, 0, time.UTC)}, + {" 2022-01-01T01:01:01+00:00", true, time.Date(2022, 1, 1, 1, 1, 1, 0, time.UTC)}, + {"2022-01-01T01:01:01+00:00 ", true, time.Date(2022, 1, 1, 1, 1, 1, 0, time.UTC)}, + {"2022-01-01T01:01:01+02:00", true, time.Date(2022, 1, 1, 1, 1, 1, 0, time.FixedZone("UTC+2", 2*60*60))}, + {"2022-01-01T01:01:01.01+02:00", true, time.Date(2022, 1, 1, 1, 1, 1, 10000000, time.FixedZone("UTC+2", 2*60*60))}, + {"Sat, 01 Jan 2022 01:01:01 +0000", true, time.Date(2022, 1, 1, 1, 1, 1, 0, time.UTC)}, + {"Sat, 01 Jan 2022 01:01:01 +0200", true, time.Date(2022, 1, 1, 1, 1, 1, 0, time.FixedZone("UTC+2", 2*60*60))}, + {"Sat, 01 Jan 2022 01:01:01 +0000", true, time.Date(2022, 1, 1, 1, 1, 1, 0, time.UTC)}, + {"", false, time.Time{}}, + {"not a date", false, time.Time{}}, + {"2022-01-01 01:01:01", false, time.Time{}}, + {"2022-01-01T01:01:01", false, time.Time{}}, + {"01 Jan 2022 01:01:01 +0000", false, time.Time{}}, + {"Sat, 01 Jan 2022 01:01:01", false, time.Time{}}, } devnull, _ := os.Open(os.DevNull) @@ -57,8 +57,8 @@ func TestTime(t *testing.T) { if err != nil { t.Errorf("Got error trying to fetch the Time flag: %v", err) } - if timeResult.Format(time.RFC3339Nano) != tc.expected { - t.Errorf("expected %q, got %q", tc.expected, timeVar.Format(time.RFC3339Nano)) + if !timeResult.Equal(tc.expected) { + t.Errorf("expected %q, got %q", tc.expected.Format(time.RFC3339Nano), timeVar.Format(time.RFC3339Nano)) } } } -- cgit v1.2.3 From d15848db482b52179577da9738cf9702d8d28466 Mon Sep 17 00:00:00 2001 From: Maximilian Frank Date: Mon, 14 Jul 2025 08:52:47 +0900 Subject: Remove unnecessary time test stderr dev null redirect --- time_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/time_test.go b/time_test.go index c002910..46a5ada 100644 --- a/time_test.go +++ b/time_test.go @@ -2,7 +2,6 @@ package pflag import ( "fmt" - "os" "testing" "time" ) @@ -35,8 +34,6 @@ func TestTime(t *testing.T) { {"Sat, 01 Jan 2022 01:01:01", false, time.Time{}}, } - devnull, _ := os.Open(os.DevNull) - os.Stderr = devnull for i := range testCases { var timeVar time.Time formats := []string{time.RFC3339Nano, time.RFC1123Z} -- cgit v1.2.3 From 7cc25e3bdd8c540b243f70c366ba1f1856fcd9e9 Mon Sep 17 00:00:00 2001 From: Tomas Aschan <1550920+tomasaschan@users.noreply.github.com> Date: Wed, 16 Jul 2025 23:37:38 +0200 Subject: Don't export `TimeValue` (yet) --- time.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/time.go b/time.go index 7964346..dbaeeab 100644 --- a/time.go +++ b/time.go @@ -7,21 +7,21 @@ import ( ) // TimeValue adapts time.Time for use as a flag. -type TimeValue struct { +type timeValue struct { *time.Time formats []string } -func newTimeValue(val time.Time, p *time.Time, formats []string) *TimeValue { +func newTimeValue(val time.Time, p *time.Time, formats []string) *timeValue { *p = val - return &TimeValue{ + return &timeValue{ Time: p, formats: formats, } } // Set time.Time value from string based on accepted formats. -func (d *TimeValue) Set(s string) error { +func (d *timeValue) Set(s string) error { s = strings.TrimSpace(s) for _, f := range d.formats { v, err := time.Parse(f, s) @@ -44,11 +44,11 @@ func (d *TimeValue) Set(s string) error { } // Type name for time.Time flags. -func (d *TimeValue) Type() string { +func (d *timeValue) Type() string { return "time" } -func (d *TimeValue) String() string { return d.Time.Format(time.RFC3339Nano) } +func (d *timeValue) String() string { return d.Time.Format(time.RFC3339Nano) } // GetTime return the time value of a flag with the given name func (f *FlagSet) GetTime(name string) (time.Time, error) { @@ -63,7 +63,7 @@ func (f *FlagSet) GetTime(name string) (time.Time, error) { return time.Time{}, err } - val, ok := flag.Value.(*TimeValue) + val, ok := flag.Value.(*timeValue) if !ok { return time.Time{}, fmt.Errorf("value %s is not a time", flag.Value) } -- cgit v1.2.3 From e3be2ebcffcc36be35e23d418d3e0ba86239826a Mon Sep 17 00:00:00 2001 From: Tomas Aschan <1550920+tomasaschan@users.noreply.github.com> Date: Wed, 16 Jul 2025 23:38:11 +0200 Subject: Reduce duplication by forwarding to sibling functions --- time.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/time.go b/time.go index dbaeeab..dc02480 100644 --- a/time.go +++ b/time.go @@ -74,7 +74,7 @@ func (f *FlagSet) GetTime(name string) (time.Time, error) { // TimeVar defines a time.Time flag with specified name, default value, and usage string. // The argument p points to a time.Time variable in which to store the value of the flag. func (f *FlagSet) TimeVar(p *time.Time, name string, value time.Time, formats []string, usage string) { - f.VarP(newTimeValue(value, p, formats), name, "", usage) + f.TimeVarP(p, name, "", value, formats, usage) } // TimeVarP is like TimeVar, but accepts a shorthand letter that can be used after a single dash. @@ -85,7 +85,7 @@ func (f *FlagSet) TimeVarP(p *time.Time, name, shorthand string, value time.Time // TimeVar defines a time.Time flag with specified name, default value, and usage string. // The argument p points to a time.Time variable in which to store the value of the flag. func TimeVar(p *time.Time, name string, value time.Time, formats []string, usage string) { - CommandLine.VarP(newTimeValue(value, p, formats), name, "", usage) + CommandLine.TimeVarP(p, name, "", value, formats, usage) } // TimeVarP is like TimeVar, but accepts a shorthand letter that can be used after a single dash. @@ -96,9 +96,7 @@ func TimeVarP(p *time.Time, name, shorthand string, value time.Time, formats []s // Time defines a time.Time flag with specified name, default value, and usage string. // The return value is the address of a time.Time variable that stores the value of the flag. func (f *FlagSet) Time(name string, value time.Time, formats []string, usage string) *time.Time { - p := new(time.Time) - f.TimeVarP(p, name, "", value, formats, usage) - return p + return f.TimeP(name, "", value, formats, usage) } // TimeP is like Time, but accepts a shorthand letter that can be used after a single dash. -- cgit v1.2.3 From 5159cdaa32a65acb63296cbd45033696aa9c7e88 Mon Sep 17 00:00:00 2001 From: Tomas Aschan <1550920+tomasaschan@users.noreply.github.com> Date: Thu, 17 Jul 2025 11:18:39 +0200 Subject: Ensure output is written to correct stream --- flag.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flag.go b/flag.go index 2723762..47c7960 100644 --- a/flag.go +++ b/flag.go @@ -1150,7 +1150,7 @@ func (f *FlagSet) Parse(arguments []string) error { case ContinueOnError: return err case ExitOnError: - fmt.Println(err) + fmt.Fprintln(f.Output(), err) os.Exit(2) case PanicOnError: panic(err) -- cgit v1.2.3 From 391036c21b8dc5e6c4b1d535e69885d8e613fb06 Mon Sep 17 00:00:00 2001 From: Tomas Aschan <1550920+tomasaschan@users.noreply.github.com> Date: Thu, 17 Jul 2025 11:18:53 +0200 Subject: Ensure output is written also from ParseAll --- flag.go | 1 + 1 file changed, 1 insertion(+) diff --git a/flag.go b/flag.go index 47c7960..0d07c36 100644 --- a/flag.go +++ b/flag.go @@ -1176,6 +1176,7 @@ func (f *FlagSet) ParseAll(arguments []string, fn func(flag *Flag, value string) case ContinueOnError: return err case ExitOnError: + fmt.Fprintln(f.Output(), err) os.Exit(2) case PanicOnError: panic(err) -- cgit v1.2.3 From 1b52f7648f880d6ef6aaa19e45b102edadd6c50a Mon Sep 17 00:00:00 2001 From: Oliver Kuckertz Date: Thu, 24 Jul 2025 17:52:43 +0200 Subject: Omit zero time.Time default from usage line This was missed in #348, there previously was no way to omit the default value. Treating zero timestamp values as canary for absence of a default is in line with other flag types, e.g. zero ints. --- time.go | 8 +++++++- time_test.go | 28 +++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/time.go b/time.go index dc02480..3dee424 100644 --- a/time.go +++ b/time.go @@ -48,7 +48,13 @@ func (d *timeValue) Type() string { return "time" } -func (d *timeValue) String() string { return d.Time.Format(time.RFC3339Nano) } +func (d *timeValue) String() string { + if d.Time.IsZero() { + return "" + } else { + return d.Time.Format(time.RFC3339Nano) + } +} // GetTime return the time value of a flag with the given name func (f *FlagSet) GetTime(name string) (time.Time, error) { diff --git a/time_test.go b/time_test.go index 46a5ada..62d9a79 100644 --- a/time_test.go +++ b/time_test.go @@ -2,13 +2,14 @@ package pflag import ( "fmt" + "strings" "testing" "time" ) func setUpTimeVar(t *time.Time, formats []string) *FlagSet { f := NewFlagSet("test", ContinueOnError) - f.TimeVar(t, "time", time.Time{}, formats, "Time") + f.TimeVar(t, "time", *t, formats, "Time") return f } @@ -60,3 +61,28 @@ func TestTime(t *testing.T) { } } } + +func usageForTimeFlagSet(t *testing.T, timeVar time.Time) string { + t.Helper() + formats := []string{time.RFC3339Nano, time.RFC1123Z} + f := setUpTimeVar(&timeVar, formats) + if err := f.Parse([]string{}); err != nil { + t.Fatalf("expected success, got %q", err) + } + return f.FlagUsages() +} + +func TestTimeDefaultZero(t *testing.T) { + usage := usageForTimeFlagSet(t, time.Time{}) + if strings.Contains(usage, "default") { + t.Errorf("expected no default value in usage, got %q", usage) + } +} + +func TestTimeDefaultNonZero(t *testing.T) { + timeVar := time.Date(2025, 1, 1, 1, 1, 1, 0, time.UTC) + usage := usageForTimeFlagSet(t, timeVar) + if !strings.Contains(usage, "default") || !strings.Contains(usage, "2025") { + t.Errorf("expected default value in usage, got %q", usage) + } +} -- cgit v1.2.3 From 44aa4aa86e19d69d62479bfa1a244d4858a62ee5 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 15 Sep 2021 12:49:31 +0200 Subject: add CopyToGoFlagSet This is useful for programs which want to define some flags with pflag (for example, in external packages) but still need to use Go flag command line parsing to preserve backward compatibility with previous releases, in particular support for single-dash flags. Without this in pflag, such tools have to resort to copying via the public API, which leads to less useful help messages (type of basic values will be unknown). --- golangflag.go | 34 +++++++++++++++++++++++++ golangflag_test.go | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/golangflag.go b/golangflag.go index f563907..e62eab5 100644 --- a/golangflag.go +++ b/golangflag.go @@ -8,6 +8,7 @@ import ( goflag "flag" "reflect" "strings" + "time" ) // go test flags prefixes @@ -113,6 +114,38 @@ func (f *FlagSet) AddGoFlagSet(newSet *goflag.FlagSet) { f.addedGoFlagSets = append(f.addedGoFlagSets, newSet) } +// CopyToGoFlagSet will add all current flags to the given Go flag set. +// Deprecation remarks get copied into the usage description. +// Whenever possible, a flag gets added for which Go flags shows +// a proper type in the help message. +func (f *FlagSet) CopyToGoFlagSet(newSet *goflag.FlagSet) { + f.VisitAll(func(flag *Flag) { + usage := flag.Usage + if flag.Deprecated != "" { + usage += " (DEPRECATED: " + flag.Deprecated + ")" + } + + switch value := flag.Value.(type) { + case *stringValue: + newSet.StringVar((*string)(value), flag.Name, flag.DefValue, usage) + case *intValue: + newSet.IntVar((*int)(value), flag.Name, *(*int)(value), usage) + case *int64Value: + newSet.Int64Var((*int64)(value), flag.Name, *(*int64)(value), usage) + case *uintValue: + newSet.UintVar((*uint)(value), flag.Name, *(*uint)(value), usage) + case *uint64Value: + newSet.Uint64Var((*uint64)(value), flag.Name, *(*uint64)(value), usage) + case *durationValue: + newSet.DurationVar((*time.Duration)(value), flag.Name, *(*time.Duration)(value), usage) + case *float64Value: + newSet.Float64Var((*float64)(value), flag.Name, *(*float64)(value), usage) + default: + newSet.Var(flag.Value, flag.Name, usage) + } + }) +} + // ParseSkippedFlags explicitly Parses go test flags (i.e. the one starting with '-test.') with goflag.Parse(), // since by default those are skipped by pflag.Parse(). // Typical usage example: `ParseGoTestFlags(os.Args[1:], goflag.CommandLine)` @@ -125,3 +158,4 @@ func ParseSkippedFlags(osArgs []string, goFlagSet *goflag.FlagSet) error { } return goFlagSet.Parse(skippedFlags) } + diff --git a/golangflag_test.go b/golangflag_test.go index 2ecefef..7309808 100644 --- a/golangflag_test.go +++ b/golangflag_test.go @@ -7,6 +7,7 @@ package pflag import ( goflag "flag" "testing" + "time" ) func TestGoflags(t *testing.T) { @@ -59,3 +60,76 @@ func TestGoflags(t *testing.T) { t.Fatal("goflag.CommandLine.Parsed() return false after f.Parse() called") } } + +func TestToGoflags(t *testing.T) { + pfs := FlagSet{} + gfs := goflag.FlagSet{} + pfs.String("StringFlag", "String value", "String flag usage") + pfs.Int("IntFlag", 1, "Int flag usage") + pfs.Uint("UintFlag", 2, "Uint flag usage") + pfs.Int64("Int64Flag", 3, "Int64 flag usage") + pfs.Uint64("Uint64Flag", 4, "Uint64 flag usage") + pfs.Int8("Int8Flag", 5, "Int8 flag usage") + pfs.Float64("Float64Flag", 6.0, "Float64 flag usage") + pfs.Duration("DurationFlag", time.Second, "Duration flag usage") + pfs.Bool("BoolFlag", true, "Bool flag usage") + pfs.String("deprecated", "Deprecated value", "Deprecated flag usage") + pfs.MarkDeprecated("deprecated", "obsolete") + + pfs.CopyToGoFlagSet(&gfs) + + // Modify via pfs. Should be visible via gfs because both share the + // same values. + for name, value := range map[string]string{ + "StringFlag": "Modified String value", + "IntFlag": "11", + "UintFlag": "12", + "Int64Flag": "13", + "Uint64Flag": "14", + "Int8Flag": "15", + "Float64Flag": "16.0", + "BoolFlag": "false", + } { + pf := pfs.Lookup(name) + if pf == nil { + t.Errorf("%s: not found in pflag flag set", name) + continue + } + if err := pf.Value.Set(value); err != nil { + t.Errorf("error setting %s = %s: %v", name, value, err) + } + } + + // Check that all flags were added and share the same value. + pfs.VisitAll(func(pf *Flag) { + gf := gfs.Lookup(pf.Name) + if gf == nil { + t.Errorf("%s: not found in Go flag set", pf.Name) + return + } + if gf.Value.String() != pf.Value.String() { + t.Errorf("%s: expected value %v from Go flag set, got %v", + pf.Name, pf.Value, gf.Value) + return + } + }) + + // Check for unexpected additional flags. + gfs.VisitAll(func(gf *goflag.Flag) { + pf := gfs.Lookup(gf.Name) + if pf == nil { + t.Errorf("%s: not found in pflag flag set", gf.Name) + return + } + }) + + deprecated := gfs.Lookup("deprecated") + if deprecated == nil { + t.Error("deprecated: not found in Go flag set") + } else { + expectedUsage := "Deprecated flag usage (DEPRECATED: obsolete)" + if deprecated.Usage != expectedUsage { + t.Errorf("deprecation remark not added, expected usage %q, got %q", expectedUsage, deprecated.Usage) + } + } +} -- cgit v1.2.3