Simplify text outputs, remove Lipgloss, BubbleTea/Bubbles #607
Conversation
This file contains hidden or 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
Adding table.go to handle output to a table format (adds dep runewidth to properly scale) Adding pager.go to handle paging Adding color.go to respect NO_COLOR
Slightly reworked metadata here to be able to output the queue cleanly in the table view
- Replaced Bubble Tea bulk stop with fmt progress bar (using shared progress bar helper) - Added ASCII-only output, delaying error printing, with a limit clamp - Removed Bubble Tea import
- Removed all bubble tea deps from internal/agent - Moved the single use functions in view.go internal to just be in view.go itself as they don't need to be exported - Removed internal/agent entirely as it was empty :~)
- Uses output.Table and pager for --output text - Refactored output once through the pager as was trying to write data on the fly
Impliments the table and pager in job list, removes lipgloss
- Added standardized table view and pager - Added a header for URL as data was pretty lacking - Removed lipgloss - Added header for all other list commands for consistency
Missed it
Allow pager to be disabled without specifying --no-pager
- Switch build view to the new table renderer and drop tea/lipgloss - Adding shared summary helpers for consistent output - Add pager to build view
Adds global check for no pager Updates color checks to ensure isatty or dumb term now too
Removes all of the old UI, cleans up packages and updates missed references
Refactored the build listing command to handle streaming output and header printing, updated the confirmation prompt to print to stderr instead of stdout so prompts are still interactive in pager mode
Slightly changed string truncation logic in annotation summaries and job views to operate on runes instead of bytes, to ensure correct handling of unicode chars, preventing cuts in the wrong places
Using slice instead as was a little in-accurate
Coverage was OK but could be better, improved to be more thorough
Was hammering resources because it was unbound, I've capped it now so that we don't spawn loads of go routines
No longer needed as we now bound go routines properly
Would break out of the table view due to /n, made it csv to avoid this, updated test expected results
Rather than showing "Connected", it will now return the actual state, e.g., running, idle, paused. Added test for this
Add a plurization helper to make agent agents when more than 1 exists, made output cleaner
Output table will now scale accordingly, truncating text where overflow would occur Added test case
Added signal handling to cancel gracefully. Improved output, too.
JoeColeman95
changed the title
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.
Description
This PR aims to remove BubbleTea alternate screens, remove lipgloss styling and implement a unicode progress bar for bulk operations. The aforementioned changes have mostly been replaced with a simple tabular view, using ANSI codes for additional styling to improve readability.
All changes have been implemented keeping in mind the variance of different terminals. As such, the new styling (dim, bold, italic, underlined and strikethrough), all respect constraints of terminals, if it's got TERM=dumb, or NO_COLOR, etc. For example, if a terminal is only able to display dim/bold, the text will be formatted as plain-text for other style types due to the ANSI, it will not show the ANSI as an output (tested this with quite a few terminal types)
Further to this, the table in TTY will scale according to the available space, truncating when it would result in line-wrap (as that'd be really ugly). When in non-TTY, this has a fixed width.
When in non-TTY, all formatting is entirely removed, including color and a fixed terminal width of
120is used. This can be adjusted withBUILDKITE_TABLE_MAX_WIDTH(this will be documented in Buildkite Docs after the fact).The following environment variables are accepted for conforming to standard practises:
NO_PAGER,BUILDKITE_NO_PAGER,NO_COLOR,TERM=dumb,PAGER=less, etc. These will also be documented.Added a pager across all of the show me something items in the CLI, too. This is pretty standard across CLI tools, so it's nice to be part of the club.
Added the availability of
no_pager: boolto the config (.bk.yaml) to disable this functionality.--no-pagerwas also added as a global flag to disable the pager (when not auto-detected, or set via env var or config)Past that, we have a progress bar on
bk agent stopnow, which is nice. I would've done the same forbk artifacts download, but that was a major refactor and would've been out of scope.We still default to JSON everywhere, so this will be invisible for the most part, anyway. These changes are specific to
--output textChanges
internal/uipackage and all TUI componentsinternal/agent/*(entire agent TUI system)internal/keys/*andinternal/list/*(TUI key handlers)pkg/style/style.go(Lipgloss styling)internal/io/pager.go) with support forPAGERenv var--no-pagerglobal flag to disable pagingBUILDKITE_NO_PAGERandNO_PAGERenvironment variables to disable pagingno_pager: boolconfig option in.bk.yamlpkg/output/table.go) with ANSI stylingpkg/output/color.gofor color detection (NO_COLOR,TERM=dumb)pkg/output/value.gofor value formattinginternal/io/progress.gofor progress bar renderingBUILDKITE_TABLE_MAX_WIDTHenvironment variable to override table widthlist,viewandwatchcommands to use new table rendererNO_COLOR,TERM=dumb, and non-TTY environmentsagent stopfor bulk operationsagent stopto use worker pool pattern instead of semaphoreTesting
go test ./...)go fmt ./...)Disclosures / Credits