fix: clamp ColumnsPrinter table width to terminal and truncate overflowing cells - #65
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| result: List[str] = [] | ||
| width = 0 | ||
| ph_width = _visible_length(placeholder) | ||
| budget = target - ph_width if target >= ph_width else target | ||
|
|
||
| for is_ansi, token, token_width in _iter_display_atoms(text): | ||
| if is_ansi: | ||
| result.append(token) | ||
| continue | ||
| if width + token_width > budget: | ||
| break | ||
| result.append(token) | ||
| width += token_width | ||
|
|
||
| if target >= ph_width: | ||
| result.append(placeholder) | ||
|
|
||
| # Stop any active ANSI style from bleeding into trailing padding. | ||
| if "\x1b" in text: | ||
| result.append("\033[0m") | ||
|
|
||
| return "".join(result) |
There was a problem hiding this comment.
🟡 Truncated cells containing wide characters come out one column too narrow, misaligning the table
When a cell value is shortened to fit its column (_fit_visible at logbar/columns.py:40-61) and the text contains a full-width character (e.g. CJK or emoji) right at the truncation boundary, the truncated text plus the ... marker ends up one column short of the intended width, so that row is drawn narrower than its border and the table stops lining up.
Impact: Rows that hold wide (double-width) characters render slightly misaligned against the surrounding borders, breaking the clean grid the fix is meant to guarantee.
Why the width falls short during truncation
In the truncation path, budget = target - ph_width and the loop stops adding clusters once the next cluster would exceed budget (logbar/columns.py:49). A double-width cluster (width 2) can leave the accumulated width at budget - 1 because adding it would overflow. The code then appends the placeholder (ph_width) giving a total visible width of target - 1, but it never pads the shortfall. Compare with _pad_visible/the current < target branch which always reaches exactly target. The border segment in _emit_border (logbar/columns.py:844-850) is sized from self._slot_widths[idx] (the full target), so the content cell is one column narrower than its border. Single-width text always fills budget exactly, so this only manifests with wide clusters.
| result: List[str] = [] | |
| width = 0 | |
| ph_width = _visible_length(placeholder) | |
| budget = target - ph_width if target >= ph_width else target | |
| for is_ansi, token, token_width in _iter_display_atoms(text): | |
| if is_ansi: | |
| result.append(token) | |
| continue | |
| if width + token_width > budget: | |
| break | |
| result.append(token) | |
| width += token_width | |
| if target >= ph_width: | |
| result.append(placeholder) | |
| # Stop any active ANSI style from bleeding into trailing padding. | |
| if "\x1b" in text: | |
| result.append("\033[0m") | |
| return "".join(result) | |
| result: List[str] = [] | |
| width = 0 | |
| ph_width = _visible_length(placeholder) | |
| budget = target - ph_width if target >= ph_width else target | |
| for is_ansi, token, token_width in _iter_display_atoms(text): | |
| if is_ansi: | |
| result.append(token) | |
| continue | |
| if width + token_width > budget: | |
| break | |
| result.append(token) | |
| width += token_width | |
| if target >= ph_width: | |
| result.append(placeholder) | |
| width += ph_width | |
| # Stop any active ANSI style from bleeding into trailing padding. | |
| if "\x1b" in text: | |
| result.append("\033[0m") | |
| if width < target: | |
| result.append(" " * (target - width)) | |
| return "".join(result) |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed — _fit_visible now pads after the placeholder so the returned string always matches the target visible width, even when a double-width character falls on the truncation boundary. Added test_columns_fit_visible_exact_width_for_wide_chars to cover CJK cells.
| desired = max(target_cells, minimal_cells) | ||
| content_budget = max(0, available_total - separator_count) | ||
| return min(desired, content_budget) |
There was a problem hiding this comment.
🔍 _get_target_width now clamps below minimal header width
The new return min(desired, content_budget) at logbar/columns.py:455-457 can return a content width smaller than minimal_cells (the width needed to fit header labels) when the terminal is narrower than the headers. Previously the function guaranteed at least minimal_cells. This is intentional per the PR (headers are now truncated with ... rather than overflowing), but it is a genuine behavioral change: any caller/test relying on the minimal-width guarantee will now see truncation instead. Worth confirming no downstream code assumes headers always fit.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed as intentional. The whole point of this PR is to prevent headers from forcing the table wider than the terminal, so headers are now truncated with ... when needed. Existing tests still pass and the new test_columns_clamp_wide_rows_to_terminal_width explicitly checks that the rendered table never exceeds the terminal width even with many long headers.
Summary
ColumnsPrintercould grow slot widths from long cell values without an upper bound, so bordered rows and separators overflowed the terminal. This fix caps the rendered table width to the available terminal budget and truncates cell text with a...placeholder while preserving ANSI codes.Changes
_fit_visible()inlogbar/columns.pyto pad or truncate text to an exact visible width usingiter_display_atoms()fromlogbar/drawing._row_width_budget()and_clamp_total_width()to ensure the full bordered row (+---+...) never exceedsterminal_columns - (level_max_length + 2).ColumnsPrinternow tracks per-slot minimum widths (_slot_min_widths) and fixed-width slots (_fixed_slots) so explicit column widths and header labels are honored during clamping._apply_initial_widths,_apply_header_widths, and_update_slot_widthsnow call_clamp_total_width()after resizing slots._render_headerand_render_rowuse_fit_visible()instead of_pad_visible()so long cell values are truncated instead of overflowing.test_columns_clamp_wide_rows_to_terminal_widthtotests/test_columns.pyusing a 17-column GPTQ-style table mocked to an 80-column terminal; it asserts no rendered line exceeds the terminal width.Example
Before, a wide 17-column row on an 80-column terminal produced a 326-character line. After:
All rendered rows (including borders and the log prefix) now fit within the terminal.
Test results
Link to Devin session: https://app.devin.ai/sessions/45e5067b04374a8381566d710d0311da
Requested by: @Qubitium