From 05e45e38637e2e385885dab28814e449d4d2d668 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 17 Feb 2021 11:27:57 +0530 Subject: [PATCH 01/14] Feature of adding new files to an existing Github gist --- pkg/cmd/gist/edit/edit.go | 177 +++++++++++++++++++-------------- pkg/cmd/gist/edit/edit_test.go | 26 ++++- 2 files changed, 129 insertions(+), 74 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 7cba14800..0d460643b 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -5,10 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "net/http" - "sort" - "strings" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/internal/config" @@ -19,6 +15,9 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" + "net/http" + "sort" + "strings" ) type EditOptions struct { @@ -28,8 +27,9 @@ type EditOptions struct { Edit func(string, string, string, *iostreams.IOStreams) (string, error) - Selector string - Filename string + Selector string + EditFilename string + AddFilename string } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -48,7 +48,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "edit { | }", - Short: "Edit one of your gists", + Short: "Edit one of your gists or add new ones to it", Args: cmdutil.MinimumArgs(1, "cannot edit: gist argument required"), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] @@ -60,7 +60,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman return editRun(&opts) }, } - cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "Select a file to edit") + cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a file") + cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit") return cmd } @@ -99,89 +100,121 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} - for { - filename := opts.Filename - candidates := []string{} - for filename := range gist.Files { - candidates = append(candidates, filename) - } - - sort.Strings(candidates) - - if filename == "" { - if len(candidates) == 1 { - filename = candidates[0] - } else { - if !opts.IO.CanPrompt() { - return errors.New("unsure what file to edit; either specify --filename or run interactively") - } - err = prompt.SurveyAskOne(&survey.Select{ - Message: "Edit which file?", - Options: candidates, - }, &filename) - - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } - } - } - - if _, ok := gist.Files[filename]; !ok { - return fmt.Errorf("gist has no file %q", filename) - } + addFilename := opts.AddFilename + if addFilename != "" { + //Add files to an existing gist editorCommand, err := cmdutil.DetermineEditor(opts.Config) if err != nil { return err } - text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) + text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) if err != nil { return err } - if text != gist.Files[filename].Content { - gistFile := gist.Files[filename] - gistFile.Content = text // so it appears if they re-edit - filesToUpdate[filename] = text + if text == "" { + return fmt.Errorf("Contents can't be empty") } - if !opts.IO.CanPrompt() { - break - } - - if len(candidates) == 1 { - break - } - - choice := "" - - err = prompt.SurveyAskOne(&survey.Select{ - Message: "What next?", - Options: []string{ - "Edit another file", - "Submit", - "Cancel", + filesToAdd := map[string]*shared.GistFile{ + addFilename: { + Filename: addFilename, + Content: text, }, - }, &choice) + } + gist.Files = filesToAdd + err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { - return fmt.Errorf("could not prompt: %w", err) + return err } + } else { + for { + filename := opts.EditFilename + candidates := []string{} + for filename := range gist.Files { + candidates = append(candidates, filename) + } - stop := false + sort.Strings(candidates) - switch choice { - case "Edit another file": - continue - case "Submit": - stop = true - case "Cancel": - return cmdutil.SilentError - } + if filename == "" { + if len(candidates) == 1 { + filename = candidates[0] + } else { + if !opts.IO.CanPrompt() { + return errors.New("unsure what file to edit; either specify --filename or run interactively") + } + err = prompt.SurveyAskOne(&survey.Select{ + Message: "Edit which file?", + Options: candidates, + }, &filename) - if stop { - break + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + } + } + + if _, ok := gist.Files[filename]; !ok { + return fmt.Errorf("gist has no file %q", filename) + } + + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) + + if err != nil { + return err + } + + if text != gist.Files[filename].Content { + gistFile := gist.Files[filename] + gistFile.Content = text // so it appears if they re-edit + filesToUpdate[filename] = text + } + + if !opts.IO.CanPrompt() { + break + } + + if len(candidates) == 1 { + break + } + + choice := "" + + err = prompt.SurveyAskOne(&survey.Select{ + Message: "What next?", + Options: []string{ + "Edit another file", + "Submit", + "Cancel", + }, + }, &choice) + + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + + stop := false + + switch choice { + case "Edit another file": + continue + case "Submit": + stop = true + case "Cancel": + return cmdutil.SilentError + } + + if stop { + break + } } } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index b9b711a38..59156dec6 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -35,7 +35,15 @@ func TestNewCmdEdit(t *testing.T) { cli: "123 --filename cool.md", wants: EditOptions{ Selector: "123", - Filename: "cool.md", + EditFilename: "cool.md", + }, + }, + { + name: "add", + cli: "123 --add cool.md", + wants: EditOptions{ + Selector: "123", + AddFilename: "cool.md", }, }, } @@ -60,7 +68,8 @@ func TestNewCmdEdit(t *testing.T) { _, err = cmd.ExecuteC() assert.NoError(t, err) - assert.Equal(t, tt.wants.Filename, gotOpts.Filename) + assert.Equal(t, tt.wants.EditFilename, gotOpts.EditFilename) + assert.Equal(t, tt.wants.AddFilename, gotOpts.AddFilename) assert.Equal(t, tt.wants.Selector, gotOpts.Selector) }) } @@ -211,6 +220,19 @@ func Test_editRun(t *testing.T) { wantErr: true, wantStderr: "You do not own this gist.", }, + { + name: "add file to existing gist", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "foo.txt": { + Filename: "foo.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + }, + }, } for _, tt := range tests { From 037343c5c2bb7994f01f6480cb881a602ab426db Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 17 Feb 2021 19:20:43 +0530 Subject: [PATCH 02/14] Add existing files in the current wd to gist --- pkg/cmd/gist/edit/edit.go | 55 +++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 0d460643b..ed36e6914 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -15,7 +15,10 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" + "io/ioutil" "net/http" + "os" + "path/filepath" "sort" "strings" ) @@ -104,27 +107,63 @@ func editRun(opts *EditOptions) error { if addFilename != "" { //Add files to an existing gist - editorCommand, err := cmdutil.DetermineEditor(opts.Config) + wd, err := os.Getwd() if err != nil { return err } - text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) + m, err := filepath.Glob(wd + "/[^.]*.*") if err != nil { return err } - if text == "" { - return fmt.Errorf("Contents can't be empty") + filesToAdd := map[string]*shared.GistFile{} + fileExists := false + + for _, f := range m { + if addFilename == filepath.Base(f) { + fileExists = true + break + } } - filesToAdd := map[string]*shared.GistFile{ - addFilename: { + if fileExists { + content, err := ioutil.ReadFile(addFilename) + if err != nil { + return fmt.Errorf("failed to read file %s: %w", addFilename, err) + } + + if string(content) == "" { + return fmt.Errorf("Contents can't be empty") + } + + filesToAdd[addFilename] = &shared.GistFile{ + Filename: addFilename, + Content: string(content), + } + gist.Files = filesToAdd + } else { + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + + text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) + if err != nil { + return err + } + + if text == "" { + return fmt.Errorf("Contents can't be empty") + } + + filesToAdd[addFilename] = &shared.GistFile{ Filename: addFilename, Content: text, - }, + } + + gist.Files = filesToAdd } - gist.Files = filesToAdd err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { From 298388745879a2978573a8822440bcc646358683 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 17 Feb 2021 20:39:43 +0530 Subject: [PATCH 03/14] Add files by absolute path to gist --- pkg/cmd/gist/edit/edit.go | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index ed36e6914..988b7c141 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -104,9 +104,21 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} addFilename := opts.AddFilename + fileExists := false if addFilename != "" { //Add files to an existing gist + if fi, err := os.Stat(addFilename); err != nil { + return err + } else { + switch mode := fi.Mode(); { + case mode.IsDir(): + return fmt.Errorf("found directory %s", addFilename) + case mode.IsRegular(): + fileExists = true + } + } + wd, err := os.Getwd() if err != nil { return err @@ -118,11 +130,18 @@ func editRun(opts *EditOptions) error { } filesToAdd := map[string]*shared.GistFile{} - fileExists := false for _, f := range m { if addFilename == filepath.Base(f) { - fileExists = true + choice := false + err := prompt.Confirm("File found in this directory. Proceed?", &choice) + if err != nil { + return err + } + + if choice { + fileExists = true + } break } } @@ -137,8 +156,10 @@ func editRun(opts *EditOptions) error { return fmt.Errorf("Contents can't be empty") } - filesToAdd[addFilename] = &shared.GistFile{ - Filename: addFilename, + filename := filepath.Base(addFilename) + + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, Content: string(content), } gist.Files = filesToAdd From bff8b3007a85aed6206beb079257b5d44fb367fc Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 18:45:43 +0530 Subject: [PATCH 04/14] Add test cases and improve errors with color schemes --- pkg/cmd/gist/edit/edit.go | 130 ++++++++++++++++----------- pkg/cmd/gist/edit/edit_test.go | 160 +++++++++++++++++++++++++++++++-- 2 files changed, 233 insertions(+), 57 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 988b7c141..b430f0d97 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -5,6 +5,13 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" + "net/http" + "os" + "path/filepath" + "sort" + "strings" + "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/internal/config" @@ -15,12 +22,6 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" - "io/ioutil" - "net/http" - "os" - "path/filepath" - "sort" - "strings" ) type EditOptions struct { @@ -28,8 +29,8 @@ type EditOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) - Edit func(string, string, string, *iostreams.IOStreams) (string, error) - + Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Add func(string, string, *iostreams.IOStreams) (string, error) Selector string EditFilename string AddFilename string @@ -47,6 +48,13 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman defaultContent, io.In, io.Out, io.ErrOut, nil) }, + Add: func(editorCmd, filename string, io *iostreams.IOStreams) (string, error) { + return surveyext.Edit( + editorCmd, + "*."+filename, + "", + io.In, io.Out, io.ErrOut, nil) + }, } cmd := &cobra.Command{ @@ -104,60 +112,34 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} addFilename := opts.AddFilename - fileExists := false + cs := opts.IO.ColorScheme() if addFilename != "" { - //Add files to an existing gist - if fi, err := os.Stat(addFilename); err != nil { - return err - } else { - switch mode := fi.Mode(); { - case mode.IsDir(): - return fmt.Errorf("found directory %s", addFilename) - case mode.IsRegular(): - fileExists = true - } - } - - wd, err := os.Getwd() + //Add files to an existing gist. Aborts when file is already present + // in the directory but the user chooses not to proceed + filenamePath, fileExists, userAbort, err := processFiles(addFilename) if err != nil { - return err - } - - m, err := filepath.Glob(wd + "/[^.]*.*") - if err != nil { - return err + return fmt.Errorf("%s %s", cs.Red("!"), err) } filesToAdd := map[string]*shared.GistFile{} - for _, f := range m { - if addFilename == filepath.Base(f) { - choice := false - err := prompt.Confirm("File found in this directory. Proceed?", &choice) - if err != nil { - return err - } - - if choice { - fileExists = true - } - break - } + if userAbort { + return nil } + filename := filepath.Base(filenamePath) + if fileExists { - content, err := ioutil.ReadFile(addFilename) + content, err := ioutil.ReadFile(filenamePath) if err != nil { - return fmt.Errorf("failed to read file %s: %w", addFilename, err) + return fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), addFilename, err) } if string(content) == "" { - return fmt.Errorf("Contents can't be empty") + return fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) } - filename := filepath.Base(addFilename) - filesToAdd[filename] = &shared.GistFile{ Filename: filename, Content: string(content), @@ -169,17 +151,17 @@ func editRun(opts *EditOptions) error { return err } - text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) + text, err := opts.Add(editorCommand, filename, opts.IO) if err != nil { return err } if text == "" { - return fmt.Errorf("Contents can't be empty") + return fmt.Errorf("%s Contents can't be empty", cs.Red("!")) } - filesToAdd[addFilename] = &shared.GistFile{ - Filename: addFilename, + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, Content: text, } @@ -190,6 +172,11 @@ func editRun(opts *EditOptions) error { if err != nil { return err } + + completionMessage := filename + " added to gist" + + fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage) + } else { for { filename := opts.EditFilename @@ -315,3 +302,46 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } + +func processFiles(filename string) (string, bool, bool, error) { + fileExists := false + + if fi, err := os.Stat(filename); err != nil { + } else { + switch mode := fi.Mode(); { + case mode.IsDir(): + return "", false, false, fmt.Errorf("found directory %s" , filename) + case mode.IsRegular(): + fileExists = true + } + } + + wd, err := os.Getwd() + if err != nil { + return "", false, false, err + } + + m, err := filepath.Glob(wd + "/[^.]*.*") + if err != nil { + return "", false, false, err + } + + for _, f := range m { + if filename == filepath.Base(f) { + choice := false + err := prompt.Confirm("File found in this directory. Proceed?", &choice) + if err != nil { + return "", false, false, err + } + + if choice { + fileExists = true + } else { + return "", true, true, nil + } + break + } + } + + return filename, fileExists, false, nil +} diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 59156dec6..6f0df8040 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -17,6 +17,32 @@ import ( "github.com/stretchr/testify/assert" ) +const ( + fixtureFile = "../fixture.txt" + nonExistentFile = "../file.txt" +) + +func Test_processFiles(t *testing.T) { + filePath, fileExists, userAbort, err := processFiles(fixtureFile) + if err != nil { + t.Fatalf("unexpected error processing files: %s", err) + } + + assert.Equal(t, "../fixture.txt", filePath) + assert.Equal(t, true, fileExists) + assert.Equal(t, false, userAbort) + + filePath, fileExists, userAbort, err = processFiles(nonExistentFile) + if err != nil { + t.Fatalf("unexpected error processing files: %s", err) + } + + assert.Equal(t, "../file.txt", filePath) + assert.Equal(t, false, fileExists) + assert.Equal(t, false, userAbort) + +} + func TestNewCmdEdit(t *testing.T) { tests := []struct { name string @@ -34,15 +60,15 @@ func TestNewCmdEdit(t *testing.T) { name: "filename", cli: "123 --filename cool.md", wants: EditOptions{ - Selector: "123", + Selector: "123", EditFilename: "cool.md", }, }, { name: "add", - cli: "123 --add cool.md", + cli: "123 --add cool.md", wants: EditOptions{ - Selector: "123", + Selector: "123", AddFilename: "cool.md", }, }, @@ -225,10 +251,126 @@ func Test_editRun(t *testing.T) { gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ - "foo.txt": { - Filename: "foo.txt", - Content: "bwhiizzzbwhuiiizzzz", - Type: "text/plain", + "sample.txt": { + Filename: "sample.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{ + AddFilename: "foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist with absolute path", + 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"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "/Users/octocat/foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist with relative path", + 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"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "../foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist in same directory", + + 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"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", }, }, }, @@ -265,6 +407,10 @@ func Test_editRun(t *testing.T) { return "new file content", nil } + tt.opts.Add = func(_, _ string, _ *iostreams.IOStreams) (string, error) { + return "new content to existing gist", nil + } + tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } From 9a4fd0d706219af9fee861cc831d0d43400b43ec Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 19:17:42 +0530 Subject: [PATCH 05/14] Remove unwanted prompt for user. Unwanted test as well --- pkg/cmd/gist/edit/edit.go | 31 +++++++------------------- pkg/cmd/gist/edit/edit_test.go | 40 ++-------------------------------- 2 files changed, 10 insertions(+), 61 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index b430f0d97..453006049 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -115,19 +115,14 @@ func editRun(opts *EditOptions) error { cs := opts.IO.ColorScheme() if addFilename != "" { - //Add files to an existing gist. Aborts when file is already present - // in the directory but the user chooses not to proceed - filenamePath, fileExists, userAbort, err := processFiles(addFilename) + //Add files to an existing gist. + filenamePath, fileExists, err := processFiles(addFilename) if err != nil { return fmt.Errorf("%s %s", cs.Red("!"), err) } filesToAdd := map[string]*shared.GistFile{} - if userAbort { - return nil - } - filename := filepath.Base(filenamePath) if fileExists { @@ -303,14 +298,14 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } -func processFiles(filename string) (string, bool, bool, error) { +func processFiles(filename string) (string, bool, error) { fileExists := false if fi, err := os.Stat(filename); err != nil { } else { switch mode := fi.Mode(); { case mode.IsDir(): - return "", false, false, fmt.Errorf("found directory %s" , filename) + return "", false, fmt.Errorf("found directory %s" , filename) case mode.IsRegular(): fileExists = true } @@ -318,30 +313,20 @@ func processFiles(filename string) (string, bool, bool, error) { wd, err := os.Getwd() if err != nil { - return "", false, false, err + return "", false, err } m, err := filepath.Glob(wd + "/[^.]*.*") if err != nil { - return "", false, false, err + return "", false, err } for _, f := range m { if filename == filepath.Base(f) { - choice := false - err := prompt.Confirm("File found in this directory. Proceed?", &choice) - if err != nil { - return "", false, false, err - } - - if choice { - fileExists = true - } else { - return "", true, true, nil - } + fileExists = true break } } - return filename, fileExists, false, nil + return filename, fileExists, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 6f0df8040..fac698abf 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -23,24 +23,21 @@ const ( ) func Test_processFiles(t *testing.T) { - filePath, fileExists, userAbort, err := processFiles(fixtureFile) + filePath, fileExists, err := processFiles(fixtureFile) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } assert.Equal(t, "../fixture.txt", filePath) assert.Equal(t, true, fileExists) - assert.Equal(t, false, userAbort) - filePath, fileExists, userAbort, err = processFiles(nonExistentFile) + filePath, fileExists, err = processFiles(nonExistentFile) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } assert.Equal(t, "../file.txt", filePath) assert.Equal(t, false, fileExists) - assert.Equal(t, false, userAbort) - } func TestNewCmdEdit(t *testing.T) { @@ -342,39 +339,6 @@ func Test_editRun(t *testing.T) { }, }, }, - { - name: "add file to existing gist in same directory", - - 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"}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("POST", "gists/1234"), - httpmock.StatusStringResponse(201, "{}")) - }, - opts: &EditOptions{ - AddFilename: "foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, - }, - }, } for _, tt := range tests { From a4a194011ff072f1c5127ae3714363b1c6cdbd9c Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 19:18:49 +0530 Subject: [PATCH 06/14] gofmt --- pkg/cmd/gist/edit/edit_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index fac698abf..3fa90db7f 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -18,7 +18,7 @@ import ( ) const ( - fixtureFile = "../fixture.txt" + fixtureFile = "../fixture.txt" nonExistentFile = "../file.txt" ) @@ -261,7 +261,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "foo.txt", + AddFilename: "foo.txt", }, wantParams: map[string]interface{}{ "description": "", @@ -293,7 +293,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "/Users/octocat/foo.txt", + AddFilename: "/Users/octocat/foo.txt", }, wantParams: map[string]interface{}{ "description": "", @@ -325,7 +325,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "../foo.txt", + AddFilename: "../foo.txt", }, wantParams: map[string]interface{}{ "description": "", From 882bd1adb1fadc1c99e67d56f71d9204b19e25cc Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 22:39:56 +0530 Subject: [PATCH 07/14] add go lint to pass checks --- pkg/cmd/gist/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 453006049..2d06ee554 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -305,7 +305,7 @@ func processFiles(filename string) (string, bool, error) { } else { switch mode := fi.Mode(); { case mode.IsDir(): - return "", false, fmt.Errorf("found directory %s" , filename) + return "", false, fmt.Errorf("found directory %s", filename) case mode.IsRegular(): fileExists = true } From 4ed10140ab191fbf0465560a51ce6be04a46e402 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Fri, 19 Feb 2021 12:02:17 +0530 Subject: [PATCH 08/14] Resolved PR review comments and test cases --- pkg/cmd/gist/edit/edit.go | 276 ++++++++++++++++----------------- pkg/cmd/gist/edit/edit_test.go | 27 +++- 2 files changed, 158 insertions(+), 145 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 2d06ee554..c3465242a 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -59,7 +59,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "edit { | }", - Short: "Edit one of your gists or add new ones to it", + Short: "Edit or add files in a gist", Args: cmdutil.MinimumArgs(1, "cannot edit: gist argument required"), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] @@ -116,147 +116,109 @@ func editRun(opts *EditOptions) error { if addFilename != "" { //Add files to an existing gist. - filenamePath, fileExists, err := processFiles(addFilename) + files, err := getFilesToAdd(addFilename, opts) if err != nil { - return fmt.Errorf("%s %s", cs.Red("!"), err) + return err } - filesToAdd := map[string]*shared.GistFile{} - - filename := filepath.Base(filenamePath) - - if fileExists { - content, err := ioutil.ReadFile(filenamePath) - if err != nil { - return fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), addFilename, err) - } - - if string(content) == "" { - return fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: string(content), - } - gist.Files = filesToAdd - } else { - editorCommand, err := cmdutil.DetermineEditor(opts.Config) - if err != nil { - return err - } - - text, err := opts.Add(editorCommand, filename, opts.IO) - if err != nil { - return err - } - - if text == "" { - return fmt.Errorf("%s Contents can't be empty", cs.Red("!")) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: text, - } - - gist.Files = filesToAdd - } + fmt.Printf("%v", files) + gist.Files = files err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { return err } - completionMessage := filename + " added to gist" + completionMessage := filepath.Base(addFilename) + " added to gist" fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage) - } else { - for { - filename := opts.EditFilename - candidates := []string{} - for filename := range gist.Files { - candidates = append(candidates, filename) - } + return nil + } - sort.Strings(candidates) + for { + filename := opts.EditFilename + candidates := []string{} + for filename := range gist.Files { + candidates = append(candidates, filename) + } - if filename == "" { - if len(candidates) == 1 { - filename = candidates[0] - } else { - if !opts.IO.CanPrompt() { - return errors.New("unsure what file to edit; either specify --filename or run interactively") - } - err = prompt.SurveyAskOne(&survey.Select{ - Message: "Edit which file?", - Options: candidates, - }, &filename) + sort.Strings(candidates) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } + if filename == "" { + if len(candidates) == 1 { + filename = candidates[0] + } else { + if !opts.IO.CanPrompt() { + return errors.New("unsure what file to edit; either specify --filename or run interactively") + } + err = prompt.SurveyAskOne(&survey.Select{ + Message: "Edit which file?", + Options: candidates, + }, &filename) + + if err != nil { + return fmt.Errorf("could not prompt: %w", err) } } + } - if _, ok := gist.Files[filename]; !ok { - return fmt.Errorf("gist has no file %q", filename) - } + if _, ok := gist.Files[filename]; !ok { + return fmt.Errorf("gist has no file %q", filename) + } - editorCommand, err := cmdutil.DetermineEditor(opts.Config) - if err != nil { - return err - } - text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) - if err != nil { - return err - } + if err != nil { + return err + } - if text != gist.Files[filename].Content { - gistFile := gist.Files[filename] - gistFile.Content = text // so it appears if they re-edit - filesToUpdate[filename] = text - } + if text != gist.Files[filename].Content { + gistFile := gist.Files[filename] + gistFile.Content = text // so it appears if they re-edit + filesToUpdate[filename] = text + } - if !opts.IO.CanPrompt() { - break - } + if !opts.IO.CanPrompt() { + break + } - if len(candidates) == 1 { - break - } + if len(candidates) == 1 { + break + } - choice := "" + choice := "" - err = prompt.SurveyAskOne(&survey.Select{ - Message: "What next?", - Options: []string{ - "Edit another file", - "Submit", - "Cancel", - }, - }, &choice) + err = prompt.SurveyAskOne(&survey.Select{ + Message: "What next?", + Options: []string{ + "Edit another file", + "Submit", + "Cancel", + }, + }, &choice) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } - stop := false + stop := false - switch choice { - case "Edit another file": - continue - case "Submit": - stop = true - case "Cancel": - return cmdutil.SilentError - } + switch choice { + case "Edit another file": + continue + case "Submit": + stop = true + case "Cancel": + return cmdutil.SilentError + } - if stop { - break - } + if stop { + break } } @@ -298,35 +260,73 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } -func processFiles(filename string) (string, bool, error) { - fileExists := false +func getFilesToAdd(file string, opts *EditOptions) (map[string]*shared.GistFile, error) { + cs := opts.IO.ColorScheme() - if fi, err := os.Stat(filename); err != nil { + fileExists, err := fileExists(file) + if err != nil { + return nil, fmt.Errorf("%s %s", cs.Red("!"), err) + } + + filesToAdd := map[string]*shared.GistFile{} + + filename := filepath.Base(file) + + if fileExists { + content, err := ioutil.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) + } + + if string(content) == "" { + return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) + } + + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, + Content: string(content), + } + return filesToAdd, nil } else { - switch mode := fi.Mode(); { - case mode.IsDir(): - return "", false, fmt.Errorf("found directory %s", filename) - case mode.IsRegular(): - fileExists = true + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return nil, err } - } - wd, err := os.Getwd() - if err != nil { - return "", false, err - } - - m, err := filepath.Glob(wd + "/[^.]*.*") - if err != nil { - return "", false, err - } - - for _, f := range m { - if filename == filepath.Base(f) { - fileExists = true - break + text, err := opts.Add(editorCommand, filename, opts.IO) + if err != nil { + return nil, err } - } - return filename, fileExists, nil + if text == "" { + return nil, fmt.Errorf("%s Contents can't be empty", cs.Red("!")) + } + + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, + Content: text, + } + + return filesToAdd, nil + } +} + +func fileExists(filename string) (bool, error) { + + fi, err := os.Stat(filename) + + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + } + + switch mode := fi.Mode(); { + case mode.IsDir(): + return false, fmt.Errorf("found directory %s", filename) + case mode.IsRegular(): + return true, nil + } + + return false, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 3fa90db7f..c6ea04126 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "io/ioutil" "net/http" + "path/filepath" "testing" "github.com/cli/cli/internal/config" @@ -22,22 +23,34 @@ const ( nonExistentFile = "../file.txt" ) -func Test_processFiles(t *testing.T) { - filePath, fileExists, err := processFiles(fixtureFile) +func Test_fileExists(t *testing.T) { + fixtureFileExists, err := fileExists(fixtureFile) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } - assert.Equal(t, "../fixture.txt", filePath) - assert.Equal(t, true, fileExists) + assert.Equal(t, true, fixtureFileExists) - filePath, fileExists, err = processFiles(nonExistentFile) + neFileExists, err := fileExists(nonExistentFile) + assert.Equal(t, nil, err) + assert.Equal(t, false, neFileExists) +} + +func Test_getFilesToAdd(t *testing.T) { + gf, err := getFilesToAdd(fixtureFile, &EditOptions{ + AddFilename: fixtureFile, + IO: &iostreams.IOStreams{}, + }) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } - assert.Equal(t, "../file.txt", filePath) - assert.Equal(t, false, fileExists) + assert.Equal(t, map[string]*shared.GistFile{ + filepath.Base(fixtureFile): { + Filename: filepath.Base(fixtureFile), + Content: "{}", + }, + }, gf) } func TestNewCmdEdit(t *testing.T) { From faffc4de95e31e8e59951956ad967f82688ab419 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Fri, 19 Feb 2021 12:05:20 +0530 Subject: [PATCH 09/14] Add go fmt to pass ci/cd --- pkg/cmd/gist/edit/edit_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index c6ea04126..854e4be05 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -38,8 +38,8 @@ func Test_fileExists(t *testing.T) { func Test_getFilesToAdd(t *testing.T) { gf, err := getFilesToAdd(fixtureFile, &EditOptions{ - AddFilename: fixtureFile, - IO: &iostreams.IOStreams{}, + AddFilename: fixtureFile, + IO: &iostreams.IOStreams{}, }) if err != nil { t.Fatalf("unexpected error processing files: %s", err) From a6fa14866be9e521b82ea0ea09cdb6653319b855 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 09:44:11 +0530 Subject: [PATCH 10/14] updating tests WIP --- pkg/cmd/gist/edit/edit.go | 69 +++++----------------------------- pkg/cmd/gist/edit/edit_test.go | 46 ----------------------- 2 files changed, 9 insertions(+), 106 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index c3465242a..65f91732f 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -7,7 +7,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "path/filepath" "sort" "strings" @@ -121,8 +120,6 @@ func editRun(opts *EditOptions) error { return err } - fmt.Printf("%v", files) - gist.Files = files err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { @@ -263,70 +260,22 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error func getFilesToAdd(file string, opts *EditOptions) (map[string]*shared.GistFile, error) { cs := opts.IO.ColorScheme() - fileExists, err := fileExists(file) - if err != nil { - return nil, fmt.Errorf("%s %s", cs.Red("!"), err) - } - filesToAdd := map[string]*shared.GistFile{} filename := filepath.Base(file) - if fileExists { - content, err := ioutil.ReadFile(file) - if err != nil { - return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) - } - - if string(content) == "" { - return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: string(content), - } - return filesToAdd, nil - } else { - editorCommand, err := cmdutil.DetermineEditor(opts.Config) - if err != nil { - return nil, err - } - - text, err := opts.Add(editorCommand, filename, opts.IO) - if err != nil { - return nil, err - } - - if text == "" { - return nil, fmt.Errorf("%s Contents can't be empty", cs.Red("!")) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: text, - } - - return filesToAdd, nil - } -} - -func fileExists(filename string) (bool, error) { - - fi, err := os.Stat(filename) - + content, err := ioutil.ReadFile(file) if err != nil { - if os.IsNotExist(err) { - return false, nil - } + return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) } - switch mode := fi.Mode(); { - case mode.IsDir(): - return false, fmt.Errorf("found directory %s", filename) - case mode.IsRegular(): - return true, nil + if string(content) == "" { + return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) } - return false, nil + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, + Content: string(content), + } + return filesToAdd, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 854e4be05..c1b411a47 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -20,22 +20,8 @@ import ( const ( fixtureFile = "../fixture.txt" - nonExistentFile = "../file.txt" ) -func Test_fileExists(t *testing.T) { - fixtureFileExists, err := fileExists(fixtureFile) - if err != nil { - t.Fatalf("unexpected error processing files: %s", err) - } - - assert.Equal(t, true, fixtureFileExists) - - neFileExists, err := fileExists(nonExistentFile) - assert.Equal(t, nil, err) - assert.Equal(t, false, neFileExists) -} - func Test_getFilesToAdd(t *testing.T) { gf, err := getFilesToAdd(fixtureFile, &EditOptions{ AddFilename: fixtureFile, @@ -256,38 +242,6 @@ func Test_editRun(t *testing.T) { wantErr: true, wantStderr: "You do not own this gist.", }, - { - name: "add file to existing gist", - 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"}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("POST", "gists/1234"), - httpmock.StatusStringResponse(201, "{}")) - }, - opts: &EditOptions{ - AddFilename: "foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, - }, - }, { name: "add file to existing gist with absolute path", gist: &shared.Gist{ From d4e14beb57880f493438849b97d6ca92090333fb Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 10:14:31 +0530 Subject: [PATCH 11/14] remove unwanted tests and unwanted functionality --- pkg/cmd/gist/edit/edit.go | 8 ----- pkg/cmd/gist/edit/edit_test.go | 53 ++-------------------------------- 2 files changed, 3 insertions(+), 58 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 65f91732f..3bea3757b 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -29,7 +29,6 @@ type EditOptions struct { Config func() (config.Config, error) Edit func(string, string, string, *iostreams.IOStreams) (string, error) - Add func(string, string, *iostreams.IOStreams) (string, error) Selector string EditFilename string AddFilename string @@ -47,13 +46,6 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman defaultContent, io.In, io.Out, io.ErrOut, nil) }, - Add: func(editorCmd, filename string, io *iostreams.IOStreams) (string, error) { - return surveyext.Edit( - editorCmd, - "*."+filename, - "", - io.In, io.Out, io.ErrOut, nil) - }, } cmd := &cobra.Command{ diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index c1b411a47..1b5ac25b4 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -19,7 +19,7 @@ import ( ) const ( - fixtureFile = "../fixture.txt" + fixtureFile = "../fixture.txt" ) func Test_getFilesToAdd(t *testing.T) { @@ -243,7 +243,7 @@ func Test_editRun(t *testing.T) { wantStderr: "You do not own this gist.", }, { - name: "add file to existing gist with absolute path", + name: "add file to existing gist", gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ @@ -260,50 +260,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "/Users/octocat/foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, - }, - }, - { - name: "add file to existing gist with relative path", - 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"}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("POST", "gists/1234"), - httpmock.StatusStringResponse(201, "{}")) - }, - opts: &EditOptions{ - AddFilename: "../foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, + AddFilename: "../fixture.txt", }, }, } @@ -338,10 +295,6 @@ func Test_editRun(t *testing.T) { return "new file content", nil } - tt.opts.Add = func(_, _ string, _ *iostreams.IOStreams) (string, error) { - return "new content to existing gist", nil - } - tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } From 3e5d5a23c083848b510a7ad6c0b6056140031680 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 10:22:56 +0530 Subject: [PATCH 12/14] add fixturefile const in tests --- pkg/cmd/gist/edit/edit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 1b5ac25b4..abbda4acd 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -260,7 +260,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "../fixture.txt", + AddFilename: fixtureFile, }, }, } From 406d7eee456f8c91da5a7ba66c2d9e8ab30c2213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 12:03:29 +0100 Subject: [PATCH 13/14] :nail_care: cleanup `gist edit -a` feature --- pkg/cmd/gist/edit/edit.go | 40 +++++++++++++++------------------- pkg/cmd/gist/edit/edit_test.go | 31 +++++++++++++------------- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 5d8146f8b..238f9c64d 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -28,7 +28,8 @@ type EditOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) - Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Selector string EditFilename string AddFilename string @@ -62,7 +63,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman return editRun(&opts) }, } - cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a file") + + cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a new file to the gist") cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit") return cmd @@ -106,8 +108,7 @@ func editRun(opts *EditOptions) error { cs := opts.IO.ColorScheme() if addFilename != "" { - //Add files to an existing gist. - files, err := getFilesToAdd(addFilename, opts) + files, err := getFilesToAdd(addFilename) if err != nil { return err } @@ -118,10 +119,7 @@ func editRun(opts *EditOptions) error { return err } - completionMessage := filepath.Base(addFilename) + " added to gist" - - fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage) - + fmt.Fprintf(opts.IO.Out, "%s %s added to gist\n", cs.SuccessIconWithColor(cs.Green), filepath.Base(addFilename)) return nil } @@ -249,25 +247,21 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } -func getFilesToAdd(file string, opts *EditOptions) (map[string]*shared.GistFile, error) { - cs := opts.IO.ColorScheme() - - filesToAdd := map[string]*shared.GistFile{} - - filename := filepath.Base(file) - +func getFilesToAdd(file string) (map[string]*shared.GistFile, error) { content, err := ioutil.ReadFile(file) if err != nil { - return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) + return nil, fmt.Errorf("failed to read file %s: %w", file, err) } - if string(content) == "" { - return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) + if len(content) == 0 { + return nil, errors.New("file contents cannot be empty") } - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: string(content), - } - return filesToAdd, nil + filename := filepath.Base(file) + return map[string]*shared.GistFile{ + filename: { + Filename: filename, + Content: string(content), + }, + }, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index abbda4acd..e96c1fc28 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -16,25 +16,22 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" -) - -const ( - fixtureFile = "../fixture.txt" + "github.com/stretchr/testify/require" ) func Test_getFilesToAdd(t *testing.T) { - gf, err := getFilesToAdd(fixtureFile, &EditOptions{ - AddFilename: fixtureFile, - IO: &iostreams.IOStreams{}, - }) - if err != nil { - t.Fatalf("unexpected error processing files: %s", err) - } + fileToAdd := filepath.Join(t.TempDir(), "gist-test.txt") + err := ioutil.WriteFile(fileToAdd, []byte("hello"), 0600) + require.NoError(t, err) + gf, err := getFilesToAdd(fileToAdd) + require.NoError(t, err) + + filename := filepath.Base(fileToAdd) assert.Equal(t, map[string]*shared.GistFile{ - filepath.Base(fixtureFile): { - Filename: filepath.Base(fixtureFile), - Content: "{}", + filename: { + Filename: filename, + Content: "hello", }, }, gf) } @@ -98,6 +95,10 @@ func TestNewCmdEdit(t *testing.T) { } func Test_editRun(t *testing.T) { + fileToAdd := filepath.Join(t.TempDir(), "gist-test.txt") + err := ioutil.WriteFile(fileToAdd, []byte("hello"), 0600) + require.NoError(t, err) + tests := []struct { name string opts *EditOptions @@ -260,7 +261,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: fixtureFile, + AddFilename: fileToAdd, }, }, } From 8f96e406ac450bdaeda7129bf75ad2341701774b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 12:23:18 +0100 Subject: [PATCH 14/14] Improve error handling and avoid writing confirmation to stdout Right now the `gist edit` command doesn't write anything to stdout, so let's keep it that way until we want to intentionally provide some feedback in the terminal. --- pkg/cmd/gist/edit/edit.go | 22 ++++++++-------------- pkg/cmd/gist/edit/edit_test.go | 22 ++++++++++------------ pkg/cmd/gist/shared/shared.go | 7 +++++++ 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 238f9c64d..27df4a104 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -90,6 +90,9 @@ func editRun(opts *EditOptions) error { gist, err := shared.GetGist(client, ghinstance.OverridableDefault(), gistID) if err != nil { + if errors.Is(err, shared.NotFoundErr) { + return fmt.Errorf("gist not found: %s", gistID) + } return err } @@ -102,27 +105,18 @@ func editRun(opts *EditOptions) error { return fmt.Errorf("You do not own this gist.") } - filesToUpdate := map[string]string{} - - addFilename := opts.AddFilename - cs := opts.IO.ColorScheme() - - if addFilename != "" { - files, err := getFilesToAdd(addFilename) + if opts.AddFilename != "" { + files, err := getFilesToAdd(opts.AddFilename) if err != nil { return err } gist.Files = files - err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) - if err != nil { - return err - } - - fmt.Fprintf(opts.IO.Out, "%s %s added to gist\n", cs.SuccessIconWithColor(cs.Green), filepath.Base(addFilename)) - return nil + return updateGist(apiClient, ghinstance.OverridableDefault(), gist) } + filesToUpdate := map[string]string{} + for { filename := opts.EditFilename candidates := []string{} diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index e96c1fc28..73a53e523 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -106,13 +106,12 @@ func Test_editRun(t *testing.T) { httpStubs func(*httpmock.Registry) askStubs func(*prompt.AskStubber) nontty bool - wantErr bool - wantStderr string + wantErr string wantParams map[string]interface{} }{ { name: "no such gist", - wantErr: true, + wantErr: "gist not found: 1234", }, { name: "one file", @@ -195,7 +194,7 @@ func Test_editRun(t *testing.T) { as.StubOne("unix.md") as.StubOne("Cancel") }, - wantErr: true, + wantErr: "SilentError", gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ @@ -240,8 +239,7 @@ func Test_editRun(t *testing.T) { }, Owner: &shared.GistOwner{Login: "octocat2"}, }, - wantErr: true, - wantStderr: "You do not own this gist.", + wantErr: "You do not own this gist.", }, { name: "add file to existing gist", @@ -299,7 +297,7 @@ func Test_editRun(t *testing.T) { tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } - io, _, _, _ := iostreams.Test() + io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(!tt.nontty) io.SetStdinTTY(!tt.nontty) tt.opts.IO = io @@ -313,11 +311,8 @@ func Test_editRun(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := editRun(tt.opts) reg.Verify(t) - if tt.wantErr { - assert.Error(t, err) - if tt.wantStderr != "" { - assert.EqualError(t, err, tt.wantStderr) - } + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) return } assert.NoError(t, err) @@ -331,6 +326,9 @@ func Test_editRun(t *testing.T) { } assert.Equal(t, tt.wantParams, reqBody) } + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) }) } } diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 04e63ce86..04fe2c33b 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -1,6 +1,7 @@ package shared import ( + "errors" "fmt" "net/http" "net/url" @@ -31,6 +32,8 @@ type Gist struct { Owner *GistOwner `json:"owner,omitempty"` } +var NotFoundErr = errors.New("not found") + func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { gist := Gist{} path := fmt.Sprintf("gists/%s", gistID) @@ -38,6 +41,10 @@ func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { apiClient := api.NewClientFromHTTP(client) err := apiClient.REST(hostname, "GET", path, nil, &gist) if err != nil { + var httpErr api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { + return nil, NotFoundErr + } return nil, err }