From 4a3ef50d2d799c9063bc39e0125ae72b990afce1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 1 Feb 2022 08:36:51 +0100 Subject: [PATCH] Standardize pager output across commands (#5141) Add pager functionality to the following commands: - gist list - pr checks - release list - run list - run view - secret list - workflow list - workflow view Additionally, normalize error handling when starting the pager has failed: only print a non-fatal notice to stderr instead of aborting the whole command. --- pkg/cmd/api/api.go | 8 +++---- pkg/cmd/gist/list/list.go | 7 ++++++ pkg/cmd/issue/list/list.go | 8 +++---- pkg/cmd/pr/checks/checks.go | 6 +++++ pkg/cmd/pr/diff/diff.go | 8 +++---- pkg/cmd/pr/view/view.go | 9 ++++---- pkg/cmd/release/list/list.go | 6 +++++ pkg/cmd/repo/view/view.go | 7 +++--- pkg/cmd/run/list/list.go | 6 +++++ pkg/cmd/run/view/view.go | 18 +++++++-------- pkg/cmd/secret/list/list.go | 6 +++++ pkg/cmd/workflow/list/list.go | 6 +++++ pkg/cmd/workflow/view/view.go | 43 ++++++++++++----------------------- 13 files changed, 81 insertions(+), 57 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 7c5eeca0e..34bde4c80 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -279,11 +279,11 @@ func apiRun(opts *ApiOptions) error { if opts.Silent { opts.IO.Out = ioutil.Discard } else { - err := opts.IO.StartPager() - if err != nil { - return err + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - defer opts.IO.StopPager() } cfg, err := opts.Config() diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 229ebdc8e..38c55425d 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -1,6 +1,7 @@ package list import ( + "fmt" "net/http" "strings" "time" @@ -84,6 +85,12 @@ func listRun(opts *ListOptions) error { return err } + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + cs := opts.IO.ColorScheme() tp := utils.NewTablePrinter(opts.IO) diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index bf0e9bae6..cba60b75f 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -160,11 +160,11 @@ func listRun(opts *ListOptions) error { return err } - err = opts.IO.StartPager() - if err != nil { - return err + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - defer opts.IO.StopPager() if opts.Exporter != nil { return opts.Exporter.Write(opts.IO, listResult.Issues) diff --git a/pkg/cmd/pr/checks/checks.go b/pkg/cmd/pr/checks/checks.go index 011856385..ecc101ae1 100644 --- a/pkg/cmd/pr/checks/checks.go +++ b/pkg/cmd/pr/checks/checks.go @@ -193,6 +193,12 @@ func checksRun(opts *ChecksOptions) error { return (b0 == "fail") || (b0 == "pending" && b1 == "success") }) + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + tp := utils.NewTablePrinter(opts.IO) for _, o := range outputs { diff --git a/pkg/cmd/pr/diff/diff.go b/pkg/cmd/pr/diff/diff.go index ebab2c44a..dc0602da8 100644 --- a/pkg/cmd/pr/diff/diff.go +++ b/pkg/cmd/pr/diff/diff.go @@ -104,11 +104,11 @@ func diffRun(opts *DiffOptions) error { } defer diff.Close() - err = opts.IO.StartPager() - if err != nil { - return err + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - defer opts.IO.StopPager() if !opts.UseColor { _, err = io.Copy(opts.IO.Out, diff) diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index f0746772d..67124b195 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -109,12 +109,11 @@ func viewRun(opts *ViewOptions) error { } opts.IO.DetectTerminalTheme() - - err = opts.IO.StartPager() - if err != nil { - return err + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - defer opts.IO.StopPager() if opts.Exporter != nil { return opts.Exporter.Write(opts.IO, pr) diff --git a/pkg/cmd/release/list/list.go b/pkg/cmd/release/list/list.go index 5d0320406..18e33621d 100644 --- a/pkg/cmd/release/list/list.go +++ b/pkg/cmd/release/list/list.go @@ -63,6 +63,12 @@ func listRun(opts *ListOptions) error { return err } + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + now := time.Now() table := utils.NewTablePrinter(opts.IO) iofmt := opts.IO.ColorScheme() diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index 2a1c0fe30..cb3c19fa2 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -142,10 +142,11 @@ func viewRun(opts *ViewOptions) error { } opts.IO.DetectTerminalTheme() - if err := opts.IO.StartPager(); err != nil { - return err + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - defer opts.IO.StopPager() if opts.Exporter != nil { return opts.Exporter.Write(opts.IO, repo) diff --git a/pkg/cmd/run/list/list.go b/pkg/cmd/run/list/list.go index d3465b114..8c1f59779 100644 --- a/pkg/cmd/run/list/list.go +++ b/pkg/cmd/run/list/list.go @@ -107,6 +107,12 @@ func listRun(opts *ListOptions) error { return fmt.Errorf("failed to get runs: %w", err) } + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + if opts.Exporter != nil { return opts.Exporter.Write(opts.IO, runs) } diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index a01e8c2f2..f41f37801 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -230,6 +230,12 @@ func runView(opts *ViewOptions) error { } } + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + if opts.Exporter != nil { return opts.Exporter.Write(opts.IO, run) } @@ -276,7 +282,7 @@ func runView(opts *ViewOptions) error { attachRunLog(runLogZip, jobs) - return displayRunLog(opts.IO, jobs, opts.LogFailed) + return displayRunLog(opts.IO.Out, jobs, opts.LogFailed) } prNumber := "" @@ -502,13 +508,7 @@ func attachRunLog(rlz *zip.ReadCloser, jobs []shared.Job) { } } -func displayRunLog(io *iostreams.IOStreams, jobs []shared.Job, failed bool) error { - err := io.StartPager() - if err != nil { - return err - } - defer io.StopPager() - +func displayRunLog(w io.Writer, jobs []shared.Job, failed bool) error { for _, job := range jobs { steps := job.Steps sort.Sort(steps) @@ -526,7 +526,7 @@ func displayRunLog(io *iostreams.IOStreams, jobs []shared.Job, failed bool) erro } scanner := bufio.NewScanner(f) for scanner.Scan() { - fmt.Fprintf(io.Out, "%s%s\n", prefix, scanner.Text()) + fmt.Fprintf(w, "%s%s\n", prefix, scanner.Text()) } f.Close() } diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index 6d1866652..27fa941bf 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -122,6 +122,12 @@ func listRun(opts *ListOptions) error { return fmt.Errorf("failed to get secrets: %w", err) } + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + tp := utils.NewTablePrinter(opts.IO) for _, secret := range secrets { tp.AddField(secret.Name, nil, nil) diff --git a/pkg/cmd/workflow/list/list.go b/pkg/cmd/workflow/list/list.go index cd3e66c25..e39c3bfc0 100644 --- a/pkg/cmd/workflow/list/list.go +++ b/pkg/cmd/workflow/list/list.go @@ -88,6 +88,12 @@ func listRun(opts *ListOptions) error { return nil } + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + tp := utils.NewTablePrinter(opts.IO) cs := opts.IO.ColorScheme() diff --git a/pkg/cmd/workflow/view/view.go b/pkg/cmd/workflow/view/view.go index 210835759..4972f92a0 100644 --- a/pkg/cmd/workflow/view/view.go +++ b/pkg/cmd/workflow/view/view.go @@ -123,10 +123,17 @@ func runView(opts *ViewOptions) error { return opts.Browser.Browse(address) } - if opts.YAML { - err = viewWorkflowContent(opts, client, workflow) + opts.IO.DetectTerminalTheme() + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() } else { - err = viewWorkflowInfo(opts, client, workflow) + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + + if opts.YAML { + err = viewWorkflowContent(opts, client, repo, workflow, opts.Ref) + } else { + err = viewWorkflowInfo(opts, client, repo, workflow) } if err != nil { return err @@ -135,19 +142,12 @@ func runView(opts *ViewOptions) error { return nil } -func viewWorkflowContent(opts *ViewOptions, client *api.Client, workflow *shared.Workflow) error { - repo, err := opts.BaseRepo() - if err != nil { - return fmt.Errorf("could not determine base repo: %w", err) - } - - opts.IO.StartProgressIndicator() - yamlBytes, err := shared.GetWorkflowContent(client, repo, *workflow, opts.Ref) - opts.IO.StopProgressIndicator() +func viewWorkflowContent(opts *ViewOptions, client *api.Client, repo ghrepo.Interface, workflow *shared.Workflow, ref string) error { + yamlBytes, err := shared.GetWorkflowContent(client, repo, *workflow, ref) if err != nil { if s, ok := err.(api.HTTPError); ok && s.StatusCode == 404 { - if opts.Ref != "" { - return fmt.Errorf("could not find workflow file %s on %s, try specifying a different ref", workflow.Base(), opts.Ref) + if ref != "" { + return fmt.Errorf("could not find workflow file %s on %s, try specifying a different ref", workflow.Base(), ref) } return fmt.Errorf("could not find workflow file %s, try specifying a branch or tag using `--ref`", workflow.Base()) } @@ -156,12 +156,6 @@ func viewWorkflowContent(opts *ViewOptions, client *api.Client, workflow *shared yaml := string(yamlBytes) - opts.IO.DetectTerminalTheme() - if err := opts.IO.StartPager(); err != nil { - fmt.Fprintf(opts.IO.ErrOut, "starting pager failed: %v\n", err) - } - defer opts.IO.StopPager() - if !opts.Raw { cs := opts.IO.ColorScheme() out := opts.IO.Out @@ -191,15 +185,8 @@ func viewWorkflowContent(opts *ViewOptions, client *api.Client, workflow *shared return nil } -func viewWorkflowInfo(opts *ViewOptions, client *api.Client, workflow *shared.Workflow) error { - repo, err := opts.BaseRepo() - if err != nil { - return fmt.Errorf("could not determine base repo: %w", err) - } - - opts.IO.StartProgressIndicator() +func viewWorkflowInfo(opts *ViewOptions, client *api.Client, repo ghrepo.Interface, workflow *shared.Workflow) error { wr, err := getWorkflowRuns(client, repo, workflow) - opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to get runs: %w", err) }