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

Add executor #25

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions executor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package goptions

import (
"errors"
"fmt"
"os"
"path/filepath"
"reflect"
)

var (
//ErrNoCommand This should never happen.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment should be more in the shape of ErrNoCommand should never be returned.

But more importantly: If it is never supposed to be returned, why return it? Seems like an appropriate place for a panic() imo.

ErrNoCommand = errors.New("No command specified")
)

//Executer used to allow auto execution of commands.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Space after //.

type Executer interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Executer needs proper documentation. I have no idea how to use this or what it does honestly. I’d like a (testable) example here.

I am gonna review the rest of this without really understanding what this is supposed to do. Apologies if I misunderstand something.

Execute(args []string) error
}

// ParseAndExecute ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

These comments are not really documenting anything. Would you mind augmenting them with an actual description or even proper examples??

func ParseAndExecute(v Executer) error {
globalFlagSet = NewFlagSet(filepath.Base(os.Args[0]), v)
if err := globalFlagSet.Parse(os.Args[1:]); err != nil {
return err
}
exe, args, err := getExecuterArgs(v)
if err != nil {
return err
}
return exe.Execute(args)
}

// ParseAndExecuteCustomArgs ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same.

func ParseAndExecuteCustomArgs(v Executer, args []string) error {
fs := NewFlagSet(filepath.Base(args[0]), v)
if err := fs.Parse(args[1:]); err != nil {
return err
}
exe, args, err := getExecuterArgs(v)
if err != nil {
return err
}
return exe.Execute(args)
}

// ParseExecuteAndFail ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same.

func ParseExecuteAndFail(v Executer) {
err := ParseAndExecute(v)
if err != nil {
errCode := 0
if err != ErrHelpRequest {
errCode = 1
fmt.Fprintf(os.Stderr, "Error: %s\n", err)
}
PrintHelp()
os.Exit(errCode)
}
}

// ParseExecuteAndFailCustomArgs ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same.

func ParseExecuteAndFailCustomArgs(v Executer, args []string) {
err := ParseAndExecuteCustomArgs(v, args)
if err != nil {
errCode := 0
if err != ErrHelpRequest {
errCode = 1
fmt.Fprintf(os.Stderr, "Error: %s\n", err)
}
PrintHelp()
os.Exit(errCode)
}
}

//GetCommands returns a list of all commands avaliable for the executer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Space after //

func GetCommands(v Executer) []string {
if globalFlagSet == nil {
panic("You must call parse before you can call get commands")
}
commands := []string{}
for k := range globalFlagSet.Verbs {
commands = append(commands, k)
}
return commands
}

func getExecuterArgs(v Executer) (Executer, []string, error) {
var cmd string
var args []string
structValue := reflect.ValueOf(v)
if structValue.Kind() == reflect.Ptr {
structValue = structValue.Elem()
}

if structValue.Kind() != reflect.Struct {
panic("Value type is not a pointer to a struct")
}

var i int
for i = 0; i < structValue.Type().NumField(); i++ {
if StartsWithLowercase(structValue.Type().Field(i).Name) {
continue
}
fieldValue := structValue.Field(i)
if fieldValue.Type().Name() == "Remainder" {
switch v := fieldValue.Interface().(type) {
case Remainder:
args = v
default:
fmt.Printf("%##v", fieldValue.Interface())
panic("Remainder was not a []string")

}
}
if fieldValue.Type().Name() == "Verbs" {
cmd = fmt.Sprint(fieldValue)
break
}
}
if cmd == "" {
return v, args, nil
}

for i++; i < structValue.Type().NumField(); i++ {
// fieldValue := structValue.Field(i)
tag := structValue.Type().Field(i).Tag.Get("goptions")
if tag == cmd {
x := structValue.Field(i).Interface()

switch y := x.(type) {
case Executer:
return getExecuterArgs(y)
default:
return nil, nil, fmt.Errorf("CMD: %s is not an Executer", structValue.Field(i).Type().Name())
}
}
}
return nil, nil, ErrNoCommand
}
4 changes: 3 additions & 1 deletion flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type Flag struct {
DefaultValue interface{}
}

// Return the name of the flag preceding the right amount of dashes.
// Name Return the name of the flag preceding the right amount of dashes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You probably ran go vet or something here. Please adjust the comment to a proper English sentence (“Name returns...”)

