Merge pull request #970 from cli/alias-set

gh alias set

cmd/gh/main.go

@@ -30,6 +30,28 @@ func main() {
 
 	hasDebug := os.Getenv("DEBUG") != ""
 
+	stderr := utils.NewColorable(os.Stderr)
+
+	expandedArgs := []string{}
+	if len(os.Args) > 0 {
+		expandedArgs = os.Args[1:]
+	}
+
+	cmd, _, err := command.RootCmd.Traverse(expandedArgs)
+	if err != nil || cmd == command.RootCmd {
+		originalArgs := expandedArgs
+		expandedArgs, err = command.ExpandAlias(os.Args)
+		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)
+		}
+	}
+
+	command.RootCmd.SetArgs(expandedArgs)
+
 	if cmd, err := command.RootCmd.ExecuteC(); err != nil {
 		printError(os.Stderr, err, cmd, hasDebug)
 		os.Exit(1)

command/alias.go

@@ -0,0 +1,114 @@
+package command
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/cli/cli/utils"
+	"github.com/google/shlex"
+	"github.com/spf13/cobra"
+)
+
+func init() {
+	RootCmd.AddCommand(aliasCmd)
+	aliasCmd.AddCommand(aliasSetCmd)
+}
+
+var aliasCmd = &cobra.Command{
+	Use:   "alias",
+	Short: "Create shortcuts for gh commands",
+}
+
+var aliasSetCmd = &cobra.Command{
+	Use: "set <alias> <expansion>",
+	// NB: Even when inside of a single-quoted string, cobra was noticing and parsing any flags
+	// used in an alias expansion string argument. Since this command needs no flags, I disabled their
+	// parsing. If we ever want to add flags to alias set we'll have to figure this out. I tested on
+	// linux in various shells against cobra 1.0; others on macos did /not/ see the same behavior.
+	DisableFlagParsing: true,
+	Short:              "Create a shortcut for a gh command",
+	Long:               `This command lets you write your own shortcuts for running gh. They can be simple strings or accept placeholder arguments.`,
+	Example: `
+	gh alias set pv 'pr view'
+	# gh pv -w 123 -> gh pr view -w 123.
+
+	gh alias set bugs 'issue list --label="bugs"'
+	# gh bugs -> gh issue list --label="bugs".
+
+	gh alias set epicsBy 'issue list --author="$1" --label="epic"'
+	# gh epicsBy vilmibm -> gh issue list --author="$1" --label="epic"
+`,
+	Args: cobra.MinimumNArgs(2),
+	RunE: aliasSet,
+}
+
+func aliasSet(cmd *cobra.Command, args []string) error {
+	ctx := contextForCommand(cmd)
+	cfg, err := ctx.Config()
+	if err != nil {
+		return err
+	}
+
+	aliasCfg, err := cfg.Aliases()
+	if err != nil {
+		return err
+	}
+
+	alias := args[0]
+	expansion := processArgs(args[1:])
+
+	expansionStr := strings.Join(expansion, " ")
+
+	out := colorableOut(cmd)
+	fmt.Fprintf(out, "- Adding alias for %s: %s\n", utils.Bold(alias), utils.Bold(expansionStr))
+
+	if validCommand([]string{alias}) {
+		return fmt.Errorf("could not create alias: %q is already a gh command", alias)
+	}
+
+	if !validCommand(expansion) {
+		return fmt.Errorf("could not create alias: %s does not correspond to a gh command", utils.Bold(expansionStr))
+	}
+
+	successMsg := fmt.Sprintf("%s Added alias.", utils.Green("✓"))
+
+	if aliasCfg.Exists(alias) {
+		successMsg = fmt.Sprintf("%s Changed alias %s from %s to %s",
+			utils.Green("✓"),
+			utils.Bold(alias),
+			utils.Bold(aliasCfg.Get(alias)),
+			utils.Bold(expansionStr),
+		)
+	}
+
+	err = aliasCfg.Add(alias, expansionStr)
+	if err != nil {
+		return fmt.Errorf("could not create alias: %s", err)
+	}
+
+	fmt.Fprintln(out, successMsg)
+
+	return nil
+}
+
+func validCommand(expansion []string) bool {
+	cmd, _, err := RootCmd.Traverse(expansion)
+	return err == nil && cmd != RootCmd
+}
+
+func processArgs(args []string) []string {
+	if len(args) == 1 {
+		split, _ := shlex.Split(args[0])
+		return split
+	}
+
+	newArgs := []string{}
+	for _, a := range args {
+		if !strings.HasPrefix(a, "-") && strings.Contains(a, " ") {
+			a = fmt.Sprintf("%q", a)
+		}
+		newArgs = append(newArgs, a)
+	}
+
+	return newArgs
+}

