From bd2738379b63b1d4924562bc9fb156531336fa36 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 21 Apr 2021 22:16:41 -0700 Subject: [PATCH 1/2] Optionally read stdin for `gh alias set` Resolves #3487 --- pkg/cmd/alias/set/set.go | 39 +++++++++++-- pkg/cmd/alias/set/set_test.go | 106 +++++++++++++++++++++++++++++----- 2 files changed, 126 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index 1ec24eb35..143d467ec 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -2,6 +2,7 @@ package set import ( "fmt" + "io/ioutil" "strings" "github.com/MakeNowJust/heredoc" @@ -37,6 +38,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command The expansion may specify additional arguments and flags. If the expansion includes positional placeholders such as '$1', '$2', etc., any extra arguments that follow the invocation of an alias will be inserted appropriately. + Reads from STDIN if not specified. If '--shell' is specified, the alias will be run through a shell interpreter (sh). This allows you to compose commands with "|" or redirect with ">". Note that extra arguments following the alias @@ -66,13 +68,21 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command $ gh alias set --shell igrep 'gh issue list --label="$1" | grep $2' $ gh igrep epic foo #=> gh issue list --label="epic" | grep "foo" + + # users.txt contains multiline 'api graphql -F name="$1" ...' with mixed quotes + $ gh alias set users < users.txt + $ gh users octocat + #=> gh api graphql -F name="octocat" ... `), - Args: cobra.ExactArgs(2), + Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { opts.RootCmd = cmd.Root() opts.Name = args[0] - opts.Expansion = args[1] + + if len(args) > 1 { + opts.Expansion = args[1] + } if runF != nil { return runF(opts) @@ -98,12 +108,16 @@ func setRun(opts *SetOptions) error { return err } - isTerminal := opts.IO.IsStdoutTTY() - if isTerminal { - fmt.Fprintf(opts.IO.ErrOut, "- Adding alias for %s: %s\n", cs.Bold(opts.Name), cs.Bold(opts.Expansion)) + expansion, err := getExpansion(opts) + if err != nil { + 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)) } - expansion := opts.Expansion isShell := opts.IsShell if isShell && !strings.HasPrefix(expansion, "!") { expansion = "!" + expansion @@ -149,3 +163,16 @@ func validCommand(rootCmd *cobra.Command, expansion string) bool { cmd, _, err := rootCmd.Traverse(split) return err == nil && cmd != rootCmd } + +func getExpansion(opts *SetOptions) (string, error) { + if opts.Expansion == "" { + stdin, err := ioutil.ReadAll(opts.IO.In) + if err != nil { + return "", fmt.Errorf("failed to read from STDIN: %w", err) + } + + return string(stdin), nil + } + + return opts.Expansion, nil +} diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 1bfbdda20..0250deec2 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -16,11 +16,12 @@ import ( "github.com/stretchr/testify/require" ) -func runCommand(cfg config.Config, isTTY bool, cli string) (*test.CmdOut, error) { - io, _, stdout, stderr := iostreams.Test() +func runCommand(cfg config.Config, isTTY bool, cli string, in string) (*test.CmdOut, error) { + io, stdin, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) io.SetStdinTTY(isTTY) io.SetStderrTTY(isTTY) + stdin.WriteString(in) factory := &cmdutil.Factory{ IOStreams: io, @@ -41,6 +42,9 @@ func runCommand(cfg config.Config, isTTY bool, cli string) (*test.CmdOut, error) 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 { @@ -48,7 +52,7 @@ func runCommand(cfg config.Config, isTTY bool, cli string) (*test.CmdOut, error) } rootCmd.SetArgs(argv) - rootCmd.SetIn(&bytes.Buffer{}) + rootCmd.SetIn(stdin) rootCmd.SetOut(ioutil.Discard) rootCmd.SetErr(ioutil.Discard) @@ -64,7 +68,7 @@ func TestAliasSet_gh_command(t *testing.T) { cfg := config.NewFromString(``) - _, err := runCommand(cfg, true, "pr 'pr status'") + _, err := runCommand(cfg, true, "pr 'pr status'", "") assert.EqualError(t, err, `could not create alias: "pr" is already a gh command`) } @@ -77,7 +81,7 @@ func TestAliasSet_empty_aliases(t *testing.T) { editor: vim `)) - output, err := runCommand(cfg, true, "co 'pr checkout'") + output, err := runCommand(cfg, true, "co 'pr checkout'", "") if err != nil { t.Fatalf("unexpected error: %s", err) @@ -104,7 +108,7 @@ func TestAliasSet_existing_alias(t *testing.T) { co: pr checkout `)) - output, err := runCommand(cfg, true, "co 'pr checkout -Rcool/repo'") + output, err := runCommand(cfg, true, "co 'pr checkout -Rcool/repo'", "") require.NoError(t, err) //nolint:staticcheck // prefer exact matchers over ExpectLines @@ -117,7 +121,7 @@ func TestAliasSet_space_args(t *testing.T) { cfg := config.NewFromString(``) - output, err := runCommand(cfg, true, `il 'issue list -l "cool story"'`) + output, err := runCommand(cfg, true, `il 'issue list -l "cool story"'`, "") require.NoError(t, err) //nolint:staticcheck // prefer exact matchers over ExpectLines @@ -153,7 +157,7 @@ func TestAliasSet_arg_processing(t *testing.T) { cfg := config.NewFromString(``) - output, err := runCommand(cfg, true, c.Cmd) + output, err := runCommand(cfg, true, c.Cmd, "") if err != nil { t.Fatalf("got unexpected error running %s: %s", c.Cmd, err) } @@ -174,7 +178,7 @@ func TestAliasSet_init_alias_cfg(t *testing.T) { editor: vim `)) - output, err := runCommand(cfg, true, "diff 'pr diff'") + output, err := runCommand(cfg, true, "diff 'pr diff'", "") require.NoError(t, err) expected := `editor: vim @@ -196,7 +200,7 @@ func TestAliasSet_existing_aliases(t *testing.T) { foo: bar `)) - output, err := runCommand(cfg, true, "view 'pr view'") + output, err := runCommand(cfg, true, "view 'pr view'", "") require.NoError(t, err) expected := `aliases: @@ -215,7 +219,7 @@ func TestAliasSet_invalid_command(t *testing.T) { cfg := config.NewFromString(``) - _, err := runCommand(cfg, true, "co 'pe checkout'") + _, err := runCommand(cfg, true, "co 'pe checkout'", "") assert.EqualError(t, err, "could not create alias: pe checkout does not correspond to a gh command") } @@ -225,7 +229,7 @@ func TestShellAlias_flag(t *testing.T) { cfg := config.NewFromString(``) - output, err := runCommand(cfg, true, "--shell igrep 'gh issue list | grep'") + output, err := runCommand(cfg, true, "--shell igrep 'gh issue list | grep'", "") if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -245,7 +249,7 @@ func TestShellAlias_bang(t *testing.T) { cfg := config.NewFromString(``) - output, err := runCommand(cfg, true, "igrep '!gh issue list | grep'") + output, err := runCommand(cfg, true, "igrep '!gh issue list | grep'", "") require.NoError(t, err) //nolint:staticcheck // prefer exact matchers over ExpectLines @@ -256,3 +260,79 @@ func TestShellAlias_bang(t *testing.T) { ` assert.Equal(t, expected, mainBuf.String()) } + +func TestShellAlias_from_stdin(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + 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) + + //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) { + tests := []struct { + name string + want string + expansionArg string + stdin string + }{ + { + name: "co", + want: "pr checkout", + expansionArg: "pr checkout", + }, + { + name: "co", + want: "pr checkout", + expansionArg: "pr checkout", + stdin: "api graphql -F name=\"$1\"", + }, + { + name: "stdin", + want: "api graphql -F name=\"$1\"", + stdin: "api graphql -F name=\"$1\"", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, stdin, _, _ := iostreams.Test() + + io.SetStdinTTY(false) + + _, err := stdin.WriteString(tt.stdin) + assert.NoError(t, err) + + expansion, err := getExpansion(&SetOptions{ + Expansion: tt.expansionArg, + IO: io, + }) + assert.NoError(t, err) + + assert.Equal(t, expansion, tt.want) + }) + } +} From aaa5a9e9495f71d75f7fc86c94f49ef7c71cb6c9 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 23 Apr 2021 17:23:27 -0700 Subject: [PATCH 2/2] Use `-` to read from stdin instead Resolves PR feedback. --- pkg/cmd/alias/set/set.go | 17 ++++++++--------- pkg/cmd/alias/set/set_test.go | 9 +++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index 143d467ec..e7c4ea6d5 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -38,7 +38,8 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command The expansion may specify additional arguments and flags. If the expansion includes positional placeholders such as '$1', '$2', etc., any extra arguments that follow the invocation of an alias will be inserted appropriately. - Reads from STDIN if not specified. + Reads from STDIN if '-' is specified as the expansion parameter. This can be useful + for commands with mixed quotes or multiple lines. If '--shell' is specified, the alias will be run through a shell interpreter (sh). This allows you to compose commands with "|" or redirect with ">". Note that extra arguments following the alias @@ -48,7 +49,8 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command Platform note: on Windows, shell aliases are executed via "sh" as installed by Git For Windows. If you have installed git on Windows in some other way, shell aliases may not work for you. - Quotes must always be used when defining a command as in the examples. + Quotes must always be used when defining a command as in the examples unless you pass '-' + as the expansion parameter and pipe your command to 'gh alias set'. `), Example: heredoc.Doc(` $ gh alias set pv 'pr view' @@ -70,19 +72,16 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command #=> gh issue list --label="epic" | grep "foo" # users.txt contains multiline 'api graphql -F name="$1" ...' with mixed quotes - $ gh alias set users < users.txt + $ gh alias set users - < users.txt $ gh users octocat #=> gh api graphql -F name="octocat" ... `), - Args: cobra.RangeArgs(1, 2), + Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { opts.RootCmd = cmd.Root() opts.Name = args[0] - - if len(args) > 1 { - opts.Expansion = args[1] - } + opts.Expansion = args[1] if runF != nil { return runF(opts) @@ -165,7 +164,7 @@ func validCommand(rootCmd *cobra.Command, expansion string) bool { } func getExpansion(opts *SetOptions) (string, error) { - if opts.Expansion == "" { + if opts.Expansion == "-" { stdin, err := ioutil.ReadAll(opts.IO.In) if err != nil { return "", fmt.Errorf("failed to read from STDIN: %w", err) diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 0250deec2..0e95a3c46 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -267,7 +267,7 @@ func TestShellAlias_from_stdin(t *testing.T) { cfg := config.NewFromString(``) - output, err := runCommand(cfg, true, "users", `api graphql -F name="$1" -f query=' + output, err := runCommand(cfg, true, "users -", `api graphql -F name="$1" -f query=' query ($name: String!) { user(login: $name) { name @@ -311,9 +311,10 @@ func TestShellAlias_getExpansion(t *testing.T) { stdin: "api graphql -F name=\"$1\"", }, { - name: "stdin", - want: "api graphql -F name=\"$1\"", - stdin: "api graphql -F name=\"$1\"", + name: "stdin", + expansionArg: "-", + want: "api graphql -F name=\"$1\"", + stdin: "api graphql -F name=\"$1\"", }, }