// The long name is preferred. If no name has been specified, "<unspecified>"
// will be returned.
func (f *Flag) Name() string {
Expand Down Expand Up @@ -61,12 +61,14 @@ func isLong(arg string) bool {
return strings.HasPrefix(arg, "--") && len(arg) >= 3
}

// Handles returns if the flag can handle the string.
func (f *Flag) Handles(arg string) bool {
return (isShort(arg) && arg[1:2] == f.Short) ||
(isLong(arg) && arg[2:] == f.Long)

}

// Parse Gets the value for this flag from the args if it is in the slice.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Lower-case “g” in “Gets”

func (f *Flag) Parse(args []string) ([]string, error) {
param, value := args[0], ""
if f.NeedsExtraValue() &&
Expand Down
6 changes: 5 additions & 1 deletion flagset.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func newFlagset(name string, structValue reflect.Value, parent *FlagSet) *FlagSe
}

var (
//ErrHelpRequest returned when help is requested.
Copy link
Collaborator

Choose a reason for hiding this comment

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

“is returned”

ErrHelpRequest = errors.New("Request for Help")
)

Expand Down Expand Up @@ -187,6 +188,7 @@ func (fs *FlagSet) hasShortFlag(fname string) bool {
return ok
}

// FlagByName get a flag by name from the flagset.
Copy link
Collaborator

Choose a reason for hiding this comment

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

“gets”

func (fs *FlagSet) FlagByName(fname string) *Flag {
if isShort(fname) && fs.hasShortFlag(fname[1:2]) {
return fs.shortMap[fname[1:2]]
Expand Down Expand Up @@ -214,11 +216,12 @@ func (fs *FlagSet) MutexGroups() map[string]MutexGroup {
return r
}

// Prints the FlagSet's help to the given writer.
// PrintHelp Prints the FlagSet's help to the given writer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Lower-case “P” in “Prints”

func (fs *FlagSet) PrintHelp(w io.Writer) {
fs.HelpFunc(w, fs)
}

// ParseAndFail parses the arguments and panics if there is an error.
func (fs *FlagSet) ParseAndFail(w io.Writer, args []string) {
err := fs.Parse(args)
if err != nil {
Expand All @@ -232,6 +235,7 @@ func (fs *FlagSet) ParseAndFail(w io.Writer, args []string) {
}
}

// StartsWithLowercase returns true if string starts with a lowercase.
func StartsWithLowercase(s string) bool {
if len(s) <= 0 {
return false
Expand Down
7 changes: 4 additions & 3 deletions helpfunc.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
// HelpFunc is the signature of a function responsible for printing the help.
type HelpFunc func(w io.Writer, fs *FlagSet)

// Generates a new HelpFunc taking a `text/template.Template`-formatted
// NewTemplatedHelpFunc Generates a new HelpFunc taking a `text/template.Template`-formatted
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Lower-case “G” in “Generates”

// string as an argument. The resulting template will be executed with the FlagSet
// as its data.
func NewTemplatedHelpFunc(tpl string) HelpFunc {
Expand All @@ -28,7 +28,8 @@ func NewTemplatedHelpFunc(tpl string) HelpFunc {
}

const (
_DEFAULT_HELP = "\xffUsage: {{.Name}} [global options] {{with .Verbs}}<verb> [verb options]{{end}}\n" +
// DefaultHelp is default help message template.
Copy link
Collaborator

Choose a reason for hiding this comment

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

“is the”

DefaultHelp = "\xffUsage: {{.Name}} [global options] {{with .Verbs}}<verb> [verb options]{{end}}\n" +
"\n" +
"Global options:\xff" +
"{{range .Flags}}" +
Expand Down Expand Up @@ -67,6 +68,6 @@ const (
// the output through a text/tabwriter.Writer before flushing it to the output.
func DefaultHelpFunc(w io.Writer, fs *FlagSet) {
tw := tabwriter.NewWriter(w, 4, 4, 1, ' ', tabwriter.StripEscape|tabwriter.DiscardEmptyColumns)
NewTemplatedHelpFunc(_DEFAULT_HELP)(tw, fs)
NewTemplatedHelpFunc(DefaultHelp)(tw, fs)
tw.Flush()
}
1 change: 0 additions & 1 deletion valueparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ func (f *Flag) setValue(s string) (err error) {
} else {
return fmt.Errorf("Unsupported flag type: %s", f.value.Type())
}
panic("Invalid execution path")
}

func boolValueParser(f *Flag, val string) (reflect.Value, error) {
Expand Down