Auto-refresh on window activation #2854

stefanhaller · 2023-07-31T16:56:20Z

PR Description

When you are making changes in your text editor or IDE, and then switch to the terminal window where lazygit is running, it doesn't immediately show you the latest changes you just made. You have to either refresh lazygit manually by typing shift-R (remembering that this command is only available in some panels but not others), or wait up to 10 seconds until the next auto-refresh timer comes along.

This PR solves this problem by automatically refreshing the files panel whenever lazygit's terminal window is brought to the front.

I have been running with this change locally for months now, and it has become indispensable for me; I always realize this whenever I have to work on a master-based branch that doesn't contain it.

Marked as a draft because it needs a change to gocui.

Note that an alternative solution to this problem might have been to subscribe to filesystem notifications and refresh whenever we are notified that files have changed. It seems that lazygit has experimented with this in the past, there are some rudiments of it in the code, but it's disabled. However, I actually think that refreshing on window activation is superior: with filesystem notifications you have to be careful not to refresh too often. Many files can be changed at once, and this probably needs some timer-based heuristic to wait until activity has died down before you refresh. Doing it on window activation doesn't have this problem.

Of all the other git clients I know, there's only one that reacts to filesystem changes (Sublime Merge), all the others refresh on window activation.

Please check if the PR fulfills these requirements

Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
Code has been formatted (see here)
Tests have been added/updated (see here for the integration test guide)
Text is internationalised (see here)
Docs (specifically docs/Config.md) have been updated if necessary
You've read through your own file changes for silly mistakes etc

jesseduffield · 2023-07-31T23:27:03Z

Holy shit this is a gamechanger

jesseduffield · 2023-07-31T23:27:38Z

Is there a PR against tcell for this?

stefanhaller · 2023-08-01T06:33:12Z

The tcell PR is mentioned in the commit message of the first commit, it's gdamore/tcell#599 (I see that you found it already meanwhile 😄). For some reason I didn't get any further reactions on it from Garrett since March; I hope he's doing well.

I also have to say that the PR can't be merged in it's current state because there's a change missing for the wScreen class (which I only realized very recently), and I don't know how to do that. Help would be appreciated. However, for our purposes it's good enough, since we don't use that class, so switching to my fork for now seems ok to me.

jesseduffield · 2023-08-01T12:58:51Z

Okay that sounds fine to me. I'm happy with this PR as-is (I see it's in draft: what else did you intend to change?)

stefanhaller · 2023-08-01T13:00:22Z

I didn't intend to change anything else. If the gocui changes are merged to gocui, I'd consider it ready.

jesseduffield · 2023-08-01T13:01:36Z

vendor/github.com/jesseduffield/gocui/gui.go

@@ -726,6 +731,7 @@ func (g *Gui) MainLoop() error {

 	if g.Mouse {
 		Screen.EnableMouse()
+		Screen.EnableFocus()


Does this only work when mouse is enabled? I.e. if I alt-tab to my terminal window, does that work?

Yes, that works.

But does it still require Mouse: true? Strange that it's a mouse event if it doesn't necessarily involve a mouse click

Oh that's what you mean. I was lazy and just put it in if g.Mouse because I thought all clients would probably want to either enable both, or none. Happy to add a separate Focus flag if you prefer.

I don't think it needs its own flag though. The purpose of having the Mouse flag (afaik) is that it's easier to drag and select text when mouse mode is off (in iterm I need to hold option to do it)

Ah ok. Just enable focus unconditionally then?

Yep, we'll see if it causes issues for anybody before we make it configurable

From gocui's perspective it's configurable anyway because nothing happens if you don't supply an on-focus callback

jesseduffield · 2023-08-01T13:06:39Z

I've added you as a collaborator in gocui so if you want you can get these changes into gocui yourself:

I often make changes in lazygit's vendor directory then copy them across to my gocui repo with
cp -r /path/to/lazygit/vendor/github.com/jesseduffield/gocui /path/to/gocui

Then I get that merged to gocui and then back in the lazygit repo I run:

scripts/bump_gocui.sh

This is temporary as long as gdamore/tcell#599 is not merged. Once that PR is merged, we can revert this.

stefanhaller · 2023-08-02T09:59:42Z

@jesseduffield This is ready to go if you want to give it a final look. It hasn't changed from the draft except for the middle commit. Feel free to merge it if you're happy with it, I'm not sure I would get around to that today.

jesseduffield · 2023-08-02T10:43:17Z

Super excited to use this myself. I'll clean up the file watcher code now that we've got this in place

jesseduffield added the feature For large enhancements that add a new chunk of functionality label Aug 1, 2023

jesseduffield reviewed Aug 1, 2023

View reviewed changes

stefanhaller added 3 commits August 2, 2023 11:35

Point tcell at stefanhaller's fork

4aca854

This is temporary as long as gdamore/tcell#599 is not merged. Once that PR is merged, we can revert this.

Bump gocui

c5acbb6

Refresh when the terminal window gets the focus

8ce6066

stefanhaller force-pushed the refresh-on-focus branch from bfdc140 to 8ce6066 Compare August 2, 2023 09:35

stefanhaller marked this pull request as ready for review August 2, 2023 09:36

jesseduffield merged commit c8520fb into master Aug 2, 2023

jesseduffield deleted the refresh-on-focus branch August 2, 2023 10:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-refresh on window activation #2854

Auto-refresh on window activation #2854

stefanhaller commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

stefanhaller commented Aug 1, 2023

jesseduffield commented Aug 1, 2023

stefanhaller commented Aug 1, 2023

jesseduffield Aug 1, 2023

stefanhaller Aug 1, 2023

jesseduffield Aug 1, 2023

stefanhaller Aug 1, 2023

jesseduffield Aug 1, 2023

stefanhaller Aug 1, 2023

jesseduffield Aug 1, 2023

jesseduffield Aug 1, 2023

jesseduffield commented Aug 1, 2023

stefanhaller commented Aug 2, 2023

jesseduffield commented Aug 2, 2023

Auto-refresh on window activation #2854

Auto-refresh on window activation #2854

Conversation

stefanhaller commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

stefanhaller commented Aug 1, 2023

jesseduffield commented Aug 1, 2023

stefanhaller commented Aug 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jesseduffield commented Aug 1, 2023

stefanhaller commented Aug 2, 2023

jesseduffield commented Aug 2, 2023