diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index ed4002748..ff1cb1073 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -18,12 +18,14 @@ func (gui *Gui) resetViewOrigin(v *gocui.View) { // Returns the number of lines that we should read initially from a cmd task so // that the scrollbar has the correct size, along with the number of lines after -// which the view is filled and we can do a first refresh. +// which the view is filled and we can do a first refresh. Both count from the top +// of the view. The task adds the lines above the view if it shows its content +// where the view is (see LinesToRead.LinesAboveView). func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { height := v.InnerHeight() - oy := v.OriginY() + linesAboveView := v.OriginY() - linesForFirstRefresh := height + oy + 10 + linesForFirstRefresh := height + 10 // A search counts the matches in everything the view holds, so a re-render of a // view that is being searched is read all the way to the end (as opening the @@ -34,6 +36,7 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { return tasks.LinesToRead{ Total: -1, InitialRefreshAfter: linesForFirstRefresh, + LinesAboveView: linesAboveView, } } @@ -44,13 +47,14 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { linesToReadForAccurateScrollbar := min( // However, cap it at some arbitrary max limit, so that we don't get // performance problems for huge monitors or tiny font sizes - height*(height-1)/minScrollbarHeight, 5000) + oy + height*(height-1)/minScrollbarHeight, 5000) return tasks.LinesToRead{ // A view no more than a few lines tall needs fewer lines for its scrollbar // than it takes to fill it, and the task must read at least those. Total: max(linesToReadForAccurateScrollbar, linesForFirstRefresh), InitialRefreshAfter: linesForFirstRefresh, + LinesAboveView: linesAboveView, } } diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 7e0c10676..37f586222 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -174,10 +174,30 @@ type LinesToRead struct { // showing the previous render. InitialRefreshAfter int + // The number of lines above the view when the task was asked for. Only set + // for the initial read request, whose Total and InitialRefreshAfter don't + // count these lines. A task that shows its content where the view is reads + // them as well, so that it fills the view there. A task that shows content + // the view isn't showing yet shows it from the top and doesn't read them. + LinesAboveView int + // Function to call after reading the lines is done Then func() } +// includingLinesAboveView returns the initial read request for a task that shows its +// content where the view is rather than from the top. Such a task reads the lines +// above the view as well. +func (self LinesToRead) includingLinesAboveView() LinesToRead { + if self.Total != -1 { + self.Total += self.LinesAboveView + } + if self.InitialRefreshAfter != -1 { + self.InitialRefreshAfter += self.LinesAboveView + } + return self +} + // RenderRestore puts a view back where it was when it re-renders content the user // is already looking at, laid out differently — a different context size, whitespace // ignored, another diff renderer — instead of showing the new render from the top. @@ -452,6 +472,14 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix // remembered just before the re-render that led here was triggered. restore := self.getRestoreForNextTask() + // Content the view isn't showing yet is shown from the top (see + // newContentPending), and content the view is showing is shown where the + // view is. + initialRequest := linesToRead + if !self.newContentPending.Load() { + initialRequest = linesToRead.includingLinesAboveView() + } + if self.throttle.Load() { self.Log.Info("throttling task") time.Sleep(THROTTLE_TIME) @@ -804,7 +832,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix close(lineWrittenChan) }) - self.readRequests.enqueue(linesToRead) + self.readRequests.enqueue(initialRequest) <-done diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index f0e9597d2..12b1ce52d 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -687,6 +687,62 @@ func TestForgetRenderedContent(t *testing.T) { assert.Equal(t, 2, getResetOriginCallCount(), "an emptied view is shown its content afresh") } +// A render of the content the view is showing keeps the view where it is, so it reads +// the lines above the view as well, both before it first paints and in all. A render +// of content the view isn't showing yet is shown from the top, so the lines above the +// view are no use to it. Reading them would only hold up the first paint of a diff +// selected while the view is scrolled far down another one. +func TestOnlyARenderInPlaceReadsTheLinesAboveTheView(t *testing.T) { + writer := bytes.NewBuffer(nil) + linesWritten := func() int { return strings.Count(writer.String(), "\n") } + linesWrittenAtFirstPaint := -1 + + manager := NewViewBufferManager( + utils.NewDummyLog(), + writer, + func() {}, // beforeStart + func() {}, // refreshView + func() {}, // onEndOfInput + func() {}, // resetOrigin + writer.Reset, + func() { linesWrittenAtFirstPaint = linesWritten() }, + func() gocui.Task { return gocui.NewFakeTask() }, + // no UI thread in the test; run the view mutations inline + func(f func()) error { f(); return nil }, + ) + defer manager.Close() + + // Renders the given command's output, which is longer than any of the reading + // asked for here, and returns once the task has read the lines it was asked for. + render := func(key string) { + start := func() (Cmd, io.Reader) { + // not actually starting this because it's not necessary + return ExecCmd{Cmd: exec.Command("blah")}, &BlankLineReader{totalLinesToYield: 1000} + } + served := make(chan struct{}) + linesToRead := LinesToRead{ + Total: 20, + InitialRefreshAfter: 15, + LinesAboveView: 100, + Then: func() { close(served) }, + } + _ = manager.NewTask(manager.NewCmdTask(start, nil, linesToRead, nil), key) + <-served + } + + render("cmd1") + assert.Equal(t, 15, linesWrittenAtFirstPaint) + assert.Equal(t, 20, linesWritten()) + + render("cmd1") + assert.Equal(t, 115, linesWrittenAtFirstPaint) + assert.Equal(t, 120, linesWritten()) + + render("cmd2") + assert.Equal(t, 15, linesWrittenAtFirstPaint) + assert.Equal(t, 20, linesWritten()) +} + func TestNewCmdTaskRefresh(t *testing.T) { type scenario struct { name string