valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: properly wrap lines in the Rust TUI (#937)

Browse Source 
As discussed on
699ec5a87f (commitcomment-156776835),
to properly support scrolling long content in Ratatui for a sequence of
cells, we need to:

* take the `Vec<Line>` for each cell
* using the wrapping logic we want to use at render time, compute the
_effective line count_ using `Paragraph::line_count()` (see
`wrapped_line_count_for_cell()` in this PR)
* sum up the effective line count to compute the height of the area
being scrolled
* given a `scroll_position: usize`, index into the list of "effective
lines" and accumulate the appropriate `Vec<Line>` for the cells that
should be displayed
* take that `Vec<Line>` to create a `Paragraph` and use the same
line-wrapping policy that was used in `wrapped_line_count_for_cell()`
* display the resulting `Paragraph` and use the accounting to display a
scrollbar with the appropriate thumb size and offset without having to
render the `Vec<Line>` for the full history

With this change, lines wrap as I expect and everything appears to
redraw correctly as I resize my terminal!
This commit is contained in:
Michael Bolin
2025-05-14 16:51:41 -07:00
committed by GitHub
parent 34aa1991f1
commit 0b9ef93da5
1 changed files with 144 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

235
codex-rs/tui/src/conversation_history_widget.rs
View File

@@ -11,11 +11,21 @@ use ratatui::style::Style;
use ratatui::widgets::*;
use serde_json::Value as JsonValue;
use std::cell::Cell as StdCell;
use std::cell::Cell;
use std::collections::HashMap;
use std::path::PathBuf;
/// A single history entry plus its cached wrapped-line count.
struct Entry {
cell: HistoryCell,
line_count: Cell<usize>,
}
pub struct ConversationHistoryWidget {
history: Vec<HistoryCell>,
entries: Vec<Entry>,
/// The width (in terminal cells/columns) that [`Entry::line_count`] was
/// computed for. When the available width changes we recompute counts.
cached_width: StdCell<u16>,
scroll_position: usize,
/// Number of lines the last time render_ref() was called
num_rendered_lines: StdCell<usize>,
@@ -27,7 +37,8 @@ pub struct ConversationHistoryWidget {
impl ConversationHistoryWidget {
pub fn new() -> Self {
Self {
history: Vec::new(),
entries: Vec::new(),
cached_width: StdCell::new(0),
scroll_position: usize::MAX,
num_rendered_lines: StdCell::new(0),
last_viewport_height: StdCell::new(0),
@@ -73,7 +84,7 @@ impl ConversationHistoryWidget {
fn scroll_up(&mut self, num_lines: u32) {
// If a user is scrolling up from the "stick to bottom" mode, we need to
// map this to a specific scroll position so we can caluate the delta.
// map this to a specific scroll position so we can calculate the delta.
// This requires us to care about how tall the screen is.
if self.scroll_position == usize::MAX {
self.scroll_position = self
@@ -97,9 +108,7 @@ impl ConversationHistoryWidget {
// Compute the maximum explicit scroll offset that still shows a full
// viewport. This mirrors the calculation in `scroll_page_down()` and
// in the render path.
let max_scroll = num_rendered_lines
.saturating_sub(viewport_height)
.saturating_add(1);
let max_scroll = num_rendered_lines.saturating_sub(viewport_height);
let new_pos = self.scroll_position.saturating_add(num_lines as usize);
@@ -144,7 +153,7 @@ impl ConversationHistoryWidget {
// Calculate the maximum explicit scroll offset that is still within
// range. This matches the logic in `scroll_down()` and the render
// method.
let max_scroll = num_lines.saturating_sub(viewport_height).saturating_add(1);
let max_scroll = num_lines.saturating_sub(viewport_height);
// Attempt to move down by a full page.
let new_pos = self.scroll_position.saturating_add(viewport_height);
@@ -166,7 +175,7 @@ impl ConversationHistoryWidget {
/// Note `model` could differ from `config.model` if the agent decided to
/// use a different model than the one requested by the user.
pub fn add_session_info(&mut self, config: &Config, event: SessionConfiguredEvent) {
let is_first_event = self.history.is_empty();
let is_first_event = self.entries.is_empty();
self.add_to_history(HistoryCell::new_session_info(config, event, is_first_event));
}
@@ -216,12 +225,22 @@ impl ConversationHistoryWidget {
}
fn add_to_history(&mut self, cell: HistoryCell) {
self.history.push(cell);
let width = self.cached_width.get();
let count = if width > 0 {
wrapped_line_count_for_cell(&cell, width)
} else {
0
};
self.entries.push(Entry {
cell,
line_count: Cell::new(count),
});
}
/// Remove all history entries and reset scrolling.
pub fn clear(&mut self) {
self.history.clear();
self.entries.clear();
self.scroll_position = usize::MAX;
}
@@ -232,7 +251,9 @@ impl ConversationHistoryWidget {
stderr: String,
exit_code: i32,
) {
for cell in self.history.iter_mut() {
let width = self.cached_width.get();
for entry in self.entries.iter_mut() {
let cell = &mut entry.cell;
if let HistoryCell::ActiveExecCommand {
call_id: history_id,
command,
@@ -250,6 +271,13 @@ impl ConversationHistoryWidget {
duration: start.elapsed(),
},
);
// Update cached line count.
if width > 0 {
entry
.line_count
.set(wrapped_line_count_for_cell(cell, width));
}
break;
}
}
@@ -269,14 +297,15 @@ impl ConversationHistoryWidget {
.unwrap_or_else(|_| serde_json::Value::String("<serialization error>".into()))
});
for cell in self.history.iter_mut() {
let width = self.cached_width.get();
for entry in self.entries.iter_mut() {
if let HistoryCell::ActiveMcpToolCall {
call_id: history_id,
fq_tool_name,
invocation,
start,
..
} = cell
} = &entry.cell
{
if &call_id == history_id {
let completed = HistoryCell::new_completed_mcp_tool_call(
@@ -286,7 +315,14 @@ impl ConversationHistoryWidget {
success,
result_val,
);
*cell = completed;
entry.cell = completed;
if width > 0 {
entry
.line_count
.set(wrapped_line_count_for_cell(&entry.cell, width));
}
break;
}
}
@@ -311,105 +347,102 @@ impl WidgetRef for ConversationHistoryWidget {
.border_type(BorderType::Rounded)
.border_style(border_style);
// ------------------------------------------------------------------
// Build a *window* into the history instead of cloning the entire
// history into a brand‑new Vec every time we are asked to render.
//
// There can be an unbounded number of `Line` objects in the history,
// but the terminal will only ever display `height` of them at once.
// By materialising only the `height` lines that are scrolled into
// view we avoid the potentially expensive clone of the full
// conversation every frame.
// ------------------------------------------------------------------
// Compute the inner area that will be available for the list after
// the surrounding `Block` is drawn.
let inner = block.inner(area);
let viewport_height = inner.height as usize;
// Collect the lines that will actually be visible in the viewport
// while keeping track of the total number of lines so the scrollbar
// stays correct.
let num_lines: usize = self.history.iter().map(|c| c.lines().len()).sum();
// Cache (and if necessary recalculate) the wrapped line counts for
// every [`HistoryCell`] so that our scrolling math accounts for text
// wrapping.
let width = inner.width; // Width of the viewport in terminal cells.
if width == 0 {
return; // Nothing to draw – avoid division by zero.
}
let max_scroll = num_lines.saturating_sub(viewport_height) + 1;
// Recompute cache if the width changed.
let num_lines: usize = if self.cached_width.get() != width {
self.cached_width.set(width);
let mut num_lines: usize = 0;
for entry in &self.entries {
let count = wrapped_line_count_for_cell(&entry.cell, width);
num_lines += count;
entry.line_count.set(count);
}
num_lines
} else {
self.entries.iter().map(|e| e.line_count.get()).sum()
};
// Determine the scroll position. Note the existing value of
// `self.scroll_position` could exceed the maximum scroll offset if the
// user made the window wider since the last render.
let max_scroll = num_lines.saturating_sub(viewport_height);
let scroll_pos = if self.scroll_position == usize::MAX {
max_scroll
} else {
self.scroll_position.min(max_scroll)
};
let mut visible_lines: Vec<Line<'static>> = Vec::with_capacity(viewport_height);
// ------------------------------------------------------------------
// Build a *window* into the history so we only clone the `Line`s that
// may actually be visible in this frame. We still hand the slice off
// to a `Paragraph` with an additional scroll offset to avoid slicing
// inside a wrapped line (we don’t have per-subline granularity).
// ------------------------------------------------------------------
if self.scroll_position == usize::MAX {
// Stick‑to‑bottom mode: walk the history backwards and keep the
// most recent `height` lines. This touches at most `height`
// lines regardless of how large the conversation grows.
'outer_rev: for cell in self.history.iter().rev() {
for line in cell.lines().iter().rev() {
visible_lines.push(line.clone());
if visible_lines.len() == viewport_height {
break 'outer_rev;
}
}
// Find the first entry that intersects the current scroll position.
let mut cumulative = 0usize;
let mut first_idx = 0usize;
for (idx, entry) in self.entries.iter().enumerate() {
let next = cumulative + entry.line_count.get();
if next > scroll_pos {
first_idx = idx;
break;
}
visible_lines.reverse();
} else {
// Arbitrary scroll position. Skip lines until we reach the
// desired offset, then emit the next `height` lines.
let start_line = scroll_pos;
let mut current_index = 0usize;
'outer_fwd: for cell in &self.history {
for line in cell.lines() {
if current_index >= start_line {
visible_lines.push(line.clone());
if visible_lines.len() == viewport_height {
break 'outer_fwd;
}
}
current_index += 1;
}
cumulative = next;
}
let offset_into_first = scroll_pos - cumulative;
// Collect enough raw lines from `first_idx` onward to cover the
// viewport. We may fetch *slightly* more than necessary (whole cells)
// but never the entire history.
let mut collected_wrapped = 0usize;
let mut visible_lines: Vec<Line<'static>> = Vec::new();
for entry in &self.entries[first_idx..] {
visible_lines.extend(entry.cell.lines().iter().cloned());
collected_wrapped += entry.line_count.get();
if collected_wrapped >= offset_into_first + viewport_height {
break;
}
}
// We track the number of lines in the struct so can let the user take over from
// something other than usize::MAX when they start scrolling up. This could be
// removed once we have the vec<Lines> in self.
self.num_rendered_lines.set(num_lines);
self.last_viewport_height.set(viewport_height);
// Build the Paragraph with wrapping enabled so long lines are not
// clipped. Apply vertical scroll so that `offset_into_first` wrapped
// lines are hidden at the top.
let paragraph = Paragraph::new(visible_lines)
.block(block)
.wrap(wrap_cfg())
.scroll((offset_into_first as u16, 0));
// The widget takes care of drawing the `block` and computing its own
// inner area, so we render it over the full `area`.
// We *manually* sliced the set of `visible_lines` to fit within the
// viewport above, so there is no need to ask the `Paragraph` widget
// to apply an additional scroll offset. Doing so would cause the
// content to be shifted *twice* – once by our own logic and then a
// second time by the widget – which manifested as the entire block
// drifting off‑screen when the user attempted to scroll.
// Currently, we do not use the `wrap` method on the `Paragraph` widget
// because it messes up our scrolling math above that assumes each Line
// contributes one line of height to the widget. Admittedly, this is
// bad because users cannot see content that is clipped without
// resizing the terminal.
let paragraph = Paragraph::new(visible_lines).block(block);
paragraph.render(area, buf);
// Draw scrollbar if necessary.
let needs_scrollbar = num_lines > viewport_height;
if needs_scrollbar {
let mut scroll_state = ScrollbarState::default()
// TODO(ragona):
// I don't totally understand this, but it appears to work exactly as expected
// if we set the content length as the lines minus the height. Maybe I was supposed
// to use viewport_content_length or something, but this works and I'm backing away.
// The Scrollbar widget expects the *content* height minus the
// viewport height, mirroring the calculation used previously.
.content_length(num_lines.saturating_sub(viewport_height))
.position(scroll_pos);
// Choose a thumb colour that stands out only when this pane has focus so that the
// Choose a thumb color that stands out only when this pane has focus so that the
// user’s attention is naturally drawn to the active viewport. When unfocused we show
// a low‑contrast thumb so the scrollbar fades into the background without becoming
// invisible.
let thumb_style = if self.has_input_focus {
Style::reset().fg(Color::LightYellow)
} else {
@@ -418,25 +451,25 @@ impl WidgetRef for ConversationHistoryWidget {
StatefulWidget::render(
// By default the Scrollbar widget inherits the style that was already present
// in the underlying buffer cells. That means if a coloured line (for example a
// in the underlying buffer cells. That means if a colored line (for example a
// background task notification that we render in blue) happens to be underneath
// the scrollbar, the track and thumb adopt that colour and the scrollbar appears
// to “change colour”. Explicitly setting the *track* and *thumb* styles ensures
// the scrollbar, the track and thumb adopt that color and the scrollbar appears
// to "change color." Explicitly setting the *track* and *thumb* styles ensures
// we always draw the scrollbar with the same palette regardless of what content
// is behind it.
//
// N.B. Only the *foreground* colour matters here because the scrollbar symbols
// N.B. Only the *foreground* color matters here because the scrollbar symbols
// themselves are filled‐in block glyphs that completely overwrite the prior
// character cells. We therefore leave the background at its default value so it
// character cells. We therefore leave the background at its default value so it
// blends nicely with the surrounding `Block`.
Scrollbar::new(ScrollbarOrientation::VerticalRight)
.begin_symbol(Some("↑"))
.end_symbol(Some("↓"))
.begin_style(Style::reset().fg(Color::DarkGray))
.end_style(Style::reset().fg(Color::DarkGray))
// A solid thumb so that we can colour it distinctly from the track.
// A solid thumb so that we can color it distinctly from the track.
.thumb_symbol("█")
// Apply the dynamic thumb colour computed above. We still start from
// Apply the dynamic thumb color computed above. We still start from
// Style::reset() to clear any inherited modifiers.
.thumb_style(thumb_style)
// Thin vertical line for the track.
@@ -447,5 +480,25 @@ impl WidgetRef for ConversationHistoryWidget {
&mut scroll_state,
);
}
// Update auxiliary stats that the scroll handlers rely on.
self.num_rendered_lines.set(num_lines);
self.last_viewport_height.set(viewport_height);
}
}
/// Common [`Wrap`] configuration used for both measurement and rendering so
/// they stay in sync.
#[inline]
const fn wrap_cfg() -> ratatui::widgets::Wrap {
ratatui::widgets::Wrap { trim: false }
}
/// Returns the wrapped line count for `cell` at the given `width` using the
/// same wrapping rules that `ConversationHistoryWidget` uses during
/// rendering.
fn wrapped_line_count_for_cell(cell: &HistoryCell, width: u16) -> usize {
Paragraph::new(cell.lines().clone())
.wrap(wrap_cfg())
.line_count(width)
}