mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
Bounce FILES model updates onto the UI thread
refreshStateFiles now does its git work on the worker and enqueues a single OnUIThread closure that writes Model.Submodules, Model.Files, and the FileTreeViewModel state together, instead of writing them directly from the worker goroutine. refreshStateSubmoduleConfigs becomes a pure getter (returns the configs; no model write) so the result can be threaded into that same bounce. The STAGING handler wraps RefreshStagingPanel in OnUIThread after fileWg.Wait() so it sees the post-bounce file model rather than the stale pre-refresh one — without this it would race the files bounce queued just above it. Bouncing the write opens a hazard the old synchronous write didn't have: if the user switches repos while this refresh is in flight, the queued closure would fire after resetState has replaced the model with a fresh one for the new repo, silently overwriting it with the previous repo's files. Guard against this with a repo generation: resetState bumps a counter on every switch, refreshStateFiles captures it before its git work, and onUIThreadUnlessRepoChanged drops the bounce if the generation has moved on. This one helper is the general mechanism the remaining scopes' bounces will use too; the same guard covers the rebase-continue prompt, which reads Model.Files right after. A generation counter, not a comparison of the *Model pointer: switching away from and back to a repo reuses that repo's cached state (the same Model pointer), which a pointer comparison would wrongly accept even though the in-flight data is stale. PromptToContinueRebase's Then callback (previous commit) now gets an explanatory comment, since this is the commit that makes it necessary. The explicit locking around these writes (RefreshingFilesMutex in refreshFilesAndSubmodules, FileTreeViewModel.RWMutex around the write in refreshStateFiles) is left in place for now even though it's becoming redundant, to keep this commit focused on the bounce itself; it's removed next. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
ea83f50dc3
commit
be897ce55e
|
|
@ -307,6 +307,10 @@ func (self *MergeAndRebaseHelper) PromptToContinueRebase() error {
|
|||
// Need to refresh the files to be really sure if this is the case.
|
||||
// We would otherwise be relying on lazygit's auto-refresh on focus,
|
||||
// but this is not supported by all terminals or on all platforms.
|
||||
//
|
||||
// The model.Files update is bounced onto the UI thread, so we have
|
||||
// to read it in Then; reading it inline here would see the previous
|
||||
// model.
|
||||
self.c.Refresh(types.RefreshOptions{
|
||||
Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES},
|
||||
Then: func() error {
|
||||
|
|
|
|||
|
|
@ -238,7 +238,14 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) {
|
|||
if scopeSet.Includes(types.STAGING) {
|
||||
refresh("staging", func() {
|
||||
fileWg.Wait()
|
||||
self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{})
|
||||
// Bounce onto the UI thread so this runs after the files
|
||||
// scope's model-update bounce — RefreshStagingPanel reads
|
||||
// Model.Files (via Files.GetSelected) and would otherwise
|
||||
// see the pre-refresh model.
|
||||
self.c.OnUIThread(func() error {
|
||||
self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{})
|
||||
return nil
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -667,15 +674,8 @@ func (self *RefreshHelper) refreshTags() error {
|
|||
return nil
|
||||
}
|
||||
|
||||
func (self *RefreshHelper) refreshStateSubmoduleConfigs() error {
|
||||
configs, err := self.c.Git().Submodule.GetConfigs(nil)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
self.c.Model().Submodules = configs
|
||||
|
||||
return nil
|
||||
func (self *RefreshHelper) refreshStateSubmoduleConfigs() ([]*models.SubmoduleConfig, error) {
|
||||
return self.c.Git().Submodule.GetConfigs(nil)
|
||||
}
|
||||
|
||||
// self.refreshStatus is called at the end of this because that's when we can
|
||||
|
|
@ -743,25 +743,40 @@ func (self *RefreshHelper) refreshFilesAndSubmodules(background bool) error {
|
|||
self.c.Mutexes().RefreshingFilesMutex.Lock()
|
||||
defer self.c.Mutexes().RefreshingFilesMutex.Unlock()
|
||||
|
||||
if err := self.refreshStateSubmoduleConfigs(); err != nil {
|
||||
configs, err := self.refreshStateSubmoduleConfigs()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := self.refreshStateFiles(background); err != nil {
|
||||
if err := self.refreshStateFiles(background, configs); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
self.c.OnUIThread(func() error {
|
||||
self.refreshView(self.c.Contexts().Submodules)
|
||||
self.refreshView(self.c.Contexts().Files)
|
||||
return nil
|
||||
})
|
||||
self.refreshView(self.c.Contexts().Submodules)
|
||||
self.refreshView(self.c.Contexts().Files)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (self *RefreshHelper) refreshStateFiles(background bool) error {
|
||||
// onUIThreadUnlessRepoChanged bounces a refresh's model/view update onto the UI
|
||||
// thread, but drops it if the repo was switched while the refresh was in flight.
|
||||
// Refresh workers do their git work off the UI thread and enqueue their model
|
||||
// writes here; a repo switch (which replaces the whole model and context tree)
|
||||
// bumps the generation, so a write captured under the old generation must not
|
||||
// clobber the new repo's state. Callers capture the generation with
|
||||
// State().GetRepoGeneration() before doing their git work and pass it in.
|
||||
func (self *RefreshHelper) onUIThreadUnlessRepoChanged(generation int, f func() error) {
|
||||
self.c.OnUIThread(func() error {
|
||||
if self.c.State().GetRepoGeneration() != generation {
|
||||
return nil
|
||||
}
|
||||
return f()
|
||||
})
|
||||
}
|
||||
|
||||
func (self *RefreshHelper) refreshStateFiles(background bool, submoduleConfigs []*models.SubmoduleConfig) error {
|
||||
fileTreeViewModel := self.c.Contexts().Files.FileTreeViewModel
|
||||
generation := self.c.State().GetRepoGeneration()
|
||||
|
||||
prevConflictFileCount := 0
|
||||
if self.c.UserConfig().Git.AutoStageResolvedConflicts {
|
||||
|
|
@ -822,7 +837,9 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error {
|
|||
// (e.g. in the user's editor). Offer to continue it. We only do this
|
||||
// for operations we started ourselves; prompting for one that was
|
||||
// started outside lazygit (e.g. by a coding agent) would be confusing.
|
||||
self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() })
|
||||
self.onUIThreadUnlessRepoChanged(generation, func() error {
|
||||
return self.mergeAndRebaseHelper.PromptToContinueRebase()
|
||||
})
|
||||
}
|
||||
} else {
|
||||
// Either there's no operation in progress any more, or new conflicts have
|
||||
|
|
@ -835,22 +852,23 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error {
|
|||
})
|
||||
}
|
||||
|
||||
fileTreeViewModel.RWMutex.Lock()
|
||||
|
||||
// only taking over the filter if it hasn't already been set by the user.
|
||||
if conflictFileCount > 0 && prevConflictFileCount == 0 {
|
||||
if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll {
|
||||
fileTreeViewModel.SetStatusFilter(filetree.DisplayConflicted)
|
||||
self.c.Contexts().Files.GetView().Subtitle = self.c.Tr.FilterLabelConflictingFiles
|
||||
self.onUIThreadUnlessRepoChanged(generation, func() error {
|
||||
// only taking over the filter if it hasn't already been set by the user.
|
||||
if conflictFileCount > 0 && prevConflictFileCount == 0 {
|
||||
if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll {
|
||||
fileTreeViewModel.SetStatusFilter(filetree.DisplayConflicted)
|
||||
self.c.Contexts().Files.GetView().Subtitle = self.c.Tr.FilterLabelConflictingFiles
|
||||
}
|
||||
} else if conflictFileCount == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted {
|
||||
fileTreeViewModel.SetStatusFilter(filetree.DisplayAll)
|
||||
self.c.Contexts().Files.GetView().Subtitle = ""
|
||||
}
|
||||
} else if conflictFileCount == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted {
|
||||
fileTreeViewModel.SetStatusFilter(filetree.DisplayAll)
|
||||
self.c.Contexts().Files.GetView().Subtitle = ""
|
||||
}
|
||||
|
||||
self.c.Model().Files = files
|
||||
fileTreeViewModel.SetTree()
|
||||
fileTreeViewModel.RWMutex.Unlock()
|
||||
self.c.Model().Submodules = submoduleConfigs
|
||||
self.c.Model().Files = files
|
||||
fileTreeViewModel.SetTree()
|
||||
return nil
|
||||
})
|
||||
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import (
|
|||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/jesseduffield/lazycore/pkg/boxlayout"
|
||||
|
|
@ -111,6 +112,11 @@ type Gui struct {
|
|||
|
||||
PopupHandler types.IPopupHandler
|
||||
|
||||
// Bumped every time we switch to a different repository (in resetState).
|
||||
// Used to drop refresh results that were computed for a repo we've since
|
||||
// navigated away from. See RefreshHelper.onUIThreadUnlessRepoChanged.
|
||||
repoGeneration atomic.Int32
|
||||
|
||||
// we use this to decide whether we'll return to the original directory that
|
||||
// lazygit was opened in, or if we'll retain the one we're currently in.
|
||||
RetainOriginalDir bool
|
||||
|
|
@ -169,6 +175,10 @@ func (self *StateAccessor) GetRepoState() types.IRepoStateAccessor {
|
|||
return self.gui.State
|
||||
}
|
||||
|
||||
func (self *StateAccessor) GetRepoGeneration() int {
|
||||
return int(self.gui.repoGeneration.Load())
|
||||
}
|
||||
|
||||
func (self *StateAccessor) GetPagerConfig() *config.PagerConfig {
|
||||
return self.gui.pagerConfig
|
||||
}
|
||||
|
|
@ -575,6 +585,11 @@ func (gui *Gui) checkForChangedConfigsThatDontAutoReload(oldConfig *config.UserC
|
|||
// resetState reuses the repo state from our repo state map, if the repo was
|
||||
// open before; otherwise it creates a new one.
|
||||
func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context {
|
||||
// Bump the repo generation so that any refresh still in flight for the
|
||||
// previous repo drops its model update instead of applying it here (see
|
||||
// RefreshHelper.onUIThreadUnlessRepoChanged).
|
||||
gui.repoGeneration.Add(1)
|
||||
|
||||
// Un-highlight the current view if there is one. The reason we do this is
|
||||
// that the repo we are switching to might have a different view focused,
|
||||
// and would then show an inactive highlight for the previous view.
|
||||
|
|
|
|||
|
|
@ -384,6 +384,13 @@ type IStateAccessor interface {
|
|||
GetItemOperation(item HasUrn) ItemOperation
|
||||
SetItemOperation(item HasUrn, operation ItemOperation)
|
||||
ClearItemOperation(item HasUrn)
|
||||
|
||||
// A counter that is bumped every time we switch to a different repository
|
||||
// (see Gui.resetState). Refresh workers capture it before doing their git
|
||||
// work and pass it to onUIThreadUnlessRepoChanged, so that a model update
|
||||
// computed for one repo can be dropped rather than applied to another if the
|
||||
// user switched repos while the refresh was in flight.
|
||||
GetRepoGeneration() int
|
||||
}
|
||||
|
||||
type IRepoStateAccessor interface {
|
||||
|
|
|
|||
Loading…
Reference in a new issue