Skip to content

Commit 4ed1014

Browse files
g14ag14a
committed
Resolved PR review comments and test cases
1 parent 882bd1a commit 4ed1014

File tree

2 files changed

+155
-142
lines changed

2 files changed

+155
-142
lines changed

‎pkg/cmd/gist/edit/edit.go‎

Lines changed: 135 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
5959

6060
cmd := &cobra.Command{
6161
Use: "edit {<id> | <url>}",
62-
Short: "Edit one of your gists or add new ones to it",
62+
Short: "Edit or add files in a gist",
6363
Args: cmdutil.MinimumArgs(1, "cannot edit: gist argument required"),
6464
RunE: func(c *cobra.Command, args []string) error {
6565
opts.Selector = args[0]
@@ -116,147 +116,109 @@ func editRun(opts *EditOptions) error {
116116

117117
if addFilename != "" {
118118
//Add files to an existing gist.
119-
filenamePath, fileExists, err := processFiles(addFilename)
119+
files, err := getFilesToAdd(addFilename, opts)
120120
if err != nil {
121-
return fmt.Errorf("%s %s", cs.Red("!"), err)
121+
return err
122122
}
123123

124-
filesToAdd := map[string]*shared.GistFile{}
125-
126-
filename := filepath.Base(filenamePath)
127-
128-
if fileExists {
129-
content, err := ioutil.ReadFile(filenamePath)
130-
if err != nil {
131-
return fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), addFilename, err)
132-
}
133-
134-
if string(content) == "" {
135-
return fmt.Errorf("%s Contents can't be empty", cs.FailureIcon())
136-
}
137-
138-
filesToAdd[filename] = &shared.GistFile{
139-
Filename: filename,
140-
Content: string(content),
141-
}
142-
gist.Files = filesToAdd
143-
} else {
144-
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
145-
if err != nil {
146-
return err
147-
}
148-
149-
text, err := opts.Add(editorCommand, filename, opts.IO)
150-
if err != nil {
151-
return err
152-
}
153-
154-
if text == "" {
155-
return fmt.Errorf("%s Contents can't be empty", cs.Red("!"))
156-
}
157-
158-
filesToAdd[filename] = &shared.GistFile{
159-
Filename: filename,
160-
Content: text,
161-
}
162-
163-
gist.Files = filesToAdd
164-
}
124+
fmt.Printf("%v", files)
165125

126+
gist.Files = files
166127
err = updateGist(apiClient, ghinstance.OverridableDefault(), gist)
167128
if err != nil {
168129
return err
169130
}
170131

171-
completionMessage := filename + " added to gist"
132+
completionMessage := filepath.Base(addFilename) + " added to gist"
172133

173134
fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage)
174135

175-
} else {
176-
for {
177-
filename := opts.EditFilename
178-
candidates := []string{}
179-
for filename := range gist.Files {
180-
candidates = append(candidates, filename)
181-
}
136+
return nil
137+
}
182138

183-
sort.Strings(candidates)
184-
185-
if filename == "" {
186-
if len(candidates) == 1 {
187-
filename = candidates[0]
188-
} else {
189-
if !opts.IO.CanPrompt() {
190-
return errors.New("unsure what file to edit; either specify --filename or run interactively")
191-
}
192-
err = prompt.SurveyAskOne(&survey.Select{
193-
Message: "Edit which file?",
194-
Options: candidates,
195-
}, &filename)
196-
197-
if err != nil {
198-
return fmt.Errorf("could not prompt: %w", err)
199-
}
139+
for {
140+
filename := opts.EditFilename
141+
candidates := []string{}
142+
for filename := range gist.Files {
143+
candidates = append(candidates, filename)
144+
}
145+
146+
sort.Strings(candidates)
147+
148+
if filename == "" {
149+
if len(candidates) == 1 {
150+
filename = candidates[0]
151+
} else {
152+
if !opts.IO.CanPrompt() {
153+
return errors.New("unsure what file to edit; either specify --filename or run interactively")
200154
}
201-
}
155+
err = prompt.SurveyAskOne(&survey.Select{
156+
Message: "Edit which file?",
157+
Options: candidates,
158+
}, &filename)
202159

203-
if _, ok := gist.Files[filename]; !ok {
204-
return fmt.Errorf("gist has no file %q", filename)
160+
if err != nil {
161+
return fmt.Errorf("could not prompt: %w", err)
162+
}
205163
}
164+
}
206165

207-
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
208-
if err != nil {
209-
return err
210-
}
211-
text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO)
166+
if _, ok := gist.Files[filename]; !ok {
167+
return fmt.Errorf("gist has no file %q", filename)
168+
}
212169

213-
if err != nil {
214-
return err
215-
}
170+
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
171+
if err != nil {
172+
return err
173+
}
174+
text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO)
216175

217-
if text != gist.Files[filename].Content {
218-
gistFile := gist.Files[filename]
219-
gistFile.Content = text // so it appears if they re-edit
220-
filesToUpdate[filename] = text
221-
}
176+
if err != nil {
177+
return err
178+
}
222179

223-
if !opts.IO.CanPrompt() {
224-
break
225-
}
180+
if text != gist.Files[filename].Content {
181+
gistFile := gist.Files[filename]
182+
gistFile.Content = text // so it appears if they re-edit
183+
filesToUpdate[filename] = text
184+
}
226185

227-
if len(candidates) == 1 {
228-
break
229-
}
186+
if !opts.IO.CanPrompt() {
187+
break
188+
}
230189

231-
choice := ""
190+
if len(candidates) == 1 {
191+
break
192+
}
232193

233-
err = prompt.SurveyAskOne(&survey.Select{
234-
Message: "What next?",
235-
Options: []string{
236-
"Edit another file",
237-
"Submit",
238-
"Cancel",
239-
},
240-
}, &choice)
194+
choice := ""
241195

242-
if err != nil {
243-
return fmt.Errorf("could not prompt: %w", err)
244-
}
196+
err = prompt.SurveyAskOne(&survey.Select{
197+
Message: "What next?",
198+
Options: []string{
199+
"Edit another file",
200+
"Submit",
201+
"Cancel",
202+
},
203+
}, &choice)
245204

246-
stop := false
205+
if err != nil {
206+
return fmt.Errorf("could not prompt: %w", err)
207+
}
247208

248-
switch choice {
249-
case "Edit another file":
250-
continue
251-
case "Submit":
252-
stop = true
253-
case "Cancel":
254-
return cmdutil.SilentError
255-
}
209+
stop := false
256210

257-
if stop {
258-
break
259-
}
211+
switch choice {
212+
case "Edit another file":
213+
continue
214+
case "Submit":
215+
stop = true
216+
case "Cancel":
217+
return cmdutil.SilentError
218+
}
219+
220+
if stop {
221+
break
260222
}
261223
}
262224

@@ -298,35 +260,73 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error
298260
return nil
299261
}
300262

