From 05c32e292e307a245eabcb2344bd1527d69f64f2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 Sep 2023 10:04:21 +0200 Subject: [PATCH 1/4] Extract StatusManager.addStatus method Avoids a bit of code duplication. --- pkg/gui/status/status_manager.go | 44 ++++++++++++++------------------ 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/pkg/gui/status/status_manager.go b/pkg/gui/status/status_manager.go index d54853c6e..ba04e14f2 100644 --- a/pkg/gui/status/status_manager.go +++ b/pkg/gui/status/status_manager.go @@ -27,19 +27,7 @@ func NewStatusManager() *StatusManager { } func (self *StatusManager) WithWaitingStatus(message string, f func()) { - self.mutex.Lock() - - self.nextId += 1 - id := self.nextId - - newStatus := appStatus{ - message: message, - statusType: "waiting", - id: id, - } - self.statuses = append([]appStatus{newStatus}, self.statuses...) - - self.mutex.Unlock() + id := self.addStatus(message, "waiting") f() @@ -47,18 +35,7 @@ func (self *StatusManager) WithWaitingStatus(message string, f func()) { } func (self *StatusManager) AddToastStatus(message string) int { - self.mutex.Lock() - defer self.mutex.Unlock() - - self.nextId++ - id := self.nextId - - newStatus := appStatus{ - message: message, - statusType: "toast", - id: id, - } - self.statuses = append([]appStatus{newStatus}, self.statuses...) + id := self.addStatus(message, "toast") go func() { time.Sleep(time.Second * 2) @@ -84,6 +61,23 @@ func (self *StatusManager) HasStatus() bool { return len(self.statuses) > 0 } +func (self *StatusManager) addStatus(message string, statusType string) int { + self.mutex.Lock() + defer self.mutex.Unlock() + + self.nextId++ + id := self.nextId + + newStatus := appStatus{ + message: message, + statusType: statusType, + id: id, + } + self.statuses = append([]appStatus{newStatus}, self.statuses...) + + return id +} + func (self *StatusManager) removeStatus(id int) { self.mutex.Lock() defer self.mutex.Unlock() From c222a618a86c2b434bbb92fc1e3ba95273daa002 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 Sep 2023 08:08:14 +0200 Subject: [PATCH 2/4] Extract WaitingStatusHandle --- .../controllers/helpers/app_status_helper.go | 4 +-- pkg/gui/status/status_manager.go | 25 ++++++++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index a6befe1a2..bdab8f1b1 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -36,9 +36,7 @@ func (self *AppStatusHelper) Toast(message string) { // withWaitingStatus wraps a function and shows a waiting status while the function is still executing func (self *AppStatusHelper) WithWaitingStatus(message string, f func(gocui.Task) error) { self.c.OnWorker(func(task gocui.Task) { - self.statusMgr().WithWaitingStatus(message, func() { - self.renderAppStatus() - + self.statusMgr().WithWaitingStatus(message, self.renderAppStatus, func() { if err := f(task); err != nil { self.c.OnUIThread(func() error { return self.c.Error(err) diff --git a/pkg/gui/status/status_manager.go b/pkg/gui/status/status_manager.go index ba04e14f2..1e2aa9cf1 100644 --- a/pkg/gui/status/status_manager.go +++ b/pkg/gui/status/status_manager.go @@ -16,6 +16,24 @@ type StatusManager struct { mutex deadlock.Mutex } +// Can be used to manipulate a waiting status while it is running (e.g. pause +// and resume it) +type WaitingStatusHandle struct { + statusManager *StatusManager + message string + renderFunc func() + id int +} + +func (self *WaitingStatusHandle) Show() { + self.id = self.statusManager.addStatus(self.message, "waiting") + self.renderFunc() +} + +func (self *WaitingStatusHandle) Hide() { + self.statusManager.removeStatus(self.id) +} + type appStatus struct { message string statusType string @@ -26,12 +44,13 @@ func NewStatusManager() *StatusManager { return &StatusManager{} } -func (self *StatusManager) WithWaitingStatus(message string, f func()) { - id := self.addStatus(message, "waiting") +func (self *StatusManager) WithWaitingStatus(message string, renderFunc func(), f func()) { + handle := &WaitingStatusHandle{statusManager: self, message: message, renderFunc: renderFunc, id: -1} + handle.Show() f() - self.removeStatus(id) + handle.Hide() } func (self *StatusManager) AddToastStatus(message string) int { From 1359fa14c1decd565d4f352721bb92d038cbe06d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 Sep 2023 08:43:42 +0200 Subject: [PATCH 3/4] When pausing a task during a waiting status, hide the status while paused We do this for two reasons: - when popping up a credentials prompt, it looks distracting if the waiting status keeps spinning while the user is typing the password - the task that updates the waiting status periodically would keep the program busy, so integration tests would wait forever for the program to become idle again --- .../controllers/helpers/app_status_helper.go | 25 +++++++++++++++++-- pkg/gui/status/status_manager.go | 4 +-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index bdab8f1b1..0ce30b460 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -33,11 +33,32 @@ func (self *AppStatusHelper) Toast(message string) { self.renderAppStatus() } +// A custom task for WithWaitingStatus calls; it wraps the original one and +// hides the status whenever the task is paused, and shows it again when +// continued. +type appStatusHelperTask struct { + gocui.Task + waitingStatusHandle *status.WaitingStatusHandle +} + +// poor man's version of explicitly saying that struct X implements interface Y +var _ gocui.Task = appStatusHelperTask{} + +func (self appStatusHelperTask) Pause() { + self.waitingStatusHandle.Hide() + self.Task.Pause() +} + +func (self appStatusHelperTask) Continue() { + self.Task.Continue() + self.waitingStatusHandle.Show() +} + // withWaitingStatus wraps a function and shows a waiting status while the function is still executing func (self *AppStatusHelper) WithWaitingStatus(message string, f func(gocui.Task) error) { self.c.OnWorker(func(task gocui.Task) { - self.statusMgr().WithWaitingStatus(message, self.renderAppStatus, func() { - if err := f(task); err != nil { + self.statusMgr().WithWaitingStatus(message, self.renderAppStatus, func(waitingStatusHandle *status.WaitingStatusHandle) { + if err := f(appStatusHelperTask{task, waitingStatusHandle}); err != nil { self.c.OnUIThread(func() error { return self.c.Error(err) }) diff --git a/pkg/gui/status/status_manager.go b/pkg/gui/status/status_manager.go index 1e2aa9cf1..1f4aaa569 100644 --- a/pkg/gui/status/status_manager.go +++ b/pkg/gui/status/status_manager.go @@ -44,11 +44,11 @@ func NewStatusManager() *StatusManager { return &StatusManager{} } -func (self *StatusManager) WithWaitingStatus(message string, renderFunc func(), f func()) { +func (self *StatusManager) WithWaitingStatus(message string, renderFunc func(), f func(*WaitingStatusHandle)) { handle := &WaitingStatusHandle{statusManager: self, message: message, renderFunc: renderFunc, id: -1} handle.Show() - f() + f(handle) handle.Hide() } From 4c5e250ed8d72a4939b9c5e46f04d1b17ebef994 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 Sep 2023 08:08:01 +0200 Subject: [PATCH 4/4] Add integration test for deleting a remote branch with credentials prompt This test is interesting because it shows a credentials prompt inside a WithWaitingStatus. Prior to this branch this test would have hung forever. --- ...te_remote_branch_with_credential_prompt.go | 87 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 88 insertions(+) create mode 100644 pkg/integration/tests/branch/delete_remote_branch_with_credential_prompt.go diff --git a/pkg/integration/tests/branch/delete_remote_branch_with_credential_prompt.go b/pkg/integration/tests/branch/delete_remote_branch_with_credential_prompt.go new file mode 100644 index 000000000..f145eceaa --- /dev/null +++ b/pkg/integration/tests/branch/delete_remote_branch_with_credential_prompt.go @@ -0,0 +1,87 @@ +package branch + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DeleteRemoteBranchWithCredentialPrompt = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Delete a remote branch where credentials are required", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("one") + + shell.CloneIntoRemote("origin") + + shell.NewBranch("mybranch") + + shell.PushBranch("origin", "mybranch") + + // actually getting a password prompt is tricky: it requires SSH'ing into localhost under a newly created, restricted, user. + // This is not easy to do in a cross-platform way, nor is it easy to do in a docker container. + // If you can think of a way to do it, please let me know! + shell.CopyHelpFile("pre-push", ".git/hooks/pre-push") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + deleteBranch := func() { + t.Views().Branches(). + Focus(). + Press(keys.Universal.Remove) + + t.ExpectPopup(). + Menu(). + Title(Equals("Delete branch 'mybranch'?")). + Select(Contains("Delete remote branch")). + Confirm() + + t.ExpectPopup(). + Confirmation(). + Title(Equals("Delete branch 'mybranch'?")). + Content(Equals("Are you sure you want to delete the remote branch 'mybranch' from 'origin'?")). + Confirm() + } + + t.Views().Status().Content(Contains("✓ repo → mybranch")) + + deleteBranch() + + // correct credentials are: username=username, password=password + + t.ExpectPopup().Prompt(). + Title(Equals("Username")). + Type("username"). + Confirm() + + // enter incorrect password + t.ExpectPopup().Prompt(). + Title(Equals("Password")). + Type("incorrect password"). + Confirm() + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("incorrect username/password")). + Confirm() + + t.Views().Status().Content(Contains("✓ repo → mybranch")) + + // try again with correct password + deleteBranch() + + t.ExpectPopup().Prompt(). + Title(Equals("Username")). + Type("username"). + Confirm() + + t.ExpectPopup().Prompt(). + Title(Equals("Password")). + Type("password"). + Confirm() + + t.Views().Status().Content(Contains("repo → mybranch").DoesNotContain("✓")) + t.Views().Branches().TopLines(Contains("mybranch (upstream gone)")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 12c4464b5..f4ec26ec2 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -38,6 +38,7 @@ var tests = []*components.IntegrationTest{ branch.CheckoutByName, branch.CreateTag, branch.Delete, + branch.DeleteRemoteBranchWithCredentialPrompt, branch.DetachedHead, branch.OpenWithCliArg, branch.Rebase,