Enhanced terraform plan and terraform apply commands
#246
Conversation
DavidS-ovm
commented
Apr 17, 2024
•
DavidS-ovm
commented
DavidS-ovm
force-pushed
the
bubbletea-pt2
branch
from
fdeac49 to
13dfc0a
Compare
DavidS-ovm
force-pushed
the
bubbletea-pt2
branch
from
13dfc0a to
e794177
Compare
DavidS-ovm
force-pushed
the
bubbletea-pt2
branch
2 times, most recently
from
1bd9425 to
3efa0f4
Compare
DavidS-ovm
changed the title
terraform plan and terraform apply commands
There was a problem hiding this comment.
This is awesome and I'm keen to get this merged, though I think there are some things we can improve:
Wrapping
We seem to be wrapping at a fixed point regardless of the terminal width which leads to things like the below where the link has been wrapped, but because it was the CLI that wrapped it and not the terminal it messes up the link and when I click it I don't get the whole link
Authenticate with browser shows >1 time
When I tried locally I was shown the "Authenticate with a browser" more than once, I would have thought that once I had done this once I'd be fine
Inconsistent when previewing steps
At some stages it seems like we're showing a preview of the next step that is going to happen. Like here it looks like we're currently "Waiting for device auth..." and then next is "Configuring AWS Access":
I don't mind this but if we are going to preview what steps are coming next I think we should be consistent about it i.e. layout the whole process at the start, or just show thw current and past steps
Does it not wait for risks?
It didn't wait for risks for me, I'm assuming this just isn't done yet?
Improvement: Can we hide things that serve no purpose after they are done?
For example, all of the text that goes along with auth is useless once you've authenticated. You're not going to want to scroll back and see what it asked you to do. Is it possible to basically collapse this to just a single line like we already have that says authentication successful?
DavidS-ovm
force-pushed
the
bubbletea-pt2
branch
2 times, most recently
from
e546cef to
1e6ca7e
Compare
Fixed this in a37f679
fixed in 35254ed : don't display the auth message after the authentication succeeded.
I've added steps for "Planned Changes" and "Processing" (also update the gif above). This is pretty flexible and we can add and remove tasks to taste.
I've changed the text on the link to mention risks:
We can add more waiting and more output after this if you want. My thinking here was that it gives folks something to look at (the graph) while they're waiting for the risk calculation.
Yes. See above change. |
Add a variable to the test tf file to test interactivity with the running terraform command.
See the code comments for limitations of this approach.
this allows bubbletea to render it as clickable URL instead. This still doesn't fix the blinking issue with the spinner causing refreshes.
|
(updated the commit links above after rebasing) |
|
Nice. Let's get this merged. I think I'm still going to have more design opinions but I'd like to tinker myself a bit to make sure they actually look good |
|
Yeah, please don't think this is the final form of the UI here! This is a
working prototype. It was hard enough to wrap my head around bubbletea's
architecture on the first pass 😂
…On Thu, May 2, 2024, 19:21 Dylan ***@***.***> wrote:
Nice. Let's get this merged. I think I'm still going to have more design
opinions but I'd like to tinker myself a bit to make sure they actually
look good
—
Reply to this email directly, view it on GitHub
<#246 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A4FUO3KKKFCATT5QLTRBJM3ZAJYYLAVCNFSM6AAAAABGKYDIAGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJRGEYTANRTGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
