From e76fa5a6cb648f8c3b642d83368ea08a48ab43b0 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 2 Oct 2022 20:31:40 -0700 Subject: [PATCH 1/2] fix glitchy render of stale data when flicking through files and directories --- go.mod | 2 +- go.sum | 4 +-- pkg/gui/main_panels.go | 32 +++++++++++++++++--- pkg/gui/window.go | 29 +++++++++++++----- vendor/github.com/jesseduffield/gocui/gui.go | 15 +++++++++ vendor/modules.txt | 2 +- 6 files changed, 69 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 026e573d1..57baf958b 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/integrii/flaggy v1.4.0 github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68 github.com/jesseduffield/go-git/v5 v5.1.2-0.20201006095850-341962be15a4 - github.com/jesseduffield/gocui v0.3.1-0.20221001154429-72c39318a83d + github.com/jesseduffield/gocui v0.3.1-0.20221003033055-3b1444b7ce1c github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10 github.com/jesseduffield/minimal/gitignore v0.3.3-0.20211018110810-9cde264e6b1e github.com/jesseduffield/yaml v2.1.0+incompatible diff --git a/go.sum b/go.sum index 87e316c87..fea88fbd6 100644 --- a/go.sum +++ b/go.sum @@ -72,8 +72,8 @@ github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68 h1:EQP2Tv8T github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68/go.mod h1:+LLj9/WUPAP8LqCchs7P+7X0R98HiFujVFANdNaxhGk= github.com/jesseduffield/go-git/v5 v5.1.2-0.20201006095850-341962be15a4 h1:GOQrmaE8i+KEdB8NzAegKYd4tPn/inM0I1uo0NXFerg= github.com/jesseduffield/go-git/v5 v5.1.2-0.20201006095850-341962be15a4/go.mod h1:nGNEErzf+NRznT+N2SWqmHnDnF9aLgANB1CUNEan09o= -github.com/jesseduffield/gocui v0.3.1-0.20221001154429-72c39318a83d h1:OTUa2dO3IvnY53QWCABkKJK9v5yvs3+uv3RMbG698S0= -github.com/jesseduffield/gocui v0.3.1-0.20221001154429-72c39318a83d/go.mod h1:znJuCDnF2Ph40YZSlBwdX/4GEofnIoWLGdT4mK5zRAU= +github.com/jesseduffield/gocui v0.3.1-0.20221003033055-3b1444b7ce1c h1:mbOoXlqOzc243zNV71pDxeiEof8IRRw2ZJzVXm/RLjc= +github.com/jesseduffield/gocui v0.3.1-0.20221003033055-3b1444b7ce1c/go.mod h1:znJuCDnF2Ph40YZSlBwdX/4GEofnIoWLGdT4mK5zRAU= github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10 h1:jmpr7KpX2+2GRiE91zTgfq49QvgiqB0nbmlwZ8UnOx0= github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10/go.mod h1:aA97kHeNA+sj2Hbki0pvLslmE4CbDyhBeSSTUUnOuVo= github.com/jesseduffield/minimal/gitignore v0.3.3-0.20211018110810-9cde264e6b1e h1:uw/oo+kg7t/oeMs6sqlAwr85ND/9cpO3up3VxphxY0U= diff --git a/pkg/gui/main_panels.go b/pkg/gui/main_panels.go index 625391480..9e36c18e9 100644 --- a/pkg/gui/main_panels.go +++ b/pkg/gui/main_panels.go @@ -27,11 +27,35 @@ func (gui *Gui) runTaskForView(view *gocui.View, task types.UpdateTask) error { } func (gui *Gui) moveMainContextPairToTop(pair types.MainContextPair) { - gui.setWindowContext(pair.Main) - gui.moveToTopOfWindow(pair.Main) + gui.moveMainContextToTop(pair.Main) if pair.Secondary != nil { - gui.setWindowContext(pair.Secondary) - gui.moveToTopOfWindow(pair.Secondary) + gui.moveMainContextToTop(pair.Secondary) + } +} + +func (gui *Gui) moveMainContextToTop(context types.Context) { + gui.setWindowContext(context) + + view := context.GetView() + + topView := gui.topViewInWindow(context.GetWindowName()) + if topView == nil { + gui.Log.Error("unexpected: topView is nil") + return + } + + if topView != view { + // We need to copy the content to avoid a flicker effect: If we're flicking + // through files in the files panel, we use a different view to render the + // files vs the directories, and if you select dir A, then file B, then dir + // C, you'll briefly see dir A's contents again before the view is updated. + // So here we're copying the content from the top window to avoid that + // flicker effect. + gui.g.CopyContent(topView, view) + + if err := gui.g.SetViewOnTopOf(view.Name(), topView.Name()); err != nil { + gui.Log.Error(err) + } } } diff --git a/pkg/gui/window.go b/pkg/gui/window.go index efee847e1..12cd31868 100644 --- a/pkg/gui/window.go +++ b/pkg/gui/window.go @@ -3,6 +3,7 @@ package gui import ( "fmt" + "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/samber/lo" @@ -70,28 +71,42 @@ func (gui *Gui) resetWindowContext(c types.Context) { } } -func (gui *Gui) moveToTopOfWindow(context types.Context) { +// moves given context's view to the top of the window and returns +// true if the view was not already on top. +func (gui *Gui) moveToTopOfWindow(context types.Context) bool { view := context.GetView() if view == nil { - return + return false } window := context.GetWindowName() + topView := gui.topViewInWindow(window) + + if view.Name() == topView.Name() { + return false + } else { + if err := gui.g.SetViewOnTopOf(view.Name(), topView.Name()); err != nil { + gui.Log.Error(err) + } + + return true + } +} + +func (gui *Gui) topViewInWindow(windowName string) *gocui.View { // now I need to find all views in that same window, via contexts. And I guess then I need to find the index of the highest view in that list. - viewNamesInWindow := gui.viewNamesInWindow(window) + viewNamesInWindow := gui.viewNamesInWindow(windowName) // The views list is ordered highest-last, so we're grabbing the last view of the window - topView := view + var topView *gocui.View for _, currentView := range gui.g.Views() { if lo.Contains(viewNamesInWindow, currentView.Name()) { topView = currentView } } - if err := gui.g.SetViewOnTopOf(view.Name(), topView.Name()); err != nil { - gui.Log.Error(err) - } + return topView } func (gui *Gui) viewNamesInWindow(windowName string) []string { diff --git a/vendor/github.com/jesseduffield/gocui/gui.go b/vendor/github.com/jesseduffield/gocui/gui.go index f02f1a93a..889e02999 100644 --- a/vendor/github.com/jesseduffield/gocui/gui.go +++ b/vendor/github.com/jesseduffield/gocui/gui.go @@ -403,6 +403,21 @@ func (g *Gui) SetViewOnTopOf(toMove string, other string) error { return nil } +// replaces the content in toView with the content in fromView +func (g *Gui) CopyContent(fromView *View, toView *View) { + g.Mutexes.ViewsMutex.Lock() + defer g.Mutexes.ViewsMutex.Unlock() + + toView.clear() + + toView.lines = fromView.lines + toView.viewLines = fromView.viewLines + toView.ox = fromView.ox + toView.oy = fromView.oy + toView.cx = fromView.cx + toView.cy = fromView.cy +} + // Views returns all the views in the GUI. func (g *Gui) Views() []*View { return g.views diff --git a/vendor/modules.txt b/vendor/modules.txt index f151d83dc..c37a93336 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -172,7 +172,7 @@ github.com/jesseduffield/go-git/v5/utils/merkletrie/filesystem github.com/jesseduffield/go-git/v5/utils/merkletrie/index github.com/jesseduffield/go-git/v5/utils/merkletrie/internal/frame github.com/jesseduffield/go-git/v5/utils/merkletrie/noder -# github.com/jesseduffield/gocui v0.3.1-0.20221001154429-72c39318a83d +# github.com/jesseduffield/gocui v0.3.1-0.20221003033055-3b1444b7ce1c ## explicit; go 1.12 github.com/jesseduffield/gocui # github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10 From ed98b60078df3b4b30f46ea6beee9181a57f3df1 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 2 Oct 2022 20:57:44 -0700 Subject: [PATCH 2/2] use thread safe map --- pkg/gui/gui.go | 2 +- pkg/gui/window.go | 19 ++++--- pkg/utils/thread_safe_map.go | 90 +++++++++++++++++++++++++++++++ pkg/utils/thread_safe_map_test.go | 59 ++++++++++++++++++++ 4 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 pkg/utils/thread_safe_map.go create mode 100644 pkg/utils/thread_safe_map_test.go diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 6ab2a916b..1d14c9f50 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -185,7 +185,7 @@ type GuiRepoState struct { // WindowViewNameMap is a mapping of windows to the current view of that window. // Some views move between windows for example the commitFiles view and when cycling through // side windows we need to know which view to give focus to for a given window - WindowViewNameMap map[string]string + WindowViewNameMap *utils.ThreadSafeMap[string, string] // tells us whether we've set up our views for the current repo. We'll need to // do this whenever we switch back and forth between repos to get the views diff --git a/pkg/gui/window.go b/pkg/gui/window.go index 12cd31868..1d69c420c 100644 --- a/pkg/gui/window.go +++ b/pkg/gui/window.go @@ -6,6 +6,7 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -15,18 +16,18 @@ import ( // space. Right now most windows are 1:1 with views, except for commitFiles which // is a view that moves between windows -func (gui *Gui) initialWindowViewNameMap(contextTree *context.ContextTree) map[string]string { - result := map[string]string{} +func (gui *Gui) initialWindowViewNameMap(contextTree *context.ContextTree) *utils.ThreadSafeMap[string, string] { + result := utils.NewThreadSafeMap[string, string]() for _, context := range contextTree.Flatten() { - result[context.GetWindowName()] = context.GetViewName() + result.Set(context.GetWindowName(), context.GetViewName()) } return result } func (gui *Gui) getViewNameForWindow(window string) string { - viewName, ok := gui.State.WindowViewNameMap[window] + viewName, ok := gui.State.WindowViewNameMap.Get(window) if !ok { panic(fmt.Sprintf("Viewname not found for window: %s", window)) } @@ -51,7 +52,7 @@ func (gui *Gui) setWindowContext(c types.Context) { gui.resetWindowContext(c) } - gui.State.WindowViewNameMap[c.GetWindowName()] = c.GetViewName() + gui.State.WindowViewNameMap.Set(c.GetWindowName(), c.GetViewName()) } func (gui *Gui) currentWindow() string { @@ -60,11 +61,15 @@ func (gui *Gui) currentWindow() string { // assumes the context's windowName has been set to the new window if necessary func (gui *Gui) resetWindowContext(c types.Context) { - for windowName, viewName := range gui.State.WindowViewNameMap { + for _, windowName := range gui.State.WindowViewNameMap.Keys() { + viewName, ok := gui.State.WindowViewNameMap.Get(windowName) + if !ok { + continue + } if viewName == c.GetViewName() && windowName != c.GetWindowName() { for _, context := range gui.State.Contexts.Flatten() { if context.GetKey() != c.GetKey() && context.GetWindowName() == windowName { - gui.State.WindowViewNameMap[windowName] = context.GetViewName() + gui.State.WindowViewNameMap.Set(windowName, context.GetViewName()) } } } diff --git a/pkg/utils/thread_safe_map.go b/pkg/utils/thread_safe_map.go new file mode 100644 index 000000000..a70cefc7d --- /dev/null +++ b/pkg/utils/thread_safe_map.go @@ -0,0 +1,90 @@ +package utils + +import "sync" + +type ThreadSafeMap[K comparable, V any] struct { + mutex sync.RWMutex + + innerMap map[K]V +} + +func NewThreadSafeMap[K comparable, V any]() *ThreadSafeMap[K, V] { + return &ThreadSafeMap[K, V]{ + innerMap: make(map[K]V), + } +} + +func (m *ThreadSafeMap[K, V]) Get(key K) (V, bool) { + m.mutex.RLock() + defer m.mutex.RUnlock() + + value, ok := m.innerMap[key] + return value, ok +} + +func (m *ThreadSafeMap[K, V]) Set(key K, value V) { + m.mutex.Lock() + defer m.mutex.Unlock() + + m.innerMap[key] = value +} + +func (m *ThreadSafeMap[K, V]) Delete(key K) { + m.mutex.Lock() + defer m.mutex.Unlock() + + delete(m.innerMap, key) +} + +func (m *ThreadSafeMap[K, V]) Keys() []K { + m.mutex.RLock() + defer m.mutex.RUnlock() + + keys := make([]K, 0, len(m.innerMap)) + for key := range m.innerMap { + keys = append(keys, key) + } + + return keys +} + +func (m *ThreadSafeMap[K, V]) Values() []V { + m.mutex.RLock() + defer m.mutex.RUnlock() + + values := make([]V, 0, len(m.innerMap)) + for _, value := range m.innerMap { + values = append(values, value) + } + + return values +} + +func (m *ThreadSafeMap[K, V]) Len() int { + m.mutex.RLock() + defer m.mutex.RUnlock() + + return len(m.innerMap) +} + +func (m *ThreadSafeMap[K, V]) Clear() { + m.mutex.Lock() + defer m.mutex.Unlock() + + m.innerMap = make(map[K]V) +} + +func (m *ThreadSafeMap[K, V]) IsEmpty() bool { + m.mutex.RLock() + defer m.mutex.RUnlock() + + return len(m.innerMap) == 0 +} + +func (m *ThreadSafeMap[K, V]) Has(key K) bool { + m.mutex.RLock() + defer m.mutex.RUnlock() + + _, ok := m.innerMap[key] + return ok +} diff --git a/pkg/utils/thread_safe_map_test.go b/pkg/utils/thread_safe_map_test.go new file mode 100644 index 000000000..9676cfe5f --- /dev/null +++ b/pkg/utils/thread_safe_map_test.go @@ -0,0 +1,59 @@ +package utils + +import ( + "testing" +) + +func TestThreadSafeMap(t *testing.T) { + m := NewThreadSafeMap[int, int]() + + m.Set(1, 1) + m.Set(2, 2) + m.Set(3, 3) + + if m.Len() != 3 { + t.Errorf("Expected length to be 3, got %d", m.Len()) + } + + if !m.Has(1) { + t.Errorf("Expected to have key 1") + } + + if m.Has(4) { + t.Errorf("Expected to not have key 4") + } + + if _, ok := m.Get(1); !ok { + t.Errorf("Expected to have key 1") + } + + if _, ok := m.Get(4); ok { + t.Errorf("Expected to not have key 4") + } + + m.Delete(1) + + if m.Has(1) { + t.Errorf("Expected to not have key 1") + } + + m.Clear() + + if m.Len() != 0 { + t.Errorf("Expected length to be 0, got %d", m.Len()) + } +} + +func TestThreadSafeMapConcurrentReadWrite(t *testing.T) { + m := NewThreadSafeMap[int, int]() + + go func() { + for i := 0; i < 10000; i++ { + m.Set(0, 0) + } + }() + + for i := 0; i < 10000; i++ { + m.Get(0) + } +}