avoid deadlock in merge panel

9 files changed, 79 insertions, 59 deletions

diff --git a/pkg/gui/diff_context_size.go b/pkg/gui/diff_context_size.go
index 68a7bf14b..ff7786c8a 100644
--- a/pkg/gui/diff_context_size.go
+++ b/pkg/gui/diff_context_size.go
@@ -4,10 +4,25 @@ import (
 	"errors"
 )
 
+var CONTEXT_KEYS_SHOWING_DIFFS = []ContextKey{
+	FILES_CONTEXT_KEY,
+	COMMIT_FILES_CONTEXT_KEY,
+	STASH_CONTEXT_KEY,
+	BRANCH_COMMITS_CONTEXT_KEY,
+	SUB_COMMITS_CONTEXT_KEY,
+	MAIN_STAGING_CONTEXT_KEY,
+	MAIN_PATCH_BUILDING_CONTEXT_KEY,
+}
+
 func isShowingDiff(gui *Gui) bool {
 	key := gui.currentStaticContext().GetKey()
 
-	return key == FILES_CONTEXT_KEY || key == COMMIT_FILES_CONTEXT_KEY || key == STASH_CONTEXT_KEY || key == BRANCH_COMMITS_CONTEXT_KEY || key == SUB_COMMITS_CONTEXT_KEY || key == MAIN_STAGING_CONTEXT_KEY || key == MAIN_PATCH_BUILDING_CONTEXT_KEY
+	for _, contextKey := range CONTEXT_KEYS_SHOWING_DIFFS {
+		if key == contextKey {
+			return true
+		}
+	}
+	return false
 }
 
 func (gui *Gui) IncreaseContextInDiffView() error {
diff --git a/pkg/gui/diff_context_size_test.go b/pkg/gui/diff_context_size_test.go
index 11bd0712b..b459e40d0 100644
--- a/pkg/gui/diff_context_size_test.go
+++ b/pkg/gui/diff_context_size_test.go
@@ -65,7 +65,9 @@ func TestDoesntIncreaseContextInDiffViewInContextWithoutDiff(t *testing.T) {
 		func(gui *Gui) Context { return gui.State.Contexts.ReflogCommits },
 		func(gui *Gui) Context { return gui.State.Contexts.RemoteBranches },
 		func(gui *Gui) Context { return gui.State.Contexts.Tags },
-		func(gui *Gui) Context { return gui.State.Contexts.Merging },
+		// not testing this because it will kick straight back to the files context
+		// upon pushing the context
+		// func(gui *Gui) Context { return gui.State.Contexts.Merging },
 		func(gui *Gui) Context { return gui.State.Contexts.CommandLog },
 	}
 
@@ -115,7 +117,9 @@ func TestDoesntDecreaseContextInDiffViewInContextWithoutDiff(t *testing.T) {
 		func(gui *Gui) Context { return gui.State.Contexts.ReflogCommits },
 		func(gui *Gui) Context { return gui.State.Contexts.RemoteBranches },
 		func(gui *Gui) Context { return gui.State.Contexts.Tags },
-		func(gui *Gui) Context { return gui.State.Contexts.Merging },
+		// not testing this because it will kick straight back to the files context
+		// upon pushing the context
+		// func(gui *Gui) Context { return gui.State.Contexts.Merging },
 		func(gui *Gui) Context { return gui.State.Contexts.CommandLog },
 	}
 
diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go
index 05bf8d4ed..d121c8275 100644
--- a/pkg/gui/files_panel.go
+++ b/pkg/gui/files_panel.go
@@ -54,7 +54,7 @@ func (gui *Gui) filesRenderToMain() error {
 	}
 
 	if node.File != nil && node.File.HasInlineMergeConflicts {
-		return gui.refreshMergePanelWithLock()
+		return gui.renderConflictsFromFilesPanel()
 	}
 
 	cmdObj := gui.Git.WorkingTree.WorktreeFileDiffCmdObj(node, false, !node.GetHasUnstagedChanges() && node.GetHasStagedChanges(), gui.State.IgnoreWhitespaceInDiffView)
@@ -182,7 +182,7 @@ func (gui *Gui) enterFile(opts OnFocusOpts) error {
 	}
 
 	if file.HasInlineMergeConflicts {
-		return gui.handleSwitchToMerge()
+		return gui.switchToMerge()
 	}
 	if file.HasMergeConflicts {
 		return gui.createErrorPanel(gui.Tr.FileStagingRequirements)
@@ -201,7 +201,7 @@ func (gui *Gui) handleFilePress() error {
 		file := node.File
 
 		if file.HasInlineMergeConflicts {
-			return gui.handleSwitchToMerge()
+			return gui.switchToMerge()
 		}
 
 		if file.HasUnstagedChanges {
@@ -880,16 +880,12 @@ func (gui *Gui) upstreamForBranchInConfig(branchName string) (string, string, er
 	return "", "", nil
 }
 
-func (gui *Gui) handleSwitchToMerge() error {
+func (gui *Gui) switchToMerge() error {
 	file := gui.getSelectedFile()
 	if file == nil {
 		return nil
 	}
 
-	if !file.HasInlineMergeConflicts {
-		return gui.createErrorPanel(gui.Tr.FileNoMergeCons)
-	}
-
 	return gui.pushContext(gui.State.Contexts.Merging)
 }
 
diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go
index be17d352d..b32b3bf44 100644
--- a/pkg/gui/menu_panel.go
+++ b/pkg/gui/menu_panel.go
@@ -39,7 +39,7 @@ func (gui *Gui) getMenuOptions() map[string]string {
 }
 
 func (gui *Gui) handleMenuClose() error {
-	return gui.returnFromContextSync()
+	return gui.returnFromContext()
 }
 
 type createMenuOptions struct {
diff --git a/pkg/gui/merge_panel.go b/pkg/gui/merge_panel.go
index 5b7502f64..942340286 100644
--- a/pkg/gui/merge_panel.go
+++ b/pkg/gui/merge_panel.go
@@ -90,6 +90,7 @@ func (gui *Gui) handlePickHunk() error {
 			if err := gui.handleCompleteMerge(); err != nil {
 				return err
 			}
+			return nil
 		}
 		return gui.refreshMergePanel()
 	})
@@ -151,11 +152,19 @@ func (gui *Gui) refreshMergePanelWithLock() error {
 	return gui.withMergeConflictLock(gui.refreshMergePanel)
 }
 
-func (gui *Gui) refreshMergePanel() error {
-	panelState := gui.State.Panels.Merging
+// not re-using state here because we can run into issues with mutexes when
+// doing that.
+func (gui *Gui) renderConflictsFromFilesPanel() error {
+	state := mergeconflicts.NewState()
+	_, err := gui.renderConflicts(state, false)
+
+	return err
+}
+
+func (gui *Gui) renderConflicts(state *mergeconflicts.State, hasFocus bool) (bool, error) {
 	cat, err := gui.catSelectedFile()
 	if err != nil {
-		return gui.refreshMainViews(refreshMainOpts{
+		return false, gui.refreshMainViews(refreshMainOpts{
 			main: &viewUpdateOpts{
 				title: "",
 				task:  NewRenderStringTask(err.Error()),
@@ -163,20 +172,20 @@ func (gui *Gui) refreshMergePanel() error {
 		})
 	}
 
-	panelState.SetConflictsFromCat(cat)
+	state.SetConflictsFromCat(cat)
 
-	if panelState.NoConflicts() {
-		return gui.handleCompleteMerge()
+	if state.NoConflicts() {
+		// we shouldn't end up here
+		return false, nil
 	}
 
-	hasFocus := gui.currentViewName() == "main"
-	content := mergeconflicts.ColoredConflictFile(cat, panelState.State, hasFocus)
+	content := mergeconflicts.ColoredConflictFile(cat, state, hasFocus)
 
-	if err := gui.scrollToConflict(); err != nil {
-		return err
+	if !gui.State.Panels.Merging.UserVerticalScrolling {
+		gui.centerYPos(gui.Views.Main, state.GetConflictMiddle())
 	}
 
-	return gui.refreshMainViews(refreshMainOpts{
+	return true, gui.refreshMainViews(refreshMainOpts{
 		main: &viewUpdateOpts{
 			title:  gui.Tr.MergeConflictsTitle,
 			task:   NewRenderStringWithoutScrollTask(content),
@@ -185,6 +194,19 @@ func (gui *Gui) refreshMergePanel() error {
 	})
 }
 
+func (gui *Gui) refreshMergePanel() error {
+	conflictsFound, err := gui.renderConflicts(gui.State.Panels.Merging.State, true)
+	if err != nil {
+		return err
+	}
+
+	if !conflictsFound {
+		return gui.handleCompleteMerge()
+	}
+
+	return nil
+}
+
 func (gui *Gui) catSelectedFile() (string, error) {
 	item := gui.getSelectedFile()
 	if item == nil {
@@ -203,21 +225,6 @@ func (gui *Gui) catSelectedFile() (string, error) {
 	return cat, nil
 }
 
-func (gui *Gui) scrollToConflict() error {
-	if gui.State.Panels.Merging.UserVerticalScrolling {
-		return nil
-	}
-
-	panelState := gui.State.Panels.Merging
-	if panelState.NoConflicts() {
-		return nil
-	}
-
-	gui.centerYPos(gui.Views.Main, panelState.GetConflictMiddle())
-
-	return nil
-}
-
 func (gui *Gui) centerYPos(view *gocui.View, y int) {
 	ox, _ := view.Origin()
 	_, height := view.Size()
@@ -238,18 +245,11 @@ func (gui *Gui) getMergingOptions() map[string]string {
 }
 
 func (gui *Gui) handleEscapeMerge() error {
-	gui.takeOverMergeConflictScrolling()
-
-	gui.State.Panels.Merging.Reset()
 	if err := gui.refreshSidePanels(refreshOptions{scope: []RefreshableView{FILES}}); err != nil {
 		return err
 	}
-	// it's possible this method won't be called from the merging view so we need to
-	// ensure we only 'return' focus if we already have it
-	if gui.g.CurrentView() == gui.Views.Main {
-		return gui.pushContext(gui.State.Contexts.Files)
-	}
-	return nil
+
+	return gui.escapeMerge()
 }
 
 func (gui *Gui) handleCompleteMerge() error {
@@ -259,16 +259,26 @@ func (gui *Gui) handleCompleteMerge() error {
 	if err := gui.refreshSidePanels(refreshOptions{scope: []RefreshableView{FILES}}); err != nil {
 		return err
 	}
-	// if we got conflicts after unstashing, we don't want to call any git
-	// commands to continue rebasing/merging here
-	if gui.Git.Status.WorkingTreeState() == enums.REBASE_MODE_NONE {
-		return gui.handleEscapeMerge()
-	}
+
 	// if there are no more files with merge conflicts, we should ask whether the user wants to continue
-	if !gui.anyFilesWithMergeConflicts() {
+	if gui.Git.Status.WorkingTreeState() != enums.REBASE_MODE_NONE && !gui.anyFilesWithMergeConflicts() {
 		return gui.promptToContinueRebase()
 	}
-	return gui.handleEscapeMerge()
+
+	return gui.escapeMerge()
+}
+
+func (gui *Gui) escapeMerge() error {
+	gui.takeOverMergeConflictScrolling()
+
+	gui.State.Panels.Merging.Reset()
+
+	// it's possible this method won't be called from the merging view so we need to
+	// ensure we only 'return' focus if we already have it
+	if gui.g.CurrentView() == gui.Views.Main {
+		return gui.pushContext(gui.State.Contexts.Files)
+	}
+	return nil
 }
 
 // promptToContinueRebase asks the user if they want to continue the rebase/merge that's in progress
diff --git a/pkg/i18n/chinese.go b/pkg/i18n/chinese.go
index 3167e0bf8..585d77b38 100644
--- a/pkg/i18n/chinese.go
+++ b/pkg/i18n/chinese.go
@@ -73,7 +73,6 @@ func chineseTranslationSet() TranslationSet {
 		PullWait:                            "拉取中……",
 		PushWait:                            "推送中……",
 		FetchWait:                           "正在抓取……",
-		FileNoMergeCons:                     "该文件没有合并冲突",
 		LcSoftReset:                         "软重置",
 		AlreadyCheckedOutBranch:             "您已经检出了这个分支",
 		SureForceCheckout:                   "您确定要强制检出吗？您将丢失所有本地更改",
diff --git a/pkg/i18n/dutch.go b/pkg/i18n/dutch.go
index 914d558fe..7e8b2dcec 100644
--- a/pkg/i18n/dutch.go
+++ b/pkg/i18n/dutch.go
@@ -44,7 +44,6 @@ func dutchTranslationSet() TranslationSet {
 		PullWait:                            "Pullen...",
 		PushWait:                            "Pushen...",
 		FetchWait:                           "Fetchen...",
-		FileNoMergeCons:                     "Dit bestand heeft geen merge conflicten",
 		LcSoftReset:                         "zacht reset",
 		AlreadyCheckedOutBranch:             "Je hebt deze branch al uitgecheckt",
 		SureForceCheckout:                   "Weet je zeker dat je het uitchecken wil forceren? Al je lokale verandering zullen worden verwijdert",
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index 2defb4ae6..4ff6c3555 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -58,7 +58,6 @@ type TranslationSet struct {
 	PullWait                            string
 	PushWait                            string
 	FetchWait                           string
-	FileNoMergeCons                     string
 	LcSoftReset                         string
 	AlreadyCheckedOutBranch             string
 	SureForceCheckout                   string
@@ -608,7 +607,6 @@ func EnglishTranslationSet() TranslationSet {
 		PullWait:                            "Pulling...",
 		PushWait:                            "Pushing...",
 		FetchWait:                           "Fetching...",
-		FileNoMergeCons:                     "This file has no inline merge conflicts",
 		LcSoftReset:                         "soft reset",
 		AlreadyCheckedOutBranch:             "You have already checked out this branch",
 		SureForceCheckout:                   "Are you sure you want force checkout? You will lose all local changes",
diff --git a/pkg/i18n/polish.go b/pkg/i18n/polish.go
index 5994a59e9..f378fa08b 100644
--- a/pkg/i18n/polish.go
+++ b/pkg/i18n/polish.go
@@ -39,7 +39,6 @@ func polishTranslationSet() TranslationSet {
 		PullWait:                            "Pobieranie zmian...",
 		PushWait:                            "Wysyłanie zmian...",
 		FetchWait:                           "Pobieram...",
-		FileNoMergeCons:                     "Brak konfliktów scalania w pliku",
 		AlreadyCheckedOutBranch:             "Już przęłączono na tą gałąź",
 		SureForceCheckout:                   "Jesteś pewny, że chcesz wymusić przełączenie? Stracisz wszystkie lokalne zmiany",
 		ForceCheckoutBranch:                 "Wymuś przełączenie gałęzi",