diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index e93eaa4ba..29864bf04 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -210,8 +210,8 @@ func (self *BranchesController) GetOnRenderToMain() func() { pr, ok := self.c.Helpers().Host.PullRequestForBranch(branch.Name) if ok && presentation.ShouldShowPrForBranch(pr, branch.Name, self.c.UserConfig()) { - rendererTask.Prefix = presentation.FormatPullRequestHeader(pr, self.c.Tr) - rendererTask.Prefix += strings.Repeat("─", self.c.Contexts().Normal.GetView().InnerWidth()) + "\n" + rendererTask.Prefix = types.StaticPrefix(presentation.FormatPullRequestHeader(pr, self.c.Tr) + + strings.Repeat("─", self.c.Contexts().Normal.GetView().InnerWidth()) + "\n") } } diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index 1f25e7e4e..ce53ef65b 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -360,7 +360,7 @@ func (self *FilesController) renderNonTextualConflict(node *filetree.FileNode) { prefix += self.c.Tr.MergeConflictCurrentDiff } prefix += "\n\n" - self.renderToMainWithTask(types.NewRunDiffRendererTaskWithPrefix(cmdObj.GetCmd(), prefix)) + self.renderToMainWithTask(types.NewRunDiffRendererTaskWithPrefix(cmdObj.GetCmd(), types.StaticPrefix(prefix))) return } diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index dcff87bf5..a6ef3dc28 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -78,7 +78,7 @@ func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff(commit *models.Comm } cmdObj := self.c.Git().Diff.DiffCmdObj(args, mode) prefix := style.FgYellow.Sprintf("%s %s-%s\n\n", self.c.Tr.ShowingDiffForRange, from.ShortRefName(), to.ShortRefName()) - return types.NewMainViewDiffTaskWithPrefix(cmdObj.GetCmd(), prefix, mode) + return types.NewMainViewDiffTaskWithPrefix(cmdObj.GetCmd(), types.StaticPrefix(prefix), mode) } cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.FilterPathsForCommit(commit), mode) @@ -141,7 +141,7 @@ func (self *DiffHelper) RenderDiff() { self.c.Tr.ShowingGitDiff, "git diff "+strings.Join(args, " "), ) - task := types.NewMainViewDiffTaskWithPrefix(cmdObj.GetCmd(), prefix, git_commands.DiffModeRendered) + task := types.NewMainViewDiffTaskWithPrefix(cmdObj.GetCmd(), types.StaticPrefix(prefix), git_commands.DiffModeRendered) self.c.RenderToMainViews(types.RefreshMainOpts{ Pair: self.c.MainViewPairs().Normal, diff --git a/pkg/gui/controllers/stash_controller.go b/pkg/gui/controllers/stash_controller.go index 4303d8851..7da678a3a 100644 --- a/pkg/gui/controllers/stash_controller.go +++ b/pkg/gui/controllers/stash_controller.go @@ -96,7 +96,7 @@ func (self *StashController) GetOnRenderToMain() func() { prefix := style.FgYellow.Sprintf("%s\n\n", stashEntry.Description()) task = types.NewMainViewDiffTaskWithPrefix( self.c.Git().Stash.ShowStashEntryCmdObj(stashEntry.Index, mode).GetCmd(), - prefix, + types.StaticPrefix(prefix), mode, ) } diff --git a/pkg/gui/controllers/submodules_controller.go b/pkg/gui/controllers/submodules_controller.go index c807e6352..5f9f8df9c 100644 --- a/pkg/gui/controllers/submodules_controller.go +++ b/pkg/gui/controllers/submodules_controller.go @@ -126,7 +126,7 @@ func (self *SubmodulesController) GetOnRenderToMain() func() { task = types.NewRenderStringTask(prefix) } else { cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(file, git_commands.DiffModeRendered, !file.HasUnstagedChanges && file.HasStagedChanges, file.Names()) - task = types.NewRunCommandTaskWithPrefix(cmdObj.GetCmd(), prefix) + task = types.NewRunCommandTaskWithPrefix(cmdObj.GetCmd(), types.StaticPrefix(prefix)) } } diff --git a/pkg/gui/controllers/tags_controller.go b/pkg/gui/controllers/tags_controller.go index 33e932013..e82264750 100644 --- a/pkg/gui/controllers/tags_controller.go +++ b/pkg/gui/controllers/tags_controller.go @@ -109,7 +109,7 @@ func (self *TagsController) GetOnRenderToMain() func() { } else { cmdObj := self.c.Git().Branch.GetGraphCmdObj(tag.FullRefName()) prefix := self.getTagInfo(tag) + "\n\n---\n\n" - task = types.NewRunCommandTaskWithPrefix(cmdObj.GetCmd(), prefix) + task = types.NewRunCommandTaskWithPrefix(cmdObj.GetCmd(), types.StaticPrefix(prefix)) } self.c.RenderToMainViews(types.RefreshMainOpts{ diff --git a/pkg/gui/main_view_render.go b/pkg/gui/main_view_render.go index 4b661bf85..04dd5120a 100644 --- a/pkg/gui/main_view_render.go +++ b/pkg/gui/main_view_render.go @@ -11,6 +11,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/tasks" "github.com/samber/lo" ) @@ -35,7 +36,7 @@ type renderSpec struct { // newRenderTask renders cmd's output into view, through the diff renderer the // user has configured. The renderer lays its rendering out to the width of the // view, which only the layout settles, so the task is created after it. -func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix string) error { +func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix types.Prefix) error { // Ask whatever renders the diff to state, in an OSC 1717 record per line, // which line of which file it is rendering. This lets us act on the line the // user is pointing at even when the rendering no longer looks like a diff. @@ -156,14 +157,21 @@ type runRender func(spec renderSpec) (startRender, onCloseRender) // view, running the command the given way. The task takes the place that the // reservation holds among the view's tasks. key names what is rendered, so that // a re-render of the same content can be told from a render of other content. -func (gui *Gui) newTaskForRender(reservation tasks.TaskReservation, spec renderSpec, prefix string, key string, run runRender) error { +func (gui *Gui) newTaskForRender(reservation tasks.TaskReservation, spec renderSpec, prefix types.Prefix, key string, run runRender) error { setColumnsEnvVar(spec.cmd, spec.width) start, onClose := run(spec) + // The prefix is laid out here, on the UI thread, now that the width is + // known; its text is produced on the task's goroutine. + var producePrefix func() string + if prefix != nil { + producePrefix = prefix(spec.width) + } + manager := gui.getManager(spec.view) linesToRead := gui.linesToReadFromCmdTask(spec.view) - return manager.NewReservedTask(reservation, manager.NewCmdTask(start, prefix, linesToRead, onClose), key) + return manager.NewReservedTask(reservation, manager.NewCmdTask(start, producePrefix, linesToRead, onClose), key) } // renderWithoutPtyEnvVar makes a render take the piped path on a platform that diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 4c37a18ee..c2db5f1ec 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -7,11 +7,12 @@ import ( "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" + "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/tasks" "github.com/sirupsen/logrus" ) -func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error { +func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix types.Prefix) error { cmdStr := strings.Join(cmd.Args, " ") gui.c.Log.WithField( "command", diff --git a/pkg/gui/types/rendering.go b/pkg/gui/types/rendering.go index 8be825791..7e07e4386 100644 --- a/pkg/gui/types/rendering.go +++ b/pkg/gui/types/rendering.go @@ -87,9 +87,32 @@ func NewRenderStringWithScrollTask(str string, originX int, originY int) *Render return &RenderStringWithScrollTask{Str: str, OriginX: originX, OriginY: originY} } +// A Prefix produces what a render of a command shows above the command's output. +// +// It is called with the width the render is laid out to, on the UI thread once the +// layout has settled that width, and returns the function that produces the text. +// That function is called on the render's own goroutine before the command starts, +// so a prefix that takes a while to produce holds up only the render and not the UI. +// Whatever it needs from the UI thread, it reads in the outer function. +type Prefix func(width int) func() string + +// StaticPrefix is a prefix that is the same at any width. +func StaticPrefix(text string) Prefix { + return PrefixForWidth(func(int) string { return text }) +} + +// PrefixForWidth is a prefix that is quick to lay out, so that it is produced in +// full on the UI thread. +func PrefixForWidth(layOut func(width int) string) Prefix { + return func(width int) func() string { + text := layOut(width) + return func() string { return text } + } +} + type RunCommandTask struct { Cmd *exec.Cmd - Prefix string + Prefix Prefix // contentIsDiff marks output that is a panel's own diff; see ContentIsDiff. contentIsDiff bool @@ -101,13 +124,13 @@ func NewRunCommandTask(cmd *exec.Cmd) *RunCommandTask { return &RunCommandTask{Cmd: cmd} } -func NewRunCommandTaskWithPrefix(cmd *exec.Cmd, prefix string) *RunCommandTask { +func NewRunCommandTaskWithPrefix(cmd *exec.Cmd, prefix Prefix) *RunCommandTask { return &RunCommandTask{Cmd: cmd, Prefix: prefix} } type RunDiffRendererTask struct { Cmd *exec.Cmd - Prefix string + Prefix Prefix // contentIsDiff marks output that is a panel's own diff; see ContentIsDiff. contentIsDiff bool @@ -119,7 +142,7 @@ func NewRunDiffRendererTask(cmd *exec.Cmd) *RunDiffRendererTask { return &RunDiffRendererTask{Cmd: cmd} } -func NewRunDiffRendererTaskWithPrefix(cmd *exec.Cmd, prefix string) *RunDiffRendererTask { +func NewRunDiffRendererTaskWithPrefix(cmd *exec.Cmd, prefix Prefix) *RunDiffRendererTask { return &RunDiffRendererTask{Cmd: cmd, Prefix: prefix} } @@ -131,10 +154,10 @@ func NewRunDiffRendererTaskWithPrefix(cmd *exec.Cmd, prefix string) *RunDiffRend // The task it returns is the one that says its output is a diff, so a pane rendering // it can be pointed at (see ContentIsDiff). func NewMainViewDiffTask(cmd *exec.Cmd, mode git_commands.DiffMode) UpdateTask { - return NewMainViewDiffTaskWithPrefix(cmd, "", mode) + return NewMainViewDiffTaskWithPrefix(cmd, nil, mode) } -func NewMainViewDiffTaskWithPrefix(cmd *exec.Cmd, prefix string, mode git_commands.DiffMode) UpdateTask { +func NewMainViewDiffTaskWithPrefix(cmd *exec.Cmd, prefix Prefix, mode git_commands.DiffMode) UpdateTask { if mode == git_commands.DiffModeRaw { task := NewRunCommandTaskWithPrefix(cmd, prefix) task.contentIsDiff = true diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 307b64834..510fffa49 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -421,7 +421,11 @@ func (self *ViewBufferManager) stopServingReadRequests() { } } -func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix string, linesToRead LinesToRead, onDoneFn func()) func(TaskOpts) error { +// NewCmdTask returns a task that renders the output of the command that start +// starts. prefix, unless nil, produces the text shown above that output. It is +// called on the task's goroutine before the command starts, so a prefix that +// takes a while to produce holds up only this task. +func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix func() string, linesToRead LinesToRead, onDoneFn func()) func(TaskOpts) error { return func(opts TaskOpts) error { var onDoneOnce sync.Once var onFirstPageShownOnce sync.Once @@ -462,6 +466,18 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix return nil } + prefixText := "" + if prefix != nil { + prefixText = prefix() + + // A task stopped while it was producing its prefix has no use for the + // command's output any more. + if stopped() { + onDone() + return nil + } + } + startTime := time.Now() cmd, r := start() timeToStart := time.Since(startTime) @@ -683,8 +699,8 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix // displayed until we swap in below; this is what keeps an async // re-render from showing a half-loaded buffer. self.beginRender() - if prefix != "" { - writeToView([]byte(prefix)) + if prefixText != "" { + writeToView([]byte(prefixText)) } loaded = true } diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index f1bd5920d..a5b195a9c 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -60,7 +60,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) { return ExecCmd{Cmd: cmd}, reader } - fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1, nil}, onDone) + fn := manager.NewCmdTask(start, func() string { return "prefix\n" }, LinesToRead{20, -1, nil}, onDone) _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: func() { task.Done() }}) @@ -131,7 +131,7 @@ func TestNewCmdTask(t *testing.T) { return ExecCmd{Cmd: cmd}, reader } - fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1, nil}, onDone) + fn := manager.NewCmdTask(start, func() string { return "prefix\n" }, LinesToRead{20, -1, nil}, onDone) wg := sync.WaitGroup{} wg.Go(func() { time.Sleep(100 * time.Millisecond) @@ -171,6 +171,39 @@ func TestNewCmdTask(t *testing.T) { } } +// A prefix is produced before the command starts, so a task stopped while it +// produces its prefix never gets as far as starting the command. +func TestNewCmdTaskStoppedWhileProducingItsPrefix(t *testing.T) { + noop := func() {} + task := gocui.NewFakeTask() + writer := bytes.NewBuffer(nil) + + manager := NewViewBufferManager( + utils.NewDummyLog(), writer, noop, noop, noop, noop, noop, noop, + func() gocui.Task { return task }, + func(f func()) error { f(); return nil }, + ) + + stop := make(chan struct{}) + started := false + start := func() (Cmd, io.Reader) { + started = true + return ExecCmd{Cmd: exec.Command("true")}, bytes.NewBufferString("output") + } + prefix := func() string { + close(stop) + return "prefix\n" + } + onDone, getOnDoneCallCount := getCounter() + + fn := manager.NewCmdTask(start, prefix, LinesToRead{20, -1, nil}, onDone) + _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: func() { task.Done() }}) + + assert.False(t, started) + assert.Equal(t, 1, getOnDoneCallCount()) + assert.Empty(t, writer.String()) +} + // A dummy reader that simply yields as many blank lines as requested. The only // thing we want to do with the output is count the number of lines. type BlankLineReader struct { @@ -244,7 +277,7 @@ func TestNewCmdTaskQueuedReadAtEndOfInput(t *testing.T) { // The initial request asks for far more lines than the reader has, so the // task reaches EOF while that request is still the one being served. - fn := manager.NewCmdTask(start, "", LinesToRead{100, -1, nil}, func() {}) + fn := manager.NewCmdTask(start, nil, LinesToRead{100, -1, nil}, func() {}) thenCalled := false wg := sync.WaitGroup{} @@ -293,7 +326,7 @@ func TestResetOriginSurvivesTaskReplacement(t *testing.T) { } // The first-paint point is far beyond what any of these readers yield, so // only reaching EOF paints. - _ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, onDone), key) + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), key) } runTaskToCompletion := func(key string) { done := make(chan struct{}) @@ -348,7 +381,7 @@ func TestLoadingIndicatorOnlyTakesOverForNewContent(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, reader } - _ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, onDone), key) + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), key) } // Starts a task whose command produces nothing at all, so that it is still // waiting for its first line when the loading indicator falls due. Returns @@ -434,7 +467,7 @@ func TestNewCmdTaskRestore(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, &BlankLineReader{totalLinesToYield: 50} } - _ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 30, nil}, func() { close(done) }), "cmd") + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 30, nil}, func() { close(done) }), "cmd") <-done assert.Equal(t, 1, applyCount, "Apply should run exactly once") @@ -487,7 +520,7 @@ func TestNewCmdTaskRestoreThatFindsNothing(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, &BlankLineReader{totalLinesToYield: 50} } - _ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 30, nil}, func() { close(done) }), "cmd") + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 30, nil}, func() { close(done) }), "cmd") <-done assert.Equal(t, 1, applyCount, "Apply should still run, to swap the render in") @@ -529,7 +562,7 @@ func TestRestoreSurvivesTaskReplacement(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, reader } - _ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, onDone), "cmd") + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), "cmd") } // The task the restore was installed for stalls before it can paint. @@ -576,7 +609,7 @@ func TestKeepScrollPositionForNextTask(t *testing.T) { // not actually starting this because it's not necessary return ExecCmd{Cmd: exec.Command("blah")}, reader } - _ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, onDone), key) + _ = manager.NewTask(manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, onDone), key) } runTaskToCompletion := func(key string) { done := make(chan struct{}) @@ -637,7 +670,7 @@ func TestForgetRenderedContent(t *testing.T) { } done := make(chan struct{}) _ = manager.NewTask( - manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, func() { close(done) }), key) + manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, func() { close(done) }), key) <-done } @@ -736,7 +769,7 @@ func TestNewCmdTaskRefresh(t *testing.T) { return ExecCmd{Cmd: cmd}, &reader } - fn := manager.NewCmdTask(start, "", s.linesToRead, func() {}) + fn := manager.NewCmdTask(start, nil, s.linesToRead, func() {}) wg := sync.WaitGroup{} wg.Go(func() { time.Sleep(100 * time.Millisecond) @@ -773,7 +806,7 @@ func TestQueuedReadRequestsAreAnsweredWhenTheTaskStops(t *testing.T) { stop := make(chan struct{}) fn := manager.NewCmdTask( func() (Cmd, io.Reader) { return ExecCmd{Cmd: exec.Command("true")}, pipeReader }, - "", LinesToRead{Total: 1, InitialRefreshAfter: -1}, noop) + nil, LinesToRead{Total: 1, InitialRefreshAfter: -1}, noop) go func() { _, _ = pipeWriter.Write([]byte("first line\n")) }() go func() { _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: noop}) }() // Let the task start and read the line it was asked for, so that the requests @@ -901,7 +934,7 @@ func TestMessageEndsTheLoadingOfTheTaskItReplaces(t *testing.T) { reservation := manager.ReserveTask() manager.StartLoading() - _ = manager.NewReservedTask(reservation, manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, nil), "cmd") + _ = manager.NewReservedTask(reservation, manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, nil), "cmd") <-stalled.blocked assert.True(t, manager.IsLoading()) @@ -940,7 +973,7 @@ func TestEarlierTaskEndingLeavesALaterTaskLoading(t *testing.T) { reservation := manager.ReserveTask() manager.StartLoading() _ = manager.NewReservedTask(reservation, - manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, func() { close(earlierDone) }), "earlier") + manager.NewCmdTask(start, nil, LinesToRead{100, 50, nil}, func() { close(earlierDone) }), "earlier") <-stalled.blocked // The later task is asked for, but not created yet, as for a render whose task