From cc9a20c4abc850e94352cf04f7484bf0ba381780 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 8 Oct 2023 17:18:37 +0200 Subject: [PATCH 1/9] Don't report errors from within a WithWaitingStatus We can just return our error to WithWaitingStatus, it will take care of reporting it. --- pkg/gui/controllers/branches_controller.go | 13 +++---------- pkg/gui/controllers/tags_controller.go | 15 +++++---------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 45f950457..4f8ebdbf8 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -590,24 +590,17 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { WorktreeGitDir: worktreeGitDir, }, ) - if err != nil { - _ = self.c.Error(err) - } - - return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + _ = self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + return err } else { self.c.LogAction(action) err := self.c.Git().Sync.FastForward( task, branch.Name, branch.UpstreamRemote, branch.UpstreamBranch, ) - if err != nil { - _ = self.c.Error(err) - } _ = self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + return err } - - return nil }) } diff --git a/pkg/gui/controllers/tags_controller.go b/pkg/gui/controllers/tags_controller.go index 91d590c32..2fa24f734 100644 --- a/pkg/gui/controllers/tags_controller.go +++ b/pkg/gui/controllers/tags_controller.go @@ -92,10 +92,9 @@ func (self *TagsController) checkout(tag *models.Tag) error { func (self *TagsController) localDelete(tag *models.Tag) error { return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.DeleteLocalTag) - if err := self.c.Git().Tag.LocalDelete(tag.Name); err != nil { - return self.c.Error(err) - } - return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + err := self.c.Git().Tag.LocalDelete(tag.Name) + _ = self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + return err }) } @@ -133,7 +132,7 @@ func (self *TagsController) remoteDelete(tag *models.Tag) error { return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(t gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.DeleteRemoteTag) if err := self.c.Git().Remote.DeleteRemoteTag(t, upstream, tag.Name); err != nil { - return self.c.Error(err) + return err } self.c.Toast(self.c.Tr.RemoteTagDeletedMessage) return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) @@ -192,11 +191,7 @@ func (self *TagsController) push(tag *models.Tag) error { return self.c.WithWaitingStatus(self.c.Tr.PushingTagStatus, func(task gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.PushTag) err := self.c.Git().Tag.Push(task, response, tag.Name) - if err != nil { - _ = self.c.Error(err) - } - - return nil + return err }) }, }) From 9d55d71fdd310886cc178a4416a9d743d8f34a03 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 8 Oct 2023 17:54:40 +0200 Subject: [PATCH 2/9] Add GetItemOperation/SetItemOperation/ClearItemOperation to IStateAccessor Not used by anything yet; committing this separately in the interest of having smaller independent commits. --- pkg/gui/gui.go | 30 ++++++++++++++++++++++++++++++ pkg/gui/types/common.go | 21 +++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 2ded4a5c5..33a2e3d02 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -110,6 +110,12 @@ type Gui struct { // lazygit was opened in, or if we'll retain the one we're currently in. RetainOriginalDir bool + // stores long-running operations associated with items (e.g. when a branch + // is being pushed). At the moment the rule is to use an item operation when + // we need to talk to the remote. + itemOperations map[string]types.ItemOperation + itemOperationsMutex *deadlock.Mutex + PrevLayout PrevLayout // this is the initial dir we are in upon opening lazygit. We hold onto this @@ -180,6 +186,27 @@ func (self *StateAccessor) SetRetainOriginalDir(value bool) { self.gui.RetainOriginalDir = value } +func (self *StateAccessor) GetItemOperation(item types.HasUrn) types.ItemOperation { + self.gui.itemOperationsMutex.Lock() + defer self.gui.itemOperationsMutex.Unlock() + + return self.gui.itemOperations[item.URN()] +} + +func (self *StateAccessor) SetItemOperation(item types.HasUrn, operation types.ItemOperation) { + self.gui.itemOperationsMutex.Lock() + defer self.gui.itemOperationsMutex.Unlock() + + self.gui.itemOperations[item.URN()] = operation +} + +func (self *StateAccessor) ClearItemOperation(item types.HasUrn) { + self.gui.itemOperationsMutex.Lock() + defer self.gui.itemOperationsMutex.Unlock() + + delete(self.gui.itemOperations, item.URN()) +} + // we keep track of some stuff from one render to the next to see if certain // things have changed type PrevLayout struct { @@ -473,6 +500,9 @@ func NewGui( }, InitialDir: initialDir, afterLayoutFuncs: make(chan func() error, 1000), + + itemOperations: make(map[string]types.ItemOperation), + itemOperationsMutex: &deadlock.Mutex{}, } gui.PopupHandler = popup.NewPopupHandler( diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 80d960eab..f4fde41c7 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -265,6 +265,24 @@ type Mutexes struct { PtyMutex *deadlock.Mutex } +// A long-running operation associated with an item. For example, we'll show +// that a branch is being pushed from so that there's visual feedback about +// what's happening and so that you can see multiple branches' concurrent +// operations +type ItemOperation int + +const ( + ItemOperationNone ItemOperation = iota + ItemOperationPushing + ItemOperationPulling + ItemOperationFastForwarding + ItemOperationDeleting +) + +type HasUrn interface { + URN() string +} + type IStateAccessor interface { GetRepoPathStack() *utils.StringStack GetRepoState() IRepoStateAccessor @@ -277,6 +295,9 @@ type IStateAccessor interface { SetShowExtrasWindow(bool) GetRetainOriginalDir() bool SetRetainOriginalDir(bool) + GetItemOperation(item HasUrn) ItemOperation + SetItemOperation(item HasUrn, operation ItemOperation) + ClearItemOperation(item HasUrn) } type IRepoStateAccessor interface { From 7075b66bc6dde80be5e0bc626db1988a9f47e2ca Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 22 Sep 2023 16:09:02 +0200 Subject: [PATCH 3/9] Add WithInlineStatus helper function Very similar to WithWaitingStatus, except that the status is shown in a view next to the affected item, rather than in the status bar. Not used by anything yet; again, committing separately to get smaller commits. --- pkg/gui/controllers.go | 1 + pkg/gui/controllers/helpers/helpers.go | 2 + .../helpers/inline_status_helper.go | 129 ++++++++++++++++++ pkg/gui/gui_common.go | 6 + pkg/gui/types/common.go | 6 + 5 files changed, 144 insertions(+) create mode 100644 pkg/gui/controllers/helpers/inline_status_helper.go diff --git a/pkg/gui/controllers.go b/pkg/gui/controllers.go index 40a482f30..43b226b58 100644 --- a/pkg/gui/controllers.go +++ b/pkg/gui/controllers.go @@ -112,6 +112,7 @@ func (gui *Gui) resetHelpersAndControllers() { Confirmation: helpers.NewConfirmationHelper(helperCommon), Mode: modeHelper, AppStatus: appStatusHelper, + InlineStatus: helpers.NewInlineStatusHelper(helperCommon), WindowArrangement: helpers.NewWindowArrangementHelper( gui.c, windowHelper, diff --git a/pkg/gui/controllers/helpers/helpers.go b/pkg/gui/controllers/helpers/helpers.go index 06c5cc973..9e05ba163 100644 --- a/pkg/gui/controllers/helpers/helpers.go +++ b/pkg/gui/controllers/helpers/helpers.go @@ -46,6 +46,7 @@ type Helpers struct { Confirmation *ConfirmationHelper Mode *ModeHelper AppStatus *AppStatusHelper + InlineStatus *InlineStatusHelper WindowArrangement *WindowArrangementHelper Search *SearchHelper Worktree *WorktreeHelper @@ -81,6 +82,7 @@ func NewStubHelpers() *Helpers { Confirmation: &ConfirmationHelper{}, Mode: &ModeHelper{}, AppStatus: &AppStatusHelper{}, + InlineStatus: &InlineStatusHelper{}, WindowArrangement: &WindowArrangementHelper{}, Search: &SearchHelper{}, Worktree: &WorktreeHelper{}, diff --git a/pkg/gui/controllers/helpers/inline_status_helper.go b/pkg/gui/controllers/helpers/inline_status_helper.go new file mode 100644 index 000000000..d4435e5b9 --- /dev/null +++ b/pkg/gui/controllers/helpers/inline_status_helper.go @@ -0,0 +1,129 @@ +package helpers + +import ( + "time" + + "github.com/jesseduffield/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/sasha-s/go-deadlock" +) + +type InlineStatusHelper struct { + c *HelperCommon + + contextsWithInlineStatus map[types.ContextKey]*inlineStatusInfo + mutex *deadlock.Mutex +} + +func NewInlineStatusHelper(c *HelperCommon) *InlineStatusHelper { + return &InlineStatusHelper{ + c: c, + contextsWithInlineStatus: make(map[types.ContextKey]*inlineStatusInfo), + mutex: &deadlock.Mutex{}, + } +} + +type InlineStatusOpts struct { + Item types.HasUrn + Operation types.ItemOperation + ContextKey types.ContextKey +} + +type inlineStatusInfo struct { + refCount int + stop chan struct{} +} + +// A custom task for WithInlineStatus calls; it wraps the original one and +// hides the status whenever the task is paused, and shows it again when +// continued. +type inlineStatusHelperTask struct { + gocui.Task + + inlineStatusHelper *InlineStatusHelper + opts InlineStatusOpts +} + +// poor man's version of explicitly saying that struct X implements interface Y +var _ gocui.Task = inlineStatusHelperTask{} + +func (self inlineStatusHelperTask) Pause() { + self.inlineStatusHelper.stop(self.opts) + self.Task.Pause() + + self.inlineStatusHelper.renderContext(self.opts.ContextKey) +} + +func (self inlineStatusHelperTask) Continue() { + self.Task.Continue() + self.inlineStatusHelper.start(self.opts) +} + +func (self *InlineStatusHelper) WithInlineStatus(opts InlineStatusOpts, f func(gocui.Task) error) { + self.c.OnWorker(func(task gocui.Task) { + self.start(opts) + + err := f(inlineStatusHelperTask{task, self, opts}) + if err != nil { + self.c.OnUIThread(func() error { + return self.c.Error(err) + }) + } + + self.stop(opts) + }) +} + +func (self *InlineStatusHelper) start(opts InlineStatusOpts) { + self.c.State().SetItemOperation(opts.Item, opts.Operation) + + self.mutex.Lock() + defer self.mutex.Unlock() + + info := self.contextsWithInlineStatus[opts.ContextKey] + if info == nil { + info = &inlineStatusInfo{refCount: 0, stop: make(chan struct{})} + self.contextsWithInlineStatus[opts.ContextKey] = info + + go utils.Safe(func() { + ticker := time.NewTicker(time.Millisecond * utils.LoaderAnimationInterval) + defer ticker.Stop() + outer: + for { + select { + case <-ticker.C: + self.renderContext(opts.ContextKey) + case <-info.stop: + break outer + } + } + }) + } + + info.refCount++ +} + +func (self *InlineStatusHelper) stop(opts InlineStatusOpts) { + self.mutex.Lock() + + if info := self.contextsWithInlineStatus[opts.ContextKey]; info != nil { + info.refCount-- + if info.refCount <= 0 { + info.stop <- struct{}{} + delete(self.contextsWithInlineStatus, opts.ContextKey) + } + + } + + self.mutex.Unlock() + + self.c.State().ClearItemOperation(opts.Item) +} + +func (self *InlineStatusHelper) renderContext(contextKey types.ContextKey) { + self.c.OnUIThread(func() error { + _ = self.c.ContextForKey(contextKey).HandleRender() + return nil + }) +} diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 0abe94f4c..4b9d3dc37 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -5,6 +5,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/config" + "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -199,3 +200,8 @@ func (self *guiCommon) RunningIntegrationTest() bool { func (self *guiCommon) InDemo() bool { return self.gui.integrationTest != nil && self.gui.integrationTest.IsDemo() } + +func (self *guiCommon) WithInlineStatus(item types.HasUrn, operation types.ItemOperation, contextKey types.ContextKey, f func(gocui.Task) error) error { + self.gui.helpers.InlineStatus.WithInlineStatus(helpers.InlineStatusOpts{Item: item, Operation: operation, ContextKey: contextKey}, f) + return nil +} diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index f4fde41c7..e9f9fc266 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -87,6 +87,12 @@ type IGuiCommon interface { // resized, if in accordion mode. AfterLayout(f func() error) + // Wraps a function, attaching the given operation to the given item while + // the function is executing, and also causes the given context to be + // redrawn periodically. This allows the operation to be visualized with a + // spinning loader animation (e.g. when a branch is being pushed). + WithInlineStatus(item HasUrn, operation ItemOperation, contextKey ContextKey, f func(gocui.Task) error) error + // returns the gocui Gui struct. There is a good chance you don't actually want to use // this struct and instead want to use another method above GocuiGui() *gocui.Gui From 707fa37160e82c1c88cc44a7ae07762ccec48008 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 8 Oct 2023 18:09:29 +0200 Subject: [PATCH 4/9] Add inline status for pushing/pulling/fast-forwarding branches When pulling/pushing/fast-forwarding a branch, show this state in the branches list for that branch for as long as the operation takes, to make it easier to see when it's done (without having to stare at the status bar in the lower left). This will hopefully help with making these operations feel more predictable, now that we no longer show a loader panel for them. --- pkg/commands/models/branch.go | 4 +++ pkg/gui/context/branches_context.go | 1 + pkg/gui/controllers/branches_controller.go | 9 +----- pkg/gui/controllers/helpers/refresh_helper.go | 2 +- pkg/gui/controllers/status_controller.go | 2 +- pkg/gui/controllers/sync_controller.go | 29 ++++++++++--------- pkg/gui/presentation/branches.go | 22 ++++++++++---- pkg/gui/presentation/item_operations.go | 23 +++++++++++++++ pkg/gui/presentation/status.go | 5 ++-- pkg/i18n/chinese.go | 2 +- pkg/i18n/dutch.go | 2 +- pkg/i18n/english.go | 2 +- pkg/i18n/korean.go | 2 +- pkg/i18n/russian.go | 2 +- pkg/i18n/traditional_chinese.go | 2 +- 15 files changed, 71 insertions(+), 38 deletions(-) create mode 100644 pkg/gui/presentation/item_operations.go diff --git a/pkg/commands/models/branch.go b/pkg/commands/models/branch.go index 5fc887f87..c5fcfdaed 100644 --- a/pkg/commands/models/branch.go +++ b/pkg/commands/models/branch.go @@ -65,6 +65,10 @@ func (b *Branch) ID() string { return b.RefName() } +func (b *Branch) URN() string { + return "branch-" + b.ID() +} + func (b *Branch) Description() string { return b.RefName() } diff --git a/pkg/gui/context/branches_context.go b/pkg/gui/context/branches_context.go index ac1fae52c..68324d020 100644 --- a/pkg/gui/context/branches_context.go +++ b/pkg/gui/context/branches_context.go @@ -27,6 +27,7 @@ func NewBranchesContext(c *ContextCommon) *BranchesContext { getDisplayStrings := func(_ int, _ int) [][]string { return presentation.GetBranchListDisplayStrings( viewModel.GetItems(), + c.State().GetItemOperation, c.State().GetRepoState().GetScreenMode() != types.SCREEN_NORMAL, c.Modes().Diffing.Ref, c.Tr, diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 4f8ebdbf8..407dc135a 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -563,14 +563,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { action := self.c.Tr.Actions.FastForwardBranch - message := utils.ResolvePlaceholderString( - self.c.Tr.FastForwarding, - map[string]string{ - "branch": branch.Name, - }, - ) - - return self.c.WithWaitingStatus(message, func(task gocui.Task) error { + return self.c.WithInlineStatus(branch, types.ItemOperationFastForwarding, context.LOCAL_BRANCHES_CONTEXT_KEY, func(task gocui.Task) error { worktree, ok := self.worktreeForBranch(branch) if ok { self.c.LogAction(action) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index f04b102e4..bec0a10e1 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -678,7 +678,7 @@ func (self *RefreshHelper) refreshStatus() { repoName := self.c.Git().RepoPaths.RepoName() - status := presentation.FormatStatus(repoName, currentBranch, linkedWorktreeName, workingTreeState, self.c.Tr) + status := presentation.FormatStatus(repoName, currentBranch, types.ItemOperationNone, linkedWorktreeName, workingTreeState, self.c.Tr) self.c.SetViewContent(self.c.Views().Status, status) } diff --git a/pkg/gui/controllers/status_controller.go b/pkg/gui/controllers/status_controller.go index 8344dfe76..2a186670b 100644 --- a/pkg/gui/controllers/status_controller.go +++ b/pkg/gui/controllers/status_controller.go @@ -106,7 +106,7 @@ func (self *StatusController) onClick() error { } cx, _ := self.c.Views().Status.Cursor() - upstreamStatus := presentation.BranchStatus(currentBranch, self.c.Tr) + upstreamStatus := presentation.BranchStatus(currentBranch, types.ItemOperationNone, self.c.Tr) repoName := self.c.Git().RepoPaths.RepoName() workingTreeState := self.c.Git().Status.WorkingTreeState() switch workingTreeState { diff --git a/pkg/gui/controllers/sync_controller.go b/pkg/gui/controllers/sync_controller.go index a77d047a6..46d553a84 100644 --- a/pkg/gui/controllers/sync_controller.go +++ b/pkg/gui/controllers/sync_controller.go @@ -7,6 +7,7 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -73,13 +74,13 @@ func (self *SyncController) push(currentBranch *models.Branch) error { if currentBranch.IsTrackingRemote() { opts := pushOpts{} if currentBranch.HasCommitsToPull() { - return self.requestToForcePush(opts) + return self.requestToForcePush(currentBranch, opts) } else { - return self.pushAux(opts) + return self.pushAux(currentBranch, opts) } } else { if self.c.Git().Config.GetPushToCurrent() { - return self.pushAux(pushOpts{setUpstream: true}) + return self.pushAux(currentBranch, pushOpts{setUpstream: true}) } else { return self.c.Helpers().Upstream.PromptForUpstreamWithInitialContent(currentBranch, func(upstream string) error { upstreamRemote, upstreamBranch, err := self.c.Helpers().Upstream.ParseUpstream(upstream) @@ -87,7 +88,7 @@ func (self *SyncController) push(currentBranch *models.Branch) error { return self.c.Error(err) } - return self.pushAux(pushOpts{ + return self.pushAux(currentBranch, pushOpts{ setUpstream: true, upstreamRemote: upstreamRemote, upstreamBranch: upstreamBranch, @@ -107,11 +108,11 @@ func (self *SyncController) pull(currentBranch *models.Branch) error { return self.c.Error(err) } - return self.PullAux(PullFilesOptions{Action: action}) + return self.PullAux(currentBranch, PullFilesOptions{Action: action}) }) } - return self.PullAux(PullFilesOptions{Action: action}) + return self.PullAux(currentBranch, PullFilesOptions{Action: action}) } func (self *SyncController) setCurrentBranchUpstream(upstream string) error { @@ -139,8 +140,8 @@ type PullFilesOptions struct { Action string } -func (self *SyncController) PullAux(opts PullFilesOptions) error { - return self.c.WithWaitingStatus(self.c.Tr.PullingStatus, func(task gocui.Task) error { +func (self *SyncController) PullAux(currentBranch *models.Branch, opts PullFilesOptions) error { + return self.c.WithInlineStatus(currentBranch, types.ItemOperationPulling, context.LOCAL_BRANCHES_CONTEXT_KEY, func(task gocui.Task) error { return self.pullWithLock(task, opts) }) } @@ -167,8 +168,8 @@ type pushOpts struct { setUpstream bool } -func (self *SyncController) pushAux(opts pushOpts) error { - return self.c.WithWaitingStatus(self.c.Tr.PushingStatus, func(task gocui.Task) error { +func (self *SyncController) pushAux(currentBranch *models.Branch, opts pushOpts) error { + return self.c.WithInlineStatus(currentBranch, types.ItemOperationPushing, context.LOCAL_BRANCHES_CONTEXT_KEY, func(task gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.Push) err := self.c.Git().Sync.Push( task, @@ -192,18 +193,18 @@ func (self *SyncController) pushAux(opts pushOpts) error { newOpts := opts newOpts.force = true - return self.pushAux(newOpts) + return self.pushAux(currentBranch, newOpts) }, }) return nil } - _ = self.c.Error(err) + return err } return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) }) } -func (self *SyncController) requestToForcePush(opts pushOpts) error { +func (self *SyncController) requestToForcePush(currentBranch *models.Branch, opts pushOpts) error { forcePushDisabled := self.c.UserConfig.Git.DisableForcePushing if forcePushDisabled { return self.c.ErrorMsg(self.c.Tr.ForcePushDisabled) @@ -214,7 +215,7 @@ func (self *SyncController) requestToForcePush(opts pushOpts) error { Prompt: self.forcePushPrompt(), HandleConfirm: func() error { opts.force = true - return self.pushAux(opts) + return self.pushAux(currentBranch, opts) }, }) } diff --git a/pkg/gui/presentation/branches.go b/pkg/gui/presentation/branches.go index 6d4620238..bd027ca62 100644 --- a/pkg/gui/presentation/branches.go +++ b/pkg/gui/presentation/branches.go @@ -9,6 +9,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gui/presentation/icons" "github.com/jesseduffield/lazygit/pkg/gui/style" + "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/i18n" "github.com/jesseduffield/lazygit/pkg/theme" "github.com/jesseduffield/lazygit/pkg/utils" @@ -19,6 +20,7 @@ var branchPrefixColorCache = make(map[string]style.TextStyle) func GetBranchListDisplayStrings( branches []*models.Branch, + getItemOperation func(item types.HasUrn) types.ItemOperation, fullDescription bool, diffName string, tr *i18n.TranslationSet, @@ -27,13 +29,14 @@ func GetBranchListDisplayStrings( ) [][]string { return lo.Map(branches, func(branch *models.Branch, _ int) []string { diffed := branch.Name == diffName - return getBranchDisplayStrings(branch, fullDescription, diffed, tr, userConfig, worktrees) + return getBranchDisplayStrings(branch, getItemOperation(branch), fullDescription, diffed, tr, userConfig, worktrees) }) } // getBranchDisplayStrings returns the display string of branch func getBranchDisplayStrings( b *models.Branch, + itemOperation types.ItemOperation, fullDescription bool, diffed bool, tr *i18n.TranslationSet, @@ -51,7 +54,7 @@ func getBranchDisplayStrings( } coloredName := nameTextStyle.Sprint(displayName) - branchStatus := utils.WithPadding(ColoredBranchStatus(b, tr), 2, utils.AlignLeft) + branchStatus := utils.WithPadding(ColoredBranchStatus(b, itemOperation, tr), 2, utils.AlignLeft) if git_commands.CheckedOutByOtherWorktree(b, worktrees) { worktreeIcon := lo.Ternary(icons.IsIconEnabled(), icons.LINKED_WORKTREE_ICON, fmt.Sprintf("(%s)", tr.LcWorktree)) coloredName = fmt.Sprintf("%s %s", coloredName, style.FgDefault.Sprint(worktreeIcon)) @@ -109,9 +112,11 @@ func GetBranchTextStyle(name string) style.TextStyle { } } -func ColoredBranchStatus(branch *models.Branch, tr *i18n.TranslationSet) string { +func ColoredBranchStatus(branch *models.Branch, itemOperation types.ItemOperation, tr *i18n.TranslationSet) string { colour := style.FgYellow - if branch.UpstreamGone { + if itemOperation != types.ItemOperationNone { + colour = style.FgCyan + } else if branch.UpstreamGone { colour = style.FgRed } else if branch.MatchesUpstream() { colour = style.FgGreen @@ -119,10 +124,15 @@ func ColoredBranchStatus(branch *models.Branch, tr *i18n.TranslationSet) string colour = style.FgMagenta } - return colour.Sprint(BranchStatus(branch, tr)) + return colour.Sprint(BranchStatus(branch, itemOperation, tr)) } -func BranchStatus(branch *models.Branch, tr *i18n.TranslationSet) string { +func BranchStatus(branch *models.Branch, itemOperation types.ItemOperation, tr *i18n.TranslationSet) string { + itemOperationStr := itemOperationToString(itemOperation, tr) + if itemOperationStr != "" { + return itemOperationStr + " " + utils.Loader() + } + if !branch.IsTrackingRemote() { return "" } diff --git a/pkg/gui/presentation/item_operations.go b/pkg/gui/presentation/item_operations.go new file mode 100644 index 000000000..afa48da4f --- /dev/null +++ b/pkg/gui/presentation/item_operations.go @@ -0,0 +1,23 @@ +package presentation + +import ( + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/i18n" +) + +func itemOperationToString(itemOperation types.ItemOperation, tr *i18n.TranslationSet) string { + switch itemOperation { + case types.ItemOperationNone: + return "" + case types.ItemOperationPushing: + return tr.PushingStatus + case types.ItemOperationPulling: + return tr.PullingStatus + case types.ItemOperationFastForwarding: + return tr.FastForwarding + case types.ItemOperationDeleting: + return tr.DeletingStatus + } + + return "" +} diff --git a/pkg/gui/presentation/status.go b/pkg/gui/presentation/status.go index f70210c27..7bf81948f 100644 --- a/pkg/gui/presentation/status.go +++ b/pkg/gui/presentation/status.go @@ -7,14 +7,15 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/gui/presentation/icons" "github.com/jesseduffield/lazygit/pkg/gui/style" + "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/i18n" ) -func FormatStatus(repoName string, currentBranch *models.Branch, linkedWorktreeName string, workingTreeState enums.RebaseMode, tr *i18n.TranslationSet) string { +func FormatStatus(repoName string, currentBranch *models.Branch, itemOperation types.ItemOperation, linkedWorktreeName string, workingTreeState enums.RebaseMode, tr *i18n.TranslationSet) string { status := "" if currentBranch.IsRealBranch() { - status += ColoredBranchStatus(currentBranch, tr) + " " + status += ColoredBranchStatus(currentBranch, itemOperation, tr) + " " } if workingTreeState != enums.REBASE_MODE_NONE { diff --git a/pkg/i18n/chinese.go b/pkg/i18n/chinese.go index ddfeb7254..234d470db 100644 --- a/pkg/i18n/chinese.go +++ b/pkg/i18n/chinese.go @@ -169,7 +169,7 @@ func chineseTranslationSet() TranslationSet { ToggleStagingPanel: `切换到其他面板`, ReturnToFilesPanel: `返回文件面板`, FastForward: `从上游快进此分支`, - FastForwarding: "抓取并快进 {{.branch}} ...", + FastForwarding: "抓取并快进", FoundConflictsTitle: "自动合并失败", ViewMergeRebaseOptions: "查看 合并/变基 选项", NotMergingOrRebasing: "您目前既不进行变基也不进行合并", diff --git a/pkg/i18n/dutch.go b/pkg/i18n/dutch.go index 83ef02698..7abfaa437 100644 --- a/pkg/i18n/dutch.go +++ b/pkg/i18n/dutch.go @@ -134,7 +134,7 @@ func dutchTranslationSet() TranslationSet { ToggleStagingPanel: `Ga naar een ander paneel`, ReturnToFilesPanel: `Ga terug naar het bestanden paneel`, FastForward: `Fast-forward deze branch vanaf zijn upstream`, - FastForwarding: "Fast-forwarding {{.branch}} ...", + FastForwarding: "Fast-forwarding", FoundConflictsTitle: "Conflicten!", ViewMergeRebaseOptions: "Bekijk merge/rebase opties", NotMergingOrRebasing: "Je bent momenteel niet aan het rebasen of mergen", diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 15deab925..0ffeccdbc 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -980,7 +980,7 @@ func EnglishTranslationSet() TranslationSet { ToggleStagingPanel: `Switch to other panel (staged/unstaged changes)`, ReturnToFilesPanel: `Return to files panel`, FastForward: `Fast-forward this branch from its upstream`, - FastForwarding: "Fast-forwarding {{.branch}}", + FastForwarding: "Fast-forwarding", FoundConflictsTitle: "Conflicts!", ViewConflictsMenuItem: "View conflicts", AbortMenuItem: "Abort the %s", diff --git a/pkg/i18n/korean.go b/pkg/i18n/korean.go index e33f74866..d6b7793a0 100644 --- a/pkg/i18n/korean.go +++ b/pkg/i18n/korean.go @@ -170,7 +170,7 @@ func koreanTranslationSet() TranslationSet { ToggleStagingPanel: `패널 전환`, ReturnToFilesPanel: `파일 목록으로 돌아가기`, FastForward: `Fast-forward this branch from its upstream`, - FastForwarding: "Fast-forwarding {{.branch}} ...", + FastForwarding: "Fast-forwarding", FoundConflictsTitle: "Auto-merge failed", ViewMergeRebaseOptions: "View merge/rebase options", NotMergingOrRebasing: "You are currently neither rebasing nor merging", diff --git a/pkg/i18n/russian.go b/pkg/i18n/russian.go index 9a4b727e0..3332f72cd 100644 --- a/pkg/i18n/russian.go +++ b/pkg/i18n/russian.go @@ -201,7 +201,7 @@ func RussianTranslationSet() TranslationSet { ToggleStagingPanel: `Переключиться на другую панель (проиндексированные/непроиндексированные изменения)`, ReturnToFilesPanel: `Вернуться к панели файлов`, FastForward: `Перемотать эту ветку вперёд из её upstream-ветки`, - FastForwarding: "Получить изменения и перемотать вперёд {{.branch}} ...", + FastForwarding: "Получить изменения и перемотать вперёд", FoundConflictsTitle: "Конфликты!", ViewConflictsMenuItem: "Просмотр конфликтов", AbortMenuItem: "Прервать %s", diff --git a/pkg/i18n/traditional_chinese.go b/pkg/i18n/traditional_chinese.go index ae95f71e8..703c6667c 100644 --- a/pkg/i18n/traditional_chinese.go +++ b/pkg/i18n/traditional_chinese.go @@ -234,7 +234,7 @@ func traditionalChineseTranslationSet() TranslationSet { ToggleStagingPanel: `切換至另一個面板 (已預存/未預存更改)`, ReturnToFilesPanel: `返回檔案面板`, FastForward: `從上游快進此分支`, - FastForwarding: "{{.branch}} 的擷取和快進中...", + FastForwarding: "的擷取和快進中", FoundConflictsTitle: "自動合併失敗", ViewMergeRebaseOptions: "查看合併/變基選項", NotMergingOrRebasing: "你當前既不在變基也不在合併中", From 3d6965ccbbd992adc7cd68015dfd38309cbc8835 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 8 Oct 2023 16:46:04 +0200 Subject: [PATCH 5/9] Add inline status for pushing tags and deleting remote tags --- pkg/commands/models/tag.go | 4 ++++ pkg/gui/context/tags_context.go | 5 ++++- pkg/gui/controllers/tags_controller.go | 13 ++++++++++--- pkg/gui/presentation/tags.go | 21 +++++++++++++++++---- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/pkg/commands/models/tag.go b/pkg/commands/models/tag.go index ab6076a7a..24cb83254 100644 --- a/pkg/commands/models/tag.go +++ b/pkg/commands/models/tag.go @@ -24,6 +24,10 @@ func (t *Tag) ID() string { return t.RefName() } +func (t *Tag) URN() string { + return "tag-" + t.ID() +} + func (t *Tag) Description() string { return t.Message } diff --git a/pkg/gui/context/tags_context.go b/pkg/gui/context/tags_context.go index 4a9f525f6..3da5a9576 100644 --- a/pkg/gui/context/tags_context.go +++ b/pkg/gui/context/tags_context.go @@ -27,7 +27,10 @@ func NewTagsContext( ) getDisplayStrings := func(_ int, _ int) [][]string { - return presentation.GetTagListDisplayStrings(viewModel.GetItems(), c.Modes().Diffing.Ref) + return presentation.GetTagListDisplayStrings( + viewModel.GetItems(), + c.State().GetItemOperation, + c.Modes().Diffing.Ref, c.Tr) } return &TagsContext{ diff --git a/pkg/gui/controllers/tags_controller.go b/pkg/gui/controllers/tags_controller.go index 2fa24f734..224ad83a4 100644 --- a/pkg/gui/controllers/tags_controller.go +++ b/pkg/gui/controllers/tags_controller.go @@ -129,9 +129,9 @@ func (self *TagsController) remoteDelete(tag *models.Tag) error { Title: confirmTitle, Prompt: confirmPrompt, HandleConfirm: func() error { - return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(t gocui.Task) error { + return self.c.WithInlineStatus(tag, types.ItemOperationDeleting, context.TAGS_CONTEXT_KEY, func(task gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.DeleteRemoteTag) - if err := self.c.Git().Remote.DeleteRemoteTag(t, upstream, tag.Name); err != nil { + if err := self.c.Git().Remote.DeleteRemoteTag(task, upstream, tag.Name); err != nil { return err } self.c.Toast(self.c.Tr.RemoteTagDeletedMessage) @@ -188,9 +188,16 @@ func (self *TagsController) push(tag *models.Tag) error { InitialContent: "origin", FindSuggestionsFunc: self.c.Helpers().Suggestions.GetRemoteSuggestionsFunc(), HandleConfirm: func(response string) error { - return self.c.WithWaitingStatus(self.c.Tr.PushingTagStatus, func(task gocui.Task) error { + return self.c.WithInlineStatus(tag, types.ItemOperationPushing, context.TAGS_CONTEXT_KEY, func(task gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.PushTag) err := self.c.Git().Tag.Push(task, response, tag.Name) + + // Render again to remove the inline status: + self.c.OnUIThread(func() error { + _ = self.c.Contexts().Tags.HandleRender() + return nil + }) + return err }) }, diff --git a/pkg/gui/presentation/tags.go b/pkg/gui/presentation/tags.go index 13a0c9d77..9bdabcf40 100644 --- a/pkg/gui/presentation/tags.go +++ b/pkg/gui/presentation/tags.go @@ -4,19 +4,27 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/presentation/icons" "github.com/jesseduffield/lazygit/pkg/gui/style" + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/i18n" "github.com/jesseduffield/lazygit/pkg/theme" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) -func GetTagListDisplayStrings(tags []*models.Tag, diffName string) [][]string { +func GetTagListDisplayStrings( + tags []*models.Tag, + getItemOperation func(item types.HasUrn) types.ItemOperation, + diffName string, + tr *i18n.TranslationSet, +) [][]string { return lo.Map(tags, func(tag *models.Tag, _ int) []string { diffed := tag.Name == diffName - return getTagDisplayStrings(tag, diffed) + return getTagDisplayStrings(tag, getItemOperation(tag), diffed, tr) }) } // getTagDisplayStrings returns the display string of branch -func getTagDisplayStrings(t *models.Tag, diffed bool) []string { +func getTagDisplayStrings(t *models.Tag, itemOperation types.ItemOperation, diffed bool, tr *i18n.TranslationSet) []string { textStyle := theme.DefaultTextColor if diffed { textStyle = theme.DiffTerminalColor @@ -26,6 +34,11 @@ func getTagDisplayStrings(t *models.Tag, diffed bool) []string { res = append(res, textStyle.Sprint(icons.IconForTag(t))) } descriptionColor := style.FgYellow - res = append(res, textStyle.Sprint(t.Name), descriptionColor.Sprint(t.Description())) + descriptionStr := descriptionColor.Sprint(t.Description()) + itemOperationStr := itemOperationToString(itemOperation, tr) + if itemOperationStr != "" { + descriptionStr = style.FgCyan.Sprint(itemOperationStr+" "+utils.Loader()) + " " + descriptionStr + } + res = append(res, textStyle.Sprint(t.Name), descriptionStr) return res } From fd9d7cb7bb6f143147f77c6481eeeaf7cf8e4734 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 25 Sep 2023 21:36:01 +0200 Subject: [PATCH 6/9] Disallow checking out another branch while the current one is being pulled --- pkg/gui/controllers/branches_controller.go | 19 ++++++++++++++++--- pkg/i18n/english.go | 2 ++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 407dc135a..403c51b4f 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -34,9 +34,10 @@ func NewBranchesController( func (self *BranchesController) GetKeybindings(opts types.KeybindingsOpts) []*types.Binding { return []*types.Binding{ { - Key: opts.GetKey(opts.Config.Universal.Select), - Handler: self.checkSelected(self.press), - Description: self.c.Tr.Checkout, + Key: opts.GetKey(opts.Config.Universal.Select), + Handler: self.checkSelected(self.press), + GetDisabledReason: self.getDisabledReasonForPress, + Description: self.c.Tr.Checkout, }, { Key: opts.GetKey(opts.Config.Universal.New), @@ -299,6 +300,18 @@ func (self *BranchesController) press(selectedBranch *models.Branch) error { return self.c.Helpers().Refs.CheckoutRef(selectedBranch.Name, types.CheckoutRefOptions{}) } +func (self *BranchesController) getDisabledReasonForPress() string { + currentBranch := self.c.Helpers().Refs.GetCheckedOutRef() + if currentBranch != nil { + op := self.c.State().GetItemOperation(currentBranch) + if op == types.ItemOperationFastForwarding || op == types.ItemOperationPulling { + return self.c.Tr.CantCheckoutBranchWhilePulling + } + } + + return "" +} + func (self *BranchesController) worktreeForBranch(branch *models.Branch) (*models.Worktree, bool) { return git_commands.WorktreeForBranch(branch, self.c.Model().Worktrees) } diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 0ffeccdbc..29f507b3c 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -57,6 +57,7 @@ type TranslationSet struct { ResetFilter string MergeConflictsTitle string Checkout string + CantCheckoutBranchWhilePulling string NoChangedFiles string SoftReset string AlreadyCheckedOutBranch string @@ -846,6 +847,7 @@ func EnglishTranslationSet() TranslationSet { Scroll: "Scroll", MergeConflictsTitle: "Merge conflicts", Checkout: "Checkout", + CantCheckoutBranchWhilePulling: "You cannot checkout another branch while pulling the current branch", FileFilter: "Filter files by status", FilterStagedFiles: "Show only staged files", FilterUnstagedFiles: "Show only unstaged files", From 67d6447e12a7f1d7af0171d4b905adde20c877ab Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 25 Sep 2023 21:34:10 +0200 Subject: [PATCH 7/9] Disallow pulling/pushing a branch while the branch is pushed or pulled --- pkg/gui/controllers/sync_controller.go | 26 ++++++++++++++++++++------ pkg/i18n/english.go | 2 ++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/pkg/gui/controllers/sync_controller.go b/pkg/gui/controllers/sync_controller.go index 46d553a84..4e3369e0a 100644 --- a/pkg/gui/controllers/sync_controller.go +++ b/pkg/gui/controllers/sync_controller.go @@ -31,14 +31,16 @@ func NewSyncController( func (self *SyncController) GetKeybindings(opts types.KeybindingsOpts) []*types.Binding { bindings := []*types.Binding{ { - Key: opts.GetKey(opts.Config.Universal.Push), - Handler: opts.Guards.NoPopupPanel(self.HandlePush), - Description: self.c.Tr.Push, + Key: opts.GetKey(opts.Config.Universal.Push), + Handler: opts.Guards.NoPopupPanel(self.HandlePush), + GetDisabledReason: self.getDisabledReasonForPushOrPull, + Description: self.c.Tr.Push, }, { - Key: opts.GetKey(opts.Config.Universal.Pull), - Handler: opts.Guards.NoPopupPanel(self.HandlePull), - Description: self.c.Tr.Pull, + Key: opts.GetKey(opts.Config.Universal.Pull), + Handler: opts.Guards.NoPopupPanel(self.HandlePull), + GetDisabledReason: self.getDisabledReasonForPushOrPull, + Description: self.c.Tr.Pull, }, } @@ -57,6 +59,18 @@ func (self *SyncController) HandlePull() error { return self.branchCheckedOut(self.pull)() } +func (self *SyncController) getDisabledReasonForPushOrPull() string { + currentBranch := self.c.Helpers().Refs.GetCheckedOutRef() + if currentBranch != nil { + op := self.c.State().GetItemOperation(currentBranch) + if op != types.ItemOperationNone { + return self.c.Tr.CantPullOrPushSameBranchTwice + } + } + + return "" +} + func (self *SyncController) branchCheckedOut(f func(*models.Branch) error) func() error { return func() error { currentBranch := self.c.Helpers().Refs.GetCheckedOutRef() diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 29f507b3c..8e3a936ce 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -58,6 +58,7 @@ type TranslationSet struct { MergeConflictsTitle string Checkout string CantCheckoutBranchWhilePulling string + CantPullOrPushSameBranchTwice string NoChangedFiles string SoftReset string AlreadyCheckedOutBranch string @@ -848,6 +849,7 @@ func EnglishTranslationSet() TranslationSet { MergeConflictsTitle: "Merge conflicts", Checkout: "Checkout", CantCheckoutBranchWhilePulling: "You cannot checkout another branch while pulling the current branch", + CantPullOrPushSameBranchTwice: "You cannot push or pull a branch while it is already being pushed or pulled", FileFilter: "Filter files by status", FilterStagedFiles: "Show only staged files", FilterUnstagedFiles: "Show only unstaged files", From be3b4bd79133df5001f89f8f8c997fbcbe9a30d9 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 24 Sep 2023 12:43:31 +0200 Subject: [PATCH 8/9] Remove sync mutex I'm pretty convinced we don't need it. Git itself does a good job of making sure that concurrent operations don't corrupt anything. --- pkg/commands/git.go | 6 +----- pkg/commands/git_commands/common.go | 5 ----- pkg/commands/git_commands/remote.go | 4 ++-- pkg/commands/git_commands/sync.go | 9 ++++----- pkg/commands/git_commands/tag.go | 2 +- pkg/gui/controllers/helpers/repos_helper.go | 5 ----- pkg/gui/gui.go | 2 -- pkg/gui/types/common.go | 1 - 8 files changed, 8 insertions(+), 26 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index f057f8d54..510661034 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/go-errors/errors" - "github.com/sasha-s/go-deadlock" "github.com/spf13/afero" gogit "github.com/jesseduffield/go-git/v5" @@ -63,7 +62,6 @@ func NewGitCommand( version *git_commands.GitVersion, osCommand *oscommands.OSCommand, gitConfig git_config.IGitConfig, - syncMutex *deadlock.Mutex, ) (*GitCommand, error) { currentPath, err := os.Getwd() if err != nil { @@ -118,7 +116,6 @@ func NewGitCommand( gitConfig, repoPaths, repository, - syncMutex, ), nil } @@ -129,7 +126,6 @@ func NewGitCommandAux( gitConfig git_config.IGitConfig, repoPaths *git_commands.RepoPaths, repo *gogit.Repository, - syncMutex *deadlock.Mutex, ) *GitCommand { cmd := NewGitCmdObjBuilder(cmn.Log, osCommand.Cmd) @@ -140,7 +136,7 @@ func NewGitCommandAux( // common ones are: cmn, osCommand, dotGitDir, configCommands configCommands := git_commands.NewConfigCommands(cmn, gitConfig, repo) - gitCommon := git_commands.NewGitCommon(cmn, version, cmd, osCommand, repoPaths, repo, configCommands, syncMutex) + gitCommon := git_commands.NewGitCommon(cmn, version, cmd, osCommand, repoPaths, repo, configCommands) fileLoader := git_commands.NewFileLoader(gitCommon, cmd, configCommands) statusCommands := git_commands.NewStatusCommands(gitCommon) diff --git a/pkg/commands/git_commands/common.go b/pkg/commands/git_commands/common.go index cf8250863..b9537165c 100644 --- a/pkg/commands/git_commands/common.go +++ b/pkg/commands/git_commands/common.go @@ -4,7 +4,6 @@ import ( gogit "github.com/jesseduffield/go-git/v5" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" - "github.com/sasha-s/go-deadlock" ) type GitCommon struct { @@ -15,8 +14,6 @@ type GitCommon struct { repoPaths *RepoPaths repo *gogit.Repository config *ConfigCommands - // mutex for doing things like push/pull/fetch - syncMutex *deadlock.Mutex } func NewGitCommon( @@ -27,7 +24,6 @@ func NewGitCommon( repoPaths *RepoPaths, repo *gogit.Repository, config *ConfigCommands, - syncMutex *deadlock.Mutex, ) *GitCommon { return &GitCommon{ Common: cmn, @@ -37,6 +33,5 @@ func NewGitCommon( repoPaths: repoPaths, repo: repo, config: config, - syncMutex: syncMutex, } } diff --git a/pkg/commands/git_commands/remote.go b/pkg/commands/git_commands/remote.go index ce8f79442..acfb51dc9 100644 --- a/pkg/commands/git_commands/remote.go +++ b/pkg/commands/git_commands/remote.go @@ -53,7 +53,7 @@ func (self *RemoteCommands) DeleteRemoteBranch(task gocui.Task, remoteName strin Arg(remoteName, "--delete", branchName). ToArgv() - return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run() + return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run() } func (self *RemoteCommands) DeleteRemoteTag(task gocui.Task, remoteName string, tagName string) error { @@ -61,7 +61,7 @@ func (self *RemoteCommands) DeleteRemoteTag(task gocui.Task, remoteName string, Arg(remoteName, "--delete", tagName). ToArgv() - return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run() + return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run() } // CheckRemoteBranchExists Returns remote branch diff --git a/pkg/commands/git_commands/sync.go b/pkg/commands/git_commands/sync.go index c32286e6d..fd7584aea 100644 --- a/pkg/commands/git_commands/sync.go +++ b/pkg/commands/git_commands/sync.go @@ -36,7 +36,7 @@ func (self *SyncCommands) PushCmdObj(task gocui.Task, opts PushOpts) (oscommands ArgIf(opts.UpstreamBranch != "", opts.UpstreamBranch). ToArgv() - cmdObj := self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex) + cmdObj := self.cmd.New(cmdArgs).PromptOnCredentialRequest(task) return cmdObj, nil } @@ -70,7 +70,6 @@ func (self *SyncCommands) FetchBackgroundCmdObj() oscommands.ICmdObj { cmdObj := self.cmd.New(cmdArgs) cmdObj.DontLog().FailOnCredentialRequest() - cmdObj.WithMutex(self.syncMutex) return cmdObj } @@ -96,7 +95,7 @@ func (self *SyncCommands) Pull(task gocui.Task, opts PullOptions) error { // setting GIT_SEQUENCE_EDITOR to ':' as a way of skipping it, in case the user // has 'pull.rebase = interactive' configured. - return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run() + return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task).Run() } func (self *SyncCommands) FastForward( @@ -110,7 +109,7 @@ func (self *SyncCommands) FastForward( Arg(remoteBranchName + ":" + branchName). ToArgv() - return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run() + return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run() } func (self *SyncCommands) FetchRemote(task gocui.Task, remoteName string) error { @@ -118,5 +117,5 @@ func (self *SyncCommands) FetchRemote(task gocui.Task, remoteName string) error Arg(remoteName). ToArgv() - return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run() + return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run() } diff --git a/pkg/commands/git_commands/tag.go b/pkg/commands/git_commands/tag.go index 0656e1e19..d2b01ba7e 100644 --- a/pkg/commands/git_commands/tag.go +++ b/pkg/commands/git_commands/tag.go @@ -52,5 +52,5 @@ func (self *TagCommands) Push(task gocui.Task, remoteName string, tagName string cmdArgs := NewGitCmd("push").Arg(remoteName, "tag", tagName). ToArgv() - return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run() + return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run() } diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index 3ebd7767d..59d45e0c1 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -173,11 +173,6 @@ func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey return err } - // these two mutexes are used by our background goroutines (triggered via `self.goEvery`. We don't want to - // switch to a repo while one of these goroutines is in the process of updating something - self.c.Mutexes().SyncMutex.Lock() - defer self.c.Mutexes().SyncMutex.Unlock() - self.c.Mutexes().RefreshingFilesMutex.Lock() defer self.c.Mutexes().RefreshingFilesMutex.Unlock() diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 33a2e3d02..787bdd169 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -300,7 +300,6 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context gui.gitVersion, gui.os, git_config.NewStdCachedGitConfig(gui.Log), - gui.Mutexes.SyncMutex, ) if err != nil { return err @@ -490,7 +489,6 @@ func NewGui( RefreshingFilesMutex: &deadlock.Mutex{}, RefreshingBranchesMutex: &deadlock.Mutex{}, RefreshingStatusMutex: &deadlock.Mutex{}, - SyncMutex: &deadlock.Mutex{}, LocalCommitsMutex: &deadlock.Mutex{}, SubCommitsMutex: &deadlock.Mutex{}, AuthorsMutex: &deadlock.Mutex{}, diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index e9f9fc266..84ad874f3 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -262,7 +262,6 @@ type Mutexes struct { RefreshingFilesMutex *deadlock.Mutex RefreshingBranchesMutex *deadlock.Mutex RefreshingStatusMutex *deadlock.Mutex - SyncMutex *deadlock.Mutex LocalCommitsMutex *deadlock.Mutex SubCommitsMutex *deadlock.Mutex AuthorsMutex *deadlock.Mutex From 235f5bb22100ad5fd7f338680b6d37f55c07005c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 26 Sep 2023 17:16:11 +0200 Subject: [PATCH 9/9] Avoid rendering branches view twice when refreshing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refreshWorktrees re-renders the branches view, because the branches view shows worktrees against branches. This means that when both BRANCHES and WORKTREES are requested to be refreshed, the branches view would be rendered twice in short succession. This causes an ugly visual glitch when force-pushing a branch, because when pushing is done, we would see the ↑4↓9 status come back from under the Pushing status for a brief moment, to be replaced with a green checkmark a moment later. Fix this by including the worktree refresh in the branches refresh when both are requested. This means that the two are no longer running in parallel for an async refresh, but hopefully that's not so bad. --- pkg/gui/controllers/helpers/refresh_helper.go | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index bec0a10e1..a299ea431 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -115,12 +115,15 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error { } } + includeWorktreesWithBranches := false if scopeSet.Includes(types.COMMITS) || scopeSet.Includes(types.BRANCHES) || scopeSet.Includes(types.REFLOG) || scopeSet.Includes(types.BISECT_INFO) { // whenever we change commits, we should update branches because the upstream/downstream // counts can change. Whenever we change branches we should also change commits // e.g. in the case of switching branches. refresh("commits and commit files", self.refreshCommitsAndCommitFiles) - refresh("reflog and branches", self.refreshReflogAndBranches) + + includeWorktreesWithBranches = scopeSet.Includes(types.WORKTREES) + refresh("reflog and branches", func() { self.refreshReflogAndBranches(includeWorktreesWithBranches) }) } else if scopeSet.Includes(types.REBASE_COMMITS) { // the above block handles rebase commits so we only need to call this one // if we've asked specifically for rebase commits and not those other things @@ -157,7 +160,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error { refresh("remotes", func() { _ = self.refreshRemotes() }) } - if scopeSet.Includes(types.WORKTREES) { + if scopeSet.Includes(types.WORKTREES) && !includeWorktreesWithBranches { refresh("worktrees", func() { _ = self.refreshWorktrees() }) } @@ -242,7 +245,7 @@ func (self *RefreshHelper) refreshReflogCommitsConsideringStartup() { case types.INITIAL: self.c.OnWorker(func(_ gocui.Task) { _ = self.refreshReflogCommits() - self.refreshBranches() + self.refreshBranches(false) self.c.State().GetRepoState().SetStartupStage(types.COMPLETE) }) @@ -251,10 +254,10 @@ func (self *RefreshHelper) refreshReflogCommitsConsideringStartup() { } } -func (self *RefreshHelper) refreshReflogAndBranches() { +func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool) { self.refreshReflogCommitsConsideringStartup() - self.refreshBranches() + self.refreshBranches(refreshWorktrees) } func (self *RefreshHelper) refreshCommitsAndCommitFiles() { @@ -419,7 +422,7 @@ func (self *RefreshHelper) refreshStateSubmoduleConfigs() error { // self.refreshStatus is called at the end of this because that's when we can // be sure there is a State.Model.Branches array to pick the current branch from -func (self *RefreshHelper) refreshBranches() { +func (self *RefreshHelper) refreshBranches(refreshWorktrees bool) { self.c.Mutexes().RefreshingBranchesMutex.Lock() defer self.c.Mutexes().RefreshingBranchesMutex.Unlock() @@ -443,6 +446,13 @@ func (self *RefreshHelper) refreshBranches() { self.c.Model().Branches = branches + if refreshWorktrees { + self.loadWorktrees() + if err := self.c.PostRefreshUpdate(self.c.Contexts().Worktrees); err != nil { + self.c.Log.Error(err) + } + } + if err := self.c.PostRefreshUpdate(self.c.Contexts().Branches); err != nil { self.c.Log.Error(err) } @@ -636,15 +646,18 @@ func (self *RefreshHelper) refreshRemotes() error { return nil } -func (self *RefreshHelper) refreshWorktrees() error { +func (self *RefreshHelper) loadWorktrees() { worktrees, err := self.c.Git().Loaders.Worktrees.GetWorktrees() if err != nil { self.c.Log.Error(err) self.c.Model().Worktrees = []*models.Worktree{} - return nil } self.c.Model().Worktrees = worktrees +} + +func (self *RefreshHelper) refreshWorktrees() error { + self.loadWorktrees() // need to refresh branches because the branches view shows worktrees against // branches