diff --git a/pkg/cmd/extension/browse/browse.go b/pkg/cmd/extension/browse/browse.go index 9f0a75669..d5f8b660f 100644 --- a/pkg/cmd/extension/browse/browse.go +++ b/pkg/cmd/extension/browse/browse.go @@ -8,7 +8,6 @@ import ( "net/http" "os" "strings" - "sync" "time" "github.com/MakeNowJust/heredoc" @@ -92,9 +91,22 @@ type extList struct { app *tview.Application filter string opts ExtBrowseOpts - QueueUpdateDraw func(func()) + QueueUpdateDraw func(func()) *tview.Application + WaitGroup wGroup } +type wGroup interface { + Add(int) + Done() + Wait() +} + +type fakeGroup struct{} + +func (w *fakeGroup) Add(int) {} +func (w *fakeGroup) Done() {} +func (w *fakeGroup) Wait() {} + func newExtList(opts ExtBrowseOpts, ui uiRegistry, extEntries []extEntry) *extList { ui.List.SetTitleColor(tcell.ColorWhite) ui.List.SetSelectedTextColor(tcell.ColorBlack) @@ -106,13 +118,12 @@ func newExtList(opts ExtBrowseOpts, ui uiRegistry, extEntries []extEntry) *extLi }) el := &extList{ - ui: ui, - extEntries: extEntries, - app: ui.App, - opts: opts, - QueueUpdateDraw: func(f func()) { - ui.App.QueueUpdateDraw(f) - }, + ui: ui, + extEntries: extEntries, + app: ui.App, + opts: opts, + QueueUpdateDraw: ui.App.QueueUpdateDraw, + WaitGroup: &fakeGroup{}, } el.Reset() @@ -130,26 +141,52 @@ func (el *extList) createModal() *tview.Modal { return m } -// TODO consolidate these two functions -func (el *extList) InstallSelected() { - // TODO do not try to install if installed +func (el *extList) toggleSelected(verb string) { ee, ix := el.FindSelected() if ix < 0 { el.opts.Logger.Println("failed to find selected entry") return } - repo, err := ghrepo.FromFullName(ee.FullName) - if err != nil { - el.opts.Logger.Println(fmt.Errorf("failed to install '%s't: %w", ee.FullName, err)) + modal := el.createModal() + + if (ee.Installed && verb == "install") || (!ee.Installed && verb == "remove") { return } - modal := el.createModal() - modal.SetText(fmt.Sprintf("Installing %s...", ee.FullName)) + var action func() error + + if !ee.Installed { + modal.SetText(fmt.Sprintf("Installing %s...", ee.FullName)) + action = func() error { + repo, err := ghrepo.FromFullName(ee.FullName) + if err != nil { + el.opts.Logger.Println(fmt.Errorf("failed to install '%s': %w", ee.FullName, err)) + return err + } + err = el.opts.Em.Install(repo, "") + if err != nil { + return fmt.Errorf("failed to install %s: %w", ee.FullName, err) + } + return nil + } + } else { + modal.SetText(fmt.Sprintf("Removing %s...", ee.FullName)) + action = func() error { + name := strings.TrimPrefix(ee.Name, "gh-") + err := el.opts.Em.Remove(name) + if err != nil { + return fmt.Errorf("failed to remove %s: %w", ee.FullName, err) + } + return nil + } + } + el.ui.CmdFlex.Clear() el.ui.CmdFlex.AddItem(modal, 0, 1, true) - wg := sync.WaitGroup{} + var err error + wg := el.WaitGroup wg.Add(1) + go func() { el.QueueUpdateDraw(func() { el.ui.Pages.SwitchToPage("command") @@ -157,11 +194,15 @@ func (el *extList) InstallSelected() { wg.Done() go func() { el.QueueUpdateDraw(func() { - err = el.opts.Em.Install(repo, "") + err = action() if err != nil { - modal.SetText(fmt.Sprintf("Failed to install %s: %s", ee.FullName, err.Error())) + modal.SetText(err.Error()) } else { - modal.SetText(fmt.Sprintf("Installed %s!", ee.FullName)) + modalText := fmt.Sprintf("Installed %s!", ee.FullName) + if verb == "remove" { + modalText = fmt.Sprintf("Removed %s!", ee.FullName) + } + modal.SetText(modalText) modal.AddButtons([]string{"ok"}) el.app.SetFocus(modal) } @@ -171,42 +212,19 @@ func (el *extList) InstallSelected() { }) }() + // TODO blocking the app's thread and deadlocking wg.Wait() if err == nil { el.toggleInstalled(ix) } } -func (el *extList) RemoveSelected() { - // TODO do not try and remove if not installed - ee, ix := el.FindSelected() - if ix < 0 { - el.opts.Logger.Println("failed to find selected entry") - return - } +func (el *extList) InstallSelected() { + el.toggleSelected("install") +} - modal := el.createModal() - modal.SetText(fmt.Sprintf("Removing %s...", ee.FullName)) - el.ui.CmdFlex.Clear() - el.ui.CmdFlex.AddItem(modal, 0, 1, true) - go func() { - el.app.QueueUpdateDraw(func() { - el.ui.Pages.SwitchToPage("command") - go func() { - el.app.QueueUpdateDraw(func() { - err := el.opts.Em.Remove(strings.TrimPrefix(ee.Name, "gh-")) - if err != nil { - modal.SetText(fmt.Sprintf("Failed to remove %s: %s", ee.FullName, err.Error())) - } else { - modal.SetText(fmt.Sprintf("Removed %s!", ee.FullName)) - modal.AddButtons([]string{"ok"}) - el.app.SetFocus(modal) - el.toggleInstalled(ix) - } - }) - }() - }) - }() +func (el *extList) RemoveSelected() { + el.toggleSelected("remove") } func (el *extList) toggleInstalled(ix int) { diff --git a/pkg/cmd/extension/browse/browse_test.go b/pkg/cmd/extension/browse/browse_test.go index 9801684c7..aacfb37b5 100644 --- a/pkg/cmd/extension/browse/browse_test.go +++ b/pkg/cmd/extension/browse/browse_test.go @@ -6,6 +6,7 @@ import ( "log" "net/http" "net/url" + "sync" "testing" "time" @@ -317,10 +318,13 @@ func Test_extList(t *testing.T) { extList := newExtList(opts, ui, extEntries) - extList.QueueUpdateDraw = func(f func()) { + extList.QueueUpdateDraw = func(f func()) *tview.Application { f() + return app } + extList.WaitGroup = &sync.WaitGroup{} + extList.Filter("cool") assert.Equal(t, 1, extList.ui.List.GetItemCount())