Extract alias expansion into a separate package

This commit is contained in:
Mislav Marohnić 2020-08-11 15:18:48 +02:00
parent 172ea2b078
commit 2b9de23637
7 changed files with 329 additions and 372 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -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{}, ""},
// } {