From 59303981f9e75c87e005a507a51d38520fa6a1d5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 28 Nov 2024 11:58:02 +0100 Subject: [PATCH 1/3] Simplify startBackgroundFetch This code had a lot of logic that (fortunately) didn't work because it was buggy: - it was supposed to wait for the auto-fetch delay before fetching for the first time in case we start with a repo that we had open in a previous session (i.e. that appears in the recent repos list). This code actually ran always, not just for known repos, because the IsNewRepo flag is only set later, after this function runs. Fortunately, the code didn't work, because time.After starts a timer but doesn't wait for it (to do that, it would have to be `<-time.After`). - if the first fetch fails with error 128, it was supposed to show an error message and not start the background fetch loop. Fortunately, this didn't work because 1) it was guarded by isNew which is always false here, and 2) because git's error message in this case is actually "exit code: 128", not "exit status 128" (maybe this has changed in git at some point). I find both of these undesirable. Whenever I open a repo I want an auto-fetch to be triggered immediately to get my branch information up to date as quickly as possible. And if the initial fetch fails (e.g. because one of my remotes is offline or doesn't exist any more), then that's no reason not to start the auto-fetch loop anyway. So let's simplify the code to do what it did before, but with much fewer lines of code. --- pkg/gui/background.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index 0272f0864..c9f0e3d40 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -3,7 +3,6 @@ package gui import ( "fmt" "runtime" - "strings" "time" "github.com/jesseduffield/gocui" @@ -76,21 +75,18 @@ func (self *BackgroundRoutineMgr) startBackgroundRoutines() { func (self *BackgroundRoutineMgr) startBackgroundFetch() { self.gui.waitForIntro.Wait() - isNew := self.gui.IsNewRepo + fetch := func() error { + err := self.backgroundFetch() + self.gui.c.Render() + return err + } + + // We want an immediate fetch at startup, and since goEvery starts by + // waiting for the interval, we need to trigger one manually first + _ = fetch() + userConfig := self.gui.UserConfig() - if !isNew { - time.After(time.Duration(userConfig.Refresher.FetchInterval) * time.Second) - } - err := self.backgroundFetch() - if err != nil && strings.Contains(err.Error(), "exit status 128") && isNew { - self.gui.c.Alert(self.gui.c.Tr.NoAutomaticGitFetchTitle, self.gui.c.Tr.NoAutomaticGitFetchBody) - } else { - self.goEvery(time.Second*time.Duration(userConfig.Refresher.FetchInterval), self.gui.stopChan, func() error { - err := self.backgroundFetch() - self.gui.c.Render() - return err - }) - } + self.goEvery(time.Second*time.Duration(userConfig.Refresher.FetchInterval), self.gui.stopChan, fetch) } func (self *BackgroundRoutineMgr) startBackgroundFilesRefresh(refreshInterval int) { From b07109de4d0c0d50d64f78aa05bf1252375b0dfa Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 28 Nov 2024 12:01:19 +0100 Subject: [PATCH 2/3] Remove unused field gui.IsNewRepo --- pkg/gui/gui.go | 2 -- pkg/gui/recent_repos_panel.go | 10 +++------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 4d5f625d9..1faeb85e4 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -108,8 +108,6 @@ type Gui struct { PopupHandler types.IPopupHandler - IsNewRepo bool - IsRefreshingFiles bool // we use this to decide whether we'll return to the original directory that diff --git a/pkg/gui/recent_repos_panel.go b/pkg/gui/recent_repos_panel.go index 0f2f2c704..ba6bc8ce1 100644 --- a/pkg/gui/recent_repos_panel.go +++ b/pkg/gui/recent_repos_panel.go @@ -21,8 +21,7 @@ func (gui *Gui) updateRecentRepoList() error { if err != nil { return err } - known, recentRepos := newRecentReposList(recentRepos, currentRepo) - gui.IsNewRepo = known + recentRepos = newRecentReposList(recentRepos, currentRepo) // TODO: migrate this file to use forward slashes on all OSes for consistency // (windows uses backslashes at the moment) gui.c.GetAppState().RecentRepos = recentRepos @@ -30,8 +29,7 @@ func (gui *Gui) updateRecentRepoList() error { } // newRecentReposList returns a new repo list with a new entry but only when it doesn't exist yet -func newRecentReposList(recentRepos []string, currentRepo string) (bool, []string) { - isNew := true +func newRecentReposList(recentRepos []string, currentRepo string) []string { newRepos := []string{currentRepo} for _, repo := range recentRepos { if repo != currentRepo { @@ -39,9 +37,7 @@ func newRecentReposList(recentRepos []string, currentRepo string) (bool, []strin continue } newRepos = append(newRepos, repo) - } else { - isNew = false } } - return isNew, newRepos + return newRepos } From 64cebfc0a8c06df83dd2faf38d856f3116b4cfe4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 28 Nov 2024 11:58:36 +0100 Subject: [PATCH 3/3] Remove unused texts --- pkg/i18n/english.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index aa942093b..2b233cd70 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -249,8 +249,6 @@ type TranslationSet struct { NoBranchOnRemote string Fetch string FetchTooltip string - NoAutomaticGitFetchTitle string - NoAutomaticGitFetchBody string FileEnter string FileEnterTooltip string FileStagingRequirements string @@ -1235,8 +1233,6 @@ func EnglishTranslationSet() *TranslationSet { NoBranchOnRemote: `This branch doesn't exist on remote. You need to push it to remote first.`, Fetch: `Fetch`, FetchTooltip: "Fetch changes from remote.", - NoAutomaticGitFetchTitle: `No automatic git fetch`, - NoAutomaticGitFetchBody: `Lazygit can't use "git fetch" in a private repo; use 'f' in the files panel to run "git fetch" manually`, FileEnter: `Stage lines / Collapse directory`, FileEnterTooltip: "If the selected item is a file, focus the staging view so you can stage individual hunks/lines. If the selected item is a directory, collapse/expand it.", FileStagingRequirements: `Can only stage individual lines for tracked files`,