feat(tui): support content reflow on terminal resize#43
Conversation
Introduce contentLine abstraction to decouple data from rendering, enabling automatic content reflow when terminal window is resized. - Add contentLine type with text and toolResultData variants - Change Model.lines from []string to []contentLine - Add recreateMDRenderer() to rebuild glamour renderer on resize - Fix renderSubagentBox() to use contentWidth() instead of m.width - Update all 70+ call sites to use textLine() wrapper
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 52 minutes and 21 seconds. ⌛ 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. 📝 WalkthroughWalkthroughThis PR refactors TUI content representation from raw strings to structured Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/tui/tui.go (1)
2654-2685:⚠️ Potential issue | 🟠 MajorReflow only covers tool results — assistant Markdown remains frozen at the width it was first rendered with.
flushText(Lines 2660–2667) renders streamed text throughm.mdRendererand stores the resulting string intom.linesviatextLine(rendered). OnWindowSizeMsg,recreateMDRendererrebuilds the renderer for future output, but every previously-flushed assistant message keeps its stale wrap width. The same applies to the assistant entries appended inSessionResumedMsg(Lines 1764–1771) and team assistant messages (Lines 2244–2251).The PR objective is "content reflows automatically when the terminal is resized," yet the most visible block of content in a normal session — the assistant's markdown — won't actually reflow. To fully achieve the goal, consider storing the raw markdown source (e.g., a third
contentLinevariantmarkdownData{source string}) and rendering it dynamically incontentLine.render(width), the same way tool results are handled.If reflowing markdown is intentionally out of scope for this PR, please call that out in the PR description / a TODO so it's clear which content is actually width-adaptive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/tui.go` around lines 2654 - 2685, flushText currently stores pre-rendered assistant Markdown into m.lines so it never reflows after recreateMDRenderer changes; update flushText (and the spots that append assistant entries in SessionResumedMsg and team assistant handling) to store the raw markdown instead of the rendered string by adding a new contentLine variant (e.g., markdownData{source string}) and implementing contentLine.render(width) to call m.mdRenderer.Render with the current width (or a renderer built by recreateMDRenderer) at draw time; ensure textLine(rendered) usage is replaced with the markdownData variant and that contentWidth/recreateMDRenderer are used when rendering so assistant Markdown reflows on WindowSizeMsg.
🧹 Nitpick comments (2)
internal/tui/tui.go (2)
1545-1582: Optional: skiprecreateMDRendererwhen width didn't change.
WindowSizeMsgfires for any size change (including height-only changes from things like the terminal tab bar appearing/disappearing). Glamour renderer creation parses styles each call, so guarding on width keeps idle resizes cheap.♻️ Suggested guard
- m.recreateMDRenderer() + if m.viewport.Width() != mainWidth || m.mdRenderer == nil { + m.recreateMDRenderer() + }(Or capture the previous content width before reassigning
m.widthand compare against the new one.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/tui.go` around lines 1545 - 1582, The WindowSizeMsg handler currently always calls m.recreateMDRenderer even for height-only changes; before assigning m.width (or immediately after computing mainWidth) capture the previous main/content width (e.g., prevMainWidth := previous m.width or compute prevMainWidth based on previous showSidebar) and only call m.recreateMDRenderer() when the newly computed mainWidth differs from prevMainWidth; this ensures recreateMDRenderer is skipped for height-only resizes while leaving the rest of the resize logic (textarea, viewport sizing, SetContent) intact.
230-234: Remove thecontentLine.containsmethod — it's unused and has incorrect logic for tool results.This method is not called anywhere in the codebase (all substring checks use
strings.Containsdirectly online.text). Additionally, fortoolResultContentLinewith an emptytextfield, it would always returnfalseeven if the rendered output contains the substring.♻️ Removal
-// contains reports whether the rendered content contains the given substring. -// For tool result lines, this checks the last-rendered text. -func (cl contentLine) contains(sub string) bool { - return strings.Contains(cl.text, sub) -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/tui.go` around lines 230 - 234, Remove the unused and incorrect method contentLine.contains: delete the contains receiver method on type contentLine so callers use strings.Contains(line.text, ...) directly; ensure there are no references to contentLine.contains (search for "contains(" and "contentLine.contains") and remove or update them if found, and be mindful of toolResultContentLine where rendered output may differ from the empty text field—rely on existing uses of strings.Contains(line.text, ...) or other rendering-aware checks instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/tui/tui.go`:
- Around line 2670-2685: The comment for contentWidth was left orphaned above
recreateMDRenderer; move the doc comment "// contentWidth returns the width
available for the main content area, accounting for the sidebar when visible."
so it directly precedes the contentWidth function definition (instead of sitting
above recreateMDRenderer), or delete that orphaned line; also ensure
recreateMDRenderer has an appropriate doc comment (e.g., about rebuilding the
glamour renderer) so comments correctly associate with the contentWidth and
recreateMDRenderer methods on type Model.
---
Outside diff comments:
In `@internal/tui/tui.go`:
- Around line 2654-2685: flushText currently stores pre-rendered assistant
Markdown into m.lines so it never reflows after recreateMDRenderer changes;
update flushText (and the spots that append assistant entries in
SessionResumedMsg and team assistant handling) to store the raw markdown instead
of the rendered string by adding a new contentLine variant (e.g.,
markdownData{source string}) and implementing contentLine.render(width) to call
m.mdRenderer.Render with the current width (or a renderer built by
recreateMDRenderer) at draw time; ensure textLine(rendered) usage is replaced
with the markdownData variant and that contentWidth/recreateMDRenderer are used
when rendering so assistant Markdown reflows on WindowSizeMsg.
---
Nitpick comments:
In `@internal/tui/tui.go`:
- Around line 1545-1582: The WindowSizeMsg handler currently always calls
m.recreateMDRenderer even for height-only changes; before assigning m.width (or
immediately after computing mainWidth) capture the previous main/content width
(e.g., prevMainWidth := previous m.width or compute prevMainWidth based on
previous showSidebar) and only call m.recreateMDRenderer() when the newly
computed mainWidth differs from prevMainWidth; this ensures recreateMDRenderer
is skipped for height-only resizes while leaving the rest of the resize logic
(textarea, viewport sizing, SetContent) intact.
- Around line 230-234: Remove the unused and incorrect method
contentLine.contains: delete the contains receiver method on type contentLine so
callers use strings.Contains(line.text, ...) directly; ensure there are no
references to contentLine.contains (search for "contains(" and
"contentLine.contains") and remove or update them if found, and be mindful of
toolResultContentLine where rendered output may differ from the empty text
field—rely on existing uses of strings.Contains(line.text, ...) or other
rendering-aware checks instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d89f5fe-8dc9-4917-81bb-fca75df71902
📒 Files selected for processing (6)
internal/tui/channel_panel.gointernal/tui/input_views.gointernal/tui/pickers.gointernal/tui/ssh_handlers.gointernal/tui/team_view.gointernal/tui/tui.go
The doc comment for contentWidth was placed above recreateMDRenderer, causing godoc to associate it with the wrong function.
1 participant
Summary
Introduce a
contentLineabstraction to decouple data from rendering, enabling automatic content reflow when the terminal window is resized.Previously, tool results and Markdown content were rendered at a fixed width at first paint. Resizing the terminal would leave content misaligned and broken.
Changes
contentLine(withtextandtoolResultDatavariants) andtoolResultDatastruct to hold raw tool result dataModel.linestype change:[]string→[]contentLine— content is now stored as structured data, rendered lazily viarender(width)recreateMDRenderer(): rebuilds the glamour Markdown renderer onWindowSizeMsgwith updatedWordWrapwidthrenderSubagentBox()now usesm.contentWidth()instead ofm.width, correctly accounting for sidebar widthtextLine()wrapper for string→contentLineconversionFiles Changed
internal/tui/tui.gocontentLinemethods, renderer recreation,Model.linestype changeinternal/tui/channel_panel.go[]contentLineinternal/tui/input_views.go[]contentLineinternal/tui/pickers.go[]contentLineinternal/tui/ssh_handlers.go[]contentLineinternal/tui/team_view.go[]contentLineTesting
make buildSummary by CodeRabbit