command/alias_test.go

@@ -0,0 +1,254 @@
+package command
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/cli/cli/internal/config"
+	"github.com/cli/cli/test"
+)
+
+func TestAliasSet_gh_command(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "trunk")
+
+	buf := bytes.NewBufferString("")
+	defer config.StubWriteConfig(buf)()
+
+	_, err := RunCommand("alias set pr pr status")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+
+	eq(t, err.Error(), `could not create alias: "pr" is already a gh command`)
+}
+
+func TestAliasSet_empty_aliases(t *testing.T) {
+	cfg := `---
+aliases:
+hosts:
+  github.com:
+    user: OWNER
+    oauth_token: token123
+`
+	initBlankContext(cfg, "OWNER/REPO", "trunk")
+
+	buf := bytes.NewBufferString("")
+	defer config.StubWriteConfig(buf)()
+	output, err := RunCommand("alias set co pr checkout")
+
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	test.ExpectLines(t, output.String(), "Added alias")
+
+	expected := `aliases:
+    co: pr checkout
+hosts:
+    github.com:
+        user: OWNER
+        oauth_token: token123
+`
+	eq(t, buf.String(), expected)
+}
+
+func TestAliasSet_existing_alias(t *testing.T) {
+	cfg := `---
+hosts:
+  github.com:
+    user: OWNER
+    oauth_token: token123
+aliases:
+  co: pr checkout
+`
+	initBlankContext(cfg, "OWNER/REPO", "trunk")
+
+	buf := bytes.NewBufferString("")
+	defer config.StubWriteConfig(buf)()
+	output, err := RunCommand("alias set co pr checkout -Rcool/repo")
+
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	test.ExpectLines(t, output.String(), "Changed alias co from pr checkout to pr checkout -Rcool/repo")
+}
+
+func TestAliasSet_space_args(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "trunk")
+
+	buf := bytes.NewBufferString("")
+	defer config.StubWriteConfig(buf)()
+
+	output, err := RunCommand(`alias set il issue list -l 'cool story'`)
+
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	test.ExpectLines(t, output.String(), `Adding alias for il: issue list -l "cool story"`)
+
+	test.ExpectLines(t, buf.String(), `il: issue list -l "cool story"`)
+}
+
+func TestAliasSet_arg_processing(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "trunk")
+	cases := []struct {
+		Cmd                string
+		ExpectedOutputLine string
+		ExpectedConfigLine string
+	}{
+		{"alias set co pr checkout", "- Adding alias for co: pr checkout", "co: pr checkout"},
+
+		{`alias set il "issue list"`, "- Adding alias for il: issue list", "il: issue list"},
+
+		{`alias set iz 'issue list'`, "- Adding alias for iz: issue list", "iz: issue list"},
+
+		{`alias set iy issue list --author=\$1 --label=\$2`,
+			`- Adding alias for iy: issue list --author=\$1 --label=\$2`,
+			`iy: issue list --author=\$1 --label=\$2`},
+
+		{`alias set ii 'issue list --author="$1" --label="$2"'`,
+			`- Adding alias for ii: issue list --author=\$1 --label=\$2`,
+			`ii: issue list --author=\$1 --label=\$2`},
+
+		{`alias set ix issue list --author='$1' --label='$2'`,
+			`- Adding alias for ix: issue list --author=\$1 --label=\$2`,
+			`ix: issue list --author=\$1 --label=\$2`},
+	}
+
+	var buf *bytes.Buffer
+	for _, c := range cases {
+		buf = bytes.NewBufferString("")
+		defer config.StubWriteConfig(buf)()
+		output, err := RunCommand(c.Cmd)
+		if err != nil {
+			t.Fatalf("got unexpected error running %s: %s", c.Cmd, err)
+		}
+
+		test.ExpectLines(t, output.String(), c.ExpectedOutputLine)
+		test.ExpectLines(t, buf.String(), c.ExpectedConfigLine)
+	}
+}
+
+func TestAliasSet_init_alias_cfg(t *testing.T) {
+	cfg := `---
+hosts:
+  github.com:
+    user: OWNER
+    oauth_token: token123
+`
+	initBlankContext(cfg, "OWNER/REPO", "trunk")
+
+	buf := bytes.NewBufferString("")
+	defer config.StubWriteConfig(buf)()
+
+	output, err := RunCommand("alias set diff pr diff")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	expected := `hosts:
+    github.com:
+        user: OWNER
+        oauth_token: token123
+aliases:
+    diff: pr diff
+`
+
+	test.ExpectLines(t, output.String(), "Adding alias for diff: pr diff", "Added alias.")
+	eq(t, buf.String(), expected)
+}
+
+func TestAliasSet_existing_aliases(t *testing.T) {
+	cfg := `---
+hosts:
+  github.com:
+    user: OWNER
+    oauth_token: token123
+aliases:
+    foo: bar
+`
+	initBlankContext(cfg, "OWNER/REPO", "trunk")
+
+	buf := bytes.NewBufferString("")
+	defer config.StubWriteConfig(buf)()
+
+	output, err := RunCommand("alias set view pr view")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	expected := `hosts:
+    github.com:
+        user: OWNER
+        oauth_token: token123
+aliases:
+    foo: bar
+    view: pr view
+`
+
+	test.ExpectLines(t, output.String(), "Adding alias for view: pr view", "Added alias.")
+	eq(t, buf.String(), expected)
+
+}
+
+func TestExpandAlias(t *testing.T) {
+	cfg := `---
+hosts:
+  github.com:
+    user: OWNER
+    oauth_token: token123
+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, " ")
+		}
+
+		out, err := ExpandAlias(args)
+
+		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
+		}
+
+		eq(t, out, c.ExpectedArgs)
+	}
+}
+
+func TestAliasSet_invalid_command(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "trunk")
+	_, err := RunCommand("alias set co pe checkout")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+
+	eq(t, err.Error(), "could not create alias: pe checkout does not correspond to a gh command")
+}

command/root.go

@@ -18,6 +18,7 @@ import (
 	"github.com/cli/cli/pkg/cmdutil"
 	"github.com/cli/cli/pkg/iostreams"
 	"github.com/cli/cli/utils"
+	"github.com/google/shlex"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -462,3 +463,48 @@ func determineEditor(cmd *cobra.Command) (string, error) {
 
 	return editorCommand, nil
 }
+
+func ExpandAlias(args []string) ([]string, error) {
+	empty := []string{}
+	if len(args) < 2 {
+		// the command is lacking a subcommand
+		return empty, nil
+	}
+
+	ctx := initContext()
+	cfg, err := ctx.Config()
+	if err != nil {
+		return empty, err
+	}
+	aliases, err := cfg.Aliases()
+	if err != nil {
+		return empty, err
+	}
+
+	if aliases.Exists(args[1]) {
+		extraArgs := []string{}
+		expansion := aliases.Get(args[1])
+		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) {
+			return empty, fmt.Errorf("not enough arguments for alias: %s", expansion)
+		}
+
+		newArgs, err := shlex.Split(expansion)
+		if err != nil {
+			return nil, err
+		}
+
+		newArgs = append(newArgs, extraArgs...)
+
+		return newArgs, nil
+	}
+
+	return args[1:], nil
+}

internal/config/alias_config.go

@@ -0,0 +1,44 @@
+package config
+
+import (
+	"fmt"
+)
+
+type AliasConfig struct {
+	ConfigMap
+	Parent Config
+}
+
+func (a *AliasConfig) Exists(alias string) bool {
+	if a.Empty() {
+		return false
+	}
+	value, _ := a.GetStringValue(alias)
+
+	return value != ""
+}
+
+func (a *AliasConfig) Get(alias string) string {
+	value, _ := a.GetStringValue(alias)
+
+	return value
+}
+
+func (a *AliasConfig) Add(alias, expansion string) error {
+	err := a.SetStringValue(alias, expansion)
+	if err != nil {
+		return fmt.Errorf("failed to update config: %w", err)
+	}
+
+	err = a.Parent.Write()
+	if err != nil {
+		return fmt.Errorf("failed to write config: %w", err)
+	}
+
+	return nil
+}
+
+func (a *AliasConfig) Delete(alias string) error {
+	// TODO when we get to gh alias delete
+	return nil
+}

internal/config/config_type.go

@@ -15,6 +15,7 @@ type Config interface {
 	Hosts() ([]*HostConfig, error)
 	Get(string, string) (string, error)
 	Set(string, string, string) error
+	Aliases() (*AliasConfig, error)
 	Write() error
 }
 
@@ -34,19 +35,25 @@ type ConfigMap struct {
 	Root *yaml.Node
 }
 
+func (cm *ConfigMap) Empty() bool {
+	return cm.Root == nil || len(cm.Root.Content) == 0
+}
+
 func (cm *ConfigMap) GetStringValue(key string) (string, error) {
-	_, valueNode, err := cm.FindEntry(key)
+	entry, err := cm.FindEntry(key)
 	if err != nil {
 		return "", err
 	}
-	return valueNode.Value, nil
+	return entry.ValueNode.Value, nil
 }
 
 func (cm *ConfigMap) SetStringValue(key, value string) error {
-	_, valueNode, err := cm.FindEntry(key)
+	entry, err := cm.FindEntry(key)
 
 	var notFound *NotFoundError
 
+	valueNode := entry.ValueNode
+
 	if err != nil && errors.As(err, &notFound) {
 		keyNode := &yaml.Node{
 			Kind:  yaml.ScalarNode,
@@ -67,19 +74,30 @@ func (cm *ConfigMap) SetStringValue(key, value string) error {
 	return nil
 }
 
-func (cm *ConfigMap) FindEntry(key string) (keyNode, valueNode *yaml.Node, err error) {
+type ConfigEntry struct {
+	KeyNode   *yaml.Node
+	ValueNode *yaml.Node
+	Index     int
+}
+
+func (cm *ConfigMap) FindEntry(key string) (ce *ConfigEntry, err error) {
 	err = nil
 
+	ce = &ConfigEntry{}
+
 	topLevelKeys := cm.Root.Content
 	for i, v := range topLevelKeys {
-		if v.Value == key && i+1 < len(topLevelKeys) {
-			keyNode = v
-			valueNode = topLevelKeys[i+1]
+		if v.Value == key {
+			ce.KeyNode = v
+			ce.Index = i
+			if i+1 < len(topLevelKeys) {
+				ce.ValueNode = topLevelKeys[i+1]
+			}
 			return
 		}
 	}
 
-	return nil, nil, &NotFoundError{errors.New("not found")}
+	return ce, &NotFoundError{errors.New("not found")}
 }
 
 func NewConfig(root *yaml.Node) Config {
@@ -96,6 +114,10 @@ type fileConfig struct {
 	hosts        []*HostConfig
 }
 
+func (c *fileConfig) Root() *yaml.Node {
+	return c.ConfigMap.Root
+}
+
 func (c *fileConfig) Get(hostname, key string) (string, error) {
 	if hostname != "" {
 		hostCfg, err := c.configForHost(hostname)
@@ -167,17 +189,71 @@ func (c *fileConfig) Write() error {
 	return WriteConfigFile(ConfigFile(), marshalled)
 }
 
+func (c *fileConfig) Aliases() (*AliasConfig, error) {
+	// The complexity here is for dealing with either a missing or empty aliases key. It's something
+	// we'll likely want for other config sections at some point.
+	entry, err := c.FindEntry("aliases")
+	var nfe *NotFoundError
+	notFound := errors.As(err, &nfe)
+	if err != nil && !notFound {
+		return nil, err
+	}
+
+	toInsert := []*yaml.Node{}
+
+	keyNode := entry.KeyNode
+	valueNode := entry.ValueNode
+
+	if keyNode == nil {
+		keyNode = &yaml.Node{
+			Kind:  yaml.ScalarNode,
+			Value: "aliases",
+		}
+		toInsert = append(toInsert, keyNode)
+	}
+
+	if valueNode == nil || valueNode.Kind != yaml.MappingNode {
+		valueNode = &yaml.Node{
+			Kind:  yaml.MappingNode,
+			Value: "",
+		}
+		toInsert = append(toInsert, valueNode)
+	}
+
+	if len(toInsert) > 0 {
+		newContent := []*yaml.Node{}
+		if notFound {
+			newContent = append(c.Root().Content, keyNode, valueNode)
+		} else {
+			for i := 0; i < len(c.Root().Content); i++ {
+				if i == entry.Index {
+					newContent = append(newContent, keyNode, valueNode)
+					i++
+				} else {
+					newContent = append(newContent, c.Root().Content[i])
+				}
+			}
+		}
+		c.Root().Content = newContent
+	}
+
+	return &AliasConfig{
+		Parent:    c,
+		ConfigMap: ConfigMap{Root: valueNode},
+	}, nil
+}
+
 func (c *fileConfig) Hosts() ([]*HostConfig, error) {
 	if len(c.hosts) > 0 {
 		return c.hosts, nil
 	}
 
-	_, hostsEntry, err := c.FindEntry("hosts")
+	entry, err := c.FindEntry("hosts")
 	if err != nil {
 		return nil, fmt.Errorf("could not find hosts config: %w", err)
 	}
 
-	hostConfigs, err := c.parseHosts(hostsEntry)
+	hostConfigs, err := c.parseHosts(entry.ValueNode)
 	if err != nil {
 		return nil, fmt.Errorf("could not parse hosts config: %w", err)
 	}