Use ArgsParser for commands that don't declare flags via pflag

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 {

commands/commands.go

 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)

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()))
 	}

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

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) {

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)
-}

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"

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)
 }