diff --git a/pkg/cmd/extension/browse/browse.go b/pkg/cmd/extension/browse/browse.go index be605ef9c56..247d43c5b04 100644 --- a/pkg/cmd/extension/browse/browse.go +++ b/pkg/cmd/extension/browse/browse.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/MakeNowJust/heredoc" "github.com/charmbracelet/glamour" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" @@ -25,16 +26,17 @@ import ( const pagingOffset = 24 type ExtBrowseOpts struct { - Cmd *cobra.Command - Browser ibrowser - IO *iostreams.IOStreams - Searcher search.Searcher - Em extensions.ExtensionManager - Client *http.Client - Logger *log.Logger - Cfg config.Config - Rg *readmeGetter - Debug bool + Cmd *cobra.Command + Browser ibrowser + IO *iostreams.IOStreams + Searcher search.Searcher + Em extensions.ExtensionManager + Client *http.Client + Logger *log.Logger + Cfg config.Config + Rg *readmeGetter + Debug bool + SingleColumn bool } type ibrowser interface { @@ -48,7 +50,8 @@ type uiRegistry struct { App *tview.Application Outerflex *tview.Flex List *tview.List - Readme *tview.TextView + Pages *tview.Pages + CmdFlex *tview.Flex } type extEntry struct { @@ -83,25 +86,44 @@ func (e extEntry) Description() string { } type extList struct { - ui uiRegistry - extEntries []extEntry - app *tview.Application - filter string - opts ExtBrowseOpts + ui uiRegistry + extEntries []extEntry + app *tview.Application + filter string + opts ExtBrowseOpts + 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) ui.List.SetSelectedBackgroundColor(tcell.ColorWhite) ui.List.SetWrapAround(false) ui.List.SetBorderPadding(1, 1, 1, 1) + ui.List.SetSelectedFunc(func(ix int, _, _ string, _ rune) { + ui.Pages.SwitchToPage("readme") + }) el := &extList{ - ui: ui, - extEntries: extEntries, - app: ui.App, - opts: opts, + ui: ui, + extEntries: extEntries, + app: ui.App, + opts: opts, + QueueUpdateDraw: ui.App.QueueUpdateDraw, + WaitGroup: &fakeGroup{}, } el.Reset() @@ -112,66 +134,97 @@ func (el *extList) createModal() *tview.Modal { m := tview.NewModal() m.SetBackgroundColor(tcell.ColorPurple) m.SetDoneFunc(func(_ int, _ string) { - el.ui.App.SetRoot(el.ui.Outerflex, true) + el.ui.Pages.SwitchToPage("main") el.Refresh() }) return m } -func (el *extList) InstallSelected() { +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() + var action func() error - modal.SetText(fmt.Sprintf("Installing %s...", ee.FullName)) - el.ui.App.SetRoot(modal, true) - // I could eliminate this with a goroutine but it seems to be working fine - el.app.ForceDraw() - err = el.opts.Em.Install(repo, "") - if err != nil { - modal.SetText(fmt.Sprintf("Failed to install %s: %s", ee.FullName, err.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("Installed %s!", ee.FullName)) - modal.AddButtons([]string{"ok"}) - el.ui.App.SetFocus(modal) + 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.toggleInstalled(ix) -} + el.ui.CmdFlex.Clear() + el.ui.CmdFlex.AddItem(modal, 0, 1, true) + var err error + wg := el.WaitGroup + wg.Add(1) + + go func() { + el.QueueUpdateDraw(func() { + el.ui.Pages.SwitchToPage("command") + wg.Add(1) + wg.Done() + go func() { + el.QueueUpdateDraw(func() { + err = action() + if err != nil { + modal.SetText(err.Error()) + } else { + 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) + } + wg.Done() + }) + }() + }) + }() -func (el *extList) RemoveSelected() { - ee, ix := el.FindSelected() - if ix < 0 { - el.opts.Logger.Println("failed to find selected extension") - return + // TODO blocking the app's thread and deadlocking + wg.Wait() + if err == nil { + el.toggleInstalled(ix) } +} - modal := el.createModal() - - modal.SetText(fmt.Sprintf("Removing %s...", ee.FullName)) - el.ui.App.SetRoot(modal, true) - // I could eliminate this with a goroutine but it seems to be working fine - el.ui.App.ForceDraw() +func (el *extList) InstallSelected() { + el.toggleSelected("install") +} - 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.ui.App.SetFocus(modal) - } - el.toggleInstalled(ix) +func (el *extList) RemoveSelected() { + el.toggleSelected("remove") } func (el *extList) toggleInstalled(ix int) { @@ -365,14 +418,19 @@ func ExtBrowse(opts ExtBrowseOpts) error { readme.SetBorder(true).SetBorderColor(tcell.ColorPurple) help := tview.NewTextView() - help.SetText( - "/: filter i/r: install/remove w: open in browser pgup/pgdn: scroll readme q: quit") - help.SetTextAlign(tview.AlignCenter) + help.SetDynamicColors(true) + help.SetText("[::b]?[-:-:-]: help [::b]j/k[-:-:-]: move [::b]i[-:-:-]: install [::b]r[-:-:-]: remove [::b]w[-:-:-]: web [::b]↵[-:-:-]: view readme [::b]q[-:-:-]: quit") + + cmdFlex := tview.NewFlex() + + pages := tview.NewPages() ui := uiRegistry{ App: app, Outerflex: outerFlex, List: list, + Pages: pages, + CmdFlex: cmdFlex, } extList := newExtList(opts, ui, extEntries) @@ -414,7 +472,9 @@ func ExtBrowse(opts ExtBrowseOpts) error { innerFlex.SetDirection(tview.FlexColumn) innerFlex.AddItem(list, 0, 1, true) - innerFlex.AddItem(readme, 0, 1, false) + if !opts.SingleColumn { + innerFlex.AddItem(readme, 0, 1, false) + } outerFlex.SetDirection(tview.FlexRow) outerFlex.AddItem(header, 1, -1, false) @@ -422,7 +482,50 @@ func ExtBrowse(opts ExtBrowseOpts) error { outerFlex.AddItem(innerFlex, 0, 1, true) outerFlex.AddItem(help, 1, -1, false) - app.SetRoot(outerFlex, true) + helpBig := tview.NewTextView() + helpBig.SetDynamicColors(true) + helpBig.SetBorderPadding(0, 0, 2, 0) + helpBig.SetText(heredoc.Doc(` + [::b]Application[-:-:-] + + ?: toggle help + q: quit + + [::b]Navigation[-:-:-] + + ↓, j: scroll list of extensions down by 1 + ↑, k: scroll list of extensions up by 1 + + shift+j, space: scroll list of extensions down by 25 + shift+k, ctrl+space (mac), shift+space (windows): scroll list of extensions up by 25 + + [::b]Extension Management[-:-:-] + + i: install highlighted extension + r: remove highlighted extension + w: open highlighted extension in web browser + + [::b]Filtering[-:-:-] + + /: focus filter + enter: finish filtering and go back to list + escape: clear filter and reset list + + [::b]Readmes[-:-:-] + + enter: open highlighted extension's readme full screen + page down: scroll readme pane down + page up: scroll readme pane up + + (On a mac, page down and page up are fn+down arrow and fn+up arrow) + `)) + + pages.AddPage("main", outerFlex, true, true) + pages.AddPage("help", helpBig, true, false) + pages.AddPage("readme", readme, true, false) + pages.AddPage("command", cmdFlex, true, false) + + app.SetRoot(pages, true) // Force fetching of initial readme by loading it just prior to the first // draw. The callback is removed immediately after draw. @@ -441,7 +544,41 @@ func ExtBrowse(opts ExtBrowseOpts) error { return event } + curPage, _ := pages.GetFrontPage() + + if curPage != "main" { + if curPage == "command" { + return event + } + if event.Rune() == 'q' || event.Key() == tcell.KeyEscape { + pages.SwitchToPage("main") + return nil + } + switch curPage { + case "readme": + switch event.Key() { + case tcell.KeyPgUp: + row, col := readme.GetScrollOffset() + if row > 0 { + readme.ScrollTo(row-2, col) + } + case tcell.KeyPgDn: + row, col := readme.GetScrollOffset() + readme.ScrollTo(row+2, col) + } + case "help": + switch event.Rune() { + case '?': + pages.SwitchToPage("main") + } + } + return nil + } + switch event.Rune() { + case '?': + pages.SwitchToPage("help") + return nil case 'q': app.Stop() case 'k': @@ -491,7 +628,7 @@ func ExtBrowse(opts ExtBrowseOpts) error { filter.SetText("") extList.Reset() case tcell.KeyCtrlSpace: - // The ctrl check works on windows/mac and not windows: + // The ctrl check works on linux/mac and not windows: extList.PageUp() go loadSelectedReadme() case tcell.KeyCtrlJ: @@ -500,25 +637,11 @@ func ExtBrowse(opts ExtBrowseOpts) error { case tcell.KeyCtrlK: extList.PageUp() go loadSelectedReadme() - case tcell.KeyPgUp: - row, col := readme.GetScrollOffset() - if row > 0 { - readme.ScrollTo(row-2, col) - } - return nil - case tcell.KeyPgDn: - row, col := readme.GetScrollOffset() - readme.ScrollTo(row+2, col) - return nil } return event }) - // Without this redirection, the git client inside of the extension manager - // will dump git output to the terminal. - opts.IO.ErrOut = io.Discard - if err := app.Run(); err != nil { return err } diff --git a/pkg/cmd/extension/browse/browse_test.go b/pkg/cmd/extension/browse/browse_test.go index 8cec46607b5..aacfb37b5a8 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" @@ -274,11 +275,15 @@ func Test_extList(t *testing.T) { }, }, } + cmdFlex := tview.NewFlex() app := tview.NewApplication() list := tview.NewList() + pages := tview.NewPages() ui := uiRegistry{ - List: list, - App: app, + List: list, + App: app, + CmdFlex: cmdFlex, + Pages: pages, } extEntries := []extEntry{ { @@ -313,6 +318,13 @@ func Test_extList(t *testing.T) { extList := newExtList(opts, ui, extEntries) + 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()) @@ -322,6 +334,8 @@ func Test_extList(t *testing.T) { extList.InstallSelected() assert.True(t, extList.extEntries[0].Installed) + // so I think the goroutines are causing a later failure because the toggleInstalled isn't seen. + extList.Refresh() assert.Equal(t, 1, extList.ui.List.GetItemCount()) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 681a2c66638..722163dd217 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -3,6 +3,7 @@ package extension import ( "errors" "fmt" + gio "io" "os" "strings" "time" @@ -24,6 +25,7 @@ import ( func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { m := f.ExtensionManager io := f.IOStreams + gc := f.GitClient prompter := f.Prompter config := f.Config browser := f.Browser @@ -410,33 +412,25 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { }, func() *cobra.Command { var debug bool + var singleColumn bool cmd := &cobra.Command{ Use: "browse", Short: "Enter a UI for browsing, adding, and removing extensions", Long: heredoc.Doc(` This command will take over your terminal and run a fully interactive - interface for browsing, adding, and removing gh extensions. + interface for browsing, adding, and removing gh extensions. A terminal + width greater than 100 columns is recommended. - The extension list is navigated with the arrow keys or with j/k. - Space and control+space (or control + j/k) page the list up and down. - Extension readmes can be scrolled with page up/page down keys - (fn + arrow up/down on a mac keyboard). - - For highlighted extensions, you can press: - - - w to open the extension in your web browser - - i to install the extension - - r to remove the extension - - Press / to focus the filter input. Press enter to scroll the results. - Press Escape to clear the filter and return to the full list. + To learn how to control this interface, press ? after running to see + the help text. Press q to quit. - The output of this command may be difficult to navigate for screen reader - users, users operating at high zoom and other users of assistive technology. It - is also not advised for automation scripts. We advise those users to use the - alternative command: + Running this command with --single-column should make this command + more intelligible for users who rely on assistive technology like screen + readers or high zoom. + + For a more traditional way to discover extensions, see: gh ext search @@ -459,21 +453,25 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { searcher := search.NewSearcher(api.NewCachedHTTPClient(client, time.Hour*24), host) + gc.Stderr = gio.Discard + opts := browse.ExtBrowseOpts{ - Cmd: cmd, - IO: io, - Browser: browser, - Searcher: searcher, - Em: m, - Client: client, - Cfg: cfg, - Debug: debug, + Cmd: cmd, + IO: io, + Browser: browser, + Searcher: searcher, + Em: m, + Client: client, + Cfg: cfg, + Debug: debug, + SingleColumn: singleColumn, } return browse.ExtBrowse(opts) }, } cmd.Flags().BoolVar(&debug, "debug", false, "log to /tmp/extBrowse-*") + cmd.Flags().BoolVarP(&singleColumn, "single-column", "s", false, "Render TUI with only one column of text") return cmd }(), &cobra.Command{ diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 21c2120ae37..4f0e6a9fba8 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -350,7 +350,7 @@ func (m *Manager) Install(repo ghrepo.Interface, target string) error { return errors.New("extension is not installable: missing executable") } - return m.installGit(repo, target, m.io.Out, m.io.ErrOut) + return m.installGit(repo, target) } func (m *Manager) installBin(repo ghrepo.Interface, target string) error { @@ -453,7 +453,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error { return nil } -func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, target string) error { protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") cloneURL := ghrepo.FormatRemoteURL(repo, protocol)