diff --git a/cmd/gen-docs/main.go b/cmd/gen-docs/main.go index 92581d07a..631185393 100644 --- a/cmd/gen-docs/main.go +++ b/cmd/gen-docs/main.go @@ -5,7 +5,9 @@ import ( "os" "strings" - "github.com/cli/cli/command" + "github.com/cli/cli/pkg/cmd/root" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" "github.com/spf13/cobra/doc" "github.com/spf13/pflag" ) @@ -35,13 +37,16 @@ func main() { fatal("no dir set") } + io, _, _, _ := iostreams.Test() + rootCmd := root.NewCmdRoot(&cmdutil.Factory{IOStreams: io}, "", "") + err := os.MkdirAll(*dir, 0755) if err != nil { fatal(err) } if *website { - err = doc.GenMarkdownTreeCustom(command.RootCmd, *dir, filePrepender, linkHandler) + err = doc.GenMarkdownTreeCustom(rootCmd, *dir, filePrepender, linkHandler) if err != nil { fatal(err) } @@ -54,7 +59,7 @@ func main() { Source: "", //source and manual are just put at the top of the manpage, before name Manual: "", //if source is an empty string, it's set to "Auto generated by spf13/cobra" } - err = doc.GenManTree(command.RootCmd, header, *dir) + err = doc.GenManTree(rootCmd, header, *dir) if err != nil { fatal(err) } diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 4d801f008..3e92b3b0d 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -12,6 +12,9 @@ import ( "github.com/cli/cli/command" "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/cmd/alias/expand" + "github.com/cli/cli/pkg/cmd/factory" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/update" @@ -32,25 +35,44 @@ func main() { hasDebug := os.Getenv("DEBUG") != "" - stderr := utils.NewColorable(os.Stderr) + cmdFactory := factory.New(command.Version) + stderr := cmdFactory.IOStreams.ErrOut + rootCmd := root.NewCmdRoot(cmdFactory, command.Version, command.BuildDate) expandedArgs := []string{} if len(os.Args) > 0 { expandedArgs = os.Args[1:] } - cmd, _, err := command.RootCmd.Traverse(expandedArgs) - if err != nil || cmd == command.RootCmd { + cmd, _, err := rootCmd.Traverse(expandedArgs) + if err != nil || cmd == rootCmd { originalArgs := expandedArgs isShell := false - expandedArgs, isShell, err = command.ExpandAlias(os.Args) + + cfg, err := cmdFactory.Config() + if err != nil { + fmt.Fprintf(stderr, "failed to read configuration: %s\n", err) + os.Exit(2) + } + + expandedArgs, isShell, err = expand.ExpandAlias(cfg, os.Args, nil) if err != nil { fmt.Fprintf(stderr, "failed to process aliases: %s\n", err) os.Exit(2) } + if hasDebug { + fmt.Fprintf(stderr, "%v -> %v\n", originalArgs, expandedArgs) + } + if isShell { - err = command.ExecuteShellAlias(expandedArgs) + externalCmd := exec.Command(expandedArgs[0], expandedArgs[1:]...) + externalCmd.Stderr = os.Stderr + externalCmd.Stdout = os.Stdout + externalCmd.Stdin = os.Stdin + preparedCmd := run.PrepareCmd(externalCmd) + + err = preparedCmd.Run() if err != nil { if ee, ok := err.(*exec.ExitError); ok { os.Exit(ee.ExitCode()) @@ -62,16 +84,12 @@ func main() { os.Exit(0) } - - if hasDebug { - fmt.Fprintf(stderr, "%v -> %v\n", originalArgs, expandedArgs) - } } - command.RootCmd.SetArgs(expandedArgs) + rootCmd.SetArgs(expandedArgs) - if cmd, err := command.RootCmd.ExecuteC(); err != nil { - printError(os.Stderr, err, cmd, hasDebug) + if cmd, err := rootCmd.ExecuteC(); err != nil { + printError(stderr, err, cmd, hasDebug) os.Exit(1) } if root.HasFailed() { @@ -86,7 +104,6 @@ func main() { ansi.Color(newRelease.Version, "cyan"), ansi.Color(newRelease.URL, "yellow")) - stderr := utils.NewColorable(os.Stderr) fmt.Fprintf(stderr, "\n\n%s\n\n", msg) } } diff --git a/command/alias_test.go b/command/alias_test.go deleted file mode 100644 index 7bb76f553..000000000 --- a/command/alias_test.go +++ /dev/null @@ -1,109 +0,0 @@ -package command - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" -) - -func stubSh(value string) func() { - orig := findSh - findSh = func() (string, error) { - return value, nil - } - return func() { - findSh = orig - } -} - -func TestExpandAlias_shell(t *testing.T) { - defer stubSh("sh")() - cfg := `--- -aliases: - ig: '!gh issue list | grep cool' -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - - expanded, isShell, err := ExpandAlias([]string{"gh", "ig"}) - - assert.True(t, isShell) - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - expected := []string{"sh", "-c", "gh issue list | grep cool"} - - assert.Equal(t, expected, expanded) -} - -func TestExpandAlias_shell_extra_args(t *testing.T) { - defer stubSh("sh")() - cfg := `--- -aliases: - ig: '!gh issue list --label=$1 | grep' -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - - expanded, isShell, err := ExpandAlias([]string{"gh", "ig", "bug", "foo"}) - - assert.True(t, isShell) - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - expected := []string{"sh", "-c", "gh issue list --label=$1 | grep", "--", "bug", "foo"} - - assert.Equal(t, expected, expanded) -} - -func TestExpandAlias(t *testing.T) { - cfg := `--- -aliases: - co: pr checkout - il: issue list --author="$1" --label="$2" - ia: issue list --author="$1" --assignee="$1" -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - for _, c := range []struct { - Args string - ExpectedArgs []string - Err string - }{ - {"gh co", []string{"pr", "checkout"}, ""}, - {"gh il", nil, `not enough arguments for alias: issue list --author="$1" --label="$2"`}, - {"gh il vilmibm", nil, `not enough arguments for alias: issue list --author="vilmibm" --label="$2"`}, - {"gh co 123", []string{"pr", "checkout", "123"}, ""}, - {"gh il vilmibm epic", []string{"issue", "list", `--author=vilmibm`, `--label=epic`}, ""}, - {"gh ia vilmibm", []string{"issue", "list", `--author=vilmibm`, `--assignee=vilmibm`}, ""}, - {"gh ia $coolmoney$", []string{"issue", "list", `--author=$coolmoney$`, `--assignee=$coolmoney$`}, ""}, - {"gh pr status", []string{"pr", "status"}, ""}, - {"gh il vilmibm epic -R vilmibm/testing", []string{"issue", "list", "--author=vilmibm", "--label=epic", "-R", "vilmibm/testing"}, ""}, - {"gh dne", []string{"dne"}, ""}, - {"gh", []string{}, ""}, - {"", []string{}, ""}, - } { - args := []string{} - if c.Args != "" { - args = strings.Split(c.Args, " ") - } - - expanded, isShell, err := ExpandAlias(args) - - assert.False(t, isShell) - - if err == nil && c.Err != "" { - t.Errorf("expected error %s for %s", c.Err, c.Args) - continue - } - - if err != nil { - eq(t, err.Error(), c.Err) - continue - } - - assert.Equal(t, c.ExpectedArgs, expanded) - } -} diff --git a/command/root.go b/command/root.go index 1ff53e783..5e5260793 100644 --- a/command/root.go +++ b/command/root.go @@ -1,26 +1,15 @@ package command import ( - "errors" "fmt" "os" - "os/exec" - "path/filepath" - "regexp" - "runtime" "runtime/debug" "strings" "github.com/cli/cli/api" - "github.com/cli/cli/context" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" - "github.com/cli/cli/internal/run" - "github.com/cli/cli/pkg/cmd/factory" - "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/utils" - "github.com/google/shlex" - "github.com/spf13/cobra" ) // Version is dynamically set by the toolchain or overridden by the Makefile. @@ -29,22 +18,12 @@ var Version = "DEV" // BuildDate is dynamically set at build time in the Makefile. var BuildDate = "" // YYYY-MM-DD -var RootCmd *cobra.Command - func init() { if Version == "DEV" { if info, ok := debug.ReadBuildInfo(); ok && info.Main.Version != "(devel)" { Version = info.Main.Version } } - - cmdFactory := factory.New(Version) - RootCmd = root.NewCmdRoot(cmdFactory, Version, BuildDate) -} - -// overridden in tests -var initContext = func() context.Context { - return context.New() } // BasicClient returns an API client for github.com only that borrows from but @@ -73,110 +52,3 @@ func apiVerboseLog() api.ClientOption { colorize := utils.IsTerminal(os.Stderr) return api.VerboseLog(utils.NewColorable(os.Stderr), logTraffic, colorize) } - -func ExecuteShellAlias(args []string) error { - externalCmd := exec.Command(args[0], args[1:]...) - externalCmd.Stderr = os.Stderr - externalCmd.Stdout = os.Stdout - externalCmd.Stdin = os.Stdin - preparedCmd := run.PrepareCmd(externalCmd) - - return preparedCmd.Run() -} - -var findSh = func() (string, error) { - shPath, err := exec.LookPath("sh") - if err == nil { - return shPath, nil - } - - if runtime.GOOS == "windows" { - winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") - // We can try and find a sh executable in a Git for Windows install - gitPath, err := exec.LookPath("git") - if err != nil { - return "", winNotFoundErr - } - - shPath = filepath.Join(filepath.Dir(gitPath), "..", "bin", "sh.exe") - _, err = os.Stat(shPath) - if err != nil { - return "", winNotFoundErr - } - - return shPath, nil - } - - return "", errors.New("unable to locate sh to execute shell alias with") -} - -// ExpandAlias processes argv to see if it should be rewritten according to a user's aliases. The -// second return value indicates whether the alias should be executed in a new shell process instead -// of running gh itself. -func ExpandAlias(args []string) (expanded []string, isShell bool, err error) { - err = nil - isShell = false - expanded = []string{} - - if len(args) < 2 { - // the command is lacking a subcommand - return - } - - ctx := initContext() - cfg, err := ctx.Config() - if err != nil { - return - } - aliases, err := cfg.Aliases() - if err != nil { - return - } - - expansion, ok := aliases.Get(args[1]) - if ok { - if strings.HasPrefix(expansion, "!") { - isShell = true - shPath, shErr := findSh() - if shErr != nil { - err = shErr - return - } - - expanded = []string{shPath, "-c", expansion[1:]} - - if len(args[2:]) > 0 { - expanded = append(expanded, "--") - expanded = append(expanded, args[2:]...) - } - - return - } - - extraArgs := []string{} - for i, a := range args[2:] { - if !strings.Contains(expansion, "$") { - extraArgs = append(extraArgs, a) - } else { - expansion = strings.ReplaceAll(expansion, fmt.Sprintf("$%d", i+1), a) - } - } - lingeringRE := regexp.MustCompile(`\$\d`) - if lingeringRE.MatchString(expansion) { - err = fmt.Errorf("not enough arguments for alias: %s", expansion) - return - } - - var newArgs []string - newArgs, err = shlex.Split(expansion) - if err != nil { - return - } - - expanded = append(newArgs, extraArgs...) - return - } - - expanded = args[1:] - return -} diff --git a/command/testing.go b/command/testing.go deleted file mode 100644 index 0668776c8..000000000 --- a/command/testing.go +++ /dev/null @@ -1,119 +0,0 @@ -package command - -import ( - "bytes" - "fmt" - "reflect" - "testing" - - "github.com/cli/cli/context" - "github.com/cli/cli/internal/config" - "github.com/cli/cli/utils" - "github.com/google/shlex" - "github.com/spf13/pflag" -) - -func eq(t *testing.T, got interface{}, expected interface{}) { - t.Helper() - if !reflect.DeepEqual(got, expected) { - t.Errorf("expected: %v, got: %v", expected, got) - } -} - -const defaultTestConfig = `hosts: - github.com: - user: OWNER - oauth_token: "1234567890" -` - -func initBlankContext(cfg, repo, branch string) { - initContext = func() context.Context { - ctx := context.NewBlank() - - if cfg == "" { - cfg = defaultTestConfig - } - - // NOTE we are not restoring the original readConfig; we never want to touch the config file on - // disk during tests. - config.StubConfig(cfg, "") - - return ctx - } -} - -type cmdOut struct { - outBuf, errBuf *bytes.Buffer -} - -func (c cmdOut) String() string { - return c.outBuf.String() -} - -func (c cmdOut) Stderr() string { - return c.errBuf.String() -} - -func RunCommand(args string) (*cmdOut, error) { - rootCmd := RootCmd - rootArgv, err := shlex.Split(args) - if err != nil { - return nil, err - } - - cmd, _, err := rootCmd.Traverse(rootArgv) - if err != nil { - return nil, err - } - - rootCmd.SetArgs(rootArgv) - - outBuf := bytes.Buffer{} - cmd.SetOut(&outBuf) - errBuf := bytes.Buffer{} - cmd.SetErr(&errBuf) - - // Reset flag values so they don't leak between tests - // FIXME: change how we initialize Cobra commands to render this hack unnecessary - cmd.Flags().VisitAll(func(f *pflag.Flag) { - f.Changed = false - switch v := f.Value.(type) { - case pflag.SliceValue: - _ = v.Replace([]string{}) - default: - switch v.Type() { - case "bool", "string", "int": - _ = v.Set(f.DefValue) - } - } - }) - - _, err = rootCmd.ExecuteC() - cmd.SetOut(nil) - cmd.SetErr(nil) - - return &cmdOut{&outBuf, &errBuf}, err -} - -func stubTerminal(connected bool) func() { - isTerminal := utils.IsTerminal - utils.IsTerminal = func(_ interface{}) bool { - return connected - } - - terminalSize := utils.TerminalSize - if connected { - utils.TerminalSize = func(_ interface{}) (int, int, error) { - return 80, 20, nil - } - } else { - utils.TerminalSize = func(_ interface{}) (int, int, error) { - return 0, 0, fmt.Errorf("terminal connection stubbed to false") - } - } - - return func() { - utils.IsTerminal = isTerminal - utils.TerminalSize = terminalSize - } -} diff --git a/pkg/cmd/alias/expand/expand.go b/pkg/cmd/alias/expand/expand.go new file mode 100644 index 000000000..b2117f198 --- /dev/null +++ b/pkg/cmd/alias/expand/expand.go @@ -0,0 +1,106 @@ +package expand + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "runtime" + "strings" + + "github.com/cli/cli/internal/config" + "github.com/google/shlex" +) + +// ExpandAlias processes argv to see if it should be rewritten according to a user's aliases. The +// second return value indicates whether the alias should be executed in a new shell process instead +// of running gh itself. +func ExpandAlias(cfg config.Config, args []string, findShFunc func() (string, error)) (expanded []string, isShell bool, err error) { + if len(args) < 2 { + // the command is lacking a subcommand + return + } + expanded = args[1:] + + aliases, err := cfg.Aliases() + if err != nil { + return + } + + expansion, ok := aliases.Get(args[1]) + if !ok { + return + } + + if strings.HasPrefix(expansion, "!") { + isShell = true + if findShFunc == nil { + findShFunc = findSh + } + shPath, shErr := findShFunc() + if shErr != nil { + err = shErr + return + } + + expanded = []string{shPath, "-c", expansion[1:]} + + if len(args[2:]) > 0 { + expanded = append(expanded, "--") + expanded = append(expanded, args[2:]...) + } + + return + } + + extraArgs := []string{} + for i, a := range args[2:] { + if !strings.Contains(expansion, "$") { + extraArgs = append(extraArgs, a) + } else { + expansion = strings.ReplaceAll(expansion, fmt.Sprintf("$%d", i+1), a) + } + } + lingeringRE := regexp.MustCompile(`\$\d`) + if lingeringRE.MatchString(expansion) { + err = fmt.Errorf("not enough arguments for alias: %s", expansion) + return + } + + var newArgs []string + newArgs, err = shlex.Split(expansion) + if err != nil { + return + } + + expanded = append(newArgs, extraArgs...) + return +} + +func findSh() (string, error) { + shPath, err := exec.LookPath("sh") + if err == nil { + return shPath, nil + } + + if runtime.GOOS == "windows" { + winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") + // We can try and find a sh executable in a Git for Windows install + gitPath, err := exec.LookPath("git") + if err != nil { + return "", winNotFoundErr + } + + shPath = filepath.Join(filepath.Dir(gitPath), "..", "bin", "sh.exe") + _, err = os.Stat(shPath) + if err != nil { + return "", winNotFoundErr + } + + return shPath, nil + } + + return "", errors.New("unable to locate sh to execute shell alias with") +} diff --git a/pkg/cmd/alias/expand/expand_test.go b/pkg/cmd/alias/expand/expand_test.go new file mode 100644 index 000000000..b9535f7c7 --- /dev/null +++ b/pkg/cmd/alias/expand/expand_test.go @@ -0,0 +1,185 @@ +package expand + +import ( + "errors" + "reflect" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" +) + +func TestExpandAlias(t *testing.T) { + findShFunc := func() (string, error) { + return "/usr/bin/sh", nil + } + + cfg := config.NewFromString(heredoc.Doc(` + aliases: + co: pr checkout + il: issue list --author="$1" --label="$2" + ia: issue list --author="$1" --assignee="$1" + `)) + + type args struct { + config config.Config + argv []string + } + tests := []struct { + name string + args args + wantExpanded []string + wantIsShell bool + wantErr error + }{ + { + name: "no arguments", + args: args{ + config: cfg, + argv: []string{}, + }, + wantExpanded: []string(nil), + wantIsShell: false, + wantErr: nil, + }, + { + name: "too few arguments", + args: args{ + config: cfg, + argv: []string{"gh"}, + }, + wantExpanded: []string(nil), + wantIsShell: false, + wantErr: nil, + }, + { + name: "no expansion", + args: args{ + config: cfg, + argv: []string{"gh", "pr", "status"}, + }, + wantExpanded: []string{"pr", "status"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "simple expansion", + args: args{ + config: cfg, + argv: []string{"gh", "co"}, + }, + wantExpanded: []string{"pr", "checkout"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "adding arguments after expansion", + args: args{ + config: cfg, + argv: []string{"gh", "co", "123"}, + }, + wantExpanded: []string{"pr", "checkout", "123"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "not enough arguments for expansion", + args: args{ + config: cfg, + argv: []string{"gh", "il"}, + }, + wantExpanded: []string{}, + wantIsShell: false, + wantErr: errors.New(`not enough arguments for alias: issue list --author="$1" --label="$2"`), + }, + { + name: "not enough arguments for expansion 2", + args: args{ + config: cfg, + argv: []string{"gh", "il", "vilmibm"}, + }, + wantExpanded: []string{}, + wantIsShell: false, + wantErr: errors.New(`not enough arguments for alias: issue list --author="vilmibm" --label="$2"`), + }, + { + name: "satisfy expansion arguments", + args: args{ + config: cfg, + argv: []string{"gh", "il", "vilmibm", "help wanted"}, + }, + wantExpanded: []string{"issue", "list", "--author=vilmibm", "--label=help wanted"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "mixed positional and non-positional arguments", + args: args{ + config: cfg, + argv: []string{"gh", "il", "vilmibm", "epic", "-R", "monalisa/testing"}, + }, + wantExpanded: []string{"issue", "list", "--author=vilmibm", "--label=epic", "-R", "monalisa/testing"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "dollar in expansion", + args: args{ + config: cfg, + argv: []string{"gh", "ia", "$coolmoney$"}, + }, + wantExpanded: []string{"issue", "list", "--author=$coolmoney$", "--assignee=$coolmoney$"}, + wantIsShell: false, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotExpanded, gotIsShell, err := ExpandAlias(tt.args.config, tt.args.argv, findShFunc) + if tt.wantErr != nil { + if err == nil { + t.Fatal("expected error") + } + if tt.wantErr.Error() != err.Error() { + t.Fatalf("expected error %q, got %q", tt.wantErr, err) + } + return + } + if err != nil { + t.Fatalf("got error: %v", err) + } + if !reflect.DeepEqual(gotExpanded, tt.wantExpanded) { + t.Errorf("ExpandAlias() gotExpanded = %v, want %v", gotExpanded, tt.wantExpanded) + } + if gotIsShell != tt.wantIsShell { + t.Errorf("ExpandAlias() gotIsShell = %v, want %v", gotIsShell, tt.wantIsShell) + } + }) + } +} + +// cfg := `--- +// aliases: +// co: pr checkout +// il: issue list --author="$1" --label="$2" +// ia: issue list --author="$1" --assignee="$1" +// ` +// initBlankContext(cfg, "OWNER/REPO", "trunk") +// for _, c := range []struct { +// Args string +// ExpectedArgs []string +// Err string +// }{ +// {"gh co", []string{"pr", "checkout"}, ""}, +// {"gh il", nil, `not enough arguments for alias: issue list --author="$1" --label="$2"`}, +// {"gh il vilmibm", nil, `not enough arguments for alias: issue list --author="vilmibm" --label="$2"`}, +// {"gh co 123", []string{"pr", "checkout", "123"}, ""}, +// {"gh il vilmibm epic", []string{"issue", "list", `--author=vilmibm`, `--label=epic`}, ""}, +// {"gh ia vilmibm", []string{"issue", "list", `--author=vilmibm`, `--assignee=vilmibm`}, ""}, +// {"gh ia $coolmoney$", []string{"issue", "list", `--author=$coolmoney$`, `--assignee=$coolmoney$`}, ""}, +// {"gh pr status", []string{"pr", "status"}, ""}, +// {"gh il vilmibm epic -R vilmibm/testing", []string{"issue", "list", "--author=vilmibm", "--label=epic", "-R", "vilmibm/testing"}, ""}, +// {"gh dne", []string{"dne"}, ""}, +// {"gh", []string{}, ""}, +// {"", []string{}, ""}, +// } {