From 3e3151f86ace9dec0af9f554cb0efaa6f43e1831 Mon Sep 17 00:00:00 2001 From: DerTeta Date: Sat, 25 Sep 2021 23:00:17 +0200 Subject: [PATCH] Fix: Don't access a view if it's `nil` The way the `if` expression in `deactivateContext` was composed, it was possible to have it to evaluate to `true` even though the `view` variable was `nil`. As far as I can tell, this seems to be only possible during tests. Nonetheless, I think the expression looks more "correct" this way. --- pkg/gui/context.go | 2 +- pkg/gui/context_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 pkg/gui/context_test.go diff --git a/pkg/gui/context.go b/pkg/gui/context.go index 66e28fc35..61c7e60a3 100644 --- a/pkg/gui/context.go +++ b/pkg/gui/context.go @@ -169,7 +169,7 @@ func (gui *Gui) deactivateContext(c Context) error { } // if we are the kind of context that is sent to back upon deactivation, we should do that - if view != nil && c.GetKind() == TEMPORARY_POPUP || c.GetKind() == PERSISTENT_POPUP || c.GetKey() == COMMIT_FILES_CONTEXT_KEY { + if view != nil && (c.GetKind() == TEMPORARY_POPUP || c.GetKind() == PERSISTENT_POPUP || c.GetKey() == COMMIT_FILES_CONTEXT_KEY) { view.Visible = false } diff --git a/pkg/gui/context_test.go b/pkg/gui/context_test.go new file mode 100644 index 000000000..abfcab4cd --- /dev/null +++ b/pkg/gui/context_test.go @@ -0,0 +1,40 @@ +package gui + +import ( + "testing" + + "github.com/jesseduffield/gocui" + "github.com/stretchr/testify/assert" +) + +func TestCanDeactivatePopupContextsWithoutViews(t *testing.T) { + contexts := []func(gui *Gui) Context { + func(gui *Gui) Context { return gui.State.Contexts.Credentials }, + func(gui *Gui) Context { return gui.State.Contexts.Confirmation }, + func(gui *Gui) Context { return gui.State.Contexts.CommitMessage }, + func(gui *Gui) Context { return gui.State.Contexts.Search }, + } + + for _, c := range contexts { + gui := NewDummyGui() + context := c(gui) + gui.g = &gocui.Gui{} + + gui.deactivateContext(context) + + // This really only checks a prerequisit, not the effect of deactivateContext + view, _ := gui.g.View(context.GetViewName()) + assert.Nil(t, view, string(context.GetKey())) + } +} + +func TestCanDeactivateCommitFilesContextsWithoutViews(t *testing.T) { + gui := NewDummyGui() + gui.g = &gocui.Gui{} + + gui.deactivateContext(gui.State.Contexts.CommitFiles) + + // This really only checks a prerequisite, not the effect of deactivateContext + view, _ := gui.g.View(gui.State.Contexts.CommitFiles.GetViewName()) + assert.Nil(t, view) +}