diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index 4a87c98dacf..fe53697c8e2 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -55,9 +55,6 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error cmd.Env = append(cmd.Env, "GIT_PAGER="+pager) - _, height := view.Size() - _, oy := view.Origin() - manager := gui.getManager(view) var ptmx *os.File @@ -82,7 +79,8 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error gui.Mutexes.PtyMutex.Unlock() } - if err := manager.NewTask(manager.NewCmdTask(start, prefix, height+oy+10, onClose), cmdStr); err != nil { + linesToRead := gui.linesToReadFromCmdTask(view) + if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil { return err } diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 33aa09eb9d6..69b64d7b12f 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -16,9 +16,6 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error cmdStr, ).Debug("RunCommand") - _, height := view.Size() - _, oy := view.Origin() - manager := gui.getManager(view) start := func() (*exec.Cmd, io.Reader) { @@ -35,7 +32,8 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error return cmd, r } - if err := manager.NewTask(manager.NewCmdTask(start, prefix, height+oy+10, nil), cmdStr); err != nil { + linesToRead := gui.linesToReadFromCmdTask(view) + if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, nil), cmdStr); err != nil { gui.c.Log.Error(err) } diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index c51942b03d2..8f205524554 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -6,6 +6,7 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/gui/keybindings" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/tasks" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/spkg/bom" ) @@ -15,6 +16,33 @@ func (gui *Gui) resetOrigin(v *gocui.View) error { return v.SetOrigin(0, 0) } +// 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. +func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { + _, height := v.Size() + _, oy := v.Origin() + + linesForFirstRefresh := height + oy + 10 + + // We want to read as many lines initially as necessary to let the + // scrollbar go to its minimum height, so that the scrollbar thumb doesn't + // change size as you scroll down. + minScrollbarHeight := 2 + linesToReadForAccurateScrollbar := height*(height-1)/minScrollbarHeight + oy + + // However, cap it at some arbitrary max limit, so that we don't get + // performance problems for huge monitors or tiny font sizes + if linesToReadForAccurateScrollbar > 5000 { + linesToReadForAccurateScrollbar = 5000 + } + + return tasks.LinesToRead{ + Total: linesToReadForAccurateScrollbar, + InitialRefreshAfter: linesForFirstRefresh, + } +} + func (gui *Gui) cleanString(s string) string { output := string(bom.Clean([]byte(s))) return utils.NormalizeLinefeeds(output) diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 448857fca68..896af91ab86 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -39,7 +39,7 @@ type ViewBufferManager struct { taskIDMutex deadlock.Mutex Log *logrus.Entry newTaskID int - readLines chan int + readLines chan LinesToRead taskKey string onNewKey func() @@ -55,6 +55,16 @@ type ViewBufferManager struct { throttle bool } +type LinesToRead struct { + // Total number of lines to read + Total int + + // Number of lines after which we have read enough to fill the view, and can + // do an initial refresh. Only set for the initial read request; -1 for + // subsequent requests. + InitialRefreshAfter int +} + func (m *ViewBufferManager) GetTaskKey() string { return m.taskKey } @@ -73,19 +83,19 @@ func NewViewBufferManager( beforeStart: beforeStart, refreshView: refreshView, onEndOfInput: onEndOfInput, - readLines: make(chan int, 1024), + readLines: make(chan LinesToRead, 1024), onNewKey: onNewKey, } } func (self *ViewBufferManager) ReadLines(n int) { go utils.Safe(func() { - self.readLines <- n + self.readLines <- LinesToRead{Total: n, InitialRefreshAfter: -1} }) } // note: onDone may be called twice -func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), prefix string, linesToRead int, onDone func()) func(chan struct{}) error { +func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), prefix string, linesToRead LinesToRead, onDone func()) func(chan struct{}) error { return func(stop chan struct{}) error { var once sync.Once var onDoneWrapper func() @@ -130,7 +140,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p loadingMutex := deadlock.Mutex{} // not sure if it's the right move to redefine this or not - self.readLines = make(chan int, 1024) + self.readLines = make(chan LinesToRead, 1024) done := make(chan struct{}) @@ -163,7 +173,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p case <-stop: break outer case linesToRead := <-self.readLines: - for i := 0; i < linesToRead; i++ { + for i := 0; i < linesToRead.Total; i++ { select { case <-stop: break outer @@ -188,6 +198,13 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p break outer } _, _ = self.writer.Write(append(scanner.Bytes(), '\n')) + + if i+1 == linesToRead.InitialRefreshAfter { + // We have read enough lines to fill the view, so do a first refresh + // here to show what we have. Continue reading and refresh again at + // the end to make sure the scrollbar has the right size. + self.refreshView() + } } self.refreshView() } diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index 9bd5521628a..f12da405f6b 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -4,6 +4,8 @@ import ( "bytes" "io" "os/exec" + "reflect" + "strings" "sync" "testing" "time" @@ -45,7 +47,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) { return cmd, reader } - fn := manager.NewCmdTask(start, "prefix\n", 20, onDone) + fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1}, onDone) _ = fn(stop) @@ -99,7 +101,7 @@ func TestNewCmdTask(t *testing.T) { return cmd, reader } - fn := manager.NewCmdTask(start, "prefix\n", 20, onDone) + fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1}, onDone) wg := sync.WaitGroup{} wg.Add(1) go func() { @@ -134,3 +136,111 @@ func TestNewCmdTask(t *testing.T) { t.Errorf("expected writer to receive the following content: \n%s\n. But instead it received: %s", expectedContent, actualContent) } } + +// 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 { + totalLinesToYield int + linesYielded int +} + +func (d *BlankLineReader) Read(p []byte) (n int, err error) { + if d.totalLinesToYield == d.linesYielded { + return 0, io.EOF + } + + d.linesYielded += 1 + p[0] = '\n' + return 1, nil +} + +func TestNewCmdTaskRefresh(t *testing.T) { + type scenario struct { + name string + totalTaskLines int + linesToRead LinesToRead + expectedLineCountsOnRefresh []int + } + + scenarios := []scenario{ + { + "total < initialRefreshAfter", + 150, + LinesToRead{100, 120}, + []int{100, 100}, + }, + { + "total == initialRefreshAfter", + 150, + LinesToRead{100, 100}, + []int{100, 100, 100}, + }, + { + "total > initialRefreshAfter", + 150, + LinesToRead{100, 50}, + []int{50, 100, 100}, + }, + { + "initialRefreshAfter == -1", + 150, + LinesToRead{100, -1}, + []int{100, 100}, + }, + { + "totalTaskLines < initialRefreshAfter", + 25, + LinesToRead{100, 50}, + []int{25}, + }, + { + "totalTaskLines between total and initialRefreshAfter", + 75, + LinesToRead{100, 50}, + []int{50, 75}, + }, + } + + for _, s := range scenarios { + writer := bytes.NewBuffer(nil) + lineCountsOnRefresh := []int{} + refreshView := func() { + lineCountsOnRefresh = append(lineCountsOnRefresh, strings.Count(writer.String(), "\n")) + } + + manager := NewViewBufferManager( + utils.NewDummyLog(), + writer, + func() {}, + refreshView, + func() {}, + func() {}, + ) + + stop := make(chan struct{}) + reader := BlankLineReader{totalLinesToYield: s.totalTaskLines} + start := func() (*exec.Cmd, io.Reader) { + // not actually starting this because it's not necessary + cmd := secureexec.Command("blah blah") + + return cmd, &reader + } + + fn := manager.NewCmdTask(start, "", s.linesToRead, func() {}) + wg := sync.WaitGroup{} + wg.Add(1) + go func() { + time.Sleep(100 * time.Millisecond) + close(stop) + wg.Done() + }() + _ = fn(stop) + + wg.Wait() + + if !reflect.DeepEqual(lineCountsOnRefresh, s.expectedLineCountsOnRefresh) { + t.Errorf("%s: expected line counts on refresh: %v, got %v", + s.name, s.expectedLineCountsOnRefresh, lineCountsOnRefresh) + } + } +}