1
0
mirror of https://github.com/jesseduffield/lazygit.git synced 2025-07-17 01:42:45 +02:00

Fix main view occasionally scrolling to the top on its own when focused (#4573)

Ok, this is a long one. (It took me all weekend to figure out.)

We seem to have a race condition between re-rendering the main view and
the [layout
code](a0ec22c251/pkg/gui/layout.go (L75))
that checks whether a view has become smaller and therefore needs to
scroll up. When rerendering the main view, we are careful not to
invalidate the ViewLines array, so the code first calls Reset on the
view, and then starts writing lines to the view until we have written
enough for its old scroll position, and only then do we trigger a
redraw. This is all well and good, but theoretically it could happen
that the above-mentioned layout code runs shortly after the task has
started writing lines to the view (say, after the first one has been
written), and then it would see that the view's height is only 1, and
scroll it all the way to the top.

I have never seen this happen, so it seems that we are being lucky and
the race condition is only a theoretical one.

However: we had a very silly and embarrassing bug in the
focused-main-view code that triggers the race condition occasionally.
The bug is that when the main view is focused, we would refresh it
multiple times in quick succession, once for every side panel that is
being refreshed (instead of just once for the side panel that it belongs
to). So the first task would call Reset, start writing lines to the
view, and then the second task would come along, kill the first, call
Reset again, and start writing lines again, and so on. Apparently this
made it more likely for the layout code to run concurrently with this,
and see the view at a moment where it only has one or two lines. I have
seen it scroll to the top on its own a few times, which is very annoying
when you are in the middle of reviewing a longer commit, for instance.

We can fix this by refreshing the main view only for the side panel that
it belongs to, which is what this PR does. I have let lazygit run over
night with a `refresher.refreshInterval` value of 3, and it hadn't
scrolled to the top when I came to look in the morning, which makes me
pretty confident that we're good now.

It would still be nice if we could fix the race condition for real too,
but it's less urgent now, and it also doesn't seem trivial. I guess
instead of writing lines directly to the view we would have to buffer
them first, and only write them to the view when the original scroll
position is reached (with some synchronization, e.g. with a OnUIThread).
There are other complications that make this tricky though, and I have
no plans right now to tackle this.
This commit is contained in:
Stefan Haller
2025-05-22 08:17:19 +02:00
committed by GitHub

View File

@ -153,7 +153,9 @@ func (gui *Gui) postRefreshUpdate(c types.Context) {
// either search or change their data, but not both at the same time.
if !currentCtx.GetView().IsSearching() {
parentCtx := currentCtx.GetParentContext()
parentCtx.HandleRenderToMain()
if parentCtx.GetKey() == c.GetKey() {
parentCtx.HandleRenderToMain()
}
}
}
}