Merge pull request #228 from jesseduffield/hotfix/226-dont-panic-when-catting

226: dont panic when catting directories

main.go

@@ -45,11 +45,8 @@ func main() {
 
 	app, err := app.NewApp(appConfig)
 	if err != nil {
-		// TODO: remove this call to panic after anonymous error reporting
-		// is setup (right now the call to panic logs nothing to the screen which
-		// would make debugging difficult
+		app.Log.Error(err.Error())
 		panic(err)
-		// app.Log.Panic(err.Error())
 	}
 	app.GitCommand.SetupGit()
 	app.Gui.RunWithSubprocesses()

pkg/commands/git.go

@@ -86,16 +86,17 @@ func (c *GitCommand) GetStatusFiles() []File {
 		change := statusString[0:2]
 		stagedChange := change[0:1]
 		unstagedChange := statusString[1:2]
-		filename := statusString[3:]
+		filename := c.OSCommand.Unquote(statusString[3:])
 		tracked := !includes([]string{"??", "A ", "AM"}, change)
 		file := File{
-			Name:               c.OSCommand.Unquote(filename),
+			Name:               filename,
 			DisplayString:      statusString,
 			HasStagedChanges:   !includes([]string{" ", "U", "?"}, stagedChange),
 			HasUnstagedChanges: unstagedChange != " ",
 			Tracked:            tracked,
 			Deleted:            unstagedChange == "D" || stagedChange == "D",
 			HasMergeConflicts:  change == "UU",
+			Type:               c.OSCommand.FileType(filename),
 		}
 		files = append(files, file)
 	}

pkg/commands/git_structs.go

@@ -1,7 +1,6 @@
 package commands
 
 // File : A staged/unstaged file
-// TODO: decide whether to give all of these the Git prefix
 type File struct {
 	Name               string
 	HasStagedChanges   bool
@@ -10,6 +9,7 @@ type File struct {
 	Deleted            bool
 	HasMergeConflicts  bool
 	DisplayString      string
+	Type               string // one of 'file', 'directory', and 'other'
 }
 
 // Commit : A git commit

pkg/commands/os.go

@@ -59,6 +59,18 @@ func (c *OSCommand) RunCommand(command string) error {
 	return err
 }
 
+// FileType tells us if the file is a file, directory or other
+func (c *OSCommand) FileType(path string) string {
+	fileInfo, err := os.Stat(path)
+	if err != nil {
+		return "other"
+	}
+	if fileInfo.IsDir() {
+		return "directory"
+	}
+	return "file"
+}
+
 // RunDirectCommand wrapper around direct commands
 func (c *OSCommand) RunDirectCommand(command string) (string, error) {
 	c.Log.WithField("command", command).Info("RunDirectCommand")

pkg/commands/os_test.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"os"
 	"os/exec"
 	"testing"
 
@@ -297,3 +298,60 @@ func TestOSCommandUnquote(t *testing.T) {
 
 	assert.EqualValues(t, expected, actual)
 }
+
+func TestOSCommandFileType(t *testing.T) {
+	type scenario struct {
+		path  string
+		setup func()
+		test  func(string)
+	}
+
+	scenarios := []scenario{
+		{
+			"testFile",
+			func() {
+				if _, err := os.Create("testFile"); err != nil {
+					panic(err)
+				}
+			},
+			func(output string) {
+				assert.EqualValues(t, "file", output)
+			},
+		},
+		{
+			"file with spaces",
+			func() {
+				if _, err := os.Create("file with spaces"); err != nil {
+					panic(err)
+				}
+			},
+			func(output string) {
+				assert.EqualValues(t, "file", output)
+			},
+		},
+		{
+			"testDirectory",
+			func() {
+				if err := os.Mkdir("testDirectory", 0644); err != nil {
+					panic(err)
+				}
+			},
+			func(output string) {
+				assert.EqualValues(t, "directory", output)
+			},
+		},
+		{
+			"nonExistant",
+			func() {},
+			func(output string) {
+				assert.EqualValues(t, "other", output)
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		s.setup()
+		s.test(newDummyOSCommand().FileType(s.path))
+		_ = os.RemoveAll(s.path)
+	}
+}

pkg/gui/files_panel.go

@@ -341,9 +341,13 @@ func (gui *Gui) catSelectedFile(g *gocui.Gui) (string, error) {
 		}
 		return "", gui.renderString(g, "main", gui.Tr.SLocalize("NoFilesDisplay"))
 	}
+	if item.Type != "file" {
+		return "", gui.renderString(g, "main", gui.Tr.SLocalize("NotAFile"))
+	}
 	cat, err := gui.GitCommand.CatFile(item.Name)
 	if err != nil {
-		panic(err)
+		gui.Log.Error(err)
+		return "", gui.renderString(g, "main", err.Error())
 	}
 	return cat, nil
 }

pkg/gui/merge_panel.go

@@ -180,6 +180,9 @@ func (gui *Gui) refreshMergePanel(g *gocui.Gui) error {
 	if err != nil {
 		return err
 	}
+	if cat == "" {
+		return nil
+	}
 	gui.State.Conflicts, err = gui.findConflicts(cat)
 	if err != nil {
 		return err

pkg/i18n/english.go

@@ -105,6 +105,9 @@ func addEnglish(i18nObject *i18n.Bundle) error {
 		}, &i18n.Message{
 			ID:    "NoFilesDisplay",
 			Other: "No file to display",
+		}, &i18n.Message{
+			ID:    "NotAFile",
+			Other: "Not a file",
 		}, &i18n.Message{
 			ID:    "PullWait",
 			Other: "Pulling...",

test/repos/merge_conflict.sh

@@ -13,9 +13,15 @@ function add_spacing {
   done
 }
 
+mkdir directory
+echo "test1" > directory/file
+echo "test1" > directory/file2
+
+
 echo "Here is a story that has been told throuhg the ages" >> file1
 
 git add file1
+git add directory
 git commit -m "first commit"
 
 git checkout -b develop
@@ -24,6 +30,11 @@ echo "once upon a time there was a dog" >> file1
 add_spacing file1
 echo "once upon a time there was another dog" >> file1
 git add file1
+
+echo "test2" > directory/file
+echo "test2" > directory/file2
+git add directory
+
 git commit -m "first commit on develop"
 
 git checkout master
@@ -32,6 +43,11 @@ echo "once upon a time there was a cat" >> file1
 add_spacing file1
 echo "once upon a time there was another cat" >> file1
 git add file1
+
+echo "test3" > directory/file
+echo "test3" > directory/file2
+git add directory
+
 git commit -m "first commit on develop"
 
 git merge develop # should have a merge conflict here