301-
func processFiles(filename string) (string, bool, error) {
302-
fileExists := false
263+
func getFilesToAdd(file string, opts *EditOptions) (map[string]*shared.GistFile, error) {
264+
cs := opts.IO.ColorScheme()
265+
266+
fileExists, err := fileExists(file)
267+
if err != nil {
268+
return nil, fmt.Errorf("%s %s", cs.Red("!"), err)
269+
}
270+
271+
filesToAdd := map[string]*shared.GistFile{}
272+
273+
filename := filepath.Base(file)
303274

304-
if fi, err := os.Stat(filename); err != nil {
275+
if fileExists {
276+
content, err := ioutil.ReadFile(file)
277+
if err != nil {
278+
return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err)
279+
}
280+
281+
if string(content) == "" {
282+
return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon())
283+
}
284+
285+
filesToAdd[filename] = &shared.GistFile{
286+
Filename: filename,
287+
Content: string(content),
288+
}
289+
return filesToAdd, nil
305290
} else {
306-
switch mode := fi.Mode(); {
307-
case mode.IsDir():
308-
return "", false, fmt.Errorf("found directory %s", filename)
309-
case mode.IsRegular():
310-
fileExists = true
291+
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
292+
if err != nil {
293+
return nil, err
311294
}
312-
}
313295

314-
wd, err := os.Getwd()
315-
if err != nil {
316-
return "", false, err
296+
text, err := opts.Add(editorCommand, filename, opts.IO)
297+
if err != nil {
298+
return nil, err
299+
}
300+
301+
if text == "" {
302+
return nil, fmt.Errorf("%s Contents can't be empty", cs.Red("!"))
303+
}
304+
305+
filesToAdd[filename] = &shared.GistFile{
306+
Filename: filename,
307+
Content: text,
308+
}
309+
310+
return filesToAdd, nil
317311
}
312+
}
313+
314+
func fileExists(filename string) (bool, error) {
315+
316+
fi, err := os.Stat(filename)
318317

319-
m, err := filepath.Glob(wd + "/[^.]*.*")
320318
if err != nil {
321-
return "", false, err
319+
if os.IsNotExist(err) {
320+
return false, nil
321+
}
322322
}
323323

324-
for _, f := range m {
325-
if filename == filepath.Base(f) {
326-
fileExists = true
327-
break
328-
}
324+
switch mode := fi.Mode(); {
325+
case mode.IsDir():
326+
return false, fmt.Errorf("found directory %s", filename)
327+
case mode.IsRegular():
328+
return true, nil
329329
}
330330

331-
return filename, fileExists, nil
331+
return false, nil
332332
}

‎pkg/cmd/gist/edit/edit_test.go‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"io/ioutil"
77
"net/http"
8+
"path/filepath"
89
"testing"
910

1011
"github.com/cli/cli/internal/config"
@@ -22,22 +23,34 @@ const (
2223
nonExistentFile = "../file.txt"
2324
)
2425

25-
func Test_processFiles(t *testing.T) {
26-
filePath, fileExists, err := processFiles(fixtureFile)
26+
func Test_fileExists(t *testing.T) {
27+
fixtureFileExists, err := fileExists(fixtureFile)
2728
if err != nil {
2829
t.Fatalf("unexpected error processing files: %s", err)
2930
}
3031

31-
assert.Equal(t, "../fixture.txt", filePath)
32-
assert.Equal(t, true, fileExists)
32+
assert.Equal(t, true, fixtureFileExists)
3333

34-
filePath, fileExists, err = processFiles(nonExistentFile)
34+
neFileExists, err := fileExists(nonExistentFile)
35+
assert.Equal(t, nil, err)
36+
assert.Equal(t, false, neFileExists)
37+
}
38+
39+
func Test_getFilesToAdd(t *testing.T) {
40+
gf, err := getFilesToAdd(fixtureFile, &EditOptions{
41+
AddFilename: fixtureFile,
42+
IO: &iostreams.IOStreams{},
43+
})
3544
if err != nil {
3645
t.Fatalf("unexpected error processing files: %s", err)
3746
}
3847

39-
assert.Equal(t, "../file.txt", filePath)
40-
assert.Equal(t, false, fileExists)
48+
assert.Equal(t, map[string]*shared.GistFile{
49+
filepath.Base(fixtureFile): {
50+
Filename: filepath.Base(fixtureFile),
51+
Content: "{}",
52+
},
53+
}, gf)
4154
}
4255

4356
func TestNewCmdEdit(t *testing.T) {

0 commit comments

Comments
 (0)