Skip to content

fix(terminal): use max line width for multiline table cells#1192

Open
zcutlip wants to merge 1 commit into
ryoppippi:mainfrom
zcutlip:fix/table-model-column-width
Open

fix(terminal): use max line width for multiline table cells#1192
zcutlip wants to merge 1 commit into
ryoppippi:mainfrom
zcutlip:fix/table-model-column-width

Conversation

@zcutlip
Copy link
Copy Markdown
Contributor

@zcutlip zcutlip commented May 30, 2026

Addresses #1168

  • The Models column used visible_width_sum (sum of all lines) instead of
    visible_width_max_line (widest single line)
  • This caused columns with multiline cells to be far wider than necessary
  • Remove the special-cased column-index-1 logic and use uniform
    visible_width_max_line for all columns
  • Remove the now-unused visible_width_sum function from width.rs
  • Add regression test that asserts Models column width is based on widest line, not sum
  • Use a multiline cell with 5 models to amplify the difference between correct
    and incorrect behavior
  • Update snapshot to reflect narrower Models column

Summary by cubic

Fixes table column sizing for multiline cells by using the widest line instead of the sum of line widths. This prevents overly wide columns (notably “Models”) and improves readability.

  • Bug Fixes
    • Use visible_width_max_line for headers and cells in all columns.
    • Remove the special-case logic for column index 1.
    • Delete unused visible_width_sum from width.rs.
    • Add a regression test and update the snapshot to reflect the narrower “Models” column.

Written for commit 4e4b28b. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved table column width calculation for cells with multiline content. Columns now correctly size based on the widest single line rather than summing all lines, preventing unnecessarily large column widths and improving overall table layout.
  • Tests

    • Added test coverage for multiline cell column width calculation.

Review Change Stack

- The Models column used visible_width_sum (sum of all lines)
  instead of visible_width_max_line (widest single line)
- This caused columns with multiline cells to be far wider
  than necessary
- Remove the special-cased column-index-1 logic and use uniform
  visible_width_max_line for all columns
- Remove the now-unused visible_width_sum function from width.rs
- Update snapshot to reflect narrower Models column
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2834acea-c877-40a0-a3e9-ff65f95c540c

📥 Commits

Reviewing files that changed from the base of the PR and between 9d90e1b and 4e4b28b.

⛔ Files ignored due to path filters (1)
  • rust/crates/ccusage-terminal/src/snapshots/ccusage_terminal__table__tests__snapshots_full_table_with_multiline_cells_and_separators.snap is excluded by !**/*.snap
📒 Files selected for processing (2)
  • rust/crates/ccusage-terminal/src/table.rs
  • rust/crates/ccusage-terminal/src/width.rs
💤 Files with no reviewable changes (1)
  • rust/crates/ccusage-terminal/src/width.rs

📝 Walkthrough

Walkthrough

This PR fixes column width calculation in SimpleTable to prevent multiline cells from inflating column sizing. The width calculation is changed from summing visible widths across all lines to using only the widest line, the unused visible_width_sum helper is removed, and a unit test verifies the corrected behavior.

Changes

Column width calculation for multiline cells

Layer / File(s) Summary
Column width logic and imports
rust/crates/ccusage-terminal/src/table.rs
Import statements are updated to remove visible_width_sum. The column_widths calculation is modified so both header and cell widths use visible_width_max_line instead, ensuring multiline content is sized based on the widest single line rather than a sum of line widths.
Helper function cleanup
rust/crates/ccusage-terminal/src/width.rs
The visible_width_sum function is removed from the width module, as the updated column width logic no longer requires it.
Test verification
rust/crates/ccusage-terminal/src/table.rs
A new snapshot-style unit test verifies that a multiline "Models" cell drives column width based on the widest line width plus minimal padding, confirming that widths no longer scale with the sum of visible widths across all lines.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

Possibly related PRs

  • ryoppippi/ccusage#1146: Modifies SimpleTable sizing behavior by setting terminal width during construction, connected to the main PR's table-width logic.
  • ryoppippi/ccusage#1127: Earlier modular refactor that introduced the width helpers (visible_width_sum and visible_width_max_line) that this PR revises.

Suggested reviewers

  • ryoppippi

Poem

🐰 A column grew wide with each wrapped line,
Until max-width stepped in so fine!
Sum-widths are gone, the Models cell shrinks,
Tests prove the fix—faster than you think! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing column width calculation for multiline table cells to use the maximum line width instead of summing widths.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant