Skip to content

Commit

Permalink
Merge #11200
Browse files Browse the repository at this point in the history
11200: [cli] Reimplement the interactive renderer r=pgavlin a=pgavlin

The display pipleline looks like this:

```
       ╭──────╮
       │Engine│
       ╰──────╯
          ⬇ engine events
   ╭────────────────╮
   │Progress Display│
   ╰────────────────╯
          ⬇ display events: ticks, resource updates, system messages
   ╭─────────────────╮
   │Progress Renderer│
   ╰─────────────────╯
          ⬇ text
      ╭────────╮
      │Terminal│
      ╰────────╯
```

The existing implementation of the interactive Progress Renderer is broken
into two parts, the display renderer and the message renderer. The display
renderer converts display events into progress messages, each of which
generally represents a single line of text at a particular position in
the output. The message renderer converts progress messages into screen
updates by identifying whether or not the contents of a particular
message have changed and if so, re-rendering its output line. In
somewhat greater detail:

```
   ╭────────────────╮
   │Display Renderer│
   ╰────────────────╯
          ⬇ convert resource rows into a tree table
          ⬇ convert the tree table and system messages into lines
          ⬇ convert each line into a progress message with an index
   ╭────────────────╮
   │Message Renderer│
   ╰────────────────╯
          ⬇ if the line identified in a progress message has changed,
          ⬇ go to that line on the terminal, clear it, and update it
      ╭────────╮
      │Terminal│
      ╰────────╯
```

This separation of concerns is unnecessary and makes it difficult to
understand where and when the terminal is updated. This approach also
makes it somewhat challenging to change the way in which the display
interacts with the terminal, as both the display renderer and the
message renderer need to e.g. understand terminal dimensions, movement,
etc.

These changes reimplement the interactive Progress Renderer using a
frame-oriented approach. The display is updated at 60 frame per second.
If nothing has happened to invalidate the display's contents (i.e. no
changes to the terminal geometry or the displayable contents have occurred),
then the frame is not redrawn. Otherwise, the contents of the display
are re-rendered and redrawn.

An advantage of this approach is that it made it relatively simple to
fix a long-standing issue with the interactive display: when the number
of rows in the output exceed the height of the terminal, the new
renderer clamps the output and allows the user to scroll the tree table
using the up and down arrow keys.

Co-authored-by: Pat Gavlin <pat@pulumi.com>
  • Loading branch information
bors[bot] and pgavlin committed Nov 1, 2022
2 parents 6754c38 + 46c1624 commit 0452793
Show file tree
Hide file tree
Showing 9 changed files with 478 additions and 76 deletions.
@@ -0,0 +1,4 @@
changes:
- type: feat
scope: cli/display
description: Improve the usability of the interactive dipslay by making the treetable scrollable
38 changes: 34 additions & 4 deletions pkg/backend/display/jsonmessage.go
Expand Up @@ -140,14 +140,37 @@ type messageRenderer struct {
nonInteractiveSpinner cmdutil.Spinner

progressOutput chan<- Progress
closed <-chan bool

// Cache of lines we've already printed. We don't print a progress message again if it hasn't
// changed between the last time we printed and now.
printedProgressCache map[string]Progress
}

func newMessageRenderer(stdout io.Writer, op string, opts Options) progressRenderer {
progressOutput, closed := make(chan Progress), make(chan bool)
go func() {
ShowProgressOutput(progressOutput, stdout, false)
close(closed)
}()

spinner, ticker := cmdutil.NewSpinnerAndTicker(
fmt.Sprintf("%s%s...", cmdutil.EmojiOr("✨ ", "@ "), op),
nil, opts.Color, 1 /*timesPerSecond*/)
ticker.Stop()

return &messageRenderer{
opts: opts,
progressOutput: progressOutput,
closed: closed,
printedProgressCache: make(map[string]Progress),
nonInteractiveSpinner: spinner,
}
}

func (r *messageRenderer) Close() error {
close(r.progressOutput)
<-r.closed
return nil
}

