v.io/x/lib/cmdline: subtle bug fix for accessing parsed flags.
The commandline package has some complex logic for handling
duplicate flags. As part of this it takes a copy of the
flagset specified for the command and possibly manipulates
this copy. However, the copy is currently inaccessible to
the application and hence calls to Visit/Narg etc are
unpredictable depending on whether the original flag set
is used or not. A new member variable, ParsedFlags is added
which is guaranteed to be set to the FlagSet that is actually
parsed.
Change-Id: I676b01575a71d7e8f9049fd649647f9a3491c87e
diff --git a/cmdline/.api b/cmdline/.api
index 0c11b43..a5db6d2 100644
--- a/cmdline/.api
+++ b/cmdline/.api
@@ -20,6 +20,7 @@
pkg cmdline, type Command struct, Long string
pkg cmdline, type Command struct, LookPath bool
pkg cmdline, type Command struct, Name string
+pkg cmdline, type Command struct, ParsedFlags *flag.FlagSet
pkg cmdline, type Command struct, Runner Runner
pkg cmdline, type Command struct, Short string
pkg cmdline, type Command struct, Topics []Topic
diff --git a/cmdline/cmdline.go b/cmdline/cmdline.go
index e548b78..678731d 100644
--- a/cmdline/cmdline.go
+++ b/cmdline/cmdline.go
@@ -69,8 +69,17 @@
// Flags defined for this command. When a flag F is defined on a command C,
// we allow F to be specified on the command line immediately after C, or
- // after any descendant of C.
+ // after any descendant of C. This FlagSet is only used to specify the
+ // flags and their associated value variables, it is never parsed and hence
+ // methods on FlagSet that are generally used after parsing cannot be
+ // used on Flags. ParsedFlags should be used instead.
Flags flag.FlagSet
+ // ParsedFlags contains the FlagSet created by the Command
+ // implementation and that has had its Parse method called. It
+ // should be used instead of the Flags field for handling methods
+ // that assume Parse has been called (e.g. Parsed, Visit,
+ // NArgs etc).
+ ParsedFlags *flag.FlagSet
// Children of the command.
Children []*Command
@@ -436,6 +445,7 @@
if err := flags.Parse(args); err != nil {
return nil, nil, err
}
+ cmd.ParsedFlags = flags
return flags.Args(), extractSetFlags(flags), nil
}
diff --git a/cmdline/cmdline_test.go b/cmdline/cmdline_test.go
index 77d6997..990ec1e 100644
--- a/cmdline/cmdline_test.go
+++ b/cmdline/cmdline_test.go
@@ -2971,3 +2971,62 @@
}
runTestCases(t, cmd, tests)
}
+
+func TestParsedFlags(t *testing.T) {
+ root := &Command{
+ Name: "root",
+ Short: "short",
+ Long: "long.",
+ Runner: RunnerFunc(runHello),
+ }
+ var v1, v2 bool
+ env := EnvFromOS()
+ root.Flags.BoolVar(&v1, "a", false, "bool")
+ root.Flags.BoolVar(&v2, "b", false, "bool")
+
+ // ParsedFlags should be nil if Parse fails.
+ _, _, err := Parse(root, env, []string{"-xx"})
+ if err == nil {
+ t.Errorf("expected an error")
+ }
+ var nilFlagSet *flag.FlagSet
+ if got, want := root.ParsedFlags, nilFlagSet; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+
+ // ParsedFlags should be set and Parsed returns true if
+ // the command line is successfully parsed.
+ _, _, err = Parse(root, env, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got, want := root.Flags.Parsed(), false; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := root.ParsedFlags.Parsed(), true; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := v1, false; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := v2, false; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+
+ _, _, err = Parse(root, env, []string{"-a"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got, want := root.Flags.Parsed(), false; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := root.ParsedFlags.Parsed(), true; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := v1, true; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := v2, false; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+}
