Add TUI table overrides for 5 high-traffic list commands#4732
Open
simonfaltum wants to merge 15 commits intosimonfaltum/list-tui-paginatedfrom
Open
Add TUI table overrides for 5 high-traffic list commands#4732simonfaltum wants to merge 15 commits intosimonfaltum/list-tui-paginatedfrom
simonfaltum wants to merge 15 commits intosimonfaltum/list-tui-paginatedfrom
Conversation
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: a725914
20 interesting tests: 9 SKIP, 7 KNOWN, 3 flaky, 1 RECOVERED
Top 31 slowest tests (at least 2 minutes):
|
simonfaltum
requested review from
andrewnester,
anton-107,
denik,
pietern and
shreyas-goenka
as code owners
Contributor
shreyas-goenka
left a comment
shreyas-goenka
left a comment
There was a problem hiding this comment.
Note: This review was posted by Claude (AI assistant). Shreyas will do a separate, more thorough review pass.
Priority: LOW — Clean mechanical PR
This is a straightforward PR that adds TUI table overrides for 5 commands. No bugs or correctness issues found.
Minor Nits
- Unused
listReqvariables: Several overrides declarelistReq(e.g.,listReq := &catalog.ListCatalogsRequest{}) that is never used after flag binding. Consider using_or removing if unneeded. - Consistency: The pattern matches the existing override style in the codebase, which is good.
What looks good
- Clean, consistent pattern across all 5 overrides
- Correct use of
tableview.RegisterConfigfor each command - Proper column selection for each resource type
No blocking issues.
Treat pipeline search input literally when users type SQL LIKE wildcards, and add package-level override tests so nested SDK field access is exercised before runtime.
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Previously, the TUI search callback overwrote req.Filter entirely with a name LIKE expression, discarding any filter the user passed via --filter. Now the name LIKE clause is combined with the existing filter using AND, so both constraints apply together.
Several list override functions declared a named request parameter (e.g. listReq) that was never used after flag binding. Replace these with blank identifiers to satisfy go vet and make intent clearer. The parameter is kept named in clusters, jobs, pipelines, and workspace overrides where it is actively used for flag binding or search closures.
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Wrap the user-provided filter in parentheses before appending the AND clause. Without this, a filter like 'a OR b' combined with a name search would parse as 'a OR (b AND name LIKE ...)' instead of the intended '(a OR b) AND name LIKE ...'. Add a test case with an OR filter to verify correct parenthesization.
Previously, RenderIterator and RunPaginated only returned the error from tea.Program.Run(), ignoring any fetch error stored in the model. An API error mid-stream would display an error screen in the TUI but the command would still exit 0. Now both functions inspect the final model via the new Err() accessor and return the fetch error if set. Also documents the destructive MaxWidth truncation behavior on ColumnDef and renderContent.
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
…ui-overrides-2 # Conflicts: # libs/cmdio/render.go
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Pipeline event messages can contain embedded newlines, carriage returns, and tabs that corrupt tab-delimited text output and TUI table rows. Add a `sanitize` template function to cmdio's renderFuncMap and use it in the text template. Also sanitize in the TUI Extract function. Increase MaxWidth from 60 to 200 so diagnostic payloads are not truncated destructively before the actionable part of the error. Co-authored-by: Isaac
Remove the PaginatedModel type alias (FinalModel interface suffices). Remove the duplicate TestPaginatedErrAccessor test that overlaps with TestPaginatedModelErr. Reduce the 5-line MaxWidth truncation comment to a single line. Co-authored-by: Isaac
shreyas-goenka
approved these changes
Mar 19, 2026
Contributor
shreyas-goenka
left a comment
shreyas-goenka
left a comment
There was a problem hiding this comment.
Stamping. I'm assuming the TUI UI is better.
|
|
||
| // sanitizeControlWhitespace replaces newlines and tabs with spaces to prevent | ||
| // corrupting tab-delimited text output. | ||
| func sanitizeControlWhitespace(s string) string { |
Contributor
There was a problem hiding this comment.
This function is also defined in cmd/workspace/pipelines/overrides.go. Consolidate?
Base automatically changed from
simonfaltum/list-tui-overrides
to
simonfaltum/list-tui-paginated
March 19, 2026 06:43
simonfaltum
changed the base branch from
simonfaltum/list-tui-paginated
to
main
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Suggested reviewersBased on git history of the changed files, these people are best suited to review:
Confidence: high Eligible reviewersBased on CODEOWNERS, these people or teams could also review: @andrewnester, @anton-107, @databricks/eng-apps-devex, @denik, @lennartkats-db, @shreyas-goenka Suggestions based on git history of 37 changed files (11 scored). See CODEOWNERS for path-specific ownership rules. |
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
changed the base branch from
main
to
simonfaltum/list-tui-paginated
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Why
PR #4731 added curated TUI table overrides for 15 list commands. This follow-up covers 5 additional commands that are among the most frequently used in the CLI, but were missing curated columns.
Changes
Before: these 5 commands used either generic text templates (secrets, cluster-policies) or raw JSON output (lakeview, pipeline events) with no curated TUI table columns.
Now: all 5 register curated TableConfig overrides so they show useful columns in the interactive TUI. Commands that had no text template override (lakeview list, pipelines list-pipeline-events) also get template annotations for the non-interactive fallback.
This PR stacks on #4731. It only adds per-command overrides, no engine changes.
Post-review fixes
\n,\r,\t) in pipeline event messages to prevent table row corruptionPaginatedModeltype alias, useFinalModelinterface insteadTestPaginatedErrAccessortestTest plan
go build ./...make checkspassesmake lintfullpasses (0 issues)secrets list-scopes,lakeview list,pipelines list-pipeline-events