From 84a19920559f940b391ab3f19686e2e85039e083 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Thu, 27 Jan 2022 20:10:25 +1100 Subject: [PATCH] better locking of merge panel state --- pkg/gui/context_config.go | 2 +- pkg/gui/files_panel.go | 34 +++------------ pkg/gui/merge_panel.go | 76 ++++++++++++++++++++++++++++++--- pkg/gui/mergeconflicts/state.go | 8 +++- 4 files changed, 84 insertions(+), 36 deletions(-) diff --git a/pkg/gui/context_config.go b/pkg/gui/context_config.go index 2a152e51765..f567f5a5c5d 100644 --- a/pkg/gui/context_config.go +++ b/pkg/gui/context_config.go @@ -167,7 +167,7 @@ func (gui *Gui) contextTree() ContextTree { Key: MAIN_PATCH_BUILDING_CONTEXT_KEY, }, Merging: &BasicContext{ - OnFocus: OnFocusWrapper(gui.renderConflictsWithFocus), + OnFocus: OnFocusWrapper(func() error { return gui.renderConflictsWithLock(true) }), Kind: MAIN_CONTEXT, ViewName: "main", Key: MAIN_MERGING_CONTEXT_KEY, diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go index d0b4d736ca4..fa8cfa79c98 100644 --- a/pkg/gui/files_panel.go +++ b/pkg/gui/files_panel.go @@ -56,18 +56,16 @@ func (gui *Gui) filesRenderToMain() error { } if node.File != nil && node.File.HasInlineMergeConflicts { - hasConflicts, err := gui.setMergeState(node.File.Name) + ok, err := gui.setConflictsAndRenderWithLock(node.GetPath(), false) if err != nil { return err } - - // if we don't have conflicts we'll fall through and show the diff - if hasConflicts { - return gui.renderConflicts(false) + if ok { + return nil } } - gui.resetMergeState() + gui.resetMergeStateWithLock() cmdObj := gui.Git.WorkingTree.WorktreeFileDiffCmdObj(node, false, !node.GetHasUnstagedChanges() && node.GetHasStagedChanges(), gui.State.IgnoreWhitespaceInDiffView) @@ -143,28 +141,6 @@ func (gui *Gui) refreshFilesAndSubmodules() error { return nil } -func (gui *Gui) refreshMergeState() error { - gui.State.Panels.Merging.Lock() - defer gui.State.Panels.Merging.Unlock() - - if gui.currentContext().GetKey() != MAIN_MERGING_CONTEXT_KEY { - return nil - } - - hasConflicts, err := gui.setMergeState(gui.State.Panels.Merging.GetPath()) - if err != nil { - return gui.surfaceError(err) - } - if hasConflicts { - _ = gui.renderConflicts(true) - } else { - _ = gui.escapeMerge() - return nil - } - - return nil -} - // specific functions func (gui *Gui) stagedFiles() []*models.File { @@ -959,7 +935,7 @@ func (gui *Gui) switchToMerge() error { gui.takeOverMergeConflictScrolling() if gui.State.Panels.Merging.GetPath() != file.Name { - hasConflicts, err := gui.setMergeState(file.Name) + hasConflicts, err := gui.setMergeStateWithLock(file.Name) if err != nil { return err } diff --git a/pkg/gui/merge_panel.go b/pkg/gui/merge_panel.go index 7700f193c8c..dbd5b3be731 100644 --- a/pkg/gui/merge_panel.go +++ b/pkg/gui/merge_panel.go @@ -138,6 +138,13 @@ func (gui *Gui) renderConflicts(hasFocus bool) error { // TODO: find a way to not have to do this OnUIThread thing. Why doesn't it work // without it given that we're calling the 'no scroll' variant below? gui.OnUIThread(func() error { + gui.State.Panels.Merging.Lock() + defer gui.State.Panels.Merging.Unlock() + + if !state.Active() { + return nil + } + gui.centerYPos(gui.Views.Main, state.GetConflictMiddle()) return nil }) @@ -156,6 +163,12 @@ func (gui *Gui) renderConflictsWithFocus() error { return gui.renderConflicts(true) } +func (gui *Gui) renderConflictsWithLock(hasFocus bool) error { + return gui.withMergeConflictLock(func() error { + return gui.renderConflicts(hasFocus) + }) +} + func (gui *Gui) centerYPos(view *gocui.View, y int) { ox, _ := view.Origin() _, height := view.Size() @@ -205,15 +218,27 @@ func (gui *Gui) setMergeState(path string) (bool, error) { return !gui.State.Panels.Merging.NoConflicts(), nil } -func (gui *Gui) escapeMerge() error { +func (gui *Gui) setMergeStateWithLock(path string) (bool, error) { + gui.State.Panels.Merging.Lock() + defer gui.State.Panels.Merging.Unlock() + + return gui.setMergeState(path) +} + +func (gui *Gui) resetMergeStateWithLock() { + gui.State.Panels.Merging.Lock() + defer gui.State.Panels.Merging.Unlock() + gui.resetMergeState() +} - // 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 +func (gui *Gui) escapeMerge() error { + gui.resetMergeState() - if gui.currentContext().GetKey() == MAIN_MERGING_CONTEXT_KEY { + // doing this in separate UI thread so that we're not still holding the lock by the time refresh the file + gui.OnUIThread(func() error { return gui.pushContext(gui.State.Contexts.Files) - } + }) return nil } @@ -236,3 +261,44 @@ func (gui *Gui) withMergeConflictLock(f func() error) error { func (gui *Gui) takeOverMergeConflictScrolling() { gui.State.Panels.Merging.UserVerticalScrolling = false } + +func (gui *Gui) setConflictsAndRender(path string, hasFocus bool) (bool, error) { + hasConflicts, err := gui.setMergeState(path) + if err != nil { + return false, err + } + + // if we don't have conflicts we'll fall through and show the diff + if hasConflicts { + return true, gui.renderConflicts(hasFocus) + } + + return false, nil +} + +func (gui *Gui) setConflictsAndRenderWithLock(path string, hasFocus bool) (bool, error) { + gui.State.Panels.Merging.Lock() + defer gui.State.Panels.Merging.Unlock() + + return gui.setConflictsAndRender(path, hasFocus) +} + +func (gui *Gui) refreshMergeState() error { + gui.State.Panels.Merging.Lock() + defer gui.State.Panels.Merging.Unlock() + + if gui.currentContext().GetKey() != MAIN_MERGING_CONTEXT_KEY { + return nil + } + + hasConflicts, err := gui.setConflictsAndRender(gui.State.Panels.Merging.GetPath(), true) + if err != nil { + return gui.surfaceError(err) + } + + if !hasConflicts { + return gui.escapeMerge() + } + + return nil +} diff --git a/pkg/gui/mergeconflicts/state.go b/pkg/gui/mergeconflicts/state.go index 0889aaf411e..d84f05545dd 100644 --- a/pkg/gui/mergeconflicts/state.go +++ b/pkg/gui/mergeconflicts/state.go @@ -155,7 +155,13 @@ func (s *State) Active() bool { } func (s *State) GetConflictMiddle() int { - return s.currentConflict().target + currentConflict := s.currentConflict() + + if currentConflict == nil { + return 0 + } + + return currentConflict.target } func (s *State) ContentAfterConflictResolve(selection Selection) (bool, string, error) {