-
-
Notifications
You must be signed in to change notification settings - Fork 320
fix: implement responsive table layout for narrow terminals #83
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
Conversation
Resolves #81 by adding proper responsive table support that prevents table layout disruption in narrow terminal widths. Changes: - Add ResponsiveTable utility class using cli-table3 built-in wordWrap feature - Implement dynamic column width calculation based on terminal width - Add intelligent column width allocation (wider for text, narrower for numbers) - Replace cli-table3 usage in daily, monthly, and session commands - Update pushBreakdownRows to work with new table interface - Maintain existing visual styling and user experience Features: - Automatic text wrapping at word boundaries for long content - Proportional column width distribution with minimum width guarantees - Special handling for Models column which tends to have longer text - Responsive behavior that adapts to any terminal width Technical details: - Uses cli-table3 wordWrap: true and colWidths options - Calculates available width after accounting for table borders/separators - Applies different width multipliers based on column content type - Preserves all existing table styling (colors, alignment, etc.)
|
Warning Rate limit exceeded@ryoppippi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
WalkthroughThe code replaces the usage of the external Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI Command (daily/monthly/session)
participant ResponsiveTable
participant Terminal
User->>CLI Command (daily/monthly/session): Run command
CLI Command (daily/monthly/session)->>ResponsiveTable: Create table with headers and options
CLI Command (daily/monthly/session)->>ResponsiveTable: Add rows via push()
CLI Command (daily/monthly/session)->>ResponsiveTable: toString()
ResponsiveTable->>Terminal: Compute column widths and render table string
CLI Command (daily/monthly/session)->>User: Display formatted table output
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @ryoppippi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the user experience for CLI commands that display tabular data. By introducing a new ResponsiveTable utility, I've enabled dynamic and intelligent adjustment of table column widths based on the terminal's available space. This change ensures that tables remain well-formatted and readable, even when viewed in very narrow terminal windows, resolving a long-standing display issue.
Highlights
- New Utility Class: I've introduced a new
ResponsiveTableutility class insrc/utils.table.tsthat wrapscli-table3to provide dynamic column width calculation and word wrapping. - Responsive Table Layout: This PR implements a responsive table layout that automatically adjusts column widths based on the terminal's width, ensuring readability and preventing table disruption in narrow terminals.
- Command Integration: The
daily,monthly, andsessioncommands have been updated to utilize this newResponsiveTableclass, replacing their directcli-table3usage. - Intelligent Column Sizing: The
ResponsiveTableintelligently allocates column widths, giving more space to text-heavy columns (like 'Models') and less to numeric columns, while maintaining minimum widths for readability.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a ResponsiveTable utility class to address table layout issues in narrow terminal widths. The command files (daily.ts, monthly.ts, session.ts) have been updated to use this new class, and type definitions in src/utils.internal.ts have been refined. The review focuses on enhancing the maintainability and reusability of the ResponsiveTable class.
| if (align === 'right') { | ||
| // Numeric columns - smaller width | ||
| return Math.max(Math.floor(baseWidth * 0.7), 8); | ||
| } | ||
| else if (index === 0) { | ||
| // First column (Date/Session) - medium width | ||
| return Math.max(Math.floor(baseWidth * 0.8), 10); | ||
| } | ||
| else if (index === 1) { | ||
| // Models column - can be longer, needs more space | ||
| return Math.max(Math.floor(baseWidth * 1.2), 12); | ||
| } | ||
| else { | ||
| // Other columns - standard width | ||
| return Math.max(baseWidth, 8); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/utils.table.ts
Outdated
| else if (index === 0) { | ||
| // First column (Date/Session) - medium width | ||
| return Math.max(Math.floor(baseWidth * 0.8), 10); | ||
| } | ||
| else if (index === 1) { | ||
| // Models column - can be longer, needs more space | ||
| return Math.max(Math.floor(baseWidth * 1.2), 12); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export function pushBreakdownRows( | ||
| table: any[], | ||
| table: { push: (row: (string | number)[]) => void }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/commands/daily.ts (1)
76-100: Same separator duplication issue as in monthly.tsSee previous comment – factoring this out once will keep the three commands aligned.
🧹 Nitpick comments (7)
src/utils.table.ts (2)
38-58: Column-width post-adjustment can still overflow the terminalAfter
Math.max()enforces minimums, the sum ofcolumnWidthscan exceedavailableWidth, especially on very narrow screens.cli-table3will silently truncate, but the resulting layout may wrap every cell and defeat the responsiveness goal.Consider a second pass to proportionally shrink columns when
columnWidths.reduce(...) > availableWidth.let widths = initialCalculation(); const overshoot = widths.reduce((s, w) => s + w, 0) - availableWidth; if (overshoot > 0) { const shrinkFactor = (availableWidth - numColumns * 3) / (widths.reduce((s, w) => s + w, 0)); widths = widths.map(w => Math.max(8, Math.floor(w * shrinkFactor))); }Makes the algorithm resilient across all terminal sizes.
61-68: Silenceno-unsafe-*ESLint warnings with explicit typing
cli-table3’s typings are generic. Adding a type assertion keeps the codebase in strict-mode compliance:-const table = new Table({ +const table: Table.Table = new Table({or import the
Tabletype separately.Purely cosmetic but removes the red squiggles.
src/commands/monthly.ts (1)
87-111: Derive the separator row programmaticallyWhenever the header array changes, the manual
'─'.repeat(...)list must be updated in three places (daily, monthly, session). A helper keeps them in sync:-// Add separator -table.push([ - '─'.repeat(12), // … - … -]); +table.push( + table.head.map(() => '─'.repeat(12)) +);Reduces duplication and prevents future off-by-one mistakes.
src/commands/session.ts (1)
77-103: Separator maintenance & magic numbersThe hard-coded repeats (
12,10,maxSessionLength) are easy to desynchronise from the header. Consider the helper suggested for the other commands, passing per-column repeat sizes where needed (e.g.maxSessionLength).src/utils.internal.ts (3)
32-33: Invalid JSDoc@param table.pushentry
@paramtags must reference actual parameter names. Document the shape in the main param description instead:- * @param table.push - Method to add rows to the table + * @param table - Any object exposing a `push(row)` method (e.g. ResponsiveTable)Prevents JSDoc lint errors and improves generated docs.
38-39: Tighten thetabletype to a named interfaceInstead of an inline anonymous type, expose a reusable interface alongside
ResponsiveTable:export interface PushableTable { push(row: (string | number)[]): void; }Then:
-function pushBreakdownRows( - table: { push: (row: (string | number)[]) => void }, +function pushBreakdownRows( + table: PushableTable,Makes intent explicit and allows IDE auto-completion.
51-52: Pre-allocate row array for minor perf/readability gainInstead of mutating with multiple
push()calls:-const row: (string | number)[] = [` └─ ${formatModelName(... )}`]; +const row: (string | number)[] = new Array(1 + extraColumns); +row[0] = ` └─ ${formatModelName(breakdown.modelName)}`;Minor, entirely optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/commands/daily.ts(2 hunks)src/commands/monthly.ts(2 hunks)src/commands/session.ts(2 hunks)src/utils.internal.ts(2 hunks)src/utils.table.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/commands/monthly.ts (1)
src/utils.table.ts (1)
ResponsiveTable(13-77)
src/commands/daily.ts (1)
src/utils.table.ts (1)
ResponsiveTable(13-77)
src/commands/session.ts (1)
src/utils.table.ts (1)
ResponsiveTable(13-77)
🪛 ESLint
src/utils.table.ts
[error] 30-30: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 30-30: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 30-30: Unsafe member access .stdout on an error typed value.
(ts/no-unsafe-member-access)
[error] 61-68: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 61-68: Unsafe construction of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 72-72: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 72-72: Unsafe member access .push on an error typed value.
(ts/no-unsafe-member-access)
[error] 75-75: Unsafe return of a value of type error.
(ts/no-unsafe-return)
[error] 75-75: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 75-75: Unsafe member access .toString on an error typed value.
(ts/no-unsafe-member-access)
| const terminalWidth = process.stdout.columns || 120; | ||
|
|
||
| // Calculate reasonable column widths for responsive layout | ||
| const numColumns = this.head.length; | ||
| const tableOverhead = 3 * numColumns + 1; // borders and separators | ||
| const availableWidth = terminalWidth - tableOverhead; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard against non-TTY output and very narrow terminals
process.stdout.columns is undefined when stdout is redirected (e.g. piping to a file).
If the code runs in such a context, availableWidth becomes 120 - tableOverhead, which can be negative for wide tables and will propagate bad values to baseWidth.
-const terminalWidth = process.stdout.columns || 120;
+const terminalWidth =
+ typeof process.stdout.columns === 'number' && process.stdout.columns > 0
+ ? process.stdout.columns
+ : 120;
+
+// Bail out early if the table is wider than the terminal fallback.
+if (terminalWidth <= tableOverhead + 8 /* minimal column allowance */) {
+ // Fallback: rely on cli-table3’s internal wrapping instead of custom widths.
+ return new Table({
+ head: this.head,
+ style: this.style,
+ colAligns: this.colAligns,
+ wordWrap: true,
+ wrapOnWordBoundary: true,
+ }).toString();
+}Prevents negative widths and keeps output readable in non-interactive scenarios.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 30-30: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 30-30: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 30-30: Unsafe member access .stdout on an error typed value.
(ts/no-unsafe-member-access)
🤖 Prompt for AI Agents
In src/utils.table.ts around lines 30 to 36, the code assumes
process.stdout.columns is always defined, but it can be undefined when output is
redirected, causing availableWidth to be negative. Fix this by first checking if
process.stdout.isTTY is true and process.stdout.columns is defined; if not, set
terminalWidth to a safe default like 120. Then calculate availableWidth ensuring
it never becomes negative by using Math.max with zero. This prevents negative
widths and maintains readable output in non-interactive environments.
- Fix nullable string value handling in utils.table.ts by using nullish coalescing operator (??) - Add explicit undefined check for dateFormatter to satisfy strict-boolean-expressions rule - Ensure proper type safety for conditional expressions
fix: implement responsive table layout for narrow terminals
Summary
Key Features
Test Plan