split shell alias execution into new function

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

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(`

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

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 {