From ef3a4c71ccf82a6b0edeb3bd3df8fce3678f22dd Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 15:09:38 +0200 Subject: [PATCH 01/10] Addition to AGENTS.md --- AGENTS.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index d65a623d9..f38a336e0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,6 +82,17 @@ while still being meaningful and self-contained. excuse bundling it in. Before committing, review your diff and split out any hunk that is behavior-preserving (an extraction, a rename, a move) into a preceding commit, by staging hunks or resetting and recommitting in order. +- **A preparatory refactor is a new commit only when it prepares something + new.** Before adding one, find the commit that introduced the code you are + about to restructure. If that commit is on this branch, the refactor is a + `fixup!` for it rather than a commit of its own: a branch must never contain + a commit whose code a later commit on the same branch tidies up. A prep + refactor earns a commit of its own only when the shape it corrects came from + before the branch. This holds across a branch stack too — if the commit that + introduced the code is in an earlier branch of the stack, the fixup belongs + there, and the branches above it get replayed. The one exception is when + fixing it there turns out to be unreasonably difficult; ask me what to do + rather than deciding to leave the repair at the tip. - **Do not use conventional commits** (no `feat:`/`fix:`/`chore:` prefixes). Match the plain English imperative style of the existing history. - **Wrap message body to 72 characters**. The subject is allowed to go up to 80 From e148959865845cef95e41e38654a68a4b2d15db5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 13:24:34 +0200 Subject: [PATCH 02/10] Add a test for read requests waiting when a task is stopped Ask a view buffer manager to read to the end of its content twice over, then stop the task before it has served either request, as a re-render replacing it does. Only the request it had already picked up is answered; the one still queued behind it is dropped, and its caller waits for a callback that never comes. Co-authored-by: Claude Opus 5 (1M context) --- pkg/tasks/tasks_test.go | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index b15b48ee4..4020fb09d 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -486,3 +486,45 @@ func TestNewCmdTaskRefresh(t *testing.T) { } } } + +// A read request is answered by the task that serves it calling the request's Then. +// This checks that requests still waiting when the task is stopped are answered too, +// which is what happens when a re-render replaces the task. +func TestQueuedReadRequestsAreAnsweredWhenTheTaskStops(t *testing.T) { + noop := func() {} + task := gocui.NewFakeTask() + + // A pipe the task blocks on, so that the requests are still waiting when it stops. + pipeReader, pipeWriter := io.Pipe() + defer pipeWriter.Close() + + manager := NewViewBufferManager( + utils.NewDummyLog(), bytes.NewBuffer(nil), noop, noop, noop, noop, noop, noop, + func() gocui.Task { return task }, + func(f func()) error { f(); return nil }, + ) + + stop := make(chan struct{}) + fn := manager.NewCmdTask( + func() (Cmd, io.Reader) { return ExecCmd{Cmd: exec.Command("true")}, pipeReader }, + "", 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 + // below are handed to a task that is waiting for them. + time.Sleep(50 * time.Millisecond) + + answered := atomic.Int32{} + manager.ReadToEnd(func() { answered.Add(1) }) + manager.ReadToEnd(func() { answered.Add(1) }) + + // Let the first request be picked up and block on the pipe, then stop the task. + time.Sleep(50 * time.Millisecond) + close(stop) + time.Sleep(50 * time.Millisecond) + + /* EXPECTED: + assert.EqualValues(t, 2, answered.Load()) + ACTUAL: */ + assert.EqualValues(t, 1, answered.Load()) +} From dd2a1a634f34d736e3480388f8f283e57ff9cb60 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 13:27:03 +0200 Subject: [PATCH 03/10] Answer every read request, whether or not a task is still serving A caller of ReadLines or ReadToEnd is told that the content it asked for has been read by the request's Then being called. A task that reaches the end of its input answers the requests still queued behind the one it was serving, but a task that is stopped drops them, and their callers wait for a callback that never comes. Pressing "/" in the focused main view opens the search prompt from such a callback, so if a re-render replaces the task at that moment the prompt never opens. Answering them as the read loop ends would leave a request handed over after that point unanswered, and there is a window for one. A caller reads the channel to send on, and can reach the send itself only once the loop has gone. So hand requests over through a queue instead. Asking whether a task is there and giving it the request are one step, as are taking the task away and handing back what it never answered; a request made in between goes back to the caller to answer. The queue is unbounded rather than a fixed-size channel, for the reasons gocui's userEventQueue is. Requests are handed over from the UI thread, where a blocking send would deadlock against the task waiting to be let go, and a fixed channel that fills up leaves only blocking, dropping, reordering or panicking to choose between. Co-authored-by: Claude Opus 5 (1M context) --- pkg/tasks/read_request_queue.go | 101 +++++++++++++++++++++++++++ pkg/tasks/read_request_queue_test.go | 58 +++++++++++++++ pkg/tasks/tasks.go | 87 ++++++++++++----------- pkg/tasks/tasks_test.go | 10 +-- 4 files changed, 206 insertions(+), 50 deletions(-) create mode 100644 pkg/tasks/read_request_queue.go create mode 100644 pkg/tasks/read_request_queue_test.go diff --git a/pkg/tasks/read_request_queue.go b/pkg/tasks/read_request_queue.go new file mode 100644 index 000000000..444e514fd --- /dev/null +++ b/pkg/tasks/read_request_queue.go @@ -0,0 +1,101 @@ +package tasks + +import "sync" + +// readRequestQueue is an unbounded, order-preserving FIFO of the read requests a +// view's running command task serves (see LinesToRead), with a reader that comes +// and goes. +// +// It's unbounded, rather than a fixed-size channel, for the same reasons as the +// user-event queue in gocui. Requests are handed over from the UI thread, where a +// blocking send would deadlock against the task that is waiting to be let go, and +// a fixed channel that fills up leaves only bad choices: blocking, dropping, +// reordering, or panicking on overflow. Appending to a slice does none of those. +// +// The reader coming and going is the other half of what it's for. A request is +// how a caller asks for content to be read and hears, through the request's Then, +// that it has been; a request nobody answers leaves that caller waiting for good. +// So asking whether a task is there and handing it the request are one step, and +// so are taking the task away and handing back what it never answered. A request +// made in between finds no task and goes back to its caller to answer. +// +// enqueue appends under the mutex and rings the doorbell; the task selects on the +// doorbell to wake, then takes requests until there are none left. The doorbell is +// buffered(1) and rung with a non-blocking send, so it's a coalescing "work +// pending" flag rather than a per-request signal: a burst of appends leaves at +// most one token, and the task takes everything the token stands for on a single +// wake. A token left over after the queue empties causes one harmless empty wake. +type readRequestQueue struct { + mutex sync.Mutex + requests []LinesToRead + doorbell chan struct{} + + // Whether a task is there to serve the requests. False before the first task + // starts, and between one task ending and the next starting. + serving bool +} + +func newReadRequestQueue() *readRequestQueue { + return &readRequestQueue{doorbell: make(chan struct{}, 1)} +} + +// beginServing says that a task is now there to serve the queue, and returns the +// doorbell that tells it when there is something to serve. +func (self *readRequestQueue) beginServing() <-chan struct{} { + self.mutex.Lock() + defer self.mutex.Unlock() + + self.serving = true + return self.doorbell +} + +// stopServing takes the task away and hands back the requests it never answered, +// for the caller to answer in its place. +func (self *readRequestQueue) stopServing() []LinesToRead { + self.mutex.Lock() + defer self.mutex.Unlock() + + self.serving = false + unanswered := self.requests + self.requests = nil + return unanswered +} + +// enqueue gives a request to the task serving the queue, and reports whether +// there was one to give it to. When there wasn't, the request is the caller's to +// answer. +func (self *readRequestQueue) enqueue(request LinesToRead) bool { + self.mutex.Lock() + if !self.serving { + self.mutex.Unlock() + return false + } + self.requests = append(self.requests, request) + self.mutex.Unlock() + + select { + case self.doorbell <- struct{}{}: + default: + } + return true +} + +// dequeue takes the oldest request, reporting false when there are none. +func (self *readRequestQueue) dequeue() (LinesToRead, bool) { + self.mutex.Lock() + defer self.mutex.Unlock() + + if len(self.requests) == 0 { + return LinesToRead{}, false + } + request := self.requests[0] + if len(self.requests) == 1 { + // Release the backing array whenever the queue drains, so a one-off burst + // doesn't pin its peak size for the rest of the session. + self.requests = nil + } else { + self.requests[0] = LinesToRead{} + self.requests = self.requests[1:] + } + return request, true +} diff --git a/pkg/tasks/read_request_queue_test.go b/pkg/tasks/read_request_queue_test.go new file mode 100644 index 000000000..423b89d09 --- /dev/null +++ b/pkg/tasks/read_request_queue_test.go @@ -0,0 +1,58 @@ +package tasks + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestReadRequestQueueHandsBackWhatNoTaskWillServe(t *testing.T) { + queue := newReadRequestQueue() + + // Nothing has begun serving, so the request comes straight back to its caller. + assert.False(t, queue.enqueue(LinesToRead{Total: 1})) + + queue.beginServing() + assert.True(t, queue.enqueue(LinesToRead{Total: 1})) + assert.True(t, queue.enqueue(LinesToRead{Total: 2})) + + request, ok := queue.dequeue() + assert.True(t, ok) + assert.Equal(t, 1, request.Total) + + // What the task never got to comes back when it stops, and nothing is taken + // from a caller after that. + unanswered := queue.stopServing() + assert.Len(t, unanswered, 1) + assert.Equal(t, 2, unanswered[0].Total) + + assert.False(t, queue.enqueue(LinesToRead{Total: 3})) + _, ok = queue.dequeue() + assert.False(t, ok) +} + +func TestReadRequestQueueRingsTheDoorbell(t *testing.T) { + queue := newReadRequestQueue() + doorbell := queue.beginServing() + + select { + case <-doorbell: + assert.Fail(t, "the doorbell rang before anything was queued") + default: + } + + // A burst leaves one token, which stands for everything queued. + queue.enqueue(LinesToRead{Total: 1}) + queue.enqueue(LinesToRead{Total: 2}) + + select { + case <-doorbell: + default: + assert.Fail(t, "the doorbell didn't ring") + } + select { + case <-doorbell: + assert.Fail(t, "the doorbell rang twice for one wake") + default: + } +} diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 17cfabb5f..377e74c0a 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -70,12 +70,11 @@ type ViewBufferManager struct { taskIDMutex deadlock.Mutex Log *logrus.Entry newTaskID int - // The channel by which the currently-running task is told to read more - // lines (e.g. as the user scrolls). Held in an atomic because it's swapped - // out as tasks come and go while ReadLines/ReadToEnd read it from the UI - // thread; nil when no task is running. - readLines atomic.Pointer[chan LinesToRead] - taskKey string + // The requests by which the currently-running task is told to read more lines + // (e.g. as the user scrolls), and which it answers once it has. The task + // serving them comes and goes; see readRequestQueue. + readRequests *readRequestQueue + taskKey string // Resets the view's scroll position to the top. A render whose content is // different from what the view last showed (a different command key) calls @@ -173,6 +172,7 @@ func NewViewBufferManager( onUIThread func(f func()) error, ) *ViewBufferManager { return &ViewBufferManager{ + readRequests: newReadRequestQueue(), Log: log, writer: writer, beforeStart: beforeStart, @@ -191,12 +191,9 @@ func NewViewBufferManager( // (e.g. as the user scrolls down, back up, and down again) don't re-read lines // that have already been read: the task only ever reads the shortfall. func (self *ViewBufferManager) ReadLines(totalLines int) { - if ch := self.readLines.Load(); ch != nil { - readLines := *ch - go utils.Safe(func() { - readLines <- LinesToRead{Total: totalLines, InitialRefreshAfter: -1} - }) - } + // A request with no Then needs no answer, so there is nothing to do when no + // task is there to take it. + self.readRequests.enqueue(LinesToRead{Total: totalLines, InitialRefreshAfter: -1}) } // IsLoading reports whether a command task is currently reading content into the @@ -215,16 +212,24 @@ func (self *ViewBufferManager) StartLoading() { } func (self *ViewBufferManager) ReadToEnd(then func()) { - if ch := self.readLines.Load(); ch != nil { - readLines := *ch - go utils.Safe(func() { - readLines <- LinesToRead{Total: -1, InitialRefreshAfter: -1, Then: then} - }) - } else if then != nil { + request := LinesToRead{Total: -1, InitialRefreshAfter: -1, Then: then} + if !self.readRequests.enqueue(request) && then != nil { + // With no task reading, everything there is to read has been read. then() } } +// stopServingReadRequests takes the task away from the read-request queue and +// answers whatever it never got to, so that nobody is left waiting for a callback +// that isn't coming. +func (self *ViewBufferManager) stopServingReadRequests() { + for _, request := range self.readRequests.stopServing() { + if request.Then != nil { + request.Then() + } + } +} + func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix string, linesToRead LinesToRead, onDoneFn func()) func(TaskOpts) error { return func(opts TaskOpts) error { var onDoneOnce sync.Once @@ -289,8 +294,9 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix loadingMutex := deadlock.Mutex{} - readLines := make(chan LinesToRead, 1024) - self.readLines.Store(&readLines) + // Begin serving before any goroutine starts, so that the first request below + // can't arrive before there is a task to take it. + readRequests := self.readRequests.beginServing() scanner := bufio.NewScanner(r) scanner.Split(utils.ScanLinesAndTruncateWhenLongerThanBuffer(bufio.MaxScanTokenSize)) @@ -423,10 +429,17 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix if stopped() { break outer } - select { - case <-opts.Stop: - break outer - case linesToRead := <-readLines: + linesToRead, ok := self.readRequests.dequeue() + if !ok { + // Nothing to read yet: wait to be told there is, or to be stopped. + select { + case <-opts.Stop: + break outer + case <-readRequests: + } + continue + } + { callThen := func() { if linesToRead.Then != nil { linesToRead.Then() @@ -493,21 +506,6 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix // means a newer task is taking over and is still loading. self.loading.Store(false) callThen() - // Any read requests that were queued while we were reading are - // now trivially satisfied, since we've read everything. Fire - // their callbacks instead of dropping them when we break out of - // the loop below (and nil out readLines). - drain: - for { - select { - case queued := <-readLines: - if queued.Then != nil { - queued.Then() - } - default: - break drain - } - } break outer } writeToView(append(line, '\n')) @@ -532,7 +530,11 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix } } - self.readLines.Store(nil) + // Whoever made a request the loop never got to is waiting to hear that the + // content it asked for has been read, and there is nothing here to read it + // any more: at end of input it has all been read already, and a task that + // was stopped is handing the view over to the one replacing it. + self.stopServingReadRequests() refreshViewIfStale() @@ -556,7 +558,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix close(lineWrittenChan) }) - readLines <- linesToRead + self.readRequests.enqueue(linesToRead) <-done @@ -670,7 +672,8 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error self.stopCurrentTask() } - self.readLines.Store(nil) + // Nothing serves read requests between one task and the next. + self.stopServingReadRequests() stop := make(chan struct{}) notifyStopped := make(chan struct{}) diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index 4020fb09d..c50a54cac 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -253,12 +253,9 @@ func TestNewCmdTaskQueuedReadAtEndOfInput(t *testing.T) { }) <-reader.blocked + // The request is queued by the time this returns, so it is outstanding when we + // let the task reach EOF below. manager.ReadToEnd(func() { thenCalled = true }) - // ReadToEnd queues its request from a goroutine; wait for it to land so that - // it is definitely outstanding by the time we let the task reach EOF. - for len(*manager.readLines.Load()) == 0 { - time.Sleep(time.Millisecond) - } close(reader.unblock) wg.Wait() @@ -523,8 +520,5 @@ func TestQueuedReadRequestsAreAnsweredWhenTheTaskStops(t *testing.T) { close(stop) time.Sleep(50 * time.Millisecond) - /* EXPECTED: assert.EqualValues(t, 2, answered.Load()) - ACTUAL: */ - assert.EqualValues(t, 1, answered.Load()) } From acb6e48a9890f307e5dde46fe9b05b607828a0f7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 10:14:45 +0200 Subject: [PATCH 04/10] Hold a task while a view is read to its end ReadToEnd reads the rest of a view's content on the render task's own goroutine, and calls back once it has. Nothing held a task for that, so lazygit counted as idle from the moment the caller returned until the callback ran. The search prompt in the focused main view opens from such a callback, so an integration test takes the idle report as its cue to carry on, and presses its next key while the prompt is not open yet. Hold the task in ReadToEnd rather than in the caller, so that every caller is covered (see docs/dev/Busy.md). Co-authored-by: Claude Opus 5 (1M context) --- .../filter_and_search/search_a_long_diff.go | 57 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + pkg/tasks/tasks.go | 16 +++++- 3 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 pkg/integration/tests/filter_and_search/search_a_long_diff.go diff --git a/pkg/integration/tests/filter_and_search/search_a_long_diff.go b/pkg/integration/tests/filter_and_search/search_a_long_diff.go new file mode 100644 index 000000000..6da59ce10 --- /dev/null +++ b/pkg/integration/tests/filter_and_search/search_a_long_diff.go @@ -0,0 +1,57 @@ +package filter_and_search + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// longFileWithThreeMatches is long enough that a render of its diff stops well short +// of the end, with two of the three matches for the search below the point it stops at. +func longFileWithThreeMatches() string { + lines := make([]string, 0, 2000) + for i := range 2000 { + switch i { + case 100: + lines = append(lines, "NEEDLE first") + case 1000: + lines = append(lines, "NEEDLE middle") + case 1900: + lines = append(lines, "NEEDLE last") + default: + lines = append(lines, fmt.Sprintf("line %d", i)) + } + } + return strings.Join(lines, "\n") + "\n" +} + +var SearchALongDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Search a diff that is longer than a single render of it reads", + ExtraCmdArgs: []string{}, + Skip: false, + // A small window, so that a render stops well short of 2000 lines. + Width: 120, + Height: 30, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "") + shell.Commit("one") + + shell.UpdateFile("file1", longFileWithThreeMatches()) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + // All three matches are counted: opening the prompt reads the whole diff + // first, however much of it the render had got to. + t.Views().Main(). + IsFocused(). + FilterOrSearch("NEEDLE") + + t.Views().Search().Content(Contains("matches for 'NEEDLE' (1 of 3)")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 0d25ddba6..1324a4160 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -277,6 +277,7 @@ var tests = []*components.IntegrationTest{ filter_and_search.NestedFilter, filter_and_search.NestedFilterTransient, filter_and_search.NewSearch, + filter_and_search.SearchALongDiff, filter_and_search.StageAllStagesOnlyTrackedFilesInTrackedOnlyFilter, filter_and_search.StagingFolderStagesOnlyTrackedFilesInTrackedOnlyFilter, filter_by_author.SelectAuthor, diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 377e74c0a..deaeee042 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -212,10 +212,20 @@ func (self *ViewBufferManager) StartLoading() { } func (self *ViewBufferManager) ReadToEnd(then func()) { - request := LinesToRead{Total: -1, InitialRefreshAfter: -1, Then: then} - if !self.readRequests.enqueue(request) && then != nil { + // The reading happens on the task's own goroutine, and the caller hears about + // it through then, so lazygit must not count as idle in between. + task := self.newGocuiTask() + answered := func() { + task.Done() + if then != nil { + then() + } + } + + request := LinesToRead{Total: -1, InitialRefreshAfter: -1, Then: answered} + if !self.readRequests.enqueue(request) { // With no task reading, everything there is to read has been read. - then() + answered() } } From b54318e80cfabf700d1436244c556e2e0046dd54 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 10:00:17 +0200 Subject: [PATCH 05/10] Add a test for the current search match after the matches change Search a view, step to the last match, then have the view re-rendered with fewer matches in it, and the status reads "3 of 1". The positions are worked out again whenever the content changes, but the index into them stays where it was. Stepping on from there indexes the positions out of range and panics. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/search_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 pkg/gocui/search_test.go diff --git a/pkg/gocui/search_test.go b/pkg/gocui/search_test.go new file mode 100644 index 000000000..ef87f7e46 --- /dev/null +++ b/pkg/gocui/search_test.go @@ -0,0 +1,39 @@ +package gocui + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +// writeLines writes the given lines to the view, as a task rendering content into it +// does: one line at a time. +func writeLines(v *View, lines ...string) { + for _, line := range lines { + fmt.Fprintf(v, "%s\n", line) + } +} + +func TestSearchStatusAfterTheMatchesChange(t *testing.T) { + v := NewView("name", 0, 0, 40, 10, OutputNormal) + writeLines(v, "match", "other", "match", "other", "match") + + v.Search("match", nil) + _ = v.gotoNextMatch() + _ = v.gotoNextMatch() + index, total := v.GetSearchStatus() + assert.Equal(t, 2, index) + assert.Equal(t, 3, total) + + // The content is re-rendered with only the first of those matches left in it. + v.Clear() + writeLines(v, "match", "other", "other") + + index, total = v.GetSearchStatus() + /* EXPECTED: + assert.Equal(t, 0, index) + ACTUAL: */ + assert.Equal(t, 2, index) + assert.Equal(t, 1, total) +} From 4c90bc334c5792f88c6e19d42197278402aad168 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 10:00:35 +0200 Subject: [PATCH 06/10] Bring the current search match back into range when the matches change Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/search_test.go | 3 --- pkg/gocui/view.go | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/gocui/search_test.go b/pkg/gocui/search_test.go index ef87f7e46..f2ecccec4 100644 --- a/pkg/gocui/search_test.go +++ b/pkg/gocui/search_test.go @@ -31,9 +31,6 @@ func TestSearchStatusAfterTheMatchesChange(t *testing.T) { writeLines(v, "match", "other", "other") index, total = v.GetSearchStatus() - /* EXPECTED: assert.Equal(t, 0, index) - ACTUAL: */ - assert.Equal(t, 2, index) assert.Equal(t, 1, total) } diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 6e7b35520..294b34f25 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -1431,6 +1431,11 @@ func (v *View) updateSearchPositions() { } } } + + // The content may hold fewer matches than it did, so the current one is brought + // back into range: readers index the positions by it. + v.searcher.currentSearchIndex = min(v.searcher.currentSearchIndex, + max(0, len(v.searcher.searchPositions)-1)) } // IsTainted tells us if the view is tainted From 0505e778b31f8ec9e100a3f5c24f83033ca6da22 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 10:02:55 +0200 Subject: [PATCH 07/10] Work the search positions out when they are read, not on each line written A view's search positions were worked out again from every write, and each of those walks the whole view. Content arrives a line at a time, so rendering into a searched view costs a walk per line. Streaming 2000 lines takes 565ms, where the same render into an unsearched view takes about 10ms. Mark the positions stale on a write instead, and work them out where they are read: when the view is drawn, when a key steps through the matches, when the status is asked for. That is at most once a frame, and the same 2000 lines now take 8ms. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/search_test.go | 22 ++++++++++++++++++++ pkg/gocui/view.go | 43 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/pkg/gocui/search_test.go b/pkg/gocui/search_test.go index f2ecccec4..ba20da9de 100644 --- a/pkg/gocui/search_test.go +++ b/pkg/gocui/search_test.go @@ -34,3 +34,25 @@ func TestSearchStatusAfterTheMatchesChange(t *testing.T) { assert.Equal(t, 0, index) assert.Equal(t, 1, total) } + +func TestSearchPositionsFollowStreamedContent(t *testing.T) { + v := NewView("name", 0, 0, 40, 10, OutputNormal) + v.Search("match", nil) + + // A render arrives a line at a time, and the status describes all of it. + writeLines(v, "other", "match", "other", "match") + + _, total := v.GetSearchStatus() + assert.Equal(t, 2, total) +} + +func BenchmarkWriteToSearchedView(b *testing.B) { + for b.Loop() { + v := NewView("name", 0, 0, 100, 40, OutputNormal) + v.Search("match", nil) + for i := range 2000 { + fmt.Fprintf(v, "line %d of a diff, most of which does not match\n", i) + } + v.GetSearchStatus() + } +} diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 294b34f25..cc5e6c0e4 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -271,6 +271,12 @@ type searcher struct { currentSearchIndex int onSelectItem func(*View, int) renderSearchStatus func(*View, int, int) + + // Whether the content has changed since the positions were worked out, so that + // they have to be worked out again before they are read. Working them out walks + // the whole view, and content arrives a line at a time, so it happens once per + // read rather than once per line written. + positionsStale bool } func (v *View) setRenderSearchStatus(renderSearchStatus func(*View, int, int)) { @@ -287,7 +293,27 @@ func (v *View) renderSearchStatus(index int, itemCount int) { } } +// refreshSearchPositions works the search positions out again if the content has +// changed since they were last worked out. Every read of the positions goes through +// this, so that no caller has to know whether the view has been drawn since the +// content it is asking about arrived. +func (v *View) refreshSearchPositions() { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + v.refreshSearchPositionsIfNeeded() +} + +// refreshSearchPositions for a caller that already holds writeMutex. +func (v *View) refreshSearchPositionsIfNeeded() { + if v.searcher.positionsStale { + v.updateSearchPositions() + } +} + func (v *View) gotoNextMatch() error { + v.refreshSearchPositions() + if len(v.searcher.searchPositions) == 0 { return nil } @@ -307,6 +333,8 @@ func (v *View) gotoNextMatch() error { } func (v *View) gotoPreviousMatch() error { + v.refreshSearchPositions() + if len(v.searcher.searchPositions) == 0 { return nil } @@ -328,6 +356,8 @@ func (v *View) gotoPreviousMatch() error { } func (v *View) SelectSearchResult(index int) { + v.refreshSearchPositions() + itemCount := len(v.searcher.searchPositions) if itemCount == 0 { return @@ -347,6 +377,8 @@ func (v *View) SelectSearchResult(index int) { // Returns , func (v *View) GetSearchStatus() (int, int) { + v.refreshSearchPositions() + return v.searcher.currentSearchIndex, len(v.searcher.searchPositions) } @@ -420,6 +452,8 @@ func (v *View) nearestSearchPosition() int { } func (v *View) SetNearestSearchPosition() { + v.refreshSearchPositions() + if len(v.searcher.searchPositions) > 0 { newPos := v.nearestSearchPosition() if newPos != v.searcher.currentSearchIndex { @@ -902,7 +936,7 @@ func (v *View) write(p []byte) { v.buf.write(v, p) - v.updateSearchPositions() + v.searcher.positionsStale = true } // write parses p into cells and appends them to the buffer at its write cursor. @@ -1353,6 +1387,8 @@ func stringToGraphemes(s string) []string { } func (v *View) updateSearchPositions() { + v.searcher.positionsStale = false + if v.searcher.searchString != "" { var normalizeRune func(s string) string var normalizedSearchStr string @@ -1466,6 +1502,7 @@ func (v *View) draw(isWindowFocused bool) { } v.refreshViewLinesIfNeeded() + v.refreshSearchPositionsIfNeeded() visibleViewLinesHeight := v.viewLineLengthIgnoringTrailingBlankLines() if v.Autoscroll && visibleViewLinesHeight > maxY { @@ -2075,6 +2112,8 @@ func (v *View) setContentLineCount(lineCount int) { // result that is visible in the view, if any, or the first one that is below the view if none is // visible. func (v *View) selectVisibleSearchResultAfterScrollUp() { + v.refreshSearchPositions() + if !v.Highlight && len(v.searcher.searchPositions) != 0 { windowBottom := v.oy + v.InnerHeight() if v.searcher.searchPositions[v.searcher.currentSearchIndex].Y >= windowBottom { @@ -2098,6 +2137,8 @@ func (v *View) selectVisibleSearchResultAfterScrollUp() { // result that is visible in the view, if any, or the last one that is above the view if none is // visible. func (v *View) selectVisibleSearchResultAfterScrollDown() { + v.refreshSearchPositions() + if !v.Highlight && len(v.searcher.searchPositions) != 0 { if v.searcher.searchPositions[v.searcher.currentSearchIndex].Y < v.oy { newSearchIndex := v.searcher.currentSearchIndex From 04a7ae3ec8418043ffad8a1a464a0f26b7003fde Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 10:17:20 +0200 Subject: [PATCH 08/10] Read a view that is being searched to the end when it renders again Opening the search prompt reads the whole of the view's content, so that the search counts every match in it. Rendering the content again reads only as much as the scrollbar needs, so the matches below that point are lost. The "x of y" drops to what the shortened content holds, and grows again as the user scrolls far enough to load more. Read to the end while a search is on, the way opening the prompt does. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gui/view_helpers.go | 12 ++++++++++++ .../tests/filter_and_search/search_a_long_diff.go | 9 +++++++++ 2 files changed, 21 insertions(+) diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 5f3e4c2ab..9b2e29c02 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -25,6 +25,18 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead { linesForFirstRefresh := height + oy + 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 + // search prompt reads it, see MainViewController.openSearch). Lines left unread + // hold matches the search doesn't know about, and would add themselves to the + // "x of y" as the user scrolled far enough to load them. + if v.IsSearching() { + return tasks.LinesToRead{ + Total: -1, + InitialRefreshAfter: linesForFirstRefresh, + } + } + // 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. diff --git a/pkg/integration/tests/filter_and_search/search_a_long_diff.go b/pkg/integration/tests/filter_and_search/search_a_long_diff.go index 6da59ce10..7f2ebf156 100644 --- a/pkg/integration/tests/filter_and_search/search_a_long_diff.go +++ b/pkg/integration/tests/filter_and_search/search_a_long_diff.go @@ -53,5 +53,14 @@ var SearchALongDiff = NewIntegrationTest(NewIntegrationTestArgs{ FilterOrSearch("NEEDLE") t.Views().Search().Content(Contains("matches for 'NEEDLE' (1 of 3)")) + + // Rendering the diff again reads it from the start, and it is read all the + // way down to the matches the search already knows about. + t.Views().Main(). + Press(keys.Universal.IncreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 4")) + }). + Content(Contains("+NEEDLE last")) }, }) From 811b3fbdd16faf0598705c39d27177c09b18d6df Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 10:20:24 +0200 Subject: [PATCH 09/10] Show the search status of what a re-rendered view now holds Rendering a view's content again while a search is on leaves the "x of y" describing the content that has just been replaced. The status is worked out when the search is typed and again when a key steps through the matches, and a render is neither. Change the diff context size while searching the focused main view, and the count stays as it was, however many matches the wider context brought in or took away. Run the search again over the new content once the render has finished putting it there. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/view.go | 13 ++++++ pkg/gui/main_panels.go | 15 +++++++ pkg/gui/tasks_adapter.go | 5 +++ .../search_status_after_a_rerender.go | 45 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 5 files changed, 79 insertions(+) create mode 100644 pkg/integration/tests/filter_and_search/search_status_after_a_rerender.go diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index cc5e6c0e4..0a06d1981 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -311,6 +311,19 @@ func (v *View) refreshSearchPositionsIfNeeded() { } } +// RefreshSearch runs the search again over content the view has just been re-rendered +// with, and shows the "x of y" status of what it finds. The view stays where it is: the +// position in the content is the user's, and the search follows it rather than moving +// it. +func (v *View) RefreshSearch() { + if !v.IsSearching() { + return + } + + v.UpdateSearchResults(v.searcher.searchString, v.searcher.modelSearchResults) + v.renderSearchStatus(v.searcher.currentSearchIndex, len(v.searcher.searchPositions)) +} + func (v *View) gotoNextMatch() error { v.refreshSearchPositions() diff --git a/pkg/gui/main_panels.go b/pkg/gui/main_panels.go index a0efcb14c..bc4a4219e 100644 --- a/pkg/gui/main_panels.go +++ b/pkg/gui/main_panels.go @@ -2,6 +2,7 @@ package gui import ( "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -139,3 +140,17 @@ func (gui *Gui) refreshMainViews(opts types.RefreshMainOpts) { func (gui *Gui) splitMainPanel(splitMainPanel bool) { gui.State.SplitMainPanel = splitMainPanel } + +// reApplySearch runs a search the view holds again over the content a render has just +// finished putting there, so that the matches highlighted and the "x of y" status +// describe what the view shows now rather than what it showed when the search was +// typed. Call it once the content is final. +func (gui *Gui) reApplySearch(view *gocui.View) { + // While the prompt is open, the search view holds what the user is typing, and the + // status would be written over it. + if gui.State.ContextMgr.Current().GetKey() == context.SEARCH_CONTEXT_KEY { + return + } + + view.RefreshSearch() +} diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 5e5295639..3ed141d68 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -91,6 +91,7 @@ func (gui *Gui) newStringTaskWithoutScroll(view *gocui.View, str string) error { f := func(tasks.TaskOpts) error { return gui.g.OnUIThreadAndWaitBackground(func() { gui.c.SetViewContent(view, str) + gui.reApplySearch(view) }) } @@ -108,6 +109,7 @@ func (gui *Gui) newStringTaskWithScroll(view *gocui.View, str string, originX in return gui.g.OnUIThreadAndWaitBackground(func() { gui.c.SetViewContent(view, str) view.SetOrigin(originX, originY) + gui.reApplySearch(view) }) } @@ -125,6 +127,7 @@ func (gui *Gui) newStringTaskWithKey(view *gocui.View, str string, key string) e return gui.g.OnUIThreadAndWaitBackground(func() { gui.c.ResetViewOrigin(view) gui.c.SetViewContent(view, str) + gui.reApplySearch(view) }) } @@ -170,6 +173,8 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager { view.SetOrigin(0, newOriginY) } + + gui.reApplySearch(view) }, func() { view.SetOrigin(0, 0) diff --git a/pkg/integration/tests/filter_and_search/search_status_after_a_rerender.go b/pkg/integration/tests/filter_and_search/search_status_after_a_rerender.go new file mode 100644 index 000000000..8116049b9 --- /dev/null +++ b/pkg/integration/tests/filter_and_search/search_status_after_a_rerender.go @@ -0,0 +1,45 @@ +package filter_and_search + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SearchStatusAfterARerender = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The search status counts the matches in a diff that has been rendered again", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", + "line 1\nline 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nNEEDLE\nline 11\nline 12\nline 13\nline 14\n") + shell.Commit("one") + + // Four lines above NEEDLE, so that it is context at a context size of 4 but + // not at 3. + shell.UpdateFile("file1", + "line 1\nline 2\nline 3\nline 4\nline 5\nchanged\nline 7\nline 8\nline 9\nNEEDLE\nline 11\nline 12\nline 13\nline 14\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Content(DoesNotContain("NEEDLE")). + FilterOrSearch("NEEDLE") + + t.Views().Search().Content(Contains("No matches for 'NEEDLE'")) + + // A wider context brings NEEDLE into the diff, and the search counts it. + t.Views().Main(). + Press(keys.Universal.IncreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 4")) + }). + Content(Contains("NEEDLE")) + + t.Views().Search().Content(Contains("matches for 'NEEDLE' (1 of 1)")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 1324a4160..3d9edadfe 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -278,6 +278,7 @@ var tests = []*components.IntegrationTest{ filter_and_search.NestedFilterTransient, filter_and_search.NewSearch, filter_and_search.SearchALongDiff, + filter_and_search.SearchStatusAfterARerender, filter_and_search.StageAllStagesOnlyTrackedFilesInTrackedOnlyFilter, filter_and_search.StagingFolderStagesOnlyTrackedFilesInTrackedOnlyFilter, filter_by_author.SelectAuthor, From 114d3a5a25aa859b45fbb7e160bc0b34e37d15fb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 5 Sep 2026 10:22:17 +0200 Subject: [PATCH 10/10] Render the focused main view again while it is being searched A refresh left the focused main view alone while a search was on, so the diff on screen stayed as it was however much the working tree had moved on underneath it. The search could not cope with the content changing under it, and leaving the content alone was the way around that. It can cope now. The positions are worked out again from whatever the view holds, and the status with them, so render it like any other. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gui/view_helpers.go | 12 ++---- .../rerender_the_searched_main_view.go | 37 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 pkg/integration/tests/filter_and_search/rerender_the_searched_main_view.go diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 9b2e29c02..3d924a566 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -171,15 +171,9 @@ func (gui *Gui) postRefreshUpdate(c types.Context, opts types.OnFocusOpts) { currentCtx := gui.State.ContextMgr.Current() if currentCtx.GetKey() == context.NORMAL_MAIN_CONTEXT_KEY || currentCtx.GetKey() == context.NORMAL_SECONDARY_CONTEXT_KEY { - // Searching can't cope well with the view being updated while it is being searched. - // We might be able to fix the problems with this, but it doesn't seem easy, so for now - // just don't rerender the view while searching, on the assumption that users will probably - // either search or change their data, but not both at the same time. - if !currentCtx.GetView().IsSearching() { - sidePanelContext := gui.State.ContextMgr.NextInStack(currentCtx) - if sidePanelContext != nil && sidePanelContext.GetKey() == c.GetKey() { - sidePanelContext.HandleRenderToMain() - } + sidePanelContext := gui.State.ContextMgr.NextInStack(currentCtx) + if sidePanelContext != nil && sidePanelContext.GetKey() == c.GetKey() { + sidePanelContext.HandleRenderToMain() } } else if c.GetKey() == gui.State.ContextMgr.CurrentStatic().GetKey() { // If our view is not the current one, but it is the current static context, then this diff --git a/pkg/integration/tests/filter_and_search/rerender_the_searched_main_view.go b/pkg/integration/tests/filter_and_search/rerender_the_searched_main_view.go new file mode 100644 index 000000000..3d631e650 --- /dev/null +++ b/pkg/integration/tests/filter_and_search/rerender_the_searched_main_view.go @@ -0,0 +1,37 @@ +package filter_and_search + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RerenderTheSearchedMainView = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "A refresh renders the focused main view again even while it is being searched", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("one") + + shell.UpdateFile("file1", "one\nNEEDLE\nthree\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + FilterOrSearch("NEEDLE"). + Content(Contains("+NEEDLE")). + Tap(func() { + t.Shell().UpdateFile("file1", "one\nOTHER\nthree\n") + }). + Press(keys.Universal.Refresh). + Content(Contains("+OTHER")). + Content(DoesNotContain("+NEEDLE")) + + t.Views().Search().Content(Contains("No matches for 'NEEDLE'")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 3d9edadfe..a4e732cf0 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -277,6 +277,7 @@ var tests = []*components.IntegrationTest{ filter_and_search.NestedFilter, filter_and_search.NestedFilterTransient, filter_and_search.NewSearch, + filter_and_search.RerenderTheSearchedMainView, filter_and_search.SearchALongDiff, filter_and_search.SearchStatusAfterARerender, filter_and_search.StageAllStagesOnlyTrackedFilesInTrackedOnlyFilter,