mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
Update the UI after stash operations in a single frame (#5905)
This fixes a regression in 0.64.0: before that version, creating or popping a stash would happen synchronously on the UI thread (including the refresh), blocking the UI until everything changed, including the panel focus. Blocking the UI was not nice of course, but at least the UI update was clean. With 0.64.0 this changed to a background refresh, so that the update to the two panels and the focus change all happened out of sync, which looks rather ugly. Fix this by using Refresh's mechanism to batch UI updates, and switch the panel focus in the Refresh's Then so that it updates at the same time. While we're at it, use a waiting status spinner for these operations; they are usually fast when only few files are involved, but when stashing a large number of files in a larger repo it can be noticeable, and it looks ugly if the confirmation prompt stays on the screen while it is running.
This commit is contained in:
commit
5dec89abfe
20
AGENTS.md
20
AGENTS.md
|
|
@ -390,12 +390,32 @@ Avoid phrasings like:
|
|||
- "cleaner than the previous approach"
|
||||
- "we used to ... but ..."
|
||||
- "after trying X, we found Y"
|
||||
- "X rather than Y", where Y is what the code did before the change
|
||||
|
||||
The iteration story is sometimes worth preserving — but it belongs in the
|
||||
commit message, which is the durable record of *why this change was made*. The
|
||||
code comment should make sense to someone who has never seen any prior version
|
||||
and is just trying to understand the file as it currently exists.
|
||||
|
||||
The tell is subtler than an explicit "we used to". A comment that justifies the
|
||||
code against an alternative — "run it on a worker rather than blocking the UI",
|
||||
"switch panels in `Then` rather than a moment earlier" — is history in disguise
|
||||
whenever that alternative is what the code did before the change. It reads as
|
||||
ordinary rationale, but the reader has no way to know the contrast is with a
|
||||
version that no longer exists.
|
||||
|
||||
So the check to apply is: would you have written this comment if you were
|
||||
writing the file from scratch, with no diff in mind? If not, the sentence
|
||||
belongs in the commit message.
|
||||
|
||||
## Don't justify routine call sites
|
||||
|
||||
If the codebase calls a helper in twenty places without explanation, your
|
||||
twenty-first call site doesn't need one either. A comment there says "something
|
||||
here is unusual"; when nothing is, it's noise — and it invites exactly the kind
|
||||
of before/after justification the section above warns about. Look at the
|
||||
neighboring call sites before writing one: if they're bare, match them.
|
||||
|
||||
## Don't present "live with the bug" as an option
|
||||
|
||||
When you're investigating a defect and laying out fix options for the user,
|
||||
|
|
|
|||
|
|
@ -1508,13 +1508,20 @@ func (self *FilesController) handleStashSave(stashFunc func(message string) erro
|
|||
self.c.Prompt(types.PromptOpts{
|
||||
Title: self.c.Tr.StashChanges,
|
||||
HandleConfirm: func(stashComment string) error {
|
||||
self.c.LogAction(action)
|
||||
return self.c.WithWaitingStatusBlockingInput(
|
||||
types.WaitingStatusOpts{Message: self.c.Tr.StashingStatus},
|
||||
func(gocui.Task) error {
|
||||
self.c.LogAction(action)
|
||||
|
||||
if err := stashFunc(stashComment); err != nil {
|
||||
return err
|
||||
}
|
||||
self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH, types.FILES}})
|
||||
return nil
|
||||
if err := stashFunc(stashComment); err != nil {
|
||||
return err
|
||||
}
|
||||
self.c.RefreshFromWorker(types.RefreshOptions{
|
||||
BatchUIUpdates: true,
|
||||
Scope: []types.RefreshableView{types.STASH, types.FILES},
|
||||
})
|
||||
return nil
|
||||
})
|
||||
},
|
||||
AllowEmptyInput: true,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import (
|
|||
"fmt"
|
||||
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/models"
|
||||
"github.com/jesseduffield/lazygit/pkg/gocui"
|
||||
"github.com/jesseduffield/lazygit/pkg/gui/context"
|
||||
"github.com/jesseduffield/lazygit/pkg/gui/style"
|
||||
"github.com/jesseduffield/lazygit/pkg/gui/types"
|
||||
|
|
@ -120,33 +121,29 @@ func (self *StashController) handleStashApply(stashEntry *models.StashEntry) err
|
|||
Title: self.c.Tr.StashApply,
|
||||
Prompt: self.c.Tr.SureApplyStashEntry,
|
||||
HandleConfirm: func() error {
|
||||
self.c.LogAction(self.c.Tr.Actions.ApplyStash)
|
||||
err := self.c.Git().Stash.Apply(stashEntry.Index)
|
||||
self.postStashRefresh()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if self.c.UserConfig().Gui.SwitchToFilesAfterStashApply {
|
||||
self.c.Context().Push(self.c.Contexts().Files, types.OnFocusOpts{})
|
||||
}
|
||||
return nil
|
||||
return self.c.WithWaitingStatusBlockingInput(
|
||||
types.WaitingStatusOpts{Message: self.c.Tr.ApplyingStashStatus},
|
||||
func(gocui.Task) error {
|
||||
self.c.LogAction(self.c.Tr.Actions.ApplyStash)
|
||||
err := self.c.Git().Stash.Apply(stashEntry.Index)
|
||||
self.postStashRefresh(err == nil && self.c.UserConfig().Gui.SwitchToFilesAfterStashApply)
|
||||
return err
|
||||
})
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func (self *StashController) handleStashPop(stashEntry *models.StashEntry) error {
|
||||
pop := func() error {
|
||||
self.c.LogAction(self.c.Tr.Actions.PopStash)
|
||||
self.c.LogCommand(fmt.Sprintf(self.c.Tr.Log.PoppingStash, stashEntry.Hash), false)
|
||||
err := self.c.Git().Stash.Pop(stashEntry.Index)
|
||||
self.postStashRefresh()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if self.c.UserConfig().Gui.SwitchToFilesAfterStashPop {
|
||||
self.c.Context().Push(self.c.Contexts().Files, types.OnFocusOpts{})
|
||||
}
|
||||
return nil
|
||||
return self.c.WithWaitingStatusBlockingInput(
|
||||
types.WaitingStatusOpts{Message: self.c.Tr.PoppingStashStatus},
|
||||
func(gocui.Task) error {
|
||||
self.c.LogAction(self.c.Tr.Actions.PopStash)
|
||||
self.c.LogCommand(fmt.Sprintf(self.c.Tr.Log.PoppingStash, stashEntry.Hash), false)
|
||||
err := self.c.Git().Stash.Pop(stashEntry.Index)
|
||||
self.postStashRefresh(err == nil && self.c.UserConfig().Gui.SwitchToFilesAfterStashPop)
|
||||
return err
|
||||
})
|
||||
}
|
||||
|
||||
if self.c.UserConfig().Gui.SkipStashWarning {
|
||||
|
|
@ -175,31 +172,60 @@ func (self *StashController) handleStashDrop(stashEntries []*models.StashEntry)
|
|||
// iteration lets the workers race and an earlier, stale result can
|
||||
// land last. The indices are captured up front and we drop
|
||||
// highest-first, so the remaining lower indices stay valid without
|
||||
// an intervening refresh. Block input until the refresh has
|
||||
// landed, so that dropping the next entry in quick succession
|
||||
// (confirming and pressing the key again right away) sees the
|
||||
// refreshed list and not the stale, pre-drop indices.
|
||||
defer self.c.RefreshBlockingInput(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH}})
|
||||
// an intervening refresh.
|
||||
var dropErr error
|
||||
for i := len(stashEntries) - 1; i >= 0; i-- {
|
||||
self.c.LogCommand(fmt.Sprintf(self.c.Tr.Log.DroppingStash, stashEntries[i].Hash), false)
|
||||
if err := self.c.Git().Stash.Drop(stashEntries[i].Index); err != nil {
|
||||
return err
|
||||
if dropErr = self.c.Git().Stash.Drop(stashEntries[i].Index); dropErr != nil {
|
||||
break
|
||||
}
|
||||
}
|
||||
self.context().CollapseRangeSelectionToTop()
|
||||
return nil
|
||||
// Block input until the refresh has landed, so that dropping the
|
||||
// next entry in quick succession (confirming and pressing the key
|
||||
// again right away) sees the refreshed list and not the stale,
|
||||
// pre-drop indices.
|
||||
self.c.RefreshBlockingInput(types.RefreshOptions{
|
||||
Scope: []types.RefreshableView{types.STASH},
|
||||
Then: func() error {
|
||||
// Collapse the range selection from here, so that it lands
|
||||
// in the same frame as the shortened list. The refresh has
|
||||
// painted the list by the time Then runs, so the new
|
||||
// selection needs a focus update of its own.
|
||||
if dropErr == nil {
|
||||
self.context().CollapseRangeSelectionToTop()
|
||||
self.context().HandleFocus(types.OnFocusOpts{})
|
||||
}
|
||||
return nil
|
||||
},
|
||||
})
|
||||
return dropErr
|
||||
},
|
||||
})
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (self *StashController) postStashRefresh() {
|
||||
// Block input until the refresh has landed: popping shifts the indices of
|
||||
// the remaining stash entries, and acting on the next entry in quick
|
||||
// succession (confirming the popup and pressing the key again right away)
|
||||
// must see the refreshed list, or it would target the wrong stash.
|
||||
self.c.RefreshBlockingInput(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH, types.FILES}})
|
||||
// postStashRefresh refreshes the panels that applying or popping a stash
|
||||
// affects, moving the focus to the files panel if switchToFiles is set.
|
||||
//
|
||||
// Call it from the worker that ran the stash command, from inside a
|
||||
// WithWaitingStatusBlockingInput: popping shifts the indices of the remaining
|
||||
// stash entries, so acting on the next entry in quick succession (confirming
|
||||
// the popup and pressing the key again right away) has to be held off until
|
||||
// the refreshed list is in place, or it would target the wrong stash.
|
||||
func (self *StashController) postStashRefresh(switchToFiles bool) {
|
||||
self.c.RefreshFromWorker(types.RefreshOptions{
|
||||
BatchUIUpdates: true,
|
||||
Scope: []types.RefreshableView{types.STASH, types.FILES},
|
||||
Then: func() error {
|
||||
// Switch panels from here, so that the focus change lands in the
|
||||
// same frame as the refreshed panel contents.
|
||||
if switchToFiles {
|
||||
self.c.Context().Push(self.c.Contexts().Files, types.OnFocusOpts{})
|
||||
}
|
||||
return nil
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func (self *StashController) handleNewBranchOffStashEntry(stashEntry *models.StashEntry) error {
|
||||
|
|
|
|||
|
|
@ -443,6 +443,9 @@ type TranslationSet struct {
|
|||
MovingCommitsToNewBranchStatus string
|
||||
ApplyingFilterStatus string
|
||||
RemovingFilterStatus string
|
||||
StashingStatus string
|
||||
ApplyingStashStatus string
|
||||
PoppingStashStatus string
|
||||
CommitFiles string
|
||||
SubCommitsDynamicTitle string
|
||||
CommitFilesDynamicTitle string
|
||||
|
|
@ -1603,6 +1606,9 @@ func EnglishTranslationSet() *TranslationSet {
|
|||
MovingCommitsToNewBranchStatus: "Moving commits to new branch",
|
||||
ApplyingFilterStatus: "Applying filter",
|
||||
RemovingFilterStatus: "Removing filter",
|
||||
StashingStatus: "Stashing",
|
||||
ApplyingStashStatus: "Applying stash",
|
||||
PoppingStashStatus: "Popping stash",
|
||||
CommitFiles: "Commit files",
|
||||
SubCommitsDynamicTitle: "Commits (%s)",
|
||||
CommitFilesDynamicTitle: "Diff files (%s)",
|
||||
|
|
|
|||
Loading…
Reference in a new issue