Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cli] Reimplement the interactive renderer #11200

Merged
merged 4 commits into from Nov 8, 2022

Conversation

pgavlin
Copy link
Member

@pgavlin pgavlin commented Oct 31, 2022

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.

@pgavlin
Copy link
Member Author

pgavlin commented Oct 31, 2022

Note: this stacks on top of #11199

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Oct 31, 2022

Changelog

[uncommitted] (2022-11-08)

Features

  • [cli/display] Improve the usability of the interactive dipslay by making the treetable scrollable
    #11200

@pgavlin
Copy link
Member Author

pgavlin commented Oct 31, 2022

Here's a brief clip that demonstrates the scrolling:

Screen.Recording.2022-10-31.at.9.36.10.AM.mov

@pgavlin pgavlin force-pushed the pgavlin/reimplement-interactive-renderer branch from dedcf02 to 20f6aed Compare October 31, 2022 22:08
@pgavlin
Copy link
Member Author

pgavlin commented Nov 1, 2022

looks like https://github.com/pulumi/pulumi/actions/runs/3371700872/jobs/5594394972 is stuck despite all its steps having finished...?

@pgavlin
Copy link
Member Author

pgavlin commented Nov 1, 2022

bors merge

bors bot added a commit that referenced this pull request Nov 1, 2022
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>
@bors
Copy link
Contributor

bors bot commented Nov 1, 2022

Build failed:

@pgavlin
Copy link
Member Author

pgavlin commented Nov 2, 2022

Unfortunately this is currently horribly broken on Windows, so I am going to have to do a bit of work prior to merging to fix that.

@pgavlin
Copy link
Member Author

pgavlin commented Nov 2, 2022

(I think that it would work in Windows Terminal, but it doesn't render hardly at all in the classic terminal)

@pgavlin
Copy link
Member Author

pgavlin commented Nov 8, 2022

Even in Windows Terminal this doesn't render nicely. I think I'll work around this by deferring to the existing implementation on Windows s.t. we can continue to move this work forwards. Should be a pretty straightforward adjustment to these changes.

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.
@pgavlin pgavlin force-pushed the pgavlin/reimplement-interactive-renderer branch from 7932312 to 7d17bba Compare November 8, 2022 06:03
@pgavlin
Copy link
Member Author

pgavlin commented Nov 8, 2022

I've verified that the fallback on Windows works as expected.

@pgavlin
Copy link
Member Author

pgavlin commented Nov 8, 2022

bors merge

@bors
Copy link
Contributor

bors bot commented Nov 8, 2022

Build succeeded:

  • bors-ok

@bors bors bot merged commit ed05626 into master Nov 8, 2022
@pulumi-bot pulumi-bot deleted the pgavlin/reimplement-interactive-renderer branch November 8, 2022 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants