feat: [#849] Add table support to the console context#1380
feat: [#849] Add table support to the console context#1380krishankumar01 merged 9 commits intomasterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1380 +/- ##
==========================================
- Coverage 68.80% 68.79% -0.02%
==========================================
Files 286 287 +1
Lines 18040 18118 +78
==========================================
+ Hits 12413 12464 +51
- Misses 5101 5116 +15
- Partials 526 538 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request adds a comprehensive table rendering feature to the console context, introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CliContext
participant TableRenderer as lipgloss/table
participant Output
Client->>CliContext: Table(headers, rows, options...)
CliContext->>CliContext: Merge user options with defaults
CliContext->>TableRenderer: Create table with headers and rows
CliContext->>TableRenderer: Apply border configuration
CliContext->>TableRenderer: Apply styling (BorderStyle, StyleFunc)
CliContext->>TableRenderer: Set dimensions (Width, Height)
TableRenderer->>TableRenderer: Render formatted table
CliContext->>Output: Write rendered table via Line()
Output->>Client: Display formatted table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
contracts/console/command.go (1)
7-8: Public contract leaks lipgloss types, coupling consumers to a specific rendering library.
TableOptionexposeslipgloss.Border,lipgloss.Style,table.StyleFunc, andmap[int]lipgloss.Styledirectly. Sincecontracts/console/command.gois the public interface that downstream consumers (and mocks) depend on, this forces every consumer to importlipglossandlipgloss/table, even if they never render a table.If the rendering library is ever swapped, this becomes a breaking change to the contract. Consider wrapping these types behind framework-owned abstractions or moving the lipgloss-specific types to the implementation layer. That said, this is a design trade-off — the direct exposure does provide maximum flexibility for users wanting to style tables.
Also applies to: 256-286
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/console/command.go` around lines 7 - 8, The public contract currently leaks lipgloss types via TableOption (exposing lipgloss.Border, lipgloss.Style, table.StyleFunc and map[int]lipgloss.Style) — remove direct references to lipgloss/table from contracts/console/command.go by introducing framework-level abstraction types (e.g., ConsoleBorder, ConsoleStyle, ConsoleTableStyleFunc or simple interfaces) and use those in the TableOption API and any other exposed signatures (including the region noted at lines ~256-286); move the actual lipgloss/table imports and conversions into the implementation package that builds renderers so consumers and mocks depend only on the new lightweight abstractions rather than lipgloss types.console/cli_context.go (1)
23-54: Shared*boolpointers betweenDefaultTableOptionand local copies.
opt := DefaultTableOption(Line 597) performs a shallow struct copy, so the*boolfields (e.g.,BorderTop) in bothoptandDefaultTableOptionpoint to the same underlyingbool. Currently safe because the code only reassigns pointers (never mutates*opt.BorderTop), but a future refactor could accidentally corrupt the global default.Consider either documenting this invariant or doing a deep copy of the pointer fields when cloning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/cli_context.go` around lines 23 - 54, DefaultTableOption currently holds several *bool pointer fields (BorderTop, BorderBottom, BorderLeft, BorderRight, BorderHeader, BorderColumn, BorderRow) created with convert.Pointer(...) which means shallow copies like opt := DefaultTableOption will share underlying bools; add a proper deep-clone so callers don't accidentally share mutable pointers: implement a CloneTableOption (or a Clone method on console.TableOption) that copies the struct and allocates new bools for each pointer field (copying their values) and update callers that do opt := DefaultTableOption to call the clone helper (e.g., opt := DefaultTableOption.Clone()) so each copy has independent *bool instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@console/cli_context.go`:
- Around line 594-675: The Table method in CliContext ignores
TableOption.ColumnStyles so per-column styles are never applied; update the
option-merging block in CliContext.Table to copy userOpt.ColumnStyles into
opt.ColumnStyles, and then replace or wrap the final StyleFunc (opt.StyleFunc /
DefaultTableStyleFunc) with a wrapper that consults opt.ColumnStyles for the
current column and falls back to the base StyleFunc (i.e., create a new
StyleFunc that takes (col, row, cell) -> if opt.ColumnStyles[col] != nil use
that style(cell) else call base StyleFunc), and set t.StyleFunc to this wrapped
function before calling r.Line(t.Render()). Ensure you reference
TableOption.ColumnStyles, CliContext.Table, opt.StyleFunc,
DefaultTableStyleFunc, and t.StyleFunc when making the change.
---
Nitpick comments:
In `@console/cli_context.go`:
- Around line 23-54: DefaultTableOption currently holds several *bool pointer
fields (BorderTop, BorderBottom, BorderLeft, BorderRight, BorderHeader,
BorderColumn, BorderRow) created with convert.Pointer(...) which means shallow
copies like opt := DefaultTableOption will share underlying bools; add a proper
deep-clone so callers don't accidentally share mutable pointers: implement a
CloneTableOption (or a Clone method on console.TableOption) that copies the
struct and allocates new bools for each pointer field (copying their values) and
update callers that do opt := DefaultTableOption to call the clone helper (e.g.,
opt := DefaultTableOption.Clone()) so each copy has independent *bool instances.
In `@contracts/console/command.go`:
- Around line 7-8: The public contract currently leaks lipgloss types via
TableOption (exposing lipgloss.Border, lipgloss.Style, table.StyleFunc and
map[int]lipgloss.Style) — remove direct references to lipgloss/table from
contracts/console/command.go by introducing framework-level abstraction types
(e.g., ConsoleBorder, ConsoleStyle, ConsoleTableStyleFunc or simple interfaces)
and use those in the TableOption API and any other exposed signatures (including
the region noted at lines ~256-286); move the actual lipgloss/table imports and
conversions into the implementation package that builds renderers so consumers
and mocks depend only on the new lightweight abstractions rather than lipgloss
types.
console/cli_context.go
Outdated
| var ( | ||
| BrandColor = lipgloss.CompleteColor{TrueColor: "#3D8C8D", ANSI256: "30", ANSI: "6"} | ||
| MutedColor = lipgloss.CompleteColor{TrueColor: "#4a4a4a", ANSI256: "240", ANSI: "8"} | ||
|
|
||
| DefaultTableHeaderColor = BrandColor | ||
| DefaultTableBorderColor = MutedColor | ||
|
|
||
| DefaultTableHeaderStyle = lipgloss.NewStyle().Foreground(DefaultTableHeaderColor).Bold(true).Padding(0, 1) | ||
| DefaultTableCellStyle = lipgloss.NewStyle().Padding(0, 1) | ||
|
|
||
| DefaultTableStyleFunc = func(row, col int) lipgloss.Style { | ||
| if row == table.HeaderRow { | ||
| return DefaultTableHeaderStyle | ||
| } | ||
| return DefaultTableCellStyle | ||
| } | ||
|
|
||
| DefaultTableOption = console.TableOption{ | ||
| Border: lipgloss.RoundedBorder(), | ||
|
|
||
| BorderStyle: lipgloss.NewStyle().Foreground(DefaultTableBorderColor), | ||
|
|
||
| StyleFunc: DefaultTableStyleFunc, | ||
|
|
||
| BorderTop: convert.Pointer(true), | ||
| BorderBottom: convert.Pointer(true), | ||
| BorderLeft: convert.Pointer(true), | ||
| BorderRight: convert.Pointer(true), | ||
| BorderHeader: convert.Pointer(true), | ||
| BorderColumn: convert.Pointer(true), | ||
| BorderRow: convert.Pointer(false), | ||
| } | ||
| ) |
There was a problem hiding this comment.
Is it better to move these code to an independent file?
| } | ||
|
|
||
| err := input.Value(&answer).Run() | ||
| err := input.Value(&answer).WithTheme(GlobalHuhTheme).Run() |
There was a problem hiding this comment.
Could you add a screenshot for the theme?
2 participants
📑 Description
Closes goravel/goravel#849
This PR adds a Table method to render data in a tabular format. The function accepts headers and rows, along with an optional third parameter,
console.TableOption, to configure the table style.Example
TableOptionScreenShot
✅ Checks
Summary by CodeRabbit
Release Notes
New Features
Tests