-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
short opt handling: fix parsing #758
Changes from all commits
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 |
---|---|---|
|
@@ -181,16 +181,49 @@ func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { | |
return set, set.Parse(append([]string{"--"}, args...)) | ||
} | ||
|
||
if c.UseShortOptionHandling { | ||
args = translateShortOptions(args) | ||
} | ||
|
||
if !c.SkipArgReorder { | ||
args = reorderArgs(args) | ||
} | ||
|
||
PARSE: | ||
err = set.Parse(args) | ||
if err != nil { | ||
if c.UseShortOptionHandling { | ||
// To enable short-option handling (e.g., "-it" vs "-i -t") | ||
// we have to iteratively catch parsing errors. This way | ||
// we achieve LR parsing without transforming any arguments. | ||
// Otherwise, there is no way we can discriminate combined | ||
// short options from common arguments that should be left | ||
// untouched. | ||
errStr := err.Error() | ||
trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: ") | ||
if errStr == trimmed { | ||
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 is dirty, why can't we return an error of a special type or ParseResult that does not rely on string matching? 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 agree, but currently, there is no other way around that as the standard lib doesn't provide a special error type for that yet. I opened an issue for golang about that which is slowly taking shape (see golang/go#26822) but that will take a longer while until all details are figured out. In other words, I don't think there is any other way around that. |
||
return nil, err | ||
} | ||
// regenerate the initial args with the split short opts | ||
newArgs := Args{} | ||
for i, arg := range args { | ||
if arg != trimmed { | ||
newArgs = append(newArgs, arg) | ||
continue | ||
} | ||
shortOpts := translateShortOptions(set, Args{trimmed}) | ||
if len(shortOpts) == 1 { | ||
return nil, err | ||
} | ||
// add each short option and all remaining arguments | ||
newArgs = append(newArgs, shortOpts...) | ||
newArgs = append(newArgs, args[i+1:]...) | ||
args = newArgs | ||
// now reset the flagset parse again | ||
set, err = flagSet(c.Name, c.Flags) | ||
if err != nil { | ||
return nil, err | ||
} | ||
set.SetOutput(ioutil.Discard) | ||
goto PARSE | ||
} | ||
} | ||
return nil, err | ||
} | ||
|
||
|
@@ -232,11 +265,25 @@ func reorderArgs(args []string) []string { | |
return append(flags, nonflags...) | ||
} | ||
|
||
func translateShortOptions(flagArgs Args) []string { | ||
func translateShortOptions(set *flag.FlagSet, flagArgs Args) []string { | ||
allCharsFlags := func (s string) bool { | ||
for i := range s { | ||
f := set.Lookup(string(s[i])) | ||
if f == nil { | ||
return false | ||
} | ||
} | ||
return true | ||
} | ||
|
||
// separate combined flags | ||
var flagArgsSeparated []string | ||
for _, flagArg := range flagArgs { | ||
if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 { | ||
if !allCharsFlags(flagArg[1:]) { | ||
flagArgsSeparated = append(flagArgsSeparated, flagArg) | ||
continue | ||
} | ||
for _, flagChar := range flagArg[1:] { | ||
flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar)) | ||
} | ||
|
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.
Don't use 2 spaces after a period, this is not a typewriter ;)
https://www.cultofpedagogy.com/two-spaces-after-period/
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.
I am much older than I look ;-)