Expand Down Expand Up @@ -196,7 +219,7 @@ func (r *messageRenderer) println(display *ProgressDisplay, line string) {

func (r *messageRenderer) tick(display *ProgressDisplay) {
if r.isTerminal {
r.render(display)
r.render(display, false)
} else {
// Update the spinner to let the user know that that work is still happening.
r.nonInteractiveSpinner.Tick()
Expand Down Expand Up @@ -233,7 +256,7 @@ func (r *messageRenderer) renderRow(display *ProgressDisplay,
func (r *messageRenderer) rowUpdated(display *ProgressDisplay, row Row) {
if r.isTerminal {
// if we're in a terminal, then refresh everything so that all our columns line up
r.render(display)
r.render(display, false)
} else {
// otherwise, just print out this single row.
colorizedColumns := row.ColorizedColumns()
Expand All @@ -246,17 +269,20 @@ func (r *messageRenderer) systemMessage(display *ProgressDisplay, payload engine
if r.isTerminal {
// if we're in a terminal, then refresh everything. The system events will come after
// all the normal rows
r.render(display)
r.render(display, false)
} else {
// otherwise, in a non-terminal, just print out the actual event.
r.writeSimpleMessage(renderStdoutColorEvent(payload, display.opts))
}
}

func (r *messageRenderer) done(display *ProgressDisplay) {
if r.isTerminal {
r.render(display, false)
}
}

func (r *messageRenderer) render(display *ProgressDisplay) {
func (r *messageRenderer) render(display *ProgressDisplay, done bool) {
if !r.isTerminal || display.headerRow == nil {
return
}
Expand Down Expand Up @@ -316,6 +342,10 @@ func (r *messageRenderer) render(display *ProgressDisplay) {
systemID++
}
}

if done {
r.println(display, "")
}
}

// Ensure our stored dimension info is up to date.
Expand Down
1 change: 1 addition & 0 deletions pkg/backend/display/options.go
Expand Up @@ -50,6 +50,7 @@ type Options struct {
JSONDisplay bool // true if we should emit the entire diff as JSON.
EventLogPath string // the path to the file to use for logging events, if any.
Debug bool // true to enable debug output.
Stdin io.Reader // the reader to use for stdin. Defaults to os.Stdin if unset.
Stdout io.Writer // the writer to use for stdout. Defaults to os.Stdout if unset.
Stderr io.Writer // the writer to use for stderr. Defaults to os.Stderr if unset.
SuppressTimings bool // true to suppress displaying timings of resource actions
Expand Down
78 changes: 14 additions & 64 deletions pkg/backend/display/progress.go
Expand Up @@ -19,17 +19,13 @@ import (
"bytes"
"fmt"
"io"
"math"
"os"
"sort"
"strings"
"time"
"unicode"
"unicode/utf8"

"github.com/moby/term"
"golang.org/x/crypto/ssh/terminal"

"github.com/pulumi/pulumi/pkg/v3/engine"
"github.com/pulumi/pulumi/pkg/v3/resource/deploy"
"github.com/pulumi/pulumi/sdk/v3/go/common/apitype"
Expand Down Expand Up @@ -228,45 +224,26 @@ func getEventUrnAndMetadata(event engine.Event) (resource.URN, *engine.StepEvent
func ShowProgressEvents(op string, action apitype.UpdateKind, stack tokens.Name, proj tokens.PackageName,
events <-chan engine.Event, done chan<- bool, opts Options, isPreview bool) {

stdin := opts.Stdin
if stdin == nil {
stdin = os.Stdin
}
stdout := opts.Stdout
if stdout == nil {
stdout = os.Stdout
}
stderr := opts.Stderr
if stderr == nil {
stderr = os.Stderr
}

// Create a ticker that will update all our status messages once a second. Any
// in-flight resources will get a varying . .. ... ticker appended to them to
// let the user know what is still being worked on.
var spinner cmdutil.Spinner
var ticker *time.Ticker
if stdout == os.Stdout && stderr == os.Stderr {
spinner, ticker = cmdutil.NewSpinnerAndTicker(
fmt.Sprintf("%s%s...", cmdutil.EmojiOr("✨ ", "@ "), op),
nil, opts.Color, 1 /*timesPerSecond*/)
} else {
spinner = &nopSpinner{}
ticker = time.NewTicker(math.MaxInt64)
}

// The channel we push progress messages into, and which ShowProgressOutput pulls
// from to display to the console.
progressOutput := make(chan Progress)

opStopwatch := newOpStopwatch()

renderer := &messageRenderer{
opts: opts,
progressOutput: progressOutput,
printedProgressCache: make(map[string]Progress),
nonInteractiveSpinner: spinner,
isTerminal := true
renderer, err := newTreeRenderer(stdin, stdout, opts)
if err != nil {
fmt.Println(err)
isTerminal, renderer = false, newMessageRenderer(stdout, op, opts)
}

display := &ProgressDisplay{
action: action,
isPreview: isPreview,
isTerminal: isTerminal,
opts: opts,
renderer: renderer,
stack: stack,
Expand All @@ -277,39 +254,12 @@ func ShowProgressEvents(op string, action apitype.UpdateKind, stack tokens.Name,
urnToID: make(map[resource.URN]string),
colorizedToUncolorized: make(map[string]string),
displayOrderCounter: 1,
opStopwatch: opStopwatch,
opStopwatch: newOpStopwatch(),
}

// Assume we are not displaying in a terminal by default.
renderer.isTerminal = false
if stdout == os.Stdout {
terminalWidth, terminalHeight, err := terminal.GetSize(int(os.Stdout.Fd()))
if err == nil {
// If the terminal has a size, use it.
renderer.isTerminal = opts.IsInteractive
renderer.terminalWidth = terminalWidth
renderer.terminalHeight = terminalHeight

// Don't bother attempting to treat this display as a terminal if it has no width/height.
if renderer.isTerminal && (renderer.terminalWidth == 0 || renderer.terminalHeight == 0) {
renderer.isTerminal = false
_, err = fmt.Fprintln(stderr, "Treating display as non-terminal due to 0 width/height.")
contract.IgnoreError(err)
}

// Fetch the canonical stdout stream, configured appropriately.
_, stdout, _ = term.StdStreams()
}
}
display.isTerminal = renderer.isTerminal

go func() {
display.processEvents(ticker, events)
contract.IgnoreClose(display.renderer)
}()

ShowProgressOutput(progressOutput, stdout, display.isTerminal)

ticker := time.NewTicker(1 * time.Second)
display.processEvents(ticker, events)
contract.IgnoreClose(display.renderer)
ticker.Stop()

// let our caller know we're done.
Expand Down

0 comments on commit 0452793

Please sign in to comment.