From bbd756a99f9e77590ee4a5718193d50e83a550b3 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 7 Jul 2020 16:10:32 -0500 Subject: [PATCH] split shell alias execution into new function --- cmd/gh/main.go | 11 +++++-- command/alias.go | 6 ++-- command/alias_test.go | 44 +++++++++++----------------- command/root.go | 68 ++++++++++++++++++++++++++----------------- 4 files changed, 69 insertions(+), 60 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index a52d23f21..2e9614a79 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -40,14 +40,19 @@ func main() { cmd, _, err := command.RootCmd.Traverse(expandedArgs) if err != nil || cmd == command.RootCmd { originalArgs := expandedArgs - expandedArgs, err = command.ExpandAlias(os.Args) + expandedArgs, isShell, err := command.ExpandAlias(os.Args) if err != nil { fmt.Fprintf(stderr, "failed to process aliases: %s\n", err) os.Exit(2) } - if expandedArgs == nil && err == nil { - // It was an external alias; we ran it and are now done. + if isShell { + err = command.ExecuteShellAlias(expandedArgs) + if err != nil { + fmt.Fprintf(stderr, "failed to run alias %q: %s\n", expandedArgs, err) + os.Exit(3) + } + os.Exit(0) } diff --git a/command/alias.go b/command/alias.go index 680763bfc..5baceff33 100644 --- a/command/alias.go +++ b/command/alias.go @@ -41,9 +41,9 @@ var aliasSetCmd = &cobra.Command{ that follow the invocation of an alias will be inserted appropriately. If '--shell' is specified, the alias will be run through a shell interpreter (sh or pwsh). This allows you - to compose commands with "|" or redirect output. Note that extra arguments are not passed to - shell-interpreted aliases; only placeholders ("$1", "$2" on *nix, "$args" in powershell) are - supported. + to compose commands with "|" or redirect output. Note that extra arguments following the alias + will not be automatically passed to the expanded expression. To have a shell alias receive + arguments, you must explicitly accept them using "$1", "$2", etc or "$@" to accept all of them. Quotes must always be used when defining a command as in the examples.`), Example: heredoc.Doc(` diff --git a/command/alias_test.go b/command/alias_test.go index 6f5077aa4..f140a13f8 100644 --- a/command/alias_test.go +++ b/command/alias_test.go @@ -187,21 +187,17 @@ aliases: ` initBlankContext(cfg, "OWNER/REPO", "trunk") - cs, teardown := test.InitCmdStubber() - defer teardown() - cs.Stub("") + expanded, isShell, err := ExpandAlias([]string{"gh", "ig"}) - _, err := ExpandAlias([]string{"gh", "ig"}) + assert.True(t, isShell) if err != nil { t.Fatalf("unexpected error: %s", err) } - assert.Equal(t, 1, len(cs.Calls)) - expected := []string{"sh", "-c", "gh issue list | grep cool"} - assert.Equal(t, expected, cs.Calls[0].Args) + assert.Equal(t, expected, expanded) } func TestExpandAlias_shell_nix_extra_args(t *testing.T) { @@ -214,21 +210,17 @@ aliases: ` initBlankContext(cfg, "OWNER/REPO", "trunk") - cs, teardown := test.InitCmdStubber() - defer teardown() - cs.Stub("") + expanded, isShell, err := ExpandAlias([]string{"gh", "ig", "bug", "foo"}) - _, err := ExpandAlias([]string{"gh", "ig", "bug", "foo"}) + assert.True(t, isShell) if err != nil { t.Fatalf("unexpected error: %s", err) } - assert.Equal(t, 1, len(cs.Calls)) - expected := []string{"sh", "-c", "gh issue list --label=$1 | grep", "--", "bug", "foo"} - assert.Equal(t, expected, cs.Calls[0].Args) + assert.Equal(t, expected, expanded) } func TestExpandAlias_shell_windows(t *testing.T) { @@ -245,17 +237,17 @@ aliases: defer teardown() cs.Stub("") - _, err := ExpandAlias([]string{"gh", "ig"}) + expanded, isShell, err := ExpandAlias([]string{"gh", "ig"}) + + assert.True(t, isShell) if err != nil { t.Fatalf("unexpected error: %s", err) } - assert.Equal(t, 1, len(cs.Calls)) - expected := []string{"pwsh", "-Command", "Invoke-Command -ScriptBlock { gh issue list | select-string -Pattern cool } "} - assert.Equal(t, expected, cs.Calls[0].Args) + assert.Equal(t, expected, expanded) } func TestExpandAlias_shell_windows_extra_args(t *testing.T) { @@ -269,21 +261,17 @@ aliases: ` initBlankContext(cfg, "OWNER/REPO", "trunk") - cs, teardown := test.InitCmdStubber() - defer teardown() - cs.Stub("") + expanded, isShell, err := ExpandAlias([]string{"gh", "ig", "bug", "foo"}) - _, err := ExpandAlias([]string{"gh", "ig", "bug", "foo"}) + assert.True(t, isShell) if err != nil { t.Fatalf("unexpected error: %s", err) } - assert.Equal(t, 1, len(cs.Calls)) - expected := []string{"pwsh", "-Command", "Invoke-Command -ScriptBlock { gh issue list --label=$args[0] | select-string -Pattern $args[1] } -ArgumentList @('bug','foo')"} - assert.Equal(t, expected, cs.Calls[0].Args) + assert.Equal(t, expected, expanded) } func TestExpandAlias(t *testing.T) { @@ -317,7 +305,9 @@ aliases: args = strings.Split(c.Args, " ") } - out, err := ExpandAlias(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) @@ -329,7 +319,7 @@ aliases: continue } - eq(t, out, c.ExpectedArgs) + assert.Equal(t, c.ExpectedArgs, expanded) } } diff --git a/command/root.go b/command/root.go index d8e6db028..7d26cfb88 100644 --- a/command/root.go +++ b/command/root.go @@ -366,29 +366,50 @@ func determineEditor(cmd *cobra.Command) (string, error) { return editorCommand, nil } -func ExpandAlias(args []string) ([]string, error) { +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) + + err := preparedCmd.Run() + if err != nil { + return fmt.Errorf("failed to run external command: %w", err) + } + + return nil +} + +// 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 []string{}, nil + return } ctx := initContext() cfg, err := ctx.Config() if err != nil { - return nil, err + return } aliases, err := cfg.Aliases() if err != nil { - return nil, err + return } expansion, ok := aliases.Get(args[1]) if ok { if strings.HasPrefix(expansion, "!") { - shellArgs := []string{"-c", expansion[1:]} - shellCmd := "sh" + isShell = true + expanded = []string{"sh", "-c", expansion[1:]} if runtime.GOOS == "windows" { - shellCmd = "pwsh" argList := "" if len(args[2:]) > 0 { argList = " -ArgumentList @(" @@ -401,25 +422,15 @@ func ExpandAlias(args []string) ([]string, error) { argList += ")" } invoke := fmt.Sprintf("Invoke-Command -ScriptBlock { %s } %s", expansion[1:], argList) - shellArgs = []string{"-Command", invoke} + expanded = []string{"pwsh", "-Command", invoke} } else { if len(args[2:]) > 0 { - shellArgs = append(shellArgs, "--") - shellArgs = append(shellArgs, args[2:]...) + expanded = append(expanded, "--") + expanded = append(expanded, args[2:]...) } } - externalCmd := exec.Command(shellCmd, shellArgs...) - externalCmd.Stderr = os.Stderr - externalCmd.Stdout = os.Stdout - externalCmd.Stdin = os.Stdin - preparedCmd := run.PrepareCmd(externalCmd) - err := preparedCmd.Run() - if err != nil { - return nil, fmt.Errorf("failed to run external command: %w", err) - } - - return nil, nil + return } extraArgs := []string{} @@ -432,19 +443,22 @@ func ExpandAlias(args []string) ([]string, error) { } lingeringRE := regexp.MustCompile(`\$\d`) if lingeringRE.MatchString(expansion) { - return nil, fmt.Errorf("not enough arguments for alias: %s", expansion) + err = fmt.Errorf("not enough arguments for alias: %s", expansion) + return } - newArgs, err := shlex.Split(expansion) + var newArgs []string + newArgs, err = shlex.Split(expansion) if err != nil { - return nil, err + return } - return append(newArgs, extraArgs...), nil - + expanded = append(newArgs, extraArgs...) + return } - return args[1:], nil + expanded = args[1:] + return } func connectedToTerminal(cmd *cobra.Command) bool {