diff options
author | Jesse Duffield <jessedduffield@gmail.com> | 2022-01-15 13:29:28 +1100 |
---|---|---|
committer | Jesse Duffield <jessedduffield@gmail.com> | 2022-01-15 14:15:41 +1100 |
commit | 8d8bdb948b089250c22f3ac4f549152a209dcdb2 (patch) | |
tree | 3687ba082abe7322fe5c77c452b618c7b5e6e2e2 /pkg/gui | |
parent | cdcfeb396fda1e61dee9b6d88ab4659152a10948 (diff) |
avoid deadlock in merge panel
Diffstat (limited to 'pkg/gui')
-rw-r--r-- | pkg/gui/diff_context_size.go | 17 | ||||
-rw-r--r-- | pkg/gui/diff_context_size_test.go | 8 | ||||
-rw-r--r-- | pkg/gui/files_panel.go | 12 | ||||
-rw-r--r-- | pkg/gui/menu_panel.go | 2 | ||||
-rw-r--r-- | pkg/gui/merge_panel.go | 94 |
5 files changed, 79 insertions, 54 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 |