Re-render the focused main view if its content changes while it is being searched - #5993
Merged
Merged
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…itten 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When searching the focused main view using
/, any updates to its content were ignored because back when we introduced the focused main view feature we couldn't make it work; search mode couldn't cope well with the view content changing under it.In this PR we make that work, and remove the limitation. Along the way we fix a bunch of other related problems; some are only theoretical race conditions that have been found by reading the code, but never observed in reality; some are real problems that are too edge-casey to describe in detail. See the individual commit messages for details.