Skip to content
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

Handle ShortOptions and SkipArgReorder #686

Merged
merged 1 commit into from
Dec 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
129 changes: 86 additions & 43 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,57 +115,29 @@ func (c Command) Run(ctx *Context) (err error) {
return err
}
set.SetOutput(ioutil.Discard)

firstFlagIndex, terminatorIndex := getIndexes(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked that you started pulling this logic into functions. What do you think about also pulling this whole flag handling logic into a separate function? It'd allow us to do things like return early to reduce nesting as well as compartmentalize this handling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have done more, but I couldn't logically see a path through this. I was thinking in V2 it might be easier ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 we can always do it as a second pass.

flagArgs, regularArgs := getAllArgs(ctx.Args(), firstFlagIndex, terminatorIndex)
if c.UseShortOptionHandling {
flagArgs = translateShortOptions(flagArgs)
}
if c.SkipFlagParsing {
err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
} else if !c.SkipArgReorder {
firstFlagIndex := -1
terminatorIndex := -1
for index, arg := range ctx.Args() {
if arg == "--" {
terminatorIndex = index
break
} else if arg == "-" {
// Do nothing. A dash alone is not really a flag.
continue
} else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 {
firstFlagIndex = index
}
}

if firstFlagIndex > -1 {
args := ctx.Args()
regularArgs := make([]string, len(args[1:firstFlagIndex]))
copy(regularArgs, args[1:firstFlagIndex])

var flagArgs []string
if terminatorIndex > -1 {
flagArgs = args[firstFlagIndex:terminatorIndex]
regularArgs = append(regularArgs, args[terminatorIndex:]...)
} else {
flagArgs = args[firstFlagIndex:]
}
// separate combined flags
if c.UseShortOptionHandling {
var flagArgsSeparated []string
for _, flagArg := range flagArgs {
if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 {
for _, flagChar := range flagArg[1:] {
flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar))
}
} else {
flagArgsSeparated = append(flagArgsSeparated, flagArg)
}
}
err = set.Parse(append(flagArgsSeparated, regularArgs...))
} else {
err = set.Parse(append(flagArgs, regularArgs...))
}
err = set.Parse(append(flagArgs, regularArgs...))
} else {
err = set.Parse(ctx.Args().Tail())
}
} else if c.UseShortOptionHandling {
if terminatorIndex == -1 && firstFlagIndex > -1 {
// Handle shortname AND no options
err = set.Parse(append(regularArgs, flagArgs...))
} else {
// Handle shortname and options
err = set.Parse(flagArgs)
}
} else {
err = set.Parse(ctx.Args().Tail())
err = set.Parse(append(regularArgs, flagArgs...))
}

nerr := normalizeFlags(c.Flags, set)
Expand Down Expand Up @@ -233,6 +205,77 @@ func (c Command) Run(ctx *Context) (err error) {
return err
}

func getIndexes(ctx *Context) (int, int) {
firstFlagIndex := -1
terminatorIndex := -1
for index, arg := range ctx.Args() {
if arg == "--" {
terminatorIndex = index
break
} else if arg == "-" {
// Do nothing. A dash alone is not really a flag.
continue
} else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 {
firstFlagIndex = index
}
}
if len(ctx.Args()) > 0 && !strings.HasPrefix(ctx.Args()[0], "-") && firstFlagIndex == -1 {
return -1, -1
}

return firstFlagIndex, terminatorIndex

}

// copyStringslice takes a string slice and copies it
func copyStringSlice(slice []string, start, end int) []string {
newSlice := make([]string, end-start)
copy(newSlice, slice[start:end])
return newSlice
}

// getAllArgs extracts and returns two string slices representing
// regularArgs and flagArgs
func getAllArgs(args []string, firstFlagIndex, terminatorIndex int) ([]string, []string) {
var regularArgs []string
// if there are no options, the we set the index to 1 manually
if firstFlagIndex == -1 {
firstFlagIndex = 1
regularArgs = copyStringSlice(args, 0, len(args))
} else {
regularArgs = copyStringSlice(args, 1, firstFlagIndex)
}
var flagArgs []string
// a flag terminatorIndex was found in the input. we need to collect
// flagArgs based on it.
if terminatorIndex > -1 {
flagArgs = copyStringSlice(args, firstFlagIndex, terminatorIndex)
additionalRegularArgs := copyStringSlice(args, terminatorIndex, len(args))
regularArgs = append(regularArgs, additionalRegularArgs...)
for _, i := range additionalRegularArgs {
regularArgs = append(regularArgs, i)
}
} else {
flagArgs = args[firstFlagIndex:]
}
return flagArgs, regularArgs
}

func translateShortOptions(flagArgs Args) []string {
// separate combined flags
var flagArgsSeparated []string
for _, flagArg := range flagArgs {
if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 {
for _, flagChar := range flagArg[1:] {
flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar))
}
} else {
flagArgsSeparated = append(flagArgsSeparated, flagArg)
}
}
return flagArgsSeparated
}

// Names returns the names including short names and aliases.
func (c Command) Names() []string {
names := []string{c.Name}
Expand Down
1 change: 1 addition & 0 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func TestCommandFlagParsing(t *testing.T) {

// Test no arg reorder
{[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false},
{[]string{"test-cmd", "blah", "blah", "-break", "ls", "-l"}, false, true, nil, true},

{[]string{"test-cmd", "blah", "blah"}, true, false, nil, false}, // Test SkipFlagParsing without any args that look like flags
{[]string{"test-cmd", "blah", "-break"}, true, false, nil, false}, // Test SkipFlagParsing with random flag arg
Expand Down