diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index 20c86c473..77de9ca5d 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -17,9 +17,10 @@ type SetOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams - Name string - Expansion string - IsShell bool + Name string + Expansion string + IsShell bool + OverwriteExisting bool validAliasName func(string) bool validAliasExpansion func(string) bool @@ -86,6 +87,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } cmd.Flags().BoolVarP(&opts.IsShell, "shell", "s", false, "Declare an alias to be passed through a shell interpreter") + cmd.Flags().BoolVar(&opts.OverwriteExisting, "clobber", false, "Overwrite existing aliases of the same name") return cmd } @@ -104,31 +106,39 @@ func setRun(opts *SetOptions) error { return fmt.Errorf("did not understand expansion: %w", err) } - isTerminal := opts.IO.IsStdoutTTY() - if isTerminal { - fmt.Fprintf(opts.IO.ErrOut, "- Adding alias for %s: %s\n", cs.Bold(opts.Name), cs.Bold(expansion)) - } - if opts.IsShell && !strings.HasPrefix(expansion, "!") { expansion = "!" + expansion } + isTerminal := opts.IO.IsStdoutTTY() + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "- Creating alias for %s: %s\n", cs.Bold(opts.Name), cs.Bold(expansion)) + } + + var existingAlias bool + if _, err := aliasCfg.Get(opts.Name); err == nil { + existingAlias = true + } + if !opts.validAliasName(opts.Name) { - return fmt.Errorf("could not create alias: %q is already a gh command, extension, or alias", opts.Name) + if !existingAlias { + return fmt.Errorf("%s Could not create alias %s: already a gh command or extension", + cs.FailureIcon(), + cs.Bold(opts.Name)) + } + + if existingAlias && !opts.OverwriteExisting { + return fmt.Errorf("%s Could not create alias %s: name already taken, use the --clobber flag to overwrite it", + cs.FailureIcon(), + cs.Bold(opts.Name), + ) + } } if !opts.validAliasExpansion(expansion) { - return fmt.Errorf("could not create alias: %s does not correspond to a gh command, extension, or alias", expansion) - } - - successMsg := fmt.Sprintf("%s Added alias.", cs.SuccessIcon()) - if oldExpansion, err := aliasCfg.Get(opts.Name); err == nil { - successMsg = fmt.Sprintf("%s Changed alias %s from %s to %s", - cs.SuccessIcon(), - cs.Bold(opts.Name), - cs.Bold(oldExpansion), - cs.Bold(expansion), - ) + return fmt.Errorf("%s Could not create alias %s: expansion does not correspond to a gh command, extension, or alias", + cs.FailureIcon(), + cs.Bold(opts.Name)) } aliasCfg.Add(opts.Name, expansion) @@ -138,6 +148,13 @@ func setRun(opts *SetOptions) error { return err } + successMsg := fmt.Sprintf("%s Added alias %s", cs.SuccessIcon(), cs.Bold(opts.Name)) + if existingAlias && opts.OverwriteExisting { + successMsg = fmt.Sprintf("%s Changed alias %s", + cs.WarningIcon(), + cs.Bold(opts.Name)) + } + if isTerminal { fmt.Fprintln(opts.IO.ErrOut, successMsg) } diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 54fcabf5b..4acb8b8b0 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -2,314 +2,313 @@ package set import ( "bytes" - "io" + "fmt" "testing" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/pkg/cmd/alias/shared" "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/test" "github.com/google/shlex" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func runCommand(cfg config.Config, isTTY bool, cli string, in string) (*test.CmdOut, error) { - ios, stdin, stdout, stderr := iostreams.Test() - ios.SetStdoutTTY(isTTY) - ios.SetStdinTTY(isTTY) - ios.SetStderrTTY(isTTY) - stdin.WriteString(in) - - factory := &cmdutil.Factory{ - IOStreams: ios, - Config: func() (config.Config, error) { - return cfg, nil +func TestNewCmdSet(t *testing.T) { + tests := []struct { + name string + input string + output SetOptions + wantErr bool + errMsg string + }{ + { + name: "no arguments", + input: "", + wantErr: true, + errMsg: "accepts 2 arg(s), received 0", }, - ExtensionManager: &extensions.ExtensionManagerMock{ - ListFunc: func() []extensions.Extension { - return []extensions.Extension{} + { + name: "only one argument", + input: "name", + wantErr: true, + errMsg: "accepts 2 arg(s), received 1", + }, + { + name: "name and expansion", + input: "alias-name alias-expansion", + output: SetOptions{ + Name: "alias-name", + Expansion: "alias-expansion", + }, + }, + { + name: "shell flag", + input: "alias-name alias-expansion --shell", + output: SetOptions{ + Name: "alias-name", + Expansion: "alias-expansion", + IsShell: true, + }, + }, + { + name: "clobber flag", + input: "alias-name alias-expansion --clobber", + output: SetOptions{ + Name: "alias-name", + Expansion: "alias-expansion", + OverwriteExisting: true, }, }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: ios, + } + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + var gotOpts *SetOptions + cmd := NewCmdSet(f, func(opts *SetOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) - cmd := NewCmdSet(factory, nil) - - // Create fake command structure for testing. - rootCmd := &cobra.Command{} - rootCmd.AddCommand(cmd) - prCmd := &cobra.Command{Use: "pr"} - prCmd.AddCommand(&cobra.Command{Use: "checkout"}) - prCmd.AddCommand(&cobra.Command{Use: "status"}) - rootCmd.AddCommand(prCmd) - issueCmd := &cobra.Command{Use: "issue"} - issueCmd.AddCommand(&cobra.Command{Use: "list"}) - rootCmd.AddCommand(issueCmd) - apiCmd := &cobra.Command{Use: "api"} - apiCmd.AddCommand(&cobra.Command{Use: "graphql"}) - rootCmd.AddCommand(apiCmd) - - argv, err := shlex.Split("set " + cli) - if err != nil { - return nil, err - } - rootCmd.SetArgs(argv) - - rootCmd.SetIn(stdin) - rootCmd.SetOut(io.Discard) - rootCmd.SetErr(io.Discard) - - _, err = rootCmd.ExecuteC() - return &test.CmdOut{ - OutBuf: stdout, - ErrBuf: stderr, - }, err -} - -func TestAliasSet_gh_command(t *testing.T) { - cfg := config.NewFromString(``) - - _, err := runCommand(cfg, true, "pr 'pr status'", "") - assert.EqualError(t, err, `could not create alias: "pr" is already a gh command, extension, or alias`) -} - -func TestAliasSet_empty_aliases(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - aliases: - editor: vim - `)) - - output, err := runCommand(cfg, true, "co 'pr checkout'", "") - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Added alias") - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.String(), "") - - expected := `aliases: - co: pr checkout -editor: vim -` - assert.Equal(t, expected, mainBuf.String()) -} - -func TestAliasSet_existing_alias(t *testing.T) { - _ = config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - aliases: - co: pr checkout - `)) - - output, err := runCommand(cfg, true, "co 'pr checkout -Rcool/repo'", "") - require.NoError(t, err) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Changed alias.*co.*from.*pr checkout.*to.*pr checkout -Rcool/repo") -} - -func TestAliasSet_space_args(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, `il 'issue list -l "cool story"'`, "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), `Adding alias for.*il.*issue list -l "cool story"`) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, mainBuf.String(), `il: issue list -l "cool story"`) -} - -func TestAliasSet_arg_processing(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cases := []struct { - Cmd string - ExpectedOutputLine string - ExpectedConfigLine string - }{ - {`il "issue list"`, "- Adding alias for.*il.*issue list", "il: issue list"}, - - {`iz 'issue list'`, "- Adding alias for.*iz.*issue list", "iz: issue list"}, - - {`ii 'issue list --author="$1" --label="$2"'`, - `- Adding alias for.*ii.*issue list --author="\$1" --label="\$2"`, - `ii: issue list --author="\$1" --label="\$2"`}, - - {`ix "issue list --author='\$1' --label='\$2'"`, - `- Adding alias for.*ix.*issue list --author='\$1' --label='\$2'`, - `ix: issue list --author='\$1' --label='\$2'`}, - } - - for _, c := range cases { - t.Run(c.Cmd, func(t *testing.T) { - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, c.Cmd, "") - if err != nil { - t.Fatalf("got unexpected error running %s: %s", c.Cmd, err) + _, err = cmd.ExecuteC() + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + return } - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), c.ExpectedOutputLine) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, mainBuf.String(), c.ExpectedConfigLine) + assert.NoError(t, err) + assert.Equal(t, tt.output.Name, gotOpts.Name) + assert.Equal(t, tt.output.Expansion, gotOpts.Expansion) + assert.Equal(t, tt.output.IsShell, gotOpts.IsShell) + assert.Equal(t, tt.output.OverwriteExisting, gotOpts.OverwriteExisting) }) } } -func TestAliasSet_init_alias_cfg(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - editor: vim - `)) - - output, err := runCommand(cfg, true, "diff 'pr diff'", "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - expected := `editor: vim -aliases: - diff: pr diff -` - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*diff.*pr diff", "Added alias.") - assert.Equal(t, expected, mainBuf.String()) -} - -func TestAliasSet_existing_aliases(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(heredoc.Doc(` - aliases: - foo: bar - `)) - - output, err := runCommand(cfg, true, "view 'pr view'", "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - expected := `aliases: - foo: bar - view: pr view -` - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*view.*pr view", "Added alias.") - assert.Equal(t, expected, mainBuf.String()) - -} - -func TestAliasSet_invalid_command(t *testing.T) { - cfg := config.NewFromString(``) - - _, err := runCommand(cfg, true, "co 'pe checkout'", "") - assert.EqualError(t, err, "could not create alias: pe checkout does not correspond to a gh command, extension, or alias") -} - -func TestShellAlias_flag(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, "--shell igrep 'gh issue list | grep'", "") - if err != nil { - t.Fatalf("unexpected error: %s", err) +func TestSetRun(t *testing.T) { + tests := []struct { + name string + tty bool + opts *SetOptions + stdin string + wantExpansion string + wantStdout string + wantStderr string + wantErrMsg string + }{ + { + name: "creates alias tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "bar", + }, + wantExpansion: "bar", + wantStderr: "- Creating alias for foo: bar\n✓ Added alias foo\n", + }, + { + name: "creates alias", + opts: &SetOptions{ + Name: "foo", + Expansion: "bar", + }, + wantExpansion: "bar", + }, + { + name: "creates shell alias tty", + tty: true, + opts: &SetOptions{ + Name: "igrep", + Expansion: "!gh issue list | grep", + }, + wantExpansion: "!gh issue list | grep", + wantStderr: "- Creating alias for igrep: !gh issue list | grep\n✓ Added alias igrep\n", + }, + { + name: "creates shell alias", + opts: &SetOptions{ + Name: "igrep", + Expansion: "!gh issue list | grep", + }, + wantExpansion: "!gh issue list | grep", + }, + { + name: "creates shell alias using flag tty", + tty: true, + opts: &SetOptions{ + Name: "igrep", + Expansion: "gh issue list | grep", + IsShell: true, + }, + wantExpansion: "!gh issue list | grep", + wantStderr: "- Creating alias for igrep: !gh issue list | grep\n✓ Added alias igrep\n", + }, + { + name: "creates shell alias using flag", + opts: &SetOptions{ + Name: "igrep", + Expansion: "gh issue list | grep", + IsShell: true, + }, + wantExpansion: "!gh issue list | grep", + }, + { + name: "creates alias where expansion has args tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "bar baz --author='$1' --label='$2'", + }, + wantExpansion: "bar baz --author='$1' --label='$2'", + wantStderr: "- Creating alias for foo: bar baz --author='$1' --label='$2'\n✓ Added alias foo\n", + }, + { + name: "creates alias where expansion has args", + opts: &SetOptions{ + Name: "foo", + Expansion: "bar baz --author='$1' --label='$2'", + }, + wantExpansion: "bar baz --author='$1' --label='$2'", + }, + { + name: "creates alias from stdin tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "-", + }, + stdin: `bar baz --author="$1" --label="$2"`, + wantExpansion: `bar baz --author="$1" --label="$2"`, + wantStderr: "- Creating alias for foo: bar baz --author=\"$1\" --label=\"$2\"\n✓ Added alias foo\n", + }, + { + name: "creates alias from stdin", + opts: &SetOptions{ + Name: "foo", + Expansion: "-", + }, + stdin: `bar baz --author="$1" --label="$2"`, + wantExpansion: `bar baz --author="$1" --label="$2"`, + }, + { + name: "overwrites existing alias tty", + tty: true, + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + OverwriteExisting: true, + }, + wantExpansion: "bar", + wantStderr: "- Creating alias for co: bar\n! Changed alias co\n", + }, + { + name: "overwrites existing alias", + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + OverwriteExisting: true, + }, + wantExpansion: "bar", + }, + { + name: "fails when alias name is an existing alias tty", + tty: true, + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + }, + wantExpansion: "pr checkout", + wantErrMsg: "X Could not create alias co: name already taken, use the --clobber flag to overwrite it", + wantStderr: "- Creating alias for co: bar\n", + }, + { + name: "fails when alias name is an existing alias", + opts: &SetOptions{ + Name: "co", + Expansion: "bar", + }, + wantExpansion: "pr checkout", + wantErrMsg: "X Could not create alias co: name already taken, use the --clobber flag to overwrite it", + }, + { + name: "fails when alias expansion is not an existing command tty", + tty: true, + opts: &SetOptions{ + Name: "foo", + Expansion: "baz", + }, + wantErrMsg: "X Could not create alias foo: expansion does not correspond to a gh command, extension, or alias", + wantStderr: "- Creating alias for foo: baz\n", + }, + { + name: "fails when alias expansion is not an existing command", + opts: &SetOptions{ + Name: "foo", + Expansion: "baz", + }, + wantErrMsg: "X Could not create alias foo: expansion does not correspond to a gh command, extension, or alias", + }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rootCmd := &cobra.Command{} + barCmd := &cobra.Command{Use: "bar"} + barCmd.AddCommand(&cobra.Command{Use: "baz"}) + rootCmd.AddCommand(barCmd) + coCmd := &cobra.Command{Use: "co"} + rootCmd.AddCommand(coCmd) - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) + tt.opts.validAliasName = shared.ValidAliasNameFunc(rootCmd) + tt.opts.validAliasExpansion = shared.ValidAliasExpansionFunc(rootCmd) - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") + ios, stdin, stdout, stderr := iostreams.Test() + ios.SetStdinTTY(tt.tty) + ios.SetStdoutTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + tt.opts.IO = ios - expected := `aliases: - igrep: '!gh issue list | grep' -` - assert.Equal(t, expected, mainBuf.String()) + if tt.stdin != "" { + fmt.Fprint(stdin, tt.stdin) + } + + cfg := config.NewBlankConfig() + cfg.WriteFunc = func() error { + return nil + } + tt.opts.Config = func() (config.Config, error) { + return cfg, nil + } + + err := setRun(tt.opts) + if tt.wantErrMsg != "" { + assert.EqualError(t, err, tt.wantErrMsg) + writeCalls := cfg.WriteCalls() + assert.Equal(t, 0, len(writeCalls)) + } else { + assert.NoError(t, err) + writeCalls := cfg.WriteCalls() + assert.Equal(t, 1, len(writeCalls)) + } + + ac := cfg.Aliases() + expansion, _ := ac.Get(tt.opts.Name) + assert.Equal(t, tt.wantExpansion, expansion) + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } } -func TestShellAlias_bang(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, "igrep '!gh issue list | grep'", "") - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") - - expected := `aliases: - igrep: '!gh issue list | grep' -` - assert.Equal(t, expected, mainBuf.String()) -} - -func TestShellAlias_from_stdin(t *testing.T) { - readConfigs := config.StubWriteConfig(t) - - cfg := config.NewFromString(``) - - output, err := runCommand(cfg, true, "users -", `api graphql -F name="$1" -f query=' - query ($name: String!) { - user(login: $name) { - name - } - }'`) - - require.NoError(t, err) - - mainBuf := bytes.Buffer{} - readConfigs(&mainBuf, io.Discard) - - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Adding alias for.*users") - - expected := `aliases: - users: |- - api graphql -F name="$1" -f query=' - query ($name: String!) { - user(login: $name) { - name - } - }' -` - - assert.Equal(t, expected, mainBuf.String()) -} - -func TestShellAlias_getExpansion(t *testing.T) { +func TestGetExpansion(t *testing.T) { tests := []struct { name string want string