From c0db6e6f0c970ff15ff4bea14a3948e97b43085e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Jan 2019 04:19:49 +0100 Subject: [PATCH] Use ArgsParser for commands that don't declare flags via pflag --- commands/args.go | 2 ++ commands/commands.go | 61 +++++++++++++++++++++----------- commands/issue.go | 5 ++- commands/release.go | 25 ++++++++++--- commands/runner.go | 78 +++++++++-------------------------------- commands/runner_test.go | 33 ----------------- features/help.feature | 6 ++++ main.go | 33 ++++++++++++++--- 8 files changed, 118 insertions(+), 125 deletions(-) diff --git a/commands/args.go b/commands/args.go index 93f5c4ed..790a1f03 100644 --- a/commands/args.go +++ b/commands/args.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/github/hub/cmd" + "github.com/github/hub/utils" ) type Args struct { @@ -19,6 +20,7 @@ type Args struct { Terminator bool noForward bool Callbacks []func() error + Flag *utils.ArgsParser } func (a *Args) Words() []string { diff --git a/commands/commands.go b/commands/commands.go index c56d1183..8f0e0c17 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -1,11 +1,12 @@ package commands import ( + "bytes" "fmt" "regexp" "strings" - "github.com/github/hub/ui" + "github.com/github/hub/utils" flag "github.com/ogier/pflag" ) @@ -32,7 +33,6 @@ type Command struct { func (c *Command) Call(args *Args) (err error) { runCommand, err := c.lookupSubCommand(args) if err != nil { - ui.Errorln(err) return } @@ -48,26 +48,56 @@ func (c *Command) Call(args *Args) (err error) { return } -func (c *Command) parseArguments(args *Args) (err error) { +type ErrHelp struct { + err string +} + +func (e ErrHelp) Error() string { + return e.err +} + +func hasFlags(fs *flag.FlagSet) (found bool) { + fs.VisitAll(func(f *flag.Flag) { + found = true + }) + return +} + +func (c *Command) parseArguments(args *Args) error { + if !hasFlags(&c.Flag) { + args.Flag = utils.NewArgsParserWithUsage("-h, --help\n" + c.Long) + if rest, err := args.Flag.Parse(args.Params); err == nil { + if args.Flag.Bool("--help") { + return &ErrHelp{err: c.Synopsis()} + } + args.Params = rest + return nil + } else { + return fmt.Errorf("%s\n%s", err, c.Synopsis()) + } + } + c.Flag.SetInterspersed(true) c.Flag.Init(c.Name(), flag.ContinueOnError) c.Flag.Usage = func() { - if args.HasFlags("-help", "--help") { - ui.Println(c.Synopsis()) - } else { - ui.Errorln(c.Synopsis()) - } } - if err = c.Flag.Parse(args.Params); err == nil { + var flagBuf bytes.Buffer + c.Flag.SetOutput(&flagBuf) + + err := c.Flag.Parse(args.Params) + if err == nil { for _, arg := range args.Params { if arg == "--" { args.Terminator = true } } args.Params = c.Flag.Args() + } else if err == flag.ErrHelp { + err = &ErrHelp{err: c.Synopsis()} + } else { + return fmt.Errorf("%s\n%s", err, c.Synopsis()) } - - return + return err } func (c *Command) FlagPassed(name string) bool { @@ -80,15 +110,6 @@ func (c *Command) FlagPassed(name string) bool { return found } -func (c *Command) Arg(idx int) string { - args := c.Flag.Args() - if idx < len(args) { - return args[idx] - } else { - return "" - } -} - func (c *Command) Use(subCommand *Command) { if c.subCommands == nil { c.subCommands = make(map[string]*Command) diff --git a/commands/issue.go b/commands/issue.go index 91f4ad70..9d6840a0 100644 --- a/commands/issue.go +++ b/commands/issue.go @@ -438,7 +438,10 @@ func formatIssue(issue github.Issue, format string, colorize bool) string { } func showIssue(cmd *Command, args *Args) { - issueNumber := cmd.Arg(0) + issueNumber := "" + if args.ParamsSize() > 0 { + issueNumber = args.GetParam(0) + } if issueNumber == "" { utils.Check(fmt.Errorf(cmd.Synopsis())) } diff --git a/commands/release.go b/commands/release.go index 3fdf5fac..e3759f85 100644 --- a/commands/release.go +++ b/commands/release.go @@ -313,7 +313,10 @@ func formatRelease(release github.Release, format string, colorize bool) string } func showRelease(cmd *Command, args *Args) { - tagName := cmd.Arg(0) + tagName := "" + if args.ParamsSize() > 0 { + tagName = args.GetParam(0) + } if tagName == "" { utils.Check(fmt.Errorf(cmdRelease.Synopsis())) } @@ -360,7 +363,10 @@ func showRelease(cmd *Command, args *Args) { } func downloadRelease(cmd *Command, args *Args) { - tagName := cmd.Arg(0) + tagName := "" + if args.ParamsSize() > 0 { + tagName = args.GetParam(0) + } if tagName == "" { utils.Check(fmt.Errorf(cmdRelease.Synopsis())) } @@ -406,7 +412,10 @@ func downloadReleaseAsset(asset github.ReleaseAsset, gh *github.Client) (err err } func createRelease(cmd *Command, args *Args) { - tagName := cmd.Arg(0) + tagName := "" + if args.ParamsSize() > 0 { + tagName = args.GetParam(0) + } if tagName == "" { utils.Check(fmt.Errorf(cmdRelease.Synopsis())) return @@ -475,7 +484,10 @@ text is the title and the rest is the description.`, tagName, project)) } func editRelease(cmd *Command, args *Args) { - tagName := cmd.Arg(0) + tagName := "" + if args.ParamsSize() > 0 { + tagName = args.GetParam(0) + } if tagName == "" { utils.Check(fmt.Errorf(cmdRelease.Synopsis())) return @@ -558,7 +570,10 @@ text is the title and the rest is the description.`, tagName, project)) } func deleteRelease(cmd *Command, args *Args) { - tagName := cmd.Arg(0) + tagName := "" + if args.ParamsSize() > 0 { + tagName = args.GetParam(0) + } if tagName == "" { utils.Check(fmt.Errorf(cmdRelease.Synopsis())) return diff --git a/commands/runner.go b/commands/runner.go index 3f79fbac..81383cc8 100644 --- a/commands/runner.go +++ b/commands/runner.go @@ -2,60 +2,21 @@ package commands import ( "fmt" - "os" - "os/exec" "strings" - "syscall" "github.com/github/hub/cmd" "github.com/github/hub/git" "github.com/github/hub/ui" "github.com/kballard/go-shellquote" - flag "github.com/ogier/pflag" ) -type ExecError struct { - Err error - Ran bool - ExitCode int -} - -func (execError *ExecError) Error() string { - return execError.Err.Error() -} - -func newExecError(err error) ExecError { - exitCode := 0 - ran := true - - if err != nil { - exitCode = 1 - switch e := err.(type) { - case *exec.ExitError: - if status, ok := e.Sys().(syscall.WaitStatus); ok { - exitCode = status.ExitStatus() - } - case *exec.Error: - ran = false - } - } - - return ExecError{ - Err: err, - Ran: ran, - ExitCode: exitCode, - } -} - type Runner struct { commands map[string]*Command - execute func([]*cmd.Cmd, bool) error } func NewRunner() *Runner { return &Runner{ commands: make(map[string]*Command), - execute: executeCommands, } } @@ -74,9 +35,9 @@ func (r *Runner) Lookup(name string) *Command { return r.commands[name] } -func (r *Runner) Execute() ExecError { - args := NewArgs(os.Args[1:]) - args.ProgramPath = os.Args[0] +func (r *Runner) Execute(cliArgs []string) error { + args := NewArgs(cliArgs[1:]) + args.ProgramPath = cliArgs[0] forceFail := false if args.Command == "" && len(args.GlobalFlags) == 0 { @@ -97,46 +58,39 @@ func (r *Runner) Execute() ExecError { cmd := r.Lookup(cmdName) if cmd != nil && cmd.Runnable() { - execErr := r.Call(cmd, args) - if execErr.ExitCode == 0 && forceFail { - execErr = newExecError(fmt.Errorf("")) + err := callRunnableCommand(cmd, args) + if err == nil && forceFail { + err = fmt.Errorf("") } - return execErr + return err } gitArgs := []string{args.Command} gitArgs = append(gitArgs, args.Params...) - err := git.Run(gitArgs...) - return newExecError(err) + return git.Run(gitArgs...) } -func (r *Runner) Call(cmd *Command, args *Args) ExecError { +func callRunnableCommand(cmd *Command, args *Args) error { err := cmd.Call(args) if err != nil { - if err == flag.ErrHelp { - err = nil - } - return newExecError(err) + return err } cmds := args.Commands() if args.Noop { printCommands(cmds) - } else { - err = r.execute(cmds, len(args.Callbacks) == 0) + } else if err = executeCommands(cmds, len(args.Callbacks) == 0); err != nil { + return err } - if err == nil { - for _, fn := range args.Callbacks { - err = fn() - if err != nil { - break - } + for _, fn := range args.Callbacks { + if err = fn(); err != nil { + return err } } - return newExecError(err) + return nil } func printCommands(cmds []*cmd.Cmd) { diff --git a/commands/runner_test.go b/commands/runner_test.go index 58b440be..f9450c63 100644 --- a/commands/runner_test.go +++ b/commands/runner_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/bmizerany/assert" - "github.com/github/hub/cmd" ) func TestRunner_splitAliasCmd(t *testing.T) { @@ -18,35 +17,3 @@ func TestRunner_splitAliasCmd(t *testing.T) { words, err = splitAliasCmd("") assert.NotEqual(t, nil, err) } - -func TestRunnerUseCommands(t *testing.T) { - r := &Runner{ - commands: make(map[string]*Command), - execute: func([]*cmd.Cmd, bool) error { return nil }, - } - c := &Command{Usage: "foo"} - r.Use(c) - - assert.Equal(t, c, r.Lookup("foo")) -} - -func TestRunnerCallCommands(t *testing.T) { - var result string - f := func(c *Command, args *Args) { - result = args.FirstParam() - args.Replace("git", "version", "") - } - - r := &Runner{ - commands: make(map[string]*Command), - execute: func([]*cmd.Cmd, bool) error { return nil }, - } - c := &Command{Usage: "foo", Run: f} - r.Use(c) - - args := NewArgs([]string{"foo", "bar"}) - err := r.Call(c, args) - - assert.Equal(t, 0, err.ExitCode) - assert.Equal(t, "bar", result) -} diff --git a/features/help.feature b/features/help.feature index 25125519..b373b9e2 100644 --- a/features/help.feature +++ b/features/help.feature @@ -9,6 +9,12 @@ Feature: hub help """ And the output should contain "usage: git " + Scenario: Shows help text with no arguments + When I run `hub` + Then the stdout should contain "usage: git " + And the stderr should contain exactly "" + And the exit status should be 1 + Scenario: Appends hub commands to `--all` output When I successfully run `hub help -a` Then the output should contain "pull-request" diff --git a/main.go b/main.go index 1d755b8b..4348f750 100644 --- a/main.go +++ b/main.go @@ -4,18 +4,43 @@ package main import ( "os" + "os/exec" + "syscall" "github.com/github/hub/commands" "github.com/github/hub/github" "github.com/github/hub/ui" + flag "github.com/ogier/pflag" ) func main() { defer github.CaptureCrash() + err := commands.CmdRunner.Execute(os.Args) + exitCode := handleError(err) + os.Exit(exitCode) +} + +func handleError(err error) int { + if err == nil { + return 0 + } else if err == flag.ErrHelp { + return 0 + } - err := commands.CmdRunner.Execute() - if !err.Ran { - ui.Errorln(err.Error()) + switch e := err.(type) { + case *exec.ExitError: + if status, ok := e.Sys().(syscall.WaitStatus); ok { + return status.ExitStatus() + } else { + return 1 + } + case *commands.ErrHelp: + ui.Println(err) + return 0 + default: + if errString := err.Error(); errString != "" { + ui.Errorln(err) + } + return 1 } - os.Exit(err.ExitCode) } -- GitLab