Simplify looking up binary types in gist

mislav · mislav · commit 77d9051d0e31 · 2021-03-02T14:53:10.000+01:00
diff --git a/pkg/cmd/gist/create/create.go b/pkg/cmd/gist/create/create.go
@@ -163,7 +163,6 @@ func processFiles(stdin io.ReadCloser, filenameOverride string, filenames []stri
 		var filename string
 		var content []byte
 		var err error
-		var isBinary bool
 
 		if f == "-" {
 			if filenameOverride != "" {
@@ -176,19 +175,22 @@ func processFiles(stdin io.ReadCloser, filenameOverride string, filenames []stri
 				return fs, fmt.Errorf("failed to read from stdin: %w", err)
 			}
 			stdin.Close()
+
+			if shared.IsBinaryContents(content) {
+				return nil, fmt.Errorf("binary file contents not supported")
+			}
 		} else {
-			content, err = ioutil.ReadFile(f)
+			isBinary, err := shared.IsBinaryFile(f)
 			if err != nil {
 				return fs, fmt.Errorf("failed to read file %s: %w", f, err)
 			}
-
-			isBinary, err = shared.IsBinaryContents(content)
-			if err != nil {
-				return nil, err
+			if isBinary {
+				return nil, fmt.Errorf("failed to upload %s: binary file not supported", f)
 			}
 
-			if isBinary {
-				return nil, fmt.Errorf("binary file not supported")
+			content, err = ioutil.ReadFile(f)
+			if err != nil {
+				return fs, fmt.Errorf("failed to read file %s: %w", f, err)
 			}
 
 			filename = path.Base(f)
diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go
@@ -144,31 +144,25 @@ func editRun(opts *EditOptions) error {
 			}
 		}
 
-		if _, ok := gist.Files[filename]; !ok {
+		gistFile, found := gist.Files[filename]
+		if !found {
 			return fmt.Errorf("gist has no file %q", filename)
 		}
-
-		isBinary, err := shared.IsBinaryContents([]byte(gist.Files[filename].Content))
-		if err != nil {
-			return err
-		}
-
-		if isBinary {
-			return fmt.Errorf("Editing binary files not supported")
+		if shared.IsBinaryContents([]byte(gistFile.Content)) {
+			return fmt.Errorf("editing binary files not supported")
 		}
 
 		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, filename, gistFile.Content, opts.IO)
 
 		if err != nil {
 			return err
 		}
 
-		if text != gist.Files[filename].Content {
-			gistFile := gist.Files[filename]
+		if text != gistFile.Content {
 			gistFile.Content = text // so it appears if they re-edit
 			filesToUpdate[filename] = text
 		}
diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go
@@ -4,17 +4,16 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/gabriel-vasile/mimetype"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
+	"github.com/cli/cli/api"
 	"github.com/cli/cli/internal/ghinstance"
+	"github.com/gabriel-vasile/mimetype"
 	"github.com/shurcooL/githubv4"
 	"github.com/shurcooL/graphql"
-
-	"github.com/cli/cli/api"
 )
 
 type GistFile struct {
@@ -156,26 +155,22 @@ func IsBinaryFile(file string) (bool, error) {
 	}
 
 	isBinary := true
-
 	for mime := detectedMime; mime != nil; mime = mime.Parent() {
 		if mime.Is("text/plain") {
 			isBinary = false
+			break
 		}
 	}
-
 	return isBinary, nil
 }
 
-func IsBinaryContents(contents []byte) (bool, error) {
-	detectedMime := mimetype.Detect(contents)
-
+func IsBinaryContents(contents []byte) bool {
 	isBinary := true
-
-	for mime := detectedMime; mime != nil; mime = mime.Parent() {
+	for mime := mimetype.Detect(contents); mime != nil; mime = mime.Parent() {
 		if mime.Is("text/plain") {
 			isBinary = false
+			break
 		}
 	}
-
-	return isBinary, nil
+	return isBinary
 }
diff --git a/pkg/cmd/gist/shared/shared_test.go b/pkg/cmd/gist/shared/shared_test.go
@@ -60,6 +60,14 @@ func TestIsBinaryContents(t *testing.T) {
 			want:        false,
 			fileContent: []byte("package main"),
 		},
+		{
+			want:        false,
+			fileContent: []byte(""),
+		},
+		{
+			want:        false,
+			fileContent: []byte(nil),
+		},
 		{
 			want: true,
 			fileContent: []byte{239, 191, 189, 239, 191, 189, 239, 191, 189, 239,
@@ -74,10 +82,6 @@ func TestIsBinaryContents(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		isBinary, err := IsBinaryContents(tt.fileContent)
-		if err != nil {
-			t.Fatal(err)
-		}
-		assert.Equal(t, tt.want, isBinary)
+		assert.Equal(t, tt.want, IsBinaryContents(tt.fileContent))
 	}
 }
diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go
@@ -118,13 +118,12 @@ func viewRun(opts *ViewOptions) error {
 	defer opts.IO.StopPager()
 
 	render := func(gf *shared.GistFile) error {
-		isBinary, err := shared.IsBinaryContents([]byte(gf.Content))
-		if err != nil {
-			return err
-		}
-
-		if isBinary {
-			gf.Content = "Skipping rendering binary content..."
+		if shared.IsBinaryContents([]byte(gf.Content)) {
+			if len(gist.Files) == 1 || opts.Filename != "" {
+				return fmt.Errorf("error: file is binary")
+			}
+			_, err = fmt.Fprintln(opts.IO.Out, cs.Gray("(skipping rendering binary content)"))
+			return nil
 		}
 
 		if strings.Contains(gf.Type, "markdown") && !opts.Raw {
@@ -152,16 +151,6 @@ func viewRun(opts *ViewOptions) error {
 		if !ok {
 			return fmt.Errorf("gist has no such file: %q", opts.Filename)
 		}
-
-		isBinary, err := shared.IsBinaryContents([]byte(gistFile.Content))
-		if err != nil {
-			return err
-		}
-
-		if isBinary {
-			return fmt.Errorf("Error: file contents is binary")
-		}
-
 		return render(gistFile)
 	}
 
@@ -175,17 +164,6 @@ func viewRun(opts *ViewOptions) error {
 		filenames = append(filenames, fn)
 	}
 
-	if len(filenames) == 1 {
-		isBinary, err := shared.IsBinaryContents([]byte(gist.Files[filenames[0]].Content))
-		if err != nil {
-			return err
-		}
-
-		if isBinary {
-			return fmt.Errorf("Error: file contents is binary")
-		}
-	}
-
 	sort.Strings(filenames)
 	if opts.ListFiles {
 		for _, fn := range filenames {

Original file line number	Diff line number	Diff line change
`@@ -144,31 +144,25 @@ func editRun(opts *EditOptions) error {`
`144`	`144`	`}`
`145`	`145`	`}`
`146`	`146`
`147`		`- if _, ok := gist.Files[filename]; !ok {`
	`147`	`+ gistFile, found := gist.Files[filename]`
	`148`	`+ if !found {`
`148`	`149`	`return fmt.Errorf("gist has no file %q", filename)`
`149`	`150`	`}`
`150`		`-`
`151`		`- isBinary, err := shared.IsBinaryContents([]byte(gist.Files[filename].Content))`
`152`		`- if err != nil {`
`153`		`- return err`
`154`		`- }`
`155`		`-`
`156`		`- if isBinary {`
`157`		`- return fmt.Errorf("Editing binary files not supported")`
	`151`	`+ if shared.IsBinaryContents([]byte(gistFile.Content)) {`
	`152`	`+ return fmt.Errorf("editing binary files not supported")`
`158`	`153`	`}`
`159`	`154`
`160`	`155`	`editorCommand, err := cmdutil.DetermineEditor(opts.Config)`
`161`	`156`	`if err != nil {`
`162`	`157`	`return err`
`163`	`158`	`}`
`164`		`- text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO)`
	`159`	`+ text, err := opts.Edit(editorCommand, filename, gistFile.Content, opts.IO)`
`165`	`160`
`166`	`161`	`if err != nil {`
`167`	`162`	`return err`
`168`	`163`	`}`
`169`	`164`
`170`		`- if text != gist.Files[filename].Content {`
`171`		`- gistFile := gist.Files[filename]`
	`165`	`+ if text != gistFile.Content {`
`172`	`166`	`gistFile.Content = text // so it appears if they re-edit`
`173`	`167`	`filesToUpdate[filename] = text`
`174`	`168`	`}`