code review 12587043: flag: export commandLine, the previously internal defau...

flag: export commandLine, the previously internal default FlagSet
s/commandLine/CommandLine/g

Fixes issue 4209.

[r, 2013-08-07 03:28:43]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[dsymonds, 2013-08-07 03:43:47]

LGTM

I think it makes sense. We expose os.Args, flag.Lookup, etc. I don't
see the benefit in hiding this if a program really wants to do
something tricky.

[bradfitz, 2013-08-07 16:43:37]

LGTM

I've wanted this before too.



On Tue, Aug 6, 2013 at 8:28 PM, <r@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> flag: export commandLine, the previously internal default FlagSet
> s/commandLine/CommandLine/g
>
> Fixes issue 4209.
>
> I'm not yet convinced this is worth doing. Comments welcome.
>
> Please review this at
https://codereview.appspot.**com/12587043/<https://codereview.appspot.com/125...
>
> Affected files:
>   M src/pkg/flag/export_test.go
>   M src/pkg/flag/flag.go
>   M src/pkg/flag/flag_test.go
>
>
> Index: src/pkg/flag/export_test.go
> ==============================**==============================**=======
> --- a/src/pkg/flag/export_test.go
> +++ b/src/pkg/flag/export_test.go
> @@ -12,11 +12,6 @@
>  // After calling ResetForTesting, parse errors in flag handling will not
>  // exit the program.
>  func ResetForTesting(usage func()) {
> -       commandLine = NewFlagSet(os.Args[0], ContinueOnError)
> +       CommandLine = NewFlagSet(os.Args[0], ContinueOnError)
>         Usage = usage
>  }
> -
> -// CommandLine returns the default FlagSet.
> -func CommandLine() *FlagSet {
> -       return commandLine
> -}
> Index: src/pkg/flag/flag.go
> ==============================**==============================**=======
> --- a/src/pkg/flag/flag.go
> +++ b/src/pkg/flag/flag.go
> @@ -322,7 +322,7 @@
>  // VisitAll visits the command-line flags in lexicographical order,
> calling
>  // fn for each.  It visits all flags, even those not set.
>  func VisitAll(fn func(*Flag)) {
> -       commandLine.VisitAll(fn)
> +       CommandLine.VisitAll(fn)
>  }
>
>  // Visit visits the flags in lexicographical order, calling fn for each.
> @@ -336,7 +336,7 @@
>  // Visit visits the command-line flags in lexicographical order, calling
> fn
>  // for each.  It visits only those flags that have been set.
>  func Visit(fn func(*Flag)) {
> -       commandLine.Visit(fn)
> +       CommandLine.Visit(fn)
>  }
>
>  // Lookup returns the Flag structure of the named flag, returning nil if
> none exists.
> @@ -347,7 +347,7 @@
>  // Lookup returns the Flag structure of the named command-line flag,
>  // returning nil if none exists.
>  func Lookup(name string) *Flag {
> -       return commandLine.formal[name]
> +       return CommandLine.formal[name]
>  }
>
>  // Set sets the value of the named flag.
> @@ -369,7 +369,7 @@
>
>  // Set sets the value of the named command-line flag.
>  func Set(name, value string) error {
> -       return commandLine.Set(name, value)
> +       return CommandLine.Set(name, value)
>  }
>
>  // PrintDefaults prints, to standard error unless configured
> @@ -387,7 +387,7 @@
>
>  // PrintDefaults prints to standard error the default values of all
> defined command-line flags.
>  func PrintDefaults() {
> -       commandLine.PrintDefaults()
> +       CommandLine.PrintDefaults()
>  }
>
>  // defaultUsage is the default function to print a usage message.
> @@ -400,7 +400,7 @@
>         f.PrintDefaults()
>  }
>
> -// NOTE: Usage is not just defaultUsage(commandLine)
> +// NOTE: Usage is not just defaultUsage(CommandLine)
>  // because it serves (via godoc flag Usage) as the example
>  // for how to write your own usage function.
>
> @@ -415,7 +415,7 @@
>  func (f *FlagSet) NFlag() int { return len(f.actual) }
>
>  // NFlag returns the number of command-line flags that have been set.
> -func NFlag() int { return len(commandLine.actual) }
> +func NFlag() int { return len(CommandLine.actual) }
>
>  // Arg returns the i'th argument.  Arg(0) is the first remaining argument
>  // after flags have been processed.
> @@ -429,20 +429,20 @@
>  // Arg returns the i'th command-line argument.  Arg(0) is the first
> remaining argument
>  // after flags have been processed.
>  func Arg(i int) string {
> -       return commandLine.Arg(i)
> +       return CommandLine.Arg(i)
>  }
>
>  // NArg is the number of arguments remaining after flags have been
> processed.
>  func (f *FlagSet) NArg() int { return len(f.args) }
>
>  // NArg is the number of arguments remaining after flags have been
> processed.
> -func NArg() int { return len(commandLine.args) }
> +func NArg() int { return len(CommandLine.args) }
>
>  // Args returns the non-flag arguments.
>  func (f *FlagSet) Args() []string { return f.args }
>
>  // Args returns the non-flag command-line arguments.
> -func Args() []string { return commandLine.args }
> +func Args() []string { return CommandLine.args }
>
>  // BoolVar defines a bool flag with specified name, default value, and
> usage string.
>  // The argument p points to a bool variable in which to store the value
> of the flag.
> @@ -453,7 +453,7 @@
>  // BoolVar defines a bool flag with specified name, default value, and
> usage string.
>  // The argument p points to a bool variable in which to store the value
> of the flag.
>  func BoolVar(p *bool, name string, value bool, usage string) {
> -       commandLine.Var(newBoolValue(**value, p), name, usage)
> +       CommandLine.Var(newBoolValue(**value, p), name, usage)
>  }
>
>  // Bool defines a bool flag with specified name, default value, and usage
> string.
> @@ -467,7 +467,7 @@
>  // Bool defines a bool flag with specified name, default value, and usage
> string.
>  // The return value is the address of a bool variable that stores the
> value of the flag.
>  func Bool(name string, value bool, usage string) *bool {
> -       return commandLine.Bool(name, value, usage)
> +       return CommandLine.Bool(name, value, usage)
>  }
>
>  // IntVar defines an int flag with specified name, default value, and
> usage string.
> @@ -479,7 +479,7 @@
>  // IntVar defines an int flag with specified name, default value, and
> usage string.
>  // The argument p points to an int variable in which to store the value
> of the flag.
>  func IntVar(p *int, name string, value int, usage string) {
> -       commandLine.Var(newIntValue(**value, p), name, usage)
> +       CommandLine.Var(newIntValue(**value, p), name, usage)
>  }
>
>  // Int defines an int flag with specified name, default value, and usage
> string.
> @@ -493,7 +493,7 @@
>  // Int defines an int 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.
>  func Int(name string, value int, usage string) *int {
> -       return commandLine.Int(name, value, usage)
> +       return CommandLine.Int(name, value, usage)
>  }
>
>  // Int64Var defines an int64 flag with specified name, default value, and
> usage string.
> @@ -505,7 +505,7 @@
>  // Int64Var defines an int64 flag with specified name, default value, and
> usage string.
>  // The argument p points to an int64 variable in which to store the value
> of the flag.
>  func Int64Var(p *int64, name string, value int64, usage string) {
> -       commandLine.Var(newInt64Value(**value, p), name, usage)
> +       CommandLine.Var(newInt64Value(**value, p), name, usage)
>  }
>
>  // Int64 defines an int64 flag with specified name, default value, and
> usage string.
> @@ -519,7 +519,7 @@
>  // Int64 defines an int64 flag with specified name, default value, and
> usage string.
>  // The return value is the address of an int64 variable that stores the
> value of the flag.
>  func Int64(name string, value int64, usage string) *int64 {
> -       return commandLine.Int64(name, value, usage)
> +       return CommandLine.Int64(name, value, usage)
>  }
>
>  // UintVar defines a uint flag with specified name, default value, and
> usage string.
> @@ -531,7 +531,7 @@
>  // UintVar defines a uint flag with specified name, default value, and
> usage string.
>  // The argument p points to a uint  variable in which to store the value
> of the flag.
>  func UintVar(p *uint, name string, value uint, usage string) {
> -       commandLine.Var(newUintValue(**value, p), name, usage)
> +       CommandLine.Var(newUintValue(**value, p), name, usage)
>  }
>
>  // Uint defines a uint flag with specified name, default value, and usage
> string.
> @@ -545,7 +545,7 @@
>  // Uint defines a uint flag with specified name, default value, and usage
> string.
>  // The return value is the address of a uint  variable that stores the
> value of the flag.
>  func Uint(name string, value uint, usage string) *uint {
> -       return commandLine.Uint(name, value, usage)
> +       return CommandLine.Uint(name, value, usage)
>  }
>
>  // Uint64Var defines a uint64 flag with specified name, default value,
> and usage string.
> @@ -557,7 +557,7 @@
>  // Uint64Var defines a uint64 flag with specified name, default value,
> and usage string.
>  // The argument p points to a uint64 variable in which to store the value
> of the flag.
>  func Uint64Var(p *uint64, name string, value uint64, usage string) {
> -       commandLine.Var(**newUint64Value(value, p), name, usage)
> +       CommandLine.Var(**newUint64Value(value, p), name, usage)
>  }
>
>  // Uint64 defines a uint64 flag with specified name, default value, and
> usage string.
> @@ -571,7 +571,7 @@
>  // Uint64 defines a uint64 flag with specified name, default value, and
> usage string.
>  // The return value is the address of a uint64 variable that stores the
> value of the flag.
>  func Uint64(name string, value uint64, usage string) *uint64 {
> -       return commandLine.Uint64(name, value, usage)
> +       return CommandLine.Uint64(name, value, usage)
>  }
>
>  // StringVar defines a string flag with specified name, default value,
> and usage string.
> @@ -583,7 +583,7 @@
>  // StringVar defines a string flag with specified name, default value,
> and usage string.
>  // The argument p points to a string variable in which to store the value
> of the flag.
>  func StringVar(p *string, name string, value string, usage string) {
> -       commandLine.Var(**newStringValue(value, p), name, usage)
> +       CommandLine.Var(**newStringValue(value, p), name, usage)
>  }
>
>  // String defines a string flag with specified name, default value, and
> usage string.
> @@ -597,7 +597,7 @@
>  // String defines a string flag with specified name, default value, and
> usage string.
>  // The return value is the address of a string variable that stores the
> value of the flag.
>  func String(name string, value string, usage string) *string {
> -       return commandLine.String(name, value, usage)
> +       return CommandLine.String(name, value, usage)
>  }
>
>  // Float64Var defines a float64 flag with specified name, default value,
> and usage string.
> @@ -609,7 +609,7 @@
>  // Float64Var defines a float64 flag with specified name, default value,
> and usage string.
>  // The argument p points to a float64 variable in which to store the
> value of the flag.
>  func Float64Var(p *float64, name string, value float64, usage string) {
> -       commandLine.Var(**newFloat64Value(value, p), name, usage)
> +       CommandLine.Var(**newFloat64Value(value, p), name, usage)
>  }
>
>  // Float64 defines a float64 flag with specified name, default value, and
> usage string.
> @@ -623,7 +623,7 @@
>  // Float64 defines a float64 flag with specified name, default value, and
> usage string.
>  // The return value is the address of a float64 variable that stores the
> value of the flag.
>  func Float64(name string, value float64, usage string) *float64 {
> -       return commandLine.Float64(name, value, usage)
> +       return CommandLine.Float64(name, value, usage)
>  }
>
>  // DurationVar defines a time.Duration flag with specified name, default
> value, and usage string.
> @@ -635,7 +635,7 @@
>  // DurationVar defines a time.Duration flag with specified name, default
> value, and usage string.
>  // The argument p points to a time.Duration variable in which to store
> the value of the flag.
>  func DurationVar(p *time.Duration, name string, value time.Duration,
> usage string) {
> -       commandLine.Var(**newDurationValue(value, p), name, usage)
> +       CommandLine.Var(**newDurationValue(value, p), name, usage)
>  }
>
>  // Duration defines a time.Duration flag with specified name, default
> value, and usage string.
> @@ -649,7 +649,7 @@
>  // Duration defines a time.Duration flag with specified name, default
> value, and usage string.
>  // The return value is the address of a time.Duration variable that
> stores the value of the flag.
>  func Duration(name string, value time.Duration, usage string)
> *time.Duration {
> -       return commandLine.Duration(name, value, usage)
> +       return CommandLine.Duration(name, value, usage)
>  }
>
>  // Var defines a flag with the specified name and usage string. The type
> and
> @@ -685,7 +685,7 @@
>  // of strings by giving the slice the methods of Value; in particular,
> Set would
>  // decompose the comma-separated string into the slice.
>  func Var(value Value, name string, usage string) {
> -       commandLine.Var(value, name, usage)
> +       CommandLine.Var(value, name, usage)
>  }
>
>  // failf prints to standard error a formatted error and usage message and
> @@ -698,9 +698,9 @@
>  }
>
>  // usage calls the Usage method for the flag set, or the usage function if
> -// the flag set is commandLine.
> +// the flag set is CommandLine.
>  func (f *FlagSet) usage() {
> -       if f == commandLine {
> +       if f == CommandLine {
>                 Usage()
>         } else if f.Usage == nil {
>                 defaultUsage(f)
> @@ -816,17 +816,19 @@
>  // Parse parses the command-line flags from os.Args[1:].  Must be called
>  // after all flags are defined and before flags are accessed by the
> program.
>  func Parse() {
> -       // Ignore errors; commandLine is set for ExitOnError.
> -       commandLine.Parse(os.Args[1:])
> +       // Ignore errors; CommandLine is set for ExitOnError.
> +       CommandLine.Parse(os.Args[1:])
>  }
>
>  // Parsed returns true if the command-line flags have been parsed.
>  func Parsed() bool {
> -       return commandLine.Parsed()
> +       return CommandLine.Parsed()
>  }
>
> -// The default set of command-line flags, parsed from os.Args.
> -var commandLine = NewFlagSet(os.Args[0], ExitOnError)
> +// CommandLine is the default set of command-line flags, parsed from
> os.Args.
> +// The top-level functions such as BoolVar, Arg, and on are wrappers for
> the
> +// methods of CommandLine.
> +var CommandLine = NewFlagSet(os.Args[0], ExitOnError)
>
>  // NewFlagSet returns a new, empty flag set with the specified name and
>  // error handling property.
> Index: src/pkg/flag/flag_test.go
> ==============================**==============================**=======
> --- a/src/pkg/flag/flag_test.go
> +++ b/src/pkg/flag/flag_test.go
> @@ -139,7 +139,7 @@
>  func TestUsage(t *testing.T) {
>         called := false
>         ResetForTesting(func() { called = true })
> -       if CommandLine().Parse([]string{"**-x"}) == nil {
> +       if CommandLine.Parse([]string{"-**x"}) == nil {
>                 t.Error("parse did not fail for unknown flag")
>         }
>         if !called {
> @@ -215,7 +215,7 @@
>
>  func TestParse(t *testing.T) {
>         ResetForTesting(func() { t.Error("bad parse") })
> -       testParse(CommandLine(), t)
> +       testParse(CommandLine, t)
>  }
>
>  func TestFlagSetParse(t *testing.T) {
> @@ -311,7 +311,7 @@
>         defer func() { os.Args = oldArgs }()
>         os.Args = []string{"cmd", "-before", "subcmd", "-after", "args"}
>         before := Bool("before", false, "")
> -       if err := CommandLine().Parse(os.Args[1:**]); err != nil {
> +       if err := CommandLine.Parse(os.Args[1:])**; err != nil {
>                 t.Fatal(err)
>         }
>         cmd := Arg(0)
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[r, 2013-08-08 01:51:08]

*** Submitted as https://code.google.com/p/go/source/detail?r=ecf3c8d0918c ***

flag: export commandLine, the previously internal default FlagSet
s/commandLine/CommandLine/g

Fixes issue 4209.

R=golang-dev, dsymonds, bradfitz
CC=golang-dev
https://codereview.appspot.com/12587043