Merge pull request #951 from github/global-args

Avoid depending on a hardcoded list of git global flags

commands/args.go

@@ -2,12 +2,9 @@ package commands
 
 import (
 	"fmt"
-	"io/ioutil"
 	"strings"
 
 	"github.com/github/hub/cmd"
-
-	flag "github.com/github/hub/Godeps/_workspace/src/github.com/ogier/pflag"
 )
 
 type Args struct {
@@ -24,7 +21,7 @@ type Args struct {
 func (a *Args) Words() []string {
 	aa := make([]string, 0)
 	for _, p := range a.Params {
-		if !strings.HasPrefix(p, "-") {
+		if !looksLikeFlag(p) {
 			aa = append(aa, p)
 		}
 	}
@@ -171,7 +168,8 @@ func NewArgs(args []string) *Args {
 		globalFlags []string
 	)
 
-	slurpGlobalFlags(&args, &globalFlags, &noop)
+	slurpGlobalFlags(&args, &globalFlags)
+	noop = removeValue(&globalFlags, noopFlag)
 
 	if len(args) == 0 {
 		params = []string{}
@@ -191,90 +189,42 @@ func NewArgs(args []string) *Args {
 	}
 }
 
-func slurpGlobalFlags(args *[]string, globalFlags *[]string, noop *bool) {
-	var (
-		globalFlagSet flag.FlagSet
-
-		configParam      mapValue = make(mapValue)
-		paginate         bool
-		noPaginate       bool
-		noReplaceObjects bool
-		bare             bool
-		version          bool
-		help             bool
-
-		execPath string
-		gitDir   string
-		workTree string
-	)
-
-	globalFlagSet.BoolVarP(noop, "noop", "", false, "")
-	globalFlagSet.VarP(configParam, "", "c", "")
-	globalFlagSet.BoolVarP(&paginate, "paginate", "p", false, "")
-	globalFlagSet.BoolVarP(&noPaginate, "no-pager", "", false, "")
-	globalFlagSet.BoolVarP(&noReplaceObjects, "no-replace-objects", "", false, "")
-	globalFlagSet.BoolVarP(&bare, "bare", "", false, "")
-	globalFlagSet.BoolVarP(&version, "version", "", false, "")
-	globalFlagSet.BoolVarP(&help, "help", "", false, "")
+const (
+	noopFlag    = "--noop"
+	versionFlag = "--version"
+	helpFlag    = "--help"
+	configFlag  = "-c"
+	chdirFlag   = "-C"
+	flagPrefix  = "-"
+)
 
-	globalFlagSet.StringVarP(&execPath, "exec-path", "", "", "")
-	globalFlagSet.StringVarP(&gitDir, "git-dir", "", "", "")
-	globalFlagSet.StringVarP(&workTree, "work-tree", "", "", "")
+func looksLikeFlag(value string) bool {
+	return strings.HasPrefix(value, flagPrefix)
+}
 
-	globalFlagSet.SetOutput(ioutil.Discard)
-	globalFlagSet.Init("hub", flag.ContinueOnError)
+func slurpGlobalFlags(args *[]string, globalFlags *[]string) {
+	slurpNextValue := false
+	commandIndex := 0
 
-	aa := make([]string, 0)
-	err := globalFlagSet.Parse(*args)
-	if err == nil {
-		aa = globalFlagSet.Args()
+	for i, arg := range *args {
+		if slurpNextValue {
+			commandIndex = i + 1
+			slurpNextValue = false
+		} else if arg == versionFlag || arg == helpFlag || !looksLikeFlag(arg) {
+			break
 		} else {
-		aa = *args
-	}
-
-	// manipulate global flags
-	if version {
-		aa = append([]string{"version"}, aa...)
-	}
-
-	if help {
-		aa = append([]string{"help"}, aa...)
-	}
-
-	for k, v := range configParam {
-		*globalFlags = append(*globalFlags, "-c")
-		*globalFlags = append(*globalFlags, fmt.Sprintf("%s=%s", k, v))
-	}
-
-	if paginate {
-		*globalFlags = append(*globalFlags, "--paginate")
-	}
-
-	if noPaginate {
-		*globalFlags = append(*globalFlags, "--no-pager")
-	}
-
-	if noReplaceObjects {
-		*globalFlags = append(*globalFlags, "--no-replace-objects")
-	}
-
-	if bare {
-		*globalFlags = append(*globalFlags, "--bare")
+			commandIndex = i + 1
+			if arg == configFlag || arg == chdirFlag {
+				slurpNextValue = true
 			}
-
-	if execPath != "" {
-		*globalFlags = append(*globalFlags, "--exec-path", execPath)
 		}
-
-	if gitDir != "" {
-		*globalFlags = append(*globalFlags, "--git-dir", gitDir)
 	}
 
-	if workTree != "" {
-		*globalFlags = append(*globalFlags, "--work-tree", workTree)
+	if commandIndex > 0 {
+		aa := *args
+		*globalFlags = aa[0:commandIndex]
+		*args = aa[commandIndex:]
 	}
-
-	*args = aa
 }
 
 func removeItem(slice []string, index int) (newSlice []string, item string) {
@@ -287,3 +237,15 @@ func removeItem(slice []string, index int) (newSlice []string, item string) {
 
 	return newSlice, item
 }
+
+func removeValue(slice *[]string, value string) (found bool) {
+	aa := *slice
+	for i := len(aa) - 1; i >= 0; i-- {
+		arg := aa[i]
+		if arg == value {
+			found = true
+			*slice, _ = removeItem(*slice, i)
+		}
+	}
+	return found
+}

commands/args_test.go

 package commands
 
 import (
-	"strings"
 	"testing"
 
 	"github.com/github/hub/Godeps/_workspace/src/github.com/bmizerany/assert"
@@ -20,34 +19,22 @@ func TestNewArgs(t *testing.T) {
 	assert.Equal(t, "command", args.Command)
 	assert.Equal(t, 1, args.ParamsSize())
 
-	args = NewArgs([]string{"--noop", "command", "args"})
-	assert.Equal(t, "command", args.Command)
-	assert.Equal(t, 1, args.ParamsSize())
-	assert.T(t, args.Noop)
-
 	args = NewArgs([]string{"--version"})
-	assert.Equal(t, "version", args.Command)
+	assert.Equal(t, "--version", args.Command)
 	assert.Equal(t, 0, args.ParamsSize())
 
 	args = NewArgs([]string{"--help"})
-	assert.Equal(t, "help", args.Command)
+	assert.Equal(t, "--help", args.Command)
 	assert.Equal(t, 0, args.ParamsSize())
-
-	args = NewArgs([]string{"--noop", "--version"})
-	assert.T(t, args.Noop)
-	assert.Equal(t, "version", args.Command)
-
-	args = NewArgs([]string{"-c", "foo=bar", "--git-dir=path", "--bare"})
-	assert.Equal(t, 5, len(args.GlobalFlags))
-	assert.Equal(t, "-c foo=bar --bare --git-dir path", strings.Join(args.GlobalFlags, " "))
 }
 
 func TestArgs_Words(t *testing.T) {
-	args := NewArgs([]string{"--no-ff", "master"})
+	args := NewArgs([]string{"merge", "--no-ff", "master", "-m", "message"})
 	a := args.Words()
 
-	assert.Equal(t, 1, len(a))
+	assert.Equal(t, 2, len(a))
 	assert.Equal(t, "master", a[0])
+	assert.Equal(t, "message", a[1])
 }
 
 func TestArgs_Insert(t *testing.T) {
@@ -86,3 +73,35 @@ func TestArgs_Remove(t *testing.T) {
 	assert.Equal(t, "2", args.FirstParam())
 	assert.Equal(t, "4", args.GetParam(1))
 }
+
+func TestArgs_GlobalFlags(t *testing.T) {
+	args := NewArgs([]string{"-c", "key=value", "status", "-s", "-b"})
+	assert.Equal(t, "status", args.Command)
+	assert.Equal(t, []string{"-c", "key=value"}, args.GlobalFlags)
+	assert.Equal(t, []string{"-s", "-b"}, args.Params)
+	assert.Equal(t, false, args.Noop)
+}
+
+func TestArgs_GlobalFlags_Noop(t *testing.T) {
+	args := NewArgs([]string{"-c", "key=value", "--noop", "--literal-pathspecs", "status", "-s", "-b"})
+	assert.Equal(t, "status", args.Command)
+	assert.Equal(t, []string{"-c", "key=value", "--literal-pathspecs"}, args.GlobalFlags)
+	assert.Equal(t, []string{"-s", "-b"}, args.Params)
+	assert.Equal(t, true, args.Noop)
+}
+
+func TestArgs_GlobalFlags_NoopTwice(t *testing.T) {
+	args := NewArgs([]string{"--noop", "--bare", "--noop", "status"})
+	assert.Equal(t, "status", args.Command)
+	assert.Equal(t, []string{"--bare"}, args.GlobalFlags)
+	assert.Equal(t, 0, len(args.Params))
+	assert.Equal(t, true, args.Noop)
+}
+
+func TestArgs_GlobalFlags_Repeated(t *testing.T) {
+	args := NewArgs([]string{"-C", "mydir", "-c", "a=b", "--bare", "-c", "c=d", "-c", "e=f", "status"})
+	assert.Equal(t, "status", args.Command)
+	assert.Equal(t, []string{"-C", "mydir", "-c", "a=b", "--bare", "-c", "c=d", "-c", "e=f"}, args.GlobalFlags)
+	assert.Equal(t, 0, len(args.Params))
+	assert.Equal(t, false, args.Noop)
+}

commands/help.go

@@ -17,7 +17,7 @@ var cmdHelp = &Command{
 func init() {
 	cmdHelp.Run = runHelp
 
-	CmdRunner.Use(cmdHelp)
+	CmdRunner.Use(cmdHelp, "--help")
 }
 
 func runHelp(cmd *Command, args *Args) {

commands/runner.go

@@ -54,8 +54,11 @@ func (r *Runner) All() map[string]*Command {
 	return r.commands
 }
 
-func (r *Runner) Use(command *Command) {
+func (r *Runner) Use(command *Command, aliases ...string) {
 	r.commands[command.Name()] = command
+	if len(aliases) > 0 {
+		r.commands[aliases[0]] = command
+	}
 }
 
 func (r *Runner) Lookup(name string) *Command {

commands/version.go

@@ -19,7 +19,7 @@ var cmdVersion = &Command{
 }
 
 func init() {
-	CmdRunner.Use(cmdVersion)
+	CmdRunner.Use(cmdVersion, "--version")
 }
 
 func runVersion(cmd *Command, args *Args) {