Merge pull request #6584 from cli/gh-ext-browse-followup

gh ext browse followup
This commit is contained in:
Nate Smith 2023-02-02 13:28:43 -08:00 committed by GitHub
commit c024e856db
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 244 additions and 109 deletions

View file

@ -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()
})
}()
})
}()
// TODO blocking the app's thread and deadlocking
wg.Wait()
if err == nil {
el.toggleInstalled(ix)
}
}
func (el *extList) InstallSelected() {
el.toggleSelected("install")
}
func (el *extList) RemoveSelected() {
ee, ix := el.FindSelected()
if ix < 0 {
el.opts.Logger.Println("failed to find selected extension")
return
}
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()
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)
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
}

View file

@ -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())

View file

@ -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{

View file

@ -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)