From 69c02b9b8a44ebd8f9a28ee9a2063722c68b23c6 Mon Sep 17 00:00:00 2001 From: rajhawaldar Date: Tue, 6 Jun 2023 23:30:16 +0530 Subject: [PATCH 01/12] update gh auth status to write to stdout on success --- pkg/cmd/auth/status/status.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 8070fe9ae..51d7b2c7d 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -66,7 +66,7 @@ func statusRun(opts *StatusOptions) error { // TODO check tty stderr := opts.IO.ErrOut - + stdout := opts.IO.Out cs := opts.IO.ColorScheme() statusInfo := map[string][]string{} @@ -166,13 +166,22 @@ func statusRun(opts *StatusOptions) error { if !ok { continue } - if prevEntry { + if prevEntry && failed { fmt.Fprint(stderr, "\n") + } else if prevEntry && failed == false { + fmt.Fprint(stdout, "\n") } prevEntry = true - fmt.Fprintf(stderr, "%s\n", cs.Bold(hostname)) - for _, line := range lines { - fmt.Fprintf(stderr, " %s\n", line) + if failed { + fmt.Fprintf(stderr, "%s\n", cs.Bold(hostname)) + for _, line := range lines { + fmt.Fprintf(stderr, " %s\n", line) + } + } else { + fmt.Fprintf(stdout, "%s\n", cs.Bold(hostname)) + for _, line := range lines { + fmt.Fprintf(stdout, " %s\n", line) + } } } From 437b78a9552f61e8100bad2ec932ecf4ef012715 Mon Sep 17 00:00:00 2001 From: rajhawaldar Date: Wed, 7 Jun 2023 08:23:59 +0530 Subject: [PATCH 02/12] Resolve tests/lint errors --- pkg/cmd/auth/status/status.go | 2 +- pkg/cmd/auth/status/status_test.go | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 51d7b2c7d..77a5f125c 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -168,7 +168,7 @@ func statusRun(opts *StatusOptions) error { } if prevEntry && failed { fmt.Fprint(stderr, "\n") - } else if prevEntry && failed == false { + } else if prevEntry && !failed { fmt.Fprint(stdout, "\n") } prevEntry = true diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index e169d1b3f..16affe624 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -310,13 +310,12 @@ func Test_statusRun(t *testing.T) { tt.opts = &StatusOptions{} } - ios, _, _, stderr := iostreams.Test() + ios, _, stdout, stderr := iostreams.Test() ios.SetStdinTTY(true) ios.SetStderrTTY(true) ios.SetStdoutTTY(true) tt.opts.IO = ios - cfg := config.NewFromString("") if tt.cfgStubs != nil { tt.cfgStubs(cfg) @@ -340,9 +339,13 @@ func Test_statusRun(t *testing.T) { } else { assert.NoError(t, err) } - - output := strings.ReplaceAll(stderr.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") - assert.Equal(t, tt.wantOut, output) + output := strings.ReplaceAll(stdout.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") + errorOutput := strings.ReplaceAll(stderr.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") + if output != "" { + assert.Equal(t, tt.wantOut, output) + } else { + assert.Equal(t, tt.wantOut, errorOutput) + } mainBuf := bytes.Buffer{} hostsBuf := bytes.Buffer{} From b8302311fa3f49fefe036266b93710e20650bbd4 Mon Sep 17 00:00:00 2001 From: rajhawaldar Date: Thu, 15 Jun 2023 09:01:16 +0530 Subject: [PATCH 03/12] Added wantErrOut property --- pkg/cmd/auth/status/status_test.go | 31 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 16affe624..e18604202 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -76,12 +76,13 @@ func Test_statusRun(t *testing.T) { readConfigs := config.StubWriteConfig(t) tests := []struct { - name string - opts *StatusOptions - httpStubs func(*httpmock.Registry) - cfgStubs func(*config.ConfigMock) - wantErr string - wantOut string + name string + opts *StatusOptions + httpStubs func(*httpmock.Registry) + cfgStubs func(*config.ConfigMock) + wantErr string + wantOut string + wantErrOut string }{ { name: "hostname set", @@ -126,7 +127,7 @@ func Test_statusRun(t *testing.T) { httpmock.StringResponse(`{"data":{"viewer":{"login":"tess"}}}`)) }, wantErr: "SilentError", - wantOut: heredoc.Doc(` + wantErrOut: heredoc.Doc(` joel.miller X joel.miller: the token in GH_CONFIG_DIR/hosts.yml is missing required scope 'read:org' - To request missing scopes, run: gh auth refresh -h joel.miller @@ -156,7 +157,7 @@ func Test_statusRun(t *testing.T) { httpmock.StringResponse(`{"data":{"viewer":{"login":"tess"}}}`)) }, wantErr: "SilentError", - wantOut: heredoc.Doc(` + wantErrOut: heredoc.Doc(` joel.miller X joel.miller: authentication failed - The joel.miller token in GH_CONFIG_DIR/hosts.yml is no longer valid. @@ -298,9 +299,9 @@ func Test_statusRun(t *testing.T) { cfgStubs: func(c *config.ConfigMock) { c.Set("github.com", "oauth_token", "abc123") }, - httpStubs: func(reg *httpmock.Registry) {}, - wantErr: "SilentError", - wantOut: "Hostname \"github.example.com\" not found among authenticated GitHub hosts\n", + httpStubs: func(reg *httpmock.Registry) {}, + wantErr: "SilentError", + wantErrOut: "Hostname \"github.example.com\" not found among authenticated GitHub hosts\n", }, } @@ -341,11 +342,9 @@ func Test_statusRun(t *testing.T) { } output := strings.ReplaceAll(stdout.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") errorOutput := strings.ReplaceAll(stderr.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") - if output != "" { - assert.Equal(t, tt.wantOut, output) - } else { - assert.Equal(t, tt.wantOut, errorOutput) - } + + assert.Equal(t, tt.wantErrOut, errorOutput) + assert.Equal(t, tt.wantOut, output) mainBuf := bytes.Buffer{} hostsBuf := bytes.Buffer{} From 9be9dc22e9b0d24cd45ef9b27e582ee673ff51c7 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 20 Jun 2023 08:41:02 +0900 Subject: [PATCH 04/12] Fix error handling for extension and shell alias commands (#7567) --- cmd/gh/main.go | 7 ++++--- pkg/cmd/root/alias.go | 4 ++++ pkg/cmd/root/extension.go | 10 ++++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index c36a4e6e6..15af90721 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -109,7 +109,7 @@ func mainRun() exitCode { if cmd, err := rootCmd.ExecuteContextC(ctx); err != nil { var pagerPipeError *iostreams.ErrClosedPagerPipe var noResultsError cmdutil.NoResultsError - var execError *exec.ExitError + var extError *root.ExternalCommandExitError var authError *root.AuthError if err == cmdutil.SilentError { return exitError @@ -130,8 +130,9 @@ func mainRun() exitCode { } // no results is not a command failure return exitOK - } else if errors.As(err, &execError) { - return exitCode(execError.ExitCode()) + } else if errors.As(err, &extError) { + // pass on exit codes from extensions and shell aliases + return exitCode(extError.ExitCode()) } printError(stderr, err, cmd, hasDebug) diff --git a/pkg/cmd/root/alias.go b/pkg/cmd/root/alias.go index ba0bfe9e2..4f504f2b8 100644 --- a/pkg/cmd/root/alias.go +++ b/pkg/cmd/root/alias.go @@ -31,6 +31,10 @@ func NewCmdShellAlias(io *iostreams.IOStreams, aliasName, aliasValue string) *co externalCmd.Stdin = io.In preparedCmd := run.PrepareCmd(externalCmd) if err = preparedCmd.Run(); err != nil { + var execError *exec.ExitError + if errors.As(err, &execError) { + return &ExternalCommandExitError{execError} + } return fmt.Errorf("failed to run external command: %w\n", err) } return nil diff --git a/pkg/cmd/root/extension.go b/pkg/cmd/root/extension.go index dea637619..d6d495103 100644 --- a/pkg/cmd/root/extension.go +++ b/pkg/cmd/root/extension.go @@ -1,13 +1,19 @@ package root import ( + "errors" "fmt" + "os/exec" "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" ) +type ExternalCommandExitError struct { + *exec.ExitError +} + func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension) *cobra.Command { return &cobra.Command{ Use: ext.Name(), @@ -15,6 +21,10 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex RunE: func(c *cobra.Command, args []string) error { args = append([]string{ext.Name()}, args...) if _, err := em.Dispatch(args, io.In, io.Out, io.ErrOut); err != nil { + var execError *exec.ExitError + if errors.As(err, &execError) { + return &ExternalCommandExitError{execError} + } return fmt.Errorf("failed to run extension: %w\n", err) } return nil From 78839dbe0b25f22490468d68ab13d363e3dbfa36 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 20 Jun 2023 09:21:31 +0900 Subject: [PATCH 05/12] Fix alias import clobber flag (#7569) --- pkg/cmd/alias/imports/import.go | 60 +++++++++++++++------------- pkg/cmd/alias/imports/import_test.go | 33 ++++++++++----- 2 files changed, 55 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/alias/imports/import.go b/pkg/cmd/alias/imports/import.go index f7de0bb78..ef2176d45 100644 --- a/pkg/cmd/alias/imports/import.go +++ b/pkg/cmd/alias/imports/import.go @@ -122,15 +122,31 @@ func importRun(opts *ImportOptions) error { var msg strings.Builder for _, alias := range getSortedKeys(aliasMap) { - if !opts.validAliasName(alias) { - msg.WriteString( - fmt.Sprintf("%s Could not import alias %s: already a gh command, extension, or alias\n", - cs.FailureIcon(), - cs.Bold(alias), - ), - ) + var existingAlias bool + if _, err := aliasCfg.Get(alias); err == nil { + existingAlias = true + } - continue + if !opts.validAliasName(alias) { + if !existingAlias { + msg.WriteString( + fmt.Sprintf("%s Could not import alias %s: already a gh command or extension\n", + cs.FailureIcon(), + cs.Bold(alias), + ), + ) + continue + } + + if existingAlias && !opts.OverwriteExisting { + msg.WriteString( + fmt.Sprintf("%s Could not import alias %s: name already taken\n", + cs.FailureIcon(), + cs.Bold(alias), + ), + ) + continue + } } expansion := aliasMap[alias] @@ -142,31 +158,19 @@ func importRun(opts *ImportOptions) error { cs.Bold(alias), ), ) - continue } - if _, err := aliasCfg.Get(alias); err == nil { - if opts.OverwriteExisting { - aliasCfg.Add(alias, expansion) + aliasCfg.Add(alias, expansion) - msg.WriteString( - fmt.Sprintf("%s Changed alias %s\n", - cs.WarningIcon(), - cs.Bold(alias), - ), - ) - } else { - msg.WriteString( - fmt.Sprintf("%s Could not import alias %s: name already taken\n", - cs.FailureIcon(), - cs.Bold(alias), - ), - ) - } + if existingAlias && opts.OverwriteExisting { + msg.WriteString( + fmt.Sprintf("%s Changed alias %s\n", + cs.WarningIcon(), + cs.Bold(alias), + ), + ) } else { - aliasCfg.Add(alias, expansion) - msg.WriteString( fmt.Sprintf("%s Added alias %s\n", cs.SuccessIcon(), diff --git a/pkg/cmd/alias/imports/import_test.go b/pkg/cmd/alias/imports/import_test.go index 0013429f6..024f52f02 100644 --- a/pkg/cmd/alias/imports/import_test.go +++ b/pkg/cmd/alias/imports/import_test.go @@ -112,13 +112,14 @@ func TestImportRun(t *testing.T) { importStdinMsg := "- Importing aliases from standard input" tests := []struct { - name string - opts *ImportOptions - stdin string - fileContents string - initConfig string - wantConfig string - wantStderr string + name string + opts *ImportOptions + stdin string + fileContents string + initConfig string + aliasCommands []*cobra.Command + wantConfig string + wantStderr string }{ { name: "with no existing aliases", @@ -158,6 +159,9 @@ func TestImportRun(t *testing.T) { igrep: '!gh issue list --label="$1" | grep "$2"' editor: vim `), + aliasCommands: []*cobra.Command{ + {Use: "igrep"}, + }, wantConfig: heredoc.Doc(` aliases: igrep: '!gh issue list --label="$1" | grep "$2"' @@ -211,6 +215,9 @@ func TestImportRun(t *testing.T) { co: pr checkout editor: vim `), + aliasCommands: []*cobra.Command{ + {Use: "co"}, + }, wantConfig: heredoc.Doc(` aliases: co: pr checkout @@ -234,6 +241,9 @@ func TestImportRun(t *testing.T) { co: pr checkout editor: vim `), + aliasCommands: []*cobra.Command{ + {Use: "co"}, + }, wantConfig: heredoc.Doc(` aliases: co: pr checkout -R cool/repo @@ -256,9 +266,9 @@ func TestImportRun(t *testing.T) { wantStderr: strings.Join( []string{ importFileMsg, - "X Could not import alias api: already a gh command, extension, or alias", - "X Could not import alias issue: already a gh command, extension, or alias", - "X Could not import alias pr: already a gh command, extension, or alias\n\n", + "X Could not import alias api: already a gh command or extension", + "X Could not import alias issue: already a gh command or extension", + "X Could not import alias pr: already a gh command or extension\n\n", }, "\n", ), @@ -315,6 +325,9 @@ func TestImportRun(t *testing.T) { apiCmd := &cobra.Command{Use: "api"} apiCmd.AddCommand(&cobra.Command{Use: "graphql"}) rootCmd.AddCommand(apiCmd) + for _, cmd := range tt.aliasCommands { + rootCmd.AddCommand(cmd) + } tt.opts.validAliasName = shared.ValidAliasNameFunc(rootCmd) tt.opts.validAliasExpansion = shared.ValidAliasExpansionFunc(rootCmd) From 08114f981ec27adf47d73a79f5c3d8679e827323 Mon Sep 17 00:00:00 2001 From: Naoya Yasuda <43776161+yanskun@users.noreply.github.com> Date: Tue, 20 Jun 2023 09:27:08 +0900 Subject: [PATCH 06/12] refactor: nest if (#7596) --- pkg/cmd/browse/browse.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 42664d77c..e4240a5b5 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -163,14 +163,12 @@ func runBrowse(opts *BrowseOptions) error { return fmt.Errorf("unable to determine base repository: %w", err) } - if opts.Commit != "" { - if opts.Commit == emptyCommitFlag { - commit, err := opts.GitClient.LastCommit() - if err != nil { - return err - } - opts.Commit = commit.Sha + if opts.Commit != "" && opts.Commit == emptyCommitFlag { + commit, err := opts.GitClient.LastCommit() + if err != nil { + return err } + opts.Commit = commit.Sha } section, err := parseSection(baseRepo, opts) From 1b497221bc987aaec60fbc32a1249c37c6d11791 Mon Sep 17 00:00:00 2001 From: Ariel Deitcher <1149246+mntlty@users.noreply.github.com> Date: Mon, 19 Jun 2023 18:09:26 -0700 Subject: [PATCH 07/12] output URL on project commands (#7578) * output URL on project commands * do not put useless URL in output --- pkg/cmd/project/close/close.go | 9 ++------- pkg/cmd/project/close/close_test.go | 4 ++-- pkg/cmd/project/copy/copy.go | 2 +- pkg/cmd/project/copy/copy_test.go | 2 +- pkg/cmd/project/create/create.go | 2 +- pkg/cmd/project/create/create_test.go | 6 +++--- pkg/cmd/project/delete/delete.go | 6 +++--- pkg/cmd/project/delete/delete_test.go | 15 +++++++++------ pkg/cmd/project/edit/edit.go | 2 +- pkg/cmd/project/edit/edit_test.go | 8 ++++---- 10 files changed, 27 insertions(+), 29 deletions(-) diff --git a/pkg/cmd/project/close/close.go b/pkg/cmd/project/close/close.go index 7f8006b6f..f4016fe50 100644 --- a/pkg/cmd/project/close/close.go +++ b/pkg/cmd/project/close/close.go @@ -128,13 +128,8 @@ func printResults(config closeConfig, project queries.Project) error { if !config.io.IsStdoutTTY() { return nil } - var action string - if config.opts.reopen { - action = "Reopened" - } else { - action = "Closed" - } - _, err := fmt.Fprintf(config.io.Out, "%s project %s\n", action, project.URL) + + _, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL) return err } diff --git a/pkg/cmd/project/close/close_test.go b/pkg/cmd/project/close/close_test.go index 3db3c3c68..a19dcdabd 100644 --- a/pkg/cmd/project/close/close_test.go +++ b/pkg/cmd/project/close/close_test.go @@ -183,7 +183,7 @@ func TestRunClose_User(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Closed project http://a-url.com\n", + "http://a-url.com\n", stdout.String()) } @@ -452,6 +452,6 @@ func TestRunClose_Reopen(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Reopened project http://a-url.com\n", + "http://a-url.com\n", stdout.String()) } diff --git a/pkg/cmd/project/copy/copy.go b/pkg/cmd/project/copy/copy.go index 6dae73e40..d3e07424b 100644 --- a/pkg/cmd/project/copy/copy.go +++ b/pkg/cmd/project/copy/copy.go @@ -139,7 +139,7 @@ func printResults(config copyConfig, project queries.Project) error { if !config.io.IsStdoutTTY() { return nil } - _, err := fmt.Fprintf(config.io.Out, "Copied project to %s\n", project.URL) + _, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL) return err } diff --git a/pkg/cmd/project/copy/copy_test.go b/pkg/cmd/project/copy/copy_test.go index d731adaff..d8abf5ec9 100644 --- a/pkg/cmd/project/copy/copy_test.go +++ b/pkg/cmd/project/copy/copy_test.go @@ -457,6 +457,6 @@ func TestRunCopy_Me(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Copied project to http://a-url.com\n", + "http://a-url.com\n", stdout.String()) } diff --git a/pkg/cmd/project/create/create.go b/pkg/cmd/project/create/create.go index 0ecd4f5f3..894a6e9ce 100644 --- a/pkg/cmd/project/create/create.go +++ b/pkg/cmd/project/create/create.go @@ -110,7 +110,7 @@ func printResults(config createConfig, project queries.Project) error { return nil } - _, err := fmt.Fprintf(config.io.Out, "Created project '%s'\n%s\n", project.Title, project.URL) + _, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL) return err } diff --git a/pkg/cmd/project/create/create_test.go b/pkg/cmd/project/create/create_test.go index 0a6cc9700..3ef773cb9 100644 --- a/pkg/cmd/project/create/create_test.go +++ b/pkg/cmd/project/create/create_test.go @@ -146,7 +146,7 @@ func TestRunCreate_User(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Created project 'a title'\nhttp://a-url.com\n", + "http://a-url.com\n", stdout.String()) } @@ -214,7 +214,7 @@ func TestRunCreate_Org(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Created project 'a title'\nhttp://a-url.com\n", + "http://a-url.com\n", stdout.String()) } @@ -273,6 +273,6 @@ func TestRunCreate_Me(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Created project 'a title'\nhttp://a-url.com\n", + "http://a-url.com\n", stdout.String()) } diff --git a/pkg/cmd/project/delete/delete.go b/pkg/cmd/project/delete/delete.go index 0ae93277b..674c30a97 100644 --- a/pkg/cmd/project/delete/delete.go +++ b/pkg/cmd/project/delete/delete.go @@ -100,7 +100,7 @@ func runDelete(config deleteConfig) error { return printJSON(config, *project) } - return printResults(config) + return printResults(config, query.DeleteProject.Project) } @@ -116,12 +116,12 @@ func deleteItemArgs(config deleteConfig) (*deleteProjectMutation, map[string]int } } -func printResults(config deleteConfig) error { +func printResults(config deleteConfig, project queries.Project) error { if !config.io.IsStdoutTTY() { return nil } - _, err := fmt.Fprintf(config.io.Out, "Deleted project\n") + _, err := fmt.Fprintf(config.io.Out, "Deleted project %d\n", project.Number) return err } diff --git a/pkg/cmd/project/delete/delete_test.go b/pkg/cmd/project/delete/delete_test.go index 28f30ebc6..a27180266 100644 --- a/pkg/cmd/project/delete/delete_test.go +++ b/pkg/cmd/project/delete/delete_test.go @@ -148,7 +148,8 @@ func TestRunDelete_User(t *testing.T) { "data": map[string]interface{}{ "deleteProjectV2": map[string]interface{}{ "projectV2": map[string]interface{}{ - "id": "project ID", + "id": "project ID", + "number": 1, }, }, }, @@ -171,7 +172,7 @@ func TestRunDelete_User(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Deleted project\n", + "Deleted project 1\n", stdout.String()) } @@ -239,7 +240,8 @@ func TestRunDelete_Org(t *testing.T) { "data": map[string]interface{}{ "deleteProjectV2": map[string]interface{}{ "projectV2": map[string]interface{}{ - "id": "project ID", + "id": "project ID", + "number": 1, }, }, }, @@ -262,7 +264,7 @@ func TestRunDelete_Org(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Deleted project\n", + "Deleted project 1\n", stdout.String()) } @@ -320,7 +322,8 @@ func TestRunDelete_Me(t *testing.T) { "data": map[string]interface{}{ "deleteProjectV2": map[string]interface{}{ "projectV2": map[string]interface{}{ - "id": "project ID", + "id": "project ID", + "number": 1, }, }, }, @@ -343,6 +346,6 @@ func TestRunDelete_Me(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Deleted project\n", + "Deleted project 1\n", stdout.String()) } diff --git a/pkg/cmd/project/edit/edit.go b/pkg/cmd/project/edit/edit.go index 64ee74d78..92fee3284 100644 --- a/pkg/cmd/project/edit/edit.go +++ b/pkg/cmd/project/edit/edit.go @@ -150,7 +150,7 @@ func printResults(config editConfig, project queries.Project) error { return nil } - _, err := fmt.Fprintf(config.io.Out, "Updated project %s\n", project.URL) + _, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL) return err } diff --git a/pkg/cmd/project/edit/edit_test.go b/pkg/cmd/project/edit/edit_test.go index 45aff1390..605c5f5cc 100644 --- a/pkg/cmd/project/edit/edit_test.go +++ b/pkg/cmd/project/edit/edit_test.go @@ -226,7 +226,7 @@ func TestRunUpdate_User(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Updated project http://a-url.com\n", + "http://a-url.com\n", stdout.String()) } @@ -324,7 +324,7 @@ func TestRunUpdate_Org(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Updated project http://a-url.com\n", + "http://a-url.com\n", stdout.String()) } @@ -412,7 +412,7 @@ func TestRunUpdate_Me(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Updated project http://a-url.com\n", + "http://a-url.com\n", stdout.String()) } @@ -507,6 +507,6 @@ func TestRunUpdate_OmitParams(t *testing.T) { assert.NoError(t, err) assert.Equal( t, - "Updated project http://a-url.com\n", + "http://a-url.com\n", stdout.String()) } From 31ffa6b4999b7a16beb7731562b6c9aa8c01d5bb Mon Sep 17 00:00:00 2001 From: ffalor <35144141+ffalor@users.noreply.github.com> Date: Mon, 19 Jun 2023 21:30:03 -0500 Subject: [PATCH 08/12] Alphabetically sort labels for `pr/issue view` (#7587) --- .../view/fixtures/issueView_previewWithMetadata.json | 10 +++++----- pkg/cmd/issue/view/view.go | 6 ++++++ pkg/cmd/issue/view/view_test.go | 4 ++-- .../fixtures/prViewPreviewWithMetadataByNumber.json | 10 +++++----- pkg/cmd/pr/view/view.go | 5 +++++ pkg/cmd/pr/view/view_test.go | 4 ++-- 6 files changed, 25 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json b/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json index 9bce5f745..186fa1bab 100644 --- a/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json +++ b/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json @@ -25,19 +25,19 @@ "labels": { "nodes": [ { - "name": "one" + "name": "Closed: Won't Fix" }, { - "name": "two" + "name": "Status: In Progress" }, { - "name": "three" + "name": "Type: Bug" }, { - "name": "four" + "name": "help wanted" }, { - "name": "five" + "name": "Closed: Duplicate" } ], "totalCount": 5 diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index b721451fa..83132291f 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "sort" "strings" "time" @@ -306,6 +307,11 @@ func issueLabelList(issue *api.Issue, cs *iostreams.ColorScheme) string { return "" } + // ignore case sort + sort.SliceStable(issue.Labels.Nodes, func(i, j int) bool { + return strings.ToLower(issue.Labels.Nodes[i].Name) < strings.ToLower(issue.Labels.Nodes[j].Name) + }) + labelNames := make([]string, len(issue.Labels.Nodes)) for i, label := range issue.Labels.Nodes { if cs == nil { diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index ff26f4bc3..2c4e7934e 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -125,7 +125,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { `author:\tmarseilles`, `state:\tOPEN`, `comments:\t9`, - `labels:\tone, two, three, four, five`, + `labels:\tClosed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug`, `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `milestone:\tuluru\n`, `number:\t123\n`, @@ -196,7 +196,7 @@ func TestIssueView_tty_Preview(t *testing.T) { `Open.*marseilles opened about 9 years ago.*9 comments`, `8 \x{1f615} • 7 \x{1f440} • 6 \x{2764}\x{fe0f} • 5 \x{1f389} • 4 \x{1f604} • 3 \x{1f680} • 2 \x{1f44e} • 1 \x{1f44d}`, `Assignees:.*marseilles, monaco\n`, - `Labels:.*one, two, three, four, five\n`, + `Labels:.*Closed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`, `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone:.*uluru\n`, `bold story`, diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json index 00f768f17..be4a7713d 100644 --- a/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json @@ -37,19 +37,19 @@ "labels": { "nodes": [ { - "name": "one" + "name": "Closed: Won't Fix" }, { - "name": "two" + "name": "Status: In Progress" }, { - "name": "three" + "name": "Type: Bug" }, { - "name": "four" + "name": "help wanted" }, { - "name": "five" + "name": "Closed: Duplicate" } ], "totalcount": 5 diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 71bda0649..4a69de2f4 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -415,6 +415,11 @@ func prLabelList(pr api.PullRequest, cs *iostreams.ColorScheme) string { return "" } + // ignore case sort + sort.SliceStable(pr.Labels.Nodes, func(i, j int) bool { + return strings.ToLower(pr.Labels.Nodes[i].Name) < strings.ToLower(pr.Labels.Nodes[j].Name) + }) + labelNames := make([]string, 0, len(pr.Labels.Nodes)) for _, label := range pr.Labels.Nodes { labelNames = append(labelNames, cs.HexToRGB(label.Color, label.Name)) diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 91b77cac8..5e2c4b922 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -229,7 +229,7 @@ func TestPRView_Preview_nontty(t *testing.T) { `title:\tBlueberries are from a fork\n`, `reviewers:\t1 \(Requested\)\n`, `assignees:\tmarseilles, monaco\n`, - `labels:\tone, two, three, four, five\n`, + `labels:\tClosed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`, `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `milestone:\tuluru\n`, `\*\*blueberries taste good\*\*`, @@ -390,7 +390,7 @@ func TestPRView_Preview(t *testing.T) { `.+100.-10`, `Reviewers:.*1 \(.*Requested.*\)\n`, `Assignees:.*marseilles, monaco\n`, - `Labels:.*one, two, three, four, five\n`, + `Labels:.*Closed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`, `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone:.*uluru\n`, `blueberries taste good`, From bf7db84ca8b795a38ee47b5e54a8109a917a55bf Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 21 Jun 2023 08:45:20 +0900 Subject: [PATCH 09/12] Add timeouts to keyring operations (#7580) --- internal/config/config.go | 2 +- internal/keyring/keyring.go | 68 +++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 internal/keyring/keyring.go diff --git a/internal/config/config.go b/internal/config/config.go index 02756dad5..bf14a1aa0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,9 +4,9 @@ import ( "os" "path/filepath" + "github.com/cli/cli/v2/internal/keyring" ghAuth "github.com/cli/go-gh/v2/pkg/auth" ghConfig "github.com/cli/go-gh/v2/pkg/config" - "github.com/zalando/go-keyring" ) const ( diff --git a/internal/keyring/keyring.go b/internal/keyring/keyring.go new file mode 100644 index 000000000..afb5025e6 --- /dev/null +++ b/internal/keyring/keyring.go @@ -0,0 +1,68 @@ +// Package keyring is a simple wrapper that adds timeouts to the zalando/go-keyring package. +package keyring + +import ( + "time" + + "github.com/zalando/go-keyring" +) + +type TimeoutError struct { + message string +} + +func (e *TimeoutError) Error() string { + return e.message +} + +// Set secret in keyring for user. +func Set(service, user, secret string) error { + ch := make(chan error, 1) + go func() { + defer close(ch) + ch <- keyring.Set(service, user, secret) + }() + select { + case err := <-ch: + return err + case <-time.After(3 * time.Second): + return &TimeoutError{"timeout while trying to set secret in keyring"} + } +} + +// Get secret from keyring given service and user name. +func Get(service, user string) (string, error) { + ch := make(chan struct { + val string + err error + }, 1) + go func() { + defer close(ch) + val, err := keyring.Get(service, user) + ch <- struct { + val string + err error + }{val, err} + }() + select { + case res := <-ch: + return res.val, res.err + case <-time.After(3 * time.Second): + return "", &TimeoutError{"timeout while trying to get secret from keyring"} + } +} + +// Delete secret from keyring. +func Delete(service, user string) error { + ch := make(chan error, 1) + go func() { + defer close(ch) + ch <- keyring.Delete(service, user) + }() + select { + case err := <-ch: + return err + case <-time.After(3 * time.Second): + return &TimeoutError{"timeout while trying to delete secret from keyring"} + } +} From fa8b514bf180263a8e2a176c8723a017918bc7c8 Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 21 Jun 2023 15:47:33 +0200 Subject: [PATCH 10/12] Early exit repo sync if merge-upstream requires workflow scope --- pkg/cmd/repo/sync/http.go | 7 +++++++ pkg/cmd/repo/sync/sync_test.go | 18 ++++++++++++++++++ pkg/httpmock/stub.go | 21 +++++++++++++++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/repo/sync/http.go b/pkg/cmd/repo/sync/http.go index d5cc0e282..61a1c3317 100644 --- a/pkg/cmd/repo/sync/http.go +++ b/pkg/cmd/repo/sync/http.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net/http" + "regexp" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" @@ -33,6 +34,9 @@ type upstreamMergeErr struct{ error } var upstreamMergeUnavailableErr = upstreamMergeErr{errors.New("upstream merge API is unavailable")} +var missingWorkflowScopeRE = regexp.MustCompile("refusing to allow.*without `workflow` scope") +var missingWorkflowScopeErr = errors.New("Upstream commits contain workflow changes, which require the `workflow` scope to merge. To request it, run: gh auth refresh -s workflow") + func triggerUpstreamMerge(client *api.Client, repo ghrepo.Interface, branch string) (string, error) { var payload bytes.Buffer if err := json.NewEncoder(&payload).Encode(map[string]interface{}{ @@ -52,6 +56,9 @@ func triggerUpstreamMerge(client *api.Client, repo ghrepo.Interface, branch stri if errors.As(err, &httpErr) { switch httpErr.StatusCode { case http.StatusUnprocessableEntity, http.StatusConflict: + if missingWorkflowScopeRE.MatchString(httpErr.Message) { + return "", missingWorkflowScopeErr + } return "", upstreamMergeErr{errors.New(httpErr.Message)} case http.StatusNotFound: return "", upstreamMergeUnavailableErr diff --git a/pkg/cmd/repo/sync/sync_test.go b/pkg/cmd/repo/sync/sync_test.go index 90a216607..a532f3992 100644 --- a/pkg/cmd/repo/sync/sync_test.go +++ b/pkg/cmd/repo/sync/sync_test.go @@ -457,6 +457,24 @@ func Test_SyncRun(t *testing.T) { wantErr: true, errMsg: "trunk branch does not exist on OWNER/REPO-FORK repository", }, + { + name: "sync remote fork with missing workflow scope on token", + opts: &SyncOptions{ + DestArg: "FORKOWNER/REPO-FORK", + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`)) + reg.Register( + httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"), + httpmock.StatusJSONResponse(422, struct { + Message string `json:"message"` + }{Message: "refusing to allow an OAuth App to create or update workflow `.github/workflows/unimportant.yml` without `workflow` scope"})) + }, + wantErr: true, + errMsg: "Upstream commits contain workflow changes, which require the `workflow` scope to merge. To request it, run: gh auth refresh -s workflow", + }, } for _, tt := range tests { reg := &httpmock.Registry{} diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index b26d433fa..9332ac9dd 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -143,7 +143,20 @@ func StatusStringResponse(status int, body string) Responder { func JSONResponse(body interface{}) Responder { return func(req *http.Request) (*http.Response, error) { b, _ := json.Marshal(body) - return httpResponse(200, req, bytes.NewBuffer(b)), nil + header := http.Header{ + "Content-Type": []string{"application/json"}, + } + return httpResponseWithHeader(200, req, bytes.NewBuffer(b), header), nil + } +} + +func StatusJSONResponse(status int, body interface{}) Responder { + return func(req *http.Request) (*http.Response, error) { + b, _ := json.Marshal(body) + header := http.Header{ + "Content-Type": []string{"application/json"}, + } + return httpResponseWithHeader(status, req, bytes.NewBuffer(b), header), nil } } @@ -215,10 +228,14 @@ func ScopesResponder(scopes string) func(*http.Request) (*http.Response, error) } func httpResponse(status int, req *http.Request, body io.Reader) *http.Response { + return httpResponseWithHeader(status, req, body, http.Header{}) +} + +func httpResponseWithHeader(status int, req *http.Request, body io.Reader, header http.Header) *http.Response { return &http.Response{ StatusCode: status, Request: req, Body: io.NopCloser(body), - Header: http.Header{}, + Header: header, } } From ddfe476ff903cf6bfe742ef033642eebbd014579 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 23 Jun 2023 20:32:48 +0900 Subject: [PATCH 11/12] Remove old code paths and improve code comments for repo sync (#7610) --- pkg/cmd/repo/sync/http.go | 4 ---- pkg/cmd/repo/sync/sync.go | 12 ++++++++++-- pkg/cmd/repo/sync/sync_test.go | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/repo/sync/http.go b/pkg/cmd/repo/sync/http.go index 61a1c3317..3e193daeb 100644 --- a/pkg/cmd/repo/sync/http.go +++ b/pkg/cmd/repo/sync/http.go @@ -32,8 +32,6 @@ func latestCommit(client *api.Client, repo ghrepo.Interface, branch string) (com type upstreamMergeErr struct{ error } -var upstreamMergeUnavailableErr = upstreamMergeErr{errors.New("upstream merge API is unavailable")} - var missingWorkflowScopeRE = regexp.MustCompile("refusing to allow.*without `workflow` scope") var missingWorkflowScopeErr = errors.New("Upstream commits contain workflow changes, which require the `workflow` scope to merge. To request it, run: gh auth refresh -s workflow") @@ -60,8 +58,6 @@ func triggerUpstreamMerge(client *api.Client, repo ghrepo.Interface, branch stri return "", missingWorkflowScopeErr } return "", upstreamMergeErr{errors.New(httpErr.Message)} - case http.StatusNotFound: - return "", upstreamMergeUnavailableErr } } return "", err diff --git a/pkg/cmd/repo/sync/sync.go b/pkg/cmd/repo/sync/sync.go index 1f734b543..dd52868d1 100644 --- a/pkg/cmd/repo/sync/sync.go +++ b/pkg/cmd/repo/sync/sync.go @@ -284,6 +284,13 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt return nil } +// ExecuteRemoteRepoSync will take several steps to sync the source and destination repositories. +// First it will try to use the merge-upstream API endpoint. If this fails due to merge conflicts +// or unknown merge issues then it will fallback to using the low level git references API endpoint. +// The reason the fallback is necessary is to better support these error cases. The git references API +// endpoint allows us to sync repositories that are not fast-forward merge compatible. Additionally, +// the git references API endpoint gives more detailed error responses as to why the sync failed. +// Unless the --force flag is specified we will not perform non-fast-forward merges. func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) (string, error) { branchName := opts.Branch if branchName == "" { @@ -317,8 +324,9 @@ func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interfac return "", err } - // This is not a great way to detect the error returned by the API - // Unfortunately API returns 422 for multiple reasons + // Using string comparison is a brittle way to determine the error returned by the API + // endpoint but unfortunately the API returns 422 for many reasons so we must + // interpret the message provide better error messaging for our users. err = syncFork(client, destRepo, branchName, commit.Object.SHA, opts.Force) var httpErr api.HTTPError if err != nil { diff --git a/pkg/cmd/repo/sync/sync_test.go b/pkg/cmd/repo/sync/sync_test.go index a532f3992..eafc0835e 100644 --- a/pkg/cmd/repo/sync/sync_test.go +++ b/pkg/cmd/repo/sync/sync_test.go @@ -292,7 +292,7 @@ func Test_SyncRun(t *testing.T) { httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`)) reg.Register( httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"), - httpmock.StatusStringResponse(404, `{}`)) + httpmock.StatusStringResponse(422, `{}`)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/trunk"), httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`)) From baba8949973244a53f6a172a03afa84a7a0ccfc4 Mon Sep 17 00:00:00 2001 From: Kousik Mitra Date: Fri, 23 Jun 2023 17:38:21 +0530 Subject: [PATCH 12/12] Add option to remove file from gist (#7560) --- pkg/cmd/gist/edit/edit.go | 35 +++++++++++-- pkg/cmd/gist/edit/edit_test.go | 96 +++++++++++++++++++++++++++++++--- 2 files changed, 119 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 3ad28b6c4..fffcc82ca 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -32,11 +32,12 @@ type EditOptions struct { Edit func(string, string, string, *iostreams.IOStreams) (string, error) - Selector string - EditFilename string - AddFilename string - SourceFile string - Description string + Selector string + EditFilename string + AddFilename string + RemoveFilename string + SourceFile string + Description string } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -82,6 +83,10 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a new file to the gist") cmd.Flags().StringVarP(&opts.Description, "desc", "d", "", "New description for the gist") cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit") + cmd.Flags().StringVarP(&opts.RemoveFilename, "remove", "r", "", "Remove a file from the gist") + + cmd.MarkFlagsMutuallyExclusive("add", "remove") + cmd.MarkFlagsMutuallyExclusive("remove", "filename") return cmd } @@ -187,6 +192,16 @@ func editRun(opts *EditOptions) error { return updateGist(apiClient, host, gist) } + // Remove a file from the gist + if opts.RemoveFilename != "" { + err := removeFile(gist, opts.RemoveFilename) + if err != nil { + return err + } + + return updateGist(apiClient, host, gist) + } + filesToUpdate := map[string]string{} for { @@ -337,3 +352,13 @@ func getFilesToAdd(file string, content []byte) (map[string]*shared.GistFile, er }, }, nil } + +func removeFile(gist *shared.Gist, filename string) error { + if _, found := gist.Files[filename]; !found { + return fmt.Errorf("gist has no file %q", filename) + } + + gist.Files[filename] = nil + + return nil +} diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 5e52a8ffb..481dd0a9c 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -36,9 +36,10 @@ func Test_getFilesToAdd(t *testing.T) { func TestNewCmdEdit(t *testing.T) { tests := []struct { - name string - cli string - wants EditOptions + name string + cli string + wants EditOptions + wantsErr bool }{ { name: "no flags", @@ -80,6 +81,24 @@ func TestNewCmdEdit(t *testing.T) { Description: "my new description", }, }, + { + name: "remove", + cli: "123 --remove cool.md", + wants: EditOptions{ + Selector: "123", + RemoveFilename: "cool.md", + }, + }, + { + name: "add and remove are mutually exclusive", + cli: "123 --add cool.md --remove great.md", + wantsErr: true, + }, + { + name: "filename and remove are mutually exclusive", + cli: "123 --filename cool.md --remove great.md", + wantsErr: true, + }, } for _, tt := range tests { @@ -100,11 +119,17 @@ func TestNewCmdEdit(t *testing.T) { cmd.SetErr(&bytes.Buffer{}) _, err = cmd.ExecuteC() - assert.NoError(t, err) + if tt.wantsErr { + require.Error(t, err) + return + } - assert.Equal(t, tt.wants.EditFilename, gotOpts.EditFilename) - assert.Equal(t, tt.wants.AddFilename, gotOpts.AddFilename) - assert.Equal(t, tt.wants.Selector, gotOpts.Selector) + require.NoError(t, err) + + require.Equal(t, tt.wants.EditFilename, gotOpts.EditFilename) + require.Equal(t, tt.wants.AddFilename, gotOpts.AddFilename) + require.Equal(t, tt.wants.Selector, gotOpts.Selector) + require.Equal(t, tt.wants.RemoveFilename, gotOpts.RemoveFilename) }) } } @@ -394,6 +419,63 @@ func Test_editRun(t *testing.T) { }, }, }, + { + name: "remove file, file does not exist", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + opts: &EditOptions{ + RemoveFilename: "sample2.txt", + }, + wantErr: "gist has no file \"sample2.txt\"", + }, + { + name: "remove file from existing gist", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "sample2.txt": { + Filename: "sample2.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + RemoveFilename: "sample2.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "sample.txt": map[string]interface{}{ + "filename": "sample.txt", + "content": "bwhiizzzbwhuiiizzzz", + "type": "text/plain", + }, + "sample2.txt": nil, + }, + }, + }, { name: "edit gist using file from source parameter", gist: &shared.Gist{