From 7e1e97d050dd38b08e0131776bf130bae1fb1a4c Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Tue, 28 Aug 2018 19:12:35 +1000 Subject: dont panic when catting directories --- main.go | 5 +--- pkg/commands/git.go | 5 ++-- pkg/commands/git_structs.go | 2 +- pkg/commands/os.go | 12 +++++++++ pkg/commands/os_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++ pkg/gui/files_panel.go | 6 ++++- pkg/gui/merge_panel.go | 3 +++ pkg/i18n/english.go | 3 +++ test/repos/merge_conflict.sh | 16 ++++++++++++ 9 files changed, 102 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index 9b3ad9edf..4303af02f 100644 --- a/main.go +++ b/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() diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 1e36aa84c..b56650e97 100644 --- a/pkg/commands/git.go +++ b/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) } diff --git a/pkg/commands/git_structs.go b/pkg/commands/git_structs.go index 6b10b18bb..2b3b3f2db 100644 --- a/pkg/commands/git_structs.go +++ b/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 diff --git a/pkg/commands/os.go b/pkg/commands/os.go index 10f78c7c3..0fa92f724 100644 --- a/pkg/commands/os.go +++ b/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") diff --git a/pkg/commands/os_test.go b/pkg/commands/os_test.go index d78391d17..a7ccef560 100644 --- a/pkg/commands/os_test.go +++ b/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) + } +} diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go index 907f14204..e7fbc5baf 100644 --- a/pkg/gui/files_panel.go +++ b/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 } diff --git a/pkg/gui/merge_panel.go b/pkg/gui/merge_panel.go index 1efea16da..e3da82f3c 100644 --- a/pkg/gui/merge_panel.go +++ b/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 diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 718e84c6f..f6e865b7b 100644 --- a/pkg/i18n/english.go +++ b/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...", diff --git a/test/repos/merge_conflict.sh b/test/repos/merge_conflict.sh index 51e4c1ce6..357597719 100755 --- a/test/repos/merge_conflict.sh +++ b/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 -- cgit v1.2.3