New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add ability to ignore unknown flags #160
Changes from 2 commits
bbf05e3
56e2774
eabdfa9
c8d7929
7245619
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,6 +138,9 @@ type FlagSet struct { | |
// help/usage messages. | ||
SortFlags bool | ||
|
||
// IgnoreUnknownFlags is used to indicate to ignore unknown flags | ||
IgnoreUnknownFlags bool | ||
|
||
name string | ||
parsed bool | ||
actual map[NormalizedName]*Flag | ||
|
@@ -896,6 +899,25 @@ func (f *FlagSet) usage() { | |
} | ||
} | ||
|
||
//--unknown (args will be empty) | ||
//--unknown --next-flag ... (args will be --next-flag ...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if I passed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this function just strips value for unknown flag, the flag itself is not included in args. for added the testcase to test this explicitly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really think you need to know Clearly it is a heuristic to know what to strip and what not to strip for an 'unknown' but I think we want to be as careful as we can... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yes, thats right. i would consider this a bug :) fixed. thank you for pointing out. |
||
//--unknown arg ... (args will be arg ...) | ||
func stripUnknownFlagValue(args []string) []string { | ||
if len(args) == 0 { | ||
//--unknown | ||
return args | ||
} | ||
|
||
first := args[0] | ||
if first[0] == '-' { | ||
//--unknown --next-flag ... | ||
return args | ||
} | ||
|
||
//--unknown arg ... (args will be arg ...) | ||
return args[1:] | ||
} | ||
|
||
func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []string, err error) { | ||
a = args | ||
name := s[2:] | ||
|
@@ -907,13 +929,18 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin | |
split := strings.SplitN(name, "=", 2) | ||
name = split[0] | ||
flag, exists := f.formal[f.normalizeFlagName(name)] | ||
|
||
if !exists { | ||
if name == "help" { // special case for nice help message. | ||
switch { | ||
case name == "help": | ||
f.usage() | ||
return a, ErrHelp | ||
case f.IgnoreUnknownFlags: | ||
return stripUnknownFlagValue(a), nil | ||
default: | ||
err = f.failf("unknown flag: --%s", name) | ||
return | ||
} | ||
err = f.failf("unknown flag: --%s", name) | ||
return | ||
} | ||
|
||
var value string | ||
|
@@ -951,13 +978,22 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse | |
|
||
flag, exists := f.shorthands[c] | ||
if !exists { | ||
if c == 'h' { // special case for nice help message. | ||
switch { | ||
case c == 'h': | ||
f.usage() | ||
err = ErrHelp | ||
return | ||
case f.IgnoreUnknownFlags: | ||
outArgs = stripUnknownFlagValue(outArgs) | ||
if len(shorthands) > 2 && shorthands[1] == '=' { | ||
// '-f=arg' | ||
outShorts = "" | ||
} | ||
return | ||
default: | ||
err = f.failf("unknown shorthand flag: %q in -%s", c, shorthands) | ||
return | ||
} | ||
err = f.failf("unknown shorthand flag: %q in -%s", c, shorthands) | ||
return | ||
} | ||
|
||
var value string | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -389,7 +389,70 @@ func testParseAll(f *FlagSet, t *testing.T) { | |
} | ||
if !reflect.DeepEqual(got, want) { | ||
t.Errorf("f.ParseAll() fail to restore the args") | ||
t.Errorf("Got: %v", got) | ||
t.Errorf("Got: %v", got) | ||
t.Errorf("Want: %v", want) | ||
} | ||
} | ||
|
||
func testParseWithUnknownFlags(f *FlagSet, t *testing.T) { | ||
if f.Parsed() { | ||
t.Error("f.Parse() = true before Parse") | ||
} | ||
f.IgnoreUnknownFlags = true | ||
|
||
f.BoolP("boola", "a", false, "bool value") | ||
f.BoolP("boolb", "b", false, "bool2 value") | ||
f.BoolP("boolc", "c", false, "bool3 value") | ||
f.BoolP("boold", "d", false, "bool4 value") | ||
f.StringP("stringa", "s", "0", "string value") | ||
f.StringP("stringz", "z", "0", "string value") | ||
f.StringP("stringx", "x", "0", "string value") | ||
f.StringP("stringy", "y", "0", "string value") | ||
f.Lookup("stringx").NoOptDefVal = "1" | ||
args := []string{ | ||
"-ab", | ||
"-cs=xx", | ||
"--stringz=something", | ||
"--unknown1", | ||
"unknown1Value", | ||
"-d=true", | ||
"-x", | ||
"--unknown2=unknown2Value", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this test need to cover the case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added the testcase, thanks |
||
"-u=unknown3Value", | ||
"-p", | ||
"unknown4Value", | ||
"-q", //another unknown with bool value | ||
"-y", | ||
"ee", | ||
"--unknown6", | ||
} | ||
want := []string{ | ||
"boola", "true", | ||
"boolb", "true", | ||
"boolc", "true", | ||
"stringa", "xx", | ||
"stringz", "something", | ||
"boold", "true", | ||
"stringx", "1", | ||
"stringy", "ee", | ||
} | ||
got := []string{} | ||
store := func(flag *Flag, value string) error { | ||
got = append(got, flag.Name) | ||
if len(value) > 0 { | ||
got = append(got, value) | ||
} | ||
return nil | ||
} | ||
if err := f.ParseAll(args, store); err != nil { | ||
t.Errorf("expected no error, got %s", err) | ||
} | ||
if !f.Parsed() { | ||
t.Errorf("f.Parse() = false after Parse") | ||
} | ||
if !reflect.DeepEqual(got, want) { | ||
t.Errorf("f.ParseAll() fail to restore the args") | ||
t.Errorf("Got: %v", got) | ||
t.Errorf("Want: %v", want) | ||
} | ||
} | ||
|
@@ -500,6 +563,11 @@ func TestParseAll(t *testing.T) { | |
testParseAll(GetCommandLine(), t) | ||
} | ||
|
||
func TestIgnoreUnknownFlags(t *testing.T) { | ||
ResetForTesting(func() { t.Error("bad parse") }) | ||
testParseWithUnknownFlags(GetCommandLine(), t) | ||
} | ||
|
||
func TestFlagSetParse(t *testing.T) { | ||
testParse(NewFlagSet("test", ContinueOnError), t) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert this into some sort of generic "set" of error handling bools which we can grow as needed and I'll merge this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do that today, thanks