-
Notifications
You must be signed in to change notification settings - Fork 7
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
revise API, enforce single root invariant, support zoomed interactive vertices #11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Alex Suraci <[email protected]>
bubbletea registers its own signal handler and exits, whereas before we relied on tea.Quit to call interrupt.
to my surprise, bubbletea already seems to cap the View result to the terminal height, which is kind of what you want, since we're abandoning scrollability. will revisit later
just to verify output going past scrollback
this param was always true in practice. it may have originally been how to toggle console mode, but console mode works entirely differently now. it seems more useful to treat this as an interactive on/off, so it can be disabled for things that require interaction, like REPLs or shells. along the way: * go back to viewport-based rendering * render chrome in the same loop as the regular render loop, so that we can take its height into account when rendering. this fixed a couple TODOs for hardcoding "- 2". * use persistent buffer for the render loop, likely major perf low hanging fruit
A single root seems like a valuable property to simplify a ton of things. Technically this means you might see multiple "intervals" of the root group, but the last one should be the outermost one.
complete vtx on command exit
previous implementation was an obvious hackjob, just rendering all zoomed vertices, which would have generated garbage
it betwen zoomed vertices and bubbletea
The async RenderLoop API was pretty complicated to use. Everything gets a lot simpler when you pass a callback instead, because you can just wrap it in a defer to restore the terminal to "cooked" mode, integrate the callback lifecycle into the Model, and just print the final render after the program exits.
Just use NewRecorder now that we have the "single root" invariant.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has grown a bit over time, and now includes 3 major changes:
Revised API
UI.RenderLoop
has been replaced with a more conventional synchronous callback-basedRun
API. This comes after Progrock has taken more responsibility in handling stdio. A synchronous callback flow makes it easier todefer
restoring the terminal state, and also seems to result in cleaner code.RecorderToContext
=>ToContext
RecorderFromContext
=>FromContext
NewPassthroughRecorder
has been removed (see next point)Single root invariant
In complicated applications like Dagger we want to be able to emit progress at various layers of the stack, such as early initialization. But we also wanted to set labels on the "root" group, and some of those labels required that initialization was already done, forcing into an uncomfortable situation where we needed either multiple "root" groups or to emit progress without a group at all (
NewPassthroughRecorder
).Dealing with both of those latter situations in the UI proved to be really clumsy. Fundamentally the notion of "mutliple roots" is pretty nasty for clients to have to deal with, and the notion of "ungrouped vertices" is similarly confounding. So there's a new invariant: all root groups have the same ID.
After dogfooding this has really simplified a lot of code. Technically there can be multiple root groups with different start/stop intervals and different labels, but that can be handled at the application layer. Nothing actually respects group start/stop timestamps at the moment, but being able to distinguish the intervals might be valuable in some contexts. Root group labels can probably all be merged into one set of labels.
Zoomed interactive sessions
Progrock now supports "Zoomed" vertices, which - while running - will take over the UI content while still being rendered above the status bar, so you can still see activity happening in the background. The goal here is to support things like
dagger shell
ordagger run bass
(i.e. a REPL).Zoomed vertices can return an optional
io.Writer
for accepting user input. Progrock will intelligently switch from passing user input to the Bubbletea UI model (for e.g. scrolling or pressingq
to quit) and sending user input to the currently zoomed vertex, supporting full-blown keyboard/mouse interaction in a nested shell for example.This might be expanded in the future to support full-blown
tmux
-style pane switching.