diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 42f7614a..7adfc939 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -13,7 +13,7 @@ jobs: strategy: fail-fast: false matrix: - go: ["1.21", "1.22", "1.23"] + go: ["1.12", "1.21", "1.22", "1.23"] steps: - name: Checkout repository diff --git a/bool_func_go1.21_test.go b/bool_func_go1.21_test.go new file mode 100644 index 00000000..5b1bd559 --- /dev/null +++ b/bool_func_go1.21_test.go @@ -0,0 +1,110 @@ +//go:build go1.21 +// +build go1.21 + +package pflag + +import ( + "errors" + "flag" + "io" + "strings" + "testing" +) + +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 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 + // 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 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) + 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/bool_func_test.go b/bool_func_test.go index c16be831..765c9c08 100644 --- a/bool_func_test.go +++ b/bool_func_test.go @@ -1,9 +1,6 @@ package pflag import ( - "errors" - "flag" - "io" "strings" "testing" ) @@ -48,104 +45,6 @@ func TestBoolFuncP(t *testing.T) { } } -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 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 - // 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 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) - 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) - } - }) -} - func TestBoolFuncUsage(t *testing.T) { t.Run("regular func flag", func(t *testing.T) { // regular boolfunc flag: diff --git a/flag.go b/flag.go index d4dfbc5e..2fd3c575 100644 --- a/flag.go +++ b/flag.go @@ -137,12 +137,17 @@ 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 } +// ParseErrorsWhitelist defines the parsing errors that can be ignored. +// +// Deprecated: use [ParseErrorsAllowlist] instead. This type will be removed in a future release. +type ParseErrorsWhitelist = ParseErrorsAllowlist + // NormalizedName is a flag name that has been normalized according to rules // for the FlagSet (e.g. making '-' and '_' equivalent). type NormalizedName string @@ -158,8 +163,13 @@ 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 + + // ParseErrorsAllowlist is used to configure an allowlist of errors. + // + // Deprecated: use [FlagSet.ParseErrorsAllowlist] instead. This field will be removed in a future release. + ParseErrorsWhitelist ParseErrorsAllowlist name string parsed bool @@ -928,7 +938,6 @@ func VarP(value Value, name, shorthand, usage string) { // returns the error. func (f *FlagSet) fail(err error) error { if f.errorHandling != ContinueOnError { - fmt.Fprintln(f.Output(), err) f.usage() } return err @@ -986,6 +995,8 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin f.usage() return a, ErrHelp case f.ParseErrorsWhitelist.UnknownFlags: + fallthrough + case f.ParseErrorsAllowlist.UnknownFlags: // --unknown=unknownval arg ... // we do not want to lose arg in this case if len(split) >= 2 { @@ -1044,6 +1055,8 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse err = ErrHelp return case f.ParseErrorsWhitelist.UnknownFlags: + fallthrough + case f.ParseErrorsAllowlist.UnknownFlags: // '-f=arg arg ...' // we do not want to lose arg in this case if len(shorthands) > 2 && shorthands[1] == '=' { @@ -1158,12 +1171,12 @@ func (f *FlagSet) Parse(arguments []string) error { } f.parsed = true + f.args = make([]string, 0, len(arguments)) + if len(arguments) == 0 { return nil } - f.args = make([]string, 0, len(arguments)) - set := func(flag *Flag, value string) error { return f.Set(flag.Name, value) } @@ -1174,7 +1187,10 @@ func (f *FlagSet) Parse(arguments []string) error { case ContinueOnError: return err case ExitOnError: - fmt.Println(err) + if err == ErrHelp { + os.Exit(0) + } + fmt.Fprintln(f.Output(), err) os.Exit(2) case PanicOnError: panic(err) @@ -1200,6 +1216,10 @@ func (f *FlagSet) ParseAll(arguments []string, fn func(flag *Flag, value string) case ContinueOnError: return err case ExitOnError: + if err == ErrHelp { + os.Exit(0) + } + fmt.Fprintln(f.Output(), err) os.Exit(2) case PanicOnError: panic(err) diff --git a/flag_test.go b/flag_test.go index ead05186..f8ee431e 100644 --- a/flag_test.go +++ b/flag_test.go @@ -447,11 +447,11 @@ func testParseAll(f *FlagSet, t *testing.T) { } } -func testParseWithUnknownFlags(f *FlagSet, t *testing.T) { +func testParseWithUnknownFlags(f *FlagSet, t *testing.T, setUnknownFlags func(f *FlagSet)) { if f.Parsed() { t.Error("f.Parse() = true before Parse") } - f.ParseErrorsWhitelist.UnknownFlags = true + setUnknownFlags(f) f.BoolP("boola", "a", false, "bool value") f.BoolP("boolb", "b", false, "bool2 value") @@ -649,13 +649,58 @@ func TestParseAll(t *testing.T) { func TestIgnoreUnknownFlags(t *testing.T) { ResetForTesting(func() { t.Error("bad parse") }) - testParseWithUnknownFlags(GetCommandLine(), t) + testParseWithUnknownFlags(GetCommandLine(), t, func(f *FlagSet) { f.ParseErrorsAllowlist.UnknownFlags = true }) +} + +func TestIgnoreUnknownFlagsBackwardsCompat(t *testing.T) { + ResetForTesting(func() { t.Error("bad parse") }) + testParseWithUnknownFlags(GetCommandLine(), t, func(f *FlagSet) { f.ParseErrorsWhitelist.UnknownFlags = true }) } func TestFlagSetParse(t *testing.T) { testParse(NewFlagSet("test", ContinueOnError), t) } +func TestParseRepeated(t *testing.T) { + fs := NewFlagSet("test repeated", ContinueOnError) + + t.Run("first parse", func(t *testing.T) { + err := fs.Parse([]string{"foo", "bar"}) + if err != nil { + t.Fatal("expected no error, got ", err) + } + + argsAfterFirst := fs.Args() + if !reflect.DeepEqual(argsAfterFirst, []string{"foo", "bar"}) { + t.Fatalf("expected args [foo bar], got %v", argsAfterFirst) + } + }) + + t.Run("re-parse with fewer args", func(t *testing.T) { + err := fs.Parse([]string{"baz"}) + if err != nil { + t.Fatal("expected no error, got ", err) + } + + argsAfterSecond := fs.Args() + if !reflect.DeepEqual(argsAfterSecond, []string{"baz"}) { + t.Fatalf("expected args [baz], got %v", argsAfterSecond) + } + }) + + t.Run("re-parse with no args", func(t *testing.T) { + err := fs.Parse([]string{}) + if err != nil { + t.Fatal("expected no error, got ", err) + } + + argsAfterThird := fs.Args() + if !reflect.DeepEqual(argsAfterThird, []string{}) { + t.Fatalf("expected args [], got %v", argsAfterThird) + } + }) +} + func TestChangedHelper(t *testing.T) { f := NewFlagSet("changedtest", ContinueOnError) f.Bool("changed", false, "changed bool") diff --git a/func_go1.21_test.go b/func_go1.21_test.go new file mode 100644 index 00000000..2d5ea31a --- /dev/null +++ b/func_go1.21_test.go @@ -0,0 +1,102 @@ +//go:build go1.21 +// +build go1.21 + +package pflag + +import ( + "errors" + "flag" + "io" + "strings" + "testing" +) + +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 triggered by the callback: + // (note: as can be seen in 'runCase()', if the callback sees "err" as a value + // for the 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) + } + }) +} diff --git a/func_test.go b/func_test.go index d492b488..4badf936 100644 --- a/func_test.go +++ b/func_test.go @@ -1,9 +1,6 @@ package pflag import ( - "errors" - "flag" - "io" "strings" "testing" ) @@ -62,96 +59,6 @@ func TestFuncP(t *testing.T) { } } -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 triggered by the callback: - // (note: as can be seen in 'runCase()', if the callback sees "err" as a value - // for the 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) - } - }) -} - func TestFuncUsage(t *testing.T) { t.Run("regular func flag", func(t *testing.T) { // regular func flag: diff --git a/golangflag.go b/golangflag.go index f563907e..e62eab53 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 2ecefefa..7309808d 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) + } + } +} diff --git a/string_to_string.go b/string_to_string.go index 890a01af..1d1e3bf9 100644 --- a/string_to_string.go +++ b/string_to_string.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/csv" "fmt" + "sort" "strings" ) @@ -62,8 +63,15 @@ func (s *stringToStringValue) Type() string { } func (s *stringToStringValue) String() string { + keys := make([]string, 0, len(*s.value)) + for k := range *s.value { + keys = append(keys, k) + } + sort.Strings(keys) + records := make([]string, 0, len(*s.value)>>1) - for k, v := range *s.value { + for _, k := range keys { + v := (*s.value)[k] records = append(records, k+"="+v) } diff --git a/time.go b/time.go index dc024807..3dee4247 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 46a5ada7..62d9a799 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) + } +}