Improve error handling and avoid writing confirmation to stdout

mislav · mislav · commit 8f96e406ac45 · 2021-02-27T12:23:18.000+01:00
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.
diff --git 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
@@ -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
@@ -1,6 +1,7 @@
 package shared
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -31,13 +32,19 @@ 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)
 
 	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
 	}
 
