From adb947e09dcdbc194d8a48b859fb620147c7d54e Mon Sep 17 00:00:00 2001 From: PTFOPlayer Date: Sat, 13 Jun 2026 18:41:01 +0200 Subject: [PATCH 1/5] fix(tui): proper Unicode width handling and combining mark support - Fix .max(1) bug that made zero-width combining mark handling dead code in screen.rs (write_str, write_str_wrapped, write_str_wrapped_clipped, write_str_wrapped_skip_clipped), conversation.rs (line_height_for_text), and input_bar.rs (input_rows). Combining marks were treated as width-1 instead of being merged into the previous cell. - Remove unused unicode-segmentation dependency from Cargo.toml - Clean up dead code: unused has_newline variable and empty if block in input_bar.rs - Dynamic input bar height based on content, resizable sidebar width, Unicode-aware text wrapping and truncation across all TUI widgets --- Cargo.lock | 5 +- tinyharness-ui/Cargo.toml | 3 +- tinyharness-ui/src/tui/app.rs | 25 +- tinyharness-ui/src/tui/screen.rs | 78 +++- tinyharness-ui/src/tui/terminal.rs | 4 - tinyharness-ui/src/tui/widget.rs | 22 + .../src/tui/widgets/conversation.rs | 141 +++--- tinyharness-ui/src/tui/widgets/input_bar.rs | 437 +++++++++--------- tinyharness-ui/src/tui/widgets/sidebar.rs | 91 ++-- tinyharness-ui/src/tui/widgets/tool_output.rs | 18 +- 10 files changed, 467 insertions(+), 357 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d64763d..564cf91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1839,9 +1839,9 @@ checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" [[package]] name = "signal-hook" -version = "0.3.18" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d881a16cf4426aa584979d30bd82cb33429027e42122b169753d6ef1085ed6e2" +checksum = "b2a0c28ca5908dbdbcd52e6fdaa00358ab88637f8ab33e1f188dd510eb44b53d" dependencies = [ "libc", "signal-hook-registry", @@ -2041,6 +2041,7 @@ dependencies = [ "serde_json", "signal-hook", "tinyharness-lib", + "unicode-width", ] [[package]] diff --git a/tinyharness-ui/Cargo.toml b/tinyharness-ui/Cargo.toml index 3e616e3..9bb053f 100644 --- a/tinyharness-ui/Cargo.toml +++ b/tinyharness-ui/Cargo.toml @@ -11,7 +11,8 @@ rustyline = { version = "18.0.0", features = ["derive"] } serde_json = "1.0.149" regex = "1.11.1" libc = "0.2" -signal-hook = "0.3" +signal-hook = "0.4" +unicode-width = "0.2" [features] default = [] diff --git a/tinyharness-ui/src/tui/app.rs b/tinyharness-ui/src/tui/app.rs index cf5700a..f1ce2b3 100644 --- a/tinyharness-ui/src/tui/app.rs +++ b/tinyharness-ui/src/tui/app.rs @@ -339,11 +339,20 @@ impl TuiApp { let size = self.terminal.size(); let total = Rect::new(0, 0, size.cols, size.rows); + // Input bar grows up to a maximum based on content but always keeps + // room for the status bar, the conversation, and (optionally) the + // tool output panel. + let max_input_rows = (size.rows / 4).clamp(3, 10); + let input_rows = self + .input_bar + .content_height(size.cols) + .clamp(2, max_input_rows); + // Vertical split: status bar | main area | input bar let vertical = Layout::new(Direction::Vertical).constraints(vec![ - Constraint::Length(1), // status bar - Constraint::Percentage(100), // main area (takes remaining) - Constraint::Length(3), // input bar + Constraint::Length(1), // status bar + Constraint::Percentage(100), // main area (takes remaining) + Constraint::Length(input_rows), // input bar ]); let vertical_areas = vertical.split(total); let status_area = vertical_areas[0]; @@ -366,9 +375,10 @@ impl TuiApp { if self.state.sidebar_visible { // Horizontal split of main area: conversation | sidebar // The sidebar shares the full main area height (including tool output) + let sidebar_width = self.sidebar.desired_width.min(size.cols / 2).max(15); let horizontal = Layout::new(Direction::Horizontal).constraints(vec![ - Constraint::Percentage(100), // conversation - Constraint::Length(25), // sidebar + Constraint::Percentage(100), // conversation + Constraint::Length(sidebar_width), // sidebar ]); let horizontal_areas = horizontal.split(conv_area); let inner_conv = horizontal_areas[0]; @@ -1849,7 +1859,8 @@ mod tests { let app = make_app(); let (status, conv, sidebar, input, _main, _tool) = app.compute_layout(); assert_eq!(status.height, 1); - assert_eq!(input.height, 3); + // Input bar is now at least 2 rows: top border + one content row. + assert!(input.height >= 2); assert!(conv.height > 0); assert!(sidebar.width > 0); } @@ -2277,7 +2288,7 @@ mod tests { #[test] fn test_tool_output_layout_without_panel() { - let mut app = make_app(); + let app = make_app(); assert!(!app.tool_output_visible); let (.., tool_area) = app.compute_layout(); assert!(tool_area.is_empty()); diff --git a/tinyharness-ui/src/tui/screen.rs b/tinyharness-ui/src/tui/screen.rs index 118b044..c4d8fb8 100644 --- a/tinyharness-ui/src/tui/screen.rs +++ b/tinyharness-ui/src/tui/screen.rs @@ -6,6 +6,8 @@ use std::fmt; +use unicode_width::UnicodeWidthChar; + use super::cell::{Cell, Color, Style}; use super::layout::Rect; @@ -80,9 +82,37 @@ impl Screen { } } + /// Merge a zero-width combining mark into the previous cell. + /// + /// Does nothing if `col` is at the start of the current rendering run + /// or if `in_view` is false. + fn merge_combining_mark( + &mut self, + row: u16, + col: u16, + start_col: u16, + ch: char, + fg: Color, + bg: Color, + style: Style, + in_view: bool, + ) { + if !in_view || col <= start_col { + return; + } + if let Some(prev) = self.get_mut(row, col - 1) { + prev.char = ch; + prev.fg = fg; + prev.bg = bg; + prev.style = style; + } + } + /// Write a string starting at the given position, with the given style. /// - /// Characters that exceed the screen width are truncated. + /// Characters that exceed the screen width are truncated. Each character + /// is placed according to its Unicode display width; zero-width chars + /// (e.g. combining marks) overwrite the previous cell. pub fn write_str( &mut self, row: u16, @@ -97,6 +127,11 @@ impl Screen { if c >= self.width { break; } + let width = ch.width().unwrap_or(1); + if width == 0 { + self.merge_combining_mark(row, c, col, ch, fg, bg, style, c < self.width); + continue; + } self.set_cell( row, c, @@ -107,14 +142,14 @@ impl Screen { style, }, ); - c += 1; + c += width as u16; } } /// Write a string starting at the given position, truncating or wrapping. /// /// If `wrap` is true, text wraps to the next line. If false, text is - /// truncated at the right edge. + /// truncated at the right edge. Uses Unicode display widths. pub fn write_str_wrapped( &mut self, start_row: u16, @@ -129,7 +164,14 @@ impl Screen { let mut col = start_col; for ch in text.chars() { - if col >= self.width { + let width = ch.width().unwrap_or(1); + if width == 0 { + let in_view = col < self.width && row < self.height; + self.merge_combining_mark(row, col, start_col, ch, fg, bg, style, in_view); + continue; + } + let width_u16 = width as u16; + if col + width_u16 > self.width { if wrap && row + 1 < self.height { row += 1; col = 0; @@ -152,7 +194,7 @@ impl Screen { style, }, ); - col += 1; + col += width_u16; } row @@ -163,7 +205,7 @@ impl Screen { /// /// `wrap_col` is the maximum column number; text wraps when `col >= wrap_col`. /// `max_row` is the maximum row; text stops when `row > max_row`. - /// `left_margin` is the column where wrapped lines start. + /// `left_margin` is the column where wrapped lines start. Uses Unicode display widths. pub fn write_str_wrapped_clipped( &mut self, start_row: u16, @@ -180,7 +222,14 @@ impl Screen { let mut col = start_col; for ch in text.chars() { - if col >= wrap_col { + let width = ch.width().unwrap_or(1); + if width == 0 { + let in_view = row <= max_row; + self.merge_combining_mark(row, col, start_col, ch, fg, bg, style, in_view); + continue; + } + let width_u16 = width as u16; + if col + width_u16 > wrap_col { // Wrap to next line row += 1; col = left_margin; @@ -207,7 +256,7 @@ impl Screen { style, }, ); - col += 1; + col += width_u16; } row @@ -219,7 +268,7 @@ impl Screen { /// `wrap_col` is the maximum column number; text wraps when `col >= wrap_col`. /// `skip_rows` is the number of visual rows to skip before rendering. /// `max_row` is the maximum row; text stops when `row > max_row`. - /// `left_margin` is the column where wrapped lines start. + /// `left_margin` is the column where wrapped lines start. Uses Unicode display widths. pub fn write_str_wrapped_skip_clipped( &mut self, start_row: u16, @@ -238,8 +287,15 @@ impl Screen { let mut screen_row = start_row; for ch in text.chars() { + let width = ch.width().unwrap_or(1); + if width == 0 { + let in_view = visual_row >= skip_rows && screen_row <= max_row; + self.merge_combining_mark(screen_row, col, start_col, ch, fg, bg, style, in_view); + continue; + } + let width_u16 = width as u16; // Check if we need to wrap before placing this character - if ch != '\n' && col >= wrap_col { + if ch != '\n' && col + width_u16 > wrap_col { // Wrap to next visual line visual_row += 1; col = left_margin; @@ -279,7 +335,7 @@ impl Screen { ); } - col += 1; + col += width_u16; } } diff --git a/tinyharness-ui/src/tui/terminal.rs b/tinyharness-ui/src/tui/terminal.rs index f2690f7..be16e1a 100644 --- a/tinyharness-ui/src/tui/terminal.rs +++ b/tinyharness-ui/src/tui/terminal.rs @@ -413,10 +413,6 @@ mod tests { fn new() -> Self { Self { buf: Vec::new() } } - - fn into_contents(self) -> Vec { - self.buf - } } impl Write for TestWriter { diff --git a/tinyharness-ui/src/tui/widget.rs b/tinyharness-ui/src/tui/widget.rs index ce4909e..a0ea1b9 100644 --- a/tinyharness-ui/src/tui/widget.rs +++ b/tinyharness-ui/src/tui/widget.rs @@ -134,6 +134,8 @@ pub mod styles { pub const BOX_CROSS: char = '┼'; } +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; + /// Safely truncate a string to at most `max_len` bytes, respecting UTF-8 /// char boundaries. Returns a string slice that fits within `max_len` bytes. /// @@ -151,6 +153,26 @@ pub fn truncate_str(s: &str, max_len: usize) -> &str { &s[..boundary] } +/// Truncate a string so that its Unicode display width is at most `max_width`. +/// +/// Unlike `truncate_str`, this respects terminal display columns, so CJK and +/// emoji characters count correctly. Returns the longest prefix whose width +/// does not exceed `max_width`. +pub fn truncate_str_width(s: &str, max_width: usize) -> &str { + if s.width() <= max_width { + return s; + } + let mut acc = 0usize; + for (idx, ch) in s.char_indices() { + let w = ch.width().unwrap_or(1).max(1); + if acc + w > max_width { + return &s[..idx]; + } + acc += w; + } + s +} + #[cfg(test)] mod tests { use super::*; diff --git a/tinyharness-ui/src/tui/widgets/conversation.rs b/tinyharness-ui/src/tui/widgets/conversation.rs index 96dd6ef..0053e46 100644 --- a/tinyharness-ui/src/tui/widgets/conversation.rs +++ b/tinyharness-ui/src/tui/widgets/conversation.rs @@ -3,11 +3,13 @@ // Displays the conversation history in a scrollable pane with // color-coded messages, tool call blocks, and thinking chains. +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; + use crate::tui::cell::{Cell, Color, Style}; use crate::tui::event::{Event, Key, KeyEvent}; use crate::tui::layout::Rect; use crate::tui::screen::Screen; -use crate::tui::widget::{Action, Widget, styles, truncate_str}; +use crate::tui::widget::{Action, Widget, styles, truncate_str_width}; /// A single line in the conversation display. #[derive(Clone, Debug)] @@ -288,105 +290,80 @@ impl ConversationWidget { // Show at most 20 lines for a tool result let visible_lines = lines.iter().copied().take(20).collect::>(); let joined = visible_lines.join("\n"); - return self.line_height_for_text(&joined, 4, area_width).max(1); // At least 1 row even for empty content + return self.line_height_for_text(&joined, area_width, 4, 4).max(1); } ConversationLine::ToolCall { name, args_summary } => { - let header = format!(" ── {}", name); - let header_len = header.len(); if args_summary.is_empty() { return 1; } - // Calculate wrapping for args_summary starting after the header - let total_text = if args_summary.is_empty() { - String::new() - } else { - args_summary.clone() - }; - return self.line_height_for_text_with_offset(&total_text, header_len, area_width); + let header = format!(" ── {}", name); + let header_len = header.len(); + return self.line_height_for_text(args_summary, area_width, header_len, header_len); } ConversationLine::Question { question, answers } => { - // " ❓ " (4 chars) for question line, plus one line per answer - let question_rows = self.line_height_for_text(question, 4, area_width); + let question_rows = self.line_height_for_text(question, area_width, 4, 4); let answer_rows: usize = answers .iter() .map(|a| { - // " N. " prefix = 6 chars for single-digit, 7 for double let prefix_len = if answers.len() >= 10 { 7 } else { 6 }; - self.line_height_for_text(a, prefix_len, area_width) + self.line_height_for_text(a, area_width, prefix_len, prefix_len) }) .sum(); return question_rows + answer_rows; } ConversationLine::Separator => return 1, ConversationLine::ConfirmPrompt { diff_preview, .. } => { - // Base prompt line = 1 row let mut rows = 1usize; - // Diff preview lines add extra rows if let Some(diff) = diff_preview && !diff.is_empty() { - let _max_content_width = (area_width as usize).saturating_sub(4); for line in diff.lines() { - rows += self.line_height_for_text(line, 4, area_width); + rows += self.line_height_for_text(line, area_width, 4, 4); } } return rows; } }; - self.line_height_for_text(text, prefix_len, area_width) - } - - /// Helper: calculate visual row count for text with a given prefix and area width. - fn line_height_for_text(&self, text: &str, prefix_len: usize, area_width: u16) -> usize { - if area_width == 0 || text.is_empty() { - return 1; - } - - let wrap_col = area_width as usize; - let mut rows = 1usize; - let mut col = prefix_len; - - for ch in text.chars() { - if ch == '\n' { - rows += 1; - col = prefix_len; - } else if col >= wrap_col { - rows += 1; - col = prefix_len + 1; - } else { - col += 1; - } - } - rows + self.line_height_for_text(text, area_width, prefix_len, prefix_len) } - /// Helper: calculate visual row count for text where the first line starts - /// at `first_line_offset` (header length) and wrapped lines indent to the - /// same offset. This is used for tool call args that wrap. - fn line_height_for_text_with_offset( + /// Calculate visual row count for text rendered in `area_width` columns. + /// + /// The first line starts at `start_col`; after a wrap or newline the cursor + /// returns to `wrap_indent`. Uses Unicode display widths. + fn line_height_for_text( &self, text: &str, - first_line_offset: usize, area_width: u16, + start_col: usize, + wrap_indent: usize, ) -> usize { - if area_width == 0 || text.is_empty() { + if area_width == 0 { + return 1; + } + if text.is_empty() { return 1; } let wrap_col = area_width as usize; let mut rows = 1usize; - let mut col = first_line_offset; + let mut col = start_col; for ch in text.chars() { + let w = ch.width().unwrap_or(1); + if w == 0 { + // Combining mark doesn't take up visual columns + continue; + } if ch == '\n' { rows += 1; - col = first_line_offset; - } else if col >= wrap_col { + col = wrap_indent; + } else if col + w > wrap_col { rows += 1; - col = first_line_offset + 1; + col = wrap_indent + w; } else { - col += 1; + col += w; } } rows @@ -610,11 +587,11 @@ impl ConversationWidget { let display = if content_line.is_empty() { line_prefix.to_string() - } else if content_line.len() > max_content_width { + } else if content_line.width() > max_content_width { format!( "{}{}…", line_prefix, - truncate_str(content_line, max_content_width.saturating_sub(1)) + truncate_str_width(content_line, max_content_width.saturating_sub(1)) ) } else { format!("{}{}", line_prefix, content_line) @@ -635,7 +612,7 @@ impl ConversationWidget { bg }; let fill_end = area.x + area.width.saturating_sub(1); - let end_col = area.x + display.len().min(area.width as usize - 1) as u16; + let end_col = area.x + display.width().min(area.width as usize - 1) as u16; if end_col < fill_end { for c in end_col..fill_end { if let Some(cell) = screen.get_mut(current_row, c) { @@ -828,11 +805,11 @@ impl ConversationWidget { let display = if line.is_empty() { prefix.to_string() - } else if line.len() > max_content_width { + } else if line.width() > max_content_width { format!( "{}{}…", prefix, - truncate_str(line, max_content_width.saturating_sub(1)) + truncate_str_width(line, max_content_width.saturating_sub(1)) ) } else { format!("{}{}", prefix, line) @@ -850,7 +827,7 @@ impl ConversationWidget { if line_bg != Color::Default { let fill_end = area.x + area.width.saturating_sub(1); let end_col = - area.x + display.len().min(area.width as usize - 1) as u16; + area.x + display.width_cjk().min(area.width as usize - 1) as u16; if end_col < fill_end { for c in end_col..fill_end { if let Some(cell) = screen.get_mut(current_row, c) { @@ -897,8 +874,12 @@ impl ConversationWidget { lines_remaining -= 1; } else if skip_top > 0 { // Skip question lines - let q_height = - self.line_height_for_text(question, question_prefix.len(), area.width); + let q_height = self.line_height_for_text( + question, + area.width, + question_prefix.len(), + question_prefix.len(), + ); if skip_top >= q_height { // Question entirely skipped; adjust for remaining skip } @@ -907,7 +888,12 @@ impl ConversationWidget { // Render answer lines: " N. " for (i, answer) in answers.iter().enumerate() { let answer_label = format!(" {}. ", i + 1); - let a_height = self.line_height_for_text(answer, answer_prefix_len, area.width); + let a_height = self.line_height_for_text( + answer, + area.width, + answer_prefix_len, + answer_prefix_len, + ); if skip_top > 0 { // Still skipping @@ -996,8 +982,8 @@ impl ConversationWidget { } // Write the warning text (truncate if wider than area) - let display = if msg.len() > area.width as usize { - truncate_str(&msg, area.width as usize).to_string() + let display = if msg.width() > area.width as usize { + truncate_str_width(&msg, area.width as usize).to_string() } else { msg }; @@ -1030,8 +1016,8 @@ impl ConversationWidget { // Search query text let query_text = &self.search.query; let max_query_width = area.width.saturating_sub(label.len() as u16 + 20); // reserve for match count - let display_query = if query_text.len() > max_query_width as usize { - truncate_str(query_text, max_query_width as usize).to_string() + let display_query = if query_text.width() > max_query_width as usize { + truncate_str_width(query_text, max_query_width as usize).to_string() } else { query_text.clone() }; @@ -1046,11 +1032,11 @@ impl ConversationWidget { // Cursor indicator (underline the character at cursor position or show block if at end) let cursor_col_in_display = if self.search.cursor > query_text.len() { - display_query.len() + display_query.width() } else { // Find the display position corresponding to the cursor byte offset let before_cursor = &query_text[..self.search.cursor.min(query_text.len())]; - let display_offset = before_cursor.chars().count(); + let display_offset = before_cursor.width(); if display_offset > max_query_width as usize { max_query_width as usize } else { @@ -1165,7 +1151,7 @@ impl ConversationWidget { let prefix = &text[..abs_pos]; let prefix_chars: Vec = prefix.chars().collect(); let query_chars: Vec = self.search.query.chars().collect(); - let query_len = query_chars.len(); + let _query_len = query_chars.len(); // Determine which visual row within this line the match starts on // Use the same wrapping logic as render (simplified) @@ -1181,11 +1167,12 @@ impl ConversationWidget { // Calculate the visual row offset for the start of the match let mut row_offset = 0usize; let mut col = line_prefix_len; - for _ in prefix_chars.iter() { - col += 1; - if col >= wrap_at { + for ch in prefix_chars.iter() { + let w = ch.width().unwrap_or(1).max(1) as usize; + col += w; + if col > wrap_at { row_offset += 1; - col = line_prefix_len; + col = line_prefix_len + w; } } @@ -1225,7 +1212,7 @@ impl ConversationWidget { }; let mut highlight_col = area.x + start_col as u16; - for _ in 0..query_len { + for ch in query_chars.iter() { if highlight_col >= area.x + area.width.saturating_sub(1) { break; // Don't overwrite scrollbar } @@ -1233,7 +1220,7 @@ impl ConversationWidget { cell.fg = highlight_fg; cell.bg = highlight_bg; } - highlight_col += 1; + highlight_col += ch.width().unwrap_or(1).max(1) as u16; } match_idx += 1; diff --git a/tinyharness-ui/src/tui/widgets/input_bar.rs b/tinyharness-ui/src/tui/widgets/input_bar.rs index 0518f96..85ff96b 100644 --- a/tinyharness-ui/src/tui/widgets/input_bar.rs +++ b/tinyharness-ui/src/tui/widgets/input_bar.rs @@ -3,13 +3,123 @@ // Multi-line input with history, cursor tracking, mode/model label, // and tab completion for slash commands. +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; + use crate::tui::cell::{Color, Style}; use crate::tui::event::{Event, Key, KeyEvent}; use crate::tui::layout::Rect; use crate::tui::screen::Screen; -use crate::tui::widget::{Action, Widget, styles}; +use crate::tui::widget::{Action, Widget, styles, truncate_str_width}; use std::collections::HashMap; +/// Find the byte offset of the start of the previous word, stopping at newlines. +fn word_start_backward(text: &str, cursor: usize) -> usize { + let before = &text[..cursor]; + let trimmed = before.trim_end_matches(|c: char| c.is_whitespace() && c != '\n'); + if trimmed.len() < before.len() { + trimmed + .rfind(|c: char| c.is_whitespace() || c == '\n') + .map(|p| p + 1) + .unwrap_or(0) + } else { + before + .rfind(|c: char| c.is_whitespace() || c == '\n') + .map(|p| p + 1) + .unwrap_or(0) + } +} + +/// Find the byte offset after the next word, including trailing whitespace (but not newlines). +fn word_end_forward(text: &str, cursor: usize) -> usize { + let after = &text[cursor..]; + let word_end = after + .find(|c: char| c.is_whitespace() || c == '\n') + .unwrap_or(after.len()); + let whitespace_skipped = after[word_end..] + .chars() + .take_while(|c| c.is_whitespace() && *c != '\n') + .count(); + cursor + word_end + whitespace_skipped +} + +/// A single visual row of input text, with byte range and screen column. +struct InputRow<'a> { + /// The text slice to display on this row. + text: &'a str, + /// Byte offset of `text` within the full content. + byte_offset: usize, + /// Screen column where this row starts. + col: u16, +} + +/// Compute the visual rows for input content, accounting for the prompt on +/// the first line and Unicode display widths. Each row fits within +/// `available_width` columns. +fn input_rows<'a>( + content: &'a str, + prompt_width: u16, + area_x: u16, + available_width: u16, +) -> Vec> { + let mut rows = Vec::new(); + let mut first_line = true; + let mut byte_offset = 0usize; + + for line in content.split_inclusive('\n') { + let (line_text, _has_newline) = if let Some(stripped) = line.strip_suffix('\n') { + (stripped, true) + } else { + (line, false) + }; + + let left_col = if first_line { + area_x + prompt_width + } else { + area_x + }; + first_line = false; + + // Split the logical line into wrapped visual rows. + let mut row_start = 0usize; + let mut row_width = 0usize; + let first_line_indent = if row_start == 0 && left_col > area_x { + prompt_width as usize + } else { + 0 + }; + + for (idx, ch) in line_text.char_indices() { + let w = ch.width().unwrap_or(1); + if w == 0 { + // Combining marks don't occupy columns; skip for wrapping. + continue; + } + if row_width + first_line_indent + w > available_width as usize && row_width > 0 { + let end = idx; + rows.push(InputRow { + text: &line_text[row_start..end], + byte_offset: byte_offset + row_start, + col: left_col, + }); + row_start = idx; + row_width = 0; + } + row_width += w; + } + + // Push the final row for this logical line. + rows.push(InputRow { + text: &line_text[row_start..], + byte_offset: byte_offset + row_start, + col: left_col, + }); + + byte_offset += line.len(); + } + + rows +} + /// The input bar at the bottom of the screen. /// /// Displays a prompt with mode and model labels, and accepts @@ -143,23 +253,14 @@ impl InputBarWidget { self.subcommands = subcommands; } - /// Calculate which line and column the cursor is on. - #[allow(dead_code)] - fn cursor_line_col(&self) -> (usize, usize) { - let text_before_cursor = &self.content[..self.cursor]; - let line = text_before_cursor.lines().count().saturating_sub(1); - let col = text_before_cursor - .lines() - .next_back() - .map(|l| l.len()) - .unwrap_or(0); - (line, col) - } - - /// Count the number of lines in the input. - #[allow(dead_code)] - fn line_count(&self) -> usize { - self.content.lines().count().max(1) + /// Total visual height of the input content for the given area width. + pub fn content_height(&self, area_width: u16) -> u16 { + let prompt = format!("[{}] ", self.mode_label); + let prompt_width = prompt.width() as u16; + let available = area_width.saturating_sub(prompt_width).max(1); + let rows = input_rows(&self.content, prompt_width, 0, available).len() as u16; + // Reserve one row for the top border and at least one row of content. + rows.max(1) + 1 } /// Check if the current input starts with a slash (for command detection). @@ -208,53 +309,32 @@ impl InputBarWidget { /// content, then moves the cursor to that position. pub fn click_to_cursor(&mut self, click_row: u16, click_col: u16, area: Rect) { if self.confirming || self.questioning { - // No cursor positioning in confirmation/question mode return; } - // The prompt is "[mode] " which takes some columns on the first input line let prompt = format!("[{}] ", self.mode_label); - let prompt_len = prompt.len() as u16; + let prompt_width = prompt.width() as u16; + let available = area.width.saturating_sub(prompt_width).max(1); - // First content line starts at area.y + 1 (below the top border) let first_input_row = area.y + 1; + let rel_row = click_row.saturating_sub(first_input_row) as usize; - // Determine which line of content was clicked (relative to first input row) - let line_offset = click_row.saturating_sub(first_input_row) as usize; - - // Calculate the cursor position from the click - if line_offset == 0 { - // Clicked on the first line — account for the prompt prefix - let col_offset = click_col.saturating_sub(area.x + prompt_len) as usize; - // Move cursor to that character position within the first line - let first_line_len = self.content.lines().next().map(|l| l.len()).unwrap_or(0); - let new_pos = col_offset.min(first_line_len); - // The cursor position in the full string is at the start + new_pos - let line_start = 0; - self.cursor = line_start + new_pos; - if self.cursor > self.content.len() { - self.cursor = self.content.len(); - } - } else { - // Clicked on a subsequent line — calculate byte offset for that line - let mut byte_offset = 0usize; - for (i, line) in self.content.lines().enumerate() { - if i == line_offset { - // Found the target line - let col_offset = click_col.saturating_sub(area.x) as usize; - let new_pos = col_offset.min(line.len()); - self.cursor = byte_offset + new_pos; - if self.cursor > self.content.len() { - self.cursor = self.content.len(); - } - return; - } - // +1 for the '\n' character - byte_offset += line.len() + 1; - } - // Click was past the last line — position cursor at end + let rows = input_rows(&self.content, prompt_width, area.x, available); + let Some(row) = rows.get(rel_row) else { self.cursor = self.content.len(); + return; + }; + + let mut col = row.col as usize; + for (idx, ch) in row.text.char_indices() { + let w = ch.width().unwrap_or(1).max(1) as usize; + if col + w > click_col as usize { + self.cursor = row.byte_offset + idx; + return; + } + col += w; } + self.cursor = row.byte_offset + row.text.len(); } /// Attempt tab completion for slash commands. @@ -365,7 +445,6 @@ impl Widget for InputBarWidget { } let row = area.y; - let _width = area.width as usize; // Draw top border screen.hline( @@ -425,7 +504,7 @@ impl Widget for InputBarWidget { // Draw input content let available_width = area.width.saturating_sub(col - area.x); - let display_text = if self.content.len() > available_width as usize { + let display_text = if self.content.width() > available_width as usize { let start = self.content.len().saturating_sub(available_width as usize); &self.content[start..] } else { @@ -443,7 +522,8 @@ impl Widget for InputBarWidget { // Draw cursor if self.focused { - let cursor_col = col + self.cursor.min(display_text.len()) as u16; + let before_cursor = &display_text[..self.cursor.min(display_text.len())]; + let cursor_col = col + before_cursor.width() as u16; if cursor_col < area.x + area.width { if let Some(cell) = screen.get_mut(input_row, cursor_col) { cell.style.underline = true; @@ -452,7 +532,6 @@ impl Widget for InputBarWidget { } } else { let prompt = format!("[{}] ", self.mode_label); - let _model_suffix = format!(" {}{}", self.model_name, Color::Default.fg_escape()); // Draw mode label let mut col = area.x; @@ -464,63 +543,22 @@ impl Widget for InputBarWidget { styles::INPUT_BAR_BG, Style::bold(), ); - col += prompt.len() as u16; + col += prompt.width() as u16; - // Draw input content (with wrapping if needed) let available_width = area.width.saturating_sub(col - area.x); - let display_text = if self.content.len() > available_width as usize { - // Show the end of the text that fits, scrolled to cursor - let start = self.content.len().saturating_sub(available_width as usize); - &self.content[start..] - } else { - &self.content - }; - screen.write_str( + // Render wrapped content over the available rows. + render_wrapped_input( + &self.content, + self.cursor, + self.focused, + screen, input_row, + area.y + area.height - 1, + area.x, col, - display_text, - Color::WHITE, - styles::INPUT_BAR_BG, - Style::default(), + available_width, ); - - // Draw cursor (blinking is handled by terminal, we just position it) - if self.focused { - let cursor_col = col + self.cursor.min(display_text.len()) as u16; - if cursor_col < area.x + area.width { - // Underline the character under the cursor - if let Some(cell) = screen.get_mut(input_row, cursor_col) { - cell.style.underline = true; - } - } - } - - // Fill the rest of the input line with background - let text_end = col + display_text.len() as u16; - if text_end < area.x + area.width { - // Background is already filled by write_str - } - - // For multi-line input, render additional lines - let lines: Vec<&str> = self.content.lines().collect(); - for (i, line) in lines.iter().enumerate() { - if i == 0 { - continue; // Already rendered the first line - } - let line_row = input_row + i as u16; - if line_row >= area.y + area.height { - break; - } - screen.write_str( - line_row, - area.x, - line, - Color::WHITE, - styles::INPUT_BAR_BG, - Style::default(), - ); - } } } @@ -645,6 +683,75 @@ impl Widget for InputBarWidget { } } +/// Render the normal input text wrapped into the available area. +/// `first_col` is where the first line starts (after the prompt); subsequent +/// lines start at `area_x`. +fn render_wrapped_input( + content: &str, + cursor: usize, + focused: bool, + screen: &mut Screen, + start_row: u16, + max_row: u16, + area_x: u16, + first_col: u16, + available_width: u16, +) { + let prompt_width = first_col.saturating_sub(area_x); + let rows = input_rows(content, prompt_width, area_x, available_width); + + for (i, row) in rows.iter().enumerate() { + let screen_row = start_row + i as u16; + if screen_row > max_row { + break; + } + + // Show only the tail of the row if it doesn't fit. + let display_text = if row.text.width() > available_width as usize { + truncate_str_width(row.text, available_width as usize) + } else { + row.text + }; + + screen.write_str( + screen_row, + row.col, + display_text, + Color::WHITE, + styles::INPUT_BAR_BG, + Style::default(), + ); + + // Cursor on the active row. + if focused && cursor >= row.byte_offset && cursor <= row.byte_offset + row.text.len() { + let before_cursor_len = cursor.saturating_sub(row.byte_offset).min(row.text.len()); + let before_cursor = &row.text[..before_cursor_len]; + let cursor_col = row.col + before_cursor.width() as u16; + if cursor_col < area_x + available_width { + if let Some(cell) = screen.get_mut(screen_row, cursor_col) { + cell.style.underline = true; + if cell.char == ' ' { + cell.fg = Color::WHITE; + } + } + } + } + } + + // Fill remaining lines with background so stale content is cleared. + let last_rendered_row = start_row + rows.len() as u16; + for r in last_rendered_row..=max_row { + for c in area_x..area_x + available_width { + if let Some(cell) = screen.get_mut(r, c) { + cell.char = ' '; + cell.fg = Color::Default; + cell.bg = styles::INPUT_BAR_BG; + cell.style = Style::default(); + } + } + } +} + impl InputBarWidget { /// Handle a key event in normal (non-confirmation) mode. fn handle_normal_key(&mut self, key: &KeyEvent) -> Action { @@ -704,27 +811,11 @@ impl InputBarWidget { modifiers, } => { if modifiers.ctrl { - // Ctrl+Left: move back one word - if self.cursor > 0 { - let text_before = &self.content[..self.cursor]; - let trimmed = - text_before.trim_end_matches(|c: char| c.is_whitespace() && c != '\n'); - let word_start = if trimmed.len() < text_before.len() { - trimmed - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - } else { - text_before - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - }; - self.cursor = word_start; - } + self.cursor = word_start_backward(&self.content, self.cursor); } else if self.cursor > 0 { - if let Some(ch) = self.content[..self.cursor].chars().next_back() { - self.cursor -= ch.len_utf8(); + if let Some((idx, _ch)) = self.content[..self.cursor].char_indices().next_back() + { + self.cursor = idx; } } self.reset_tab_cycle(); @@ -735,23 +826,10 @@ impl InputBarWidget { modifiers, } => { if modifiers.ctrl { - // Ctrl+Right: move forward one word - if self.cursor < self.content.len() { - let text_after = &self.content[self.cursor..]; - // Skip the current word, then skip trailing whitespace - let word_end = text_after - .find(|c: char| c.is_whitespace() || c == '\n') - .unwrap_or(text_after.len()); - let after_word = &text_after[word_end..]; - let whitespace_skipped = after_word - .chars() - .take_while(|c| c.is_whitespace() && *c != '\n') - .count(); - self.cursor += word_end + whitespace_skipped; - } + self.cursor = word_end_forward(&self.content, self.cursor); } else if self.cursor < self.content.len() { - if let Some(ch) = self.content[self.cursor..].chars().next() { - self.cursor += ch.len_utf8(); + if let Some((idx, ch)) = self.content[self.cursor..].char_indices().next() { + self.cursor = idx + ch.len_utf8(); } } self.reset_tab_cycle(); @@ -892,23 +970,7 @@ impl InputBarWidget { 'w' => { // Ctrl+W: delete word backward if self.cursor > 0 { - // Find the start of the previous word - let text_before = &self.content[..self.cursor]; - let trimmed = text_before - .trim_end_matches(|c: char| c.is_whitespace() && c != '\n'); - let word_start = if trimmed.len() < text_before.len() { - // There was trailing whitespace — skip it then find the word - trimmed - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - } else { - // No trailing whitespace — find the word boundary - text_before - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - }; + let word_start = word_start_backward(&self.content, self.cursor); let killed = self.content[word_start..self.cursor].to_string(); if !killed.is_empty() { self.kill_ring = killed; @@ -935,61 +997,20 @@ impl InputBarWidget { match c { 'b' => { // Alt+B: move cursor back one word - if self.cursor > 0 { - let text_before = &self.content[..self.cursor]; - let trimmed = text_before - .trim_end_matches(|c: char| c.is_whitespace() && c != '\n'); - let word_start = if trimmed.len() < text_before.len() { - trimmed - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - } else { - text_before - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - }; - self.cursor = word_start; - } + self.cursor = word_start_backward(&self.content, self.cursor); self.reset_tab_cycle(); Action::None } 'f' => { // Alt+F: move cursor forward one word - if self.cursor < self.content.len() { - let text_after = &self.content[self.cursor..]; - // Skip the current word, then skip trailing whitespace - let word_end = text_after - .find(|c: char| c.is_whitespace() || c == '\n') - .unwrap_or(text_after.len()); - let after_word = &text_after[word_end..]; - let whitespace_skipped = after_word - .chars() - .take_while(|c| c.is_whitespace() && *c != '\n') - .count(); - self.cursor += word_end + whitespace_skipped; - } + self.cursor = word_end_forward(&self.content, self.cursor); self.reset_tab_cycle(); Action::None } '\x08' | '\x7f' => { // Alt+Backspace: delete word backward (same as Ctrl+W) if self.cursor > 0 { - let text_before = &self.content[..self.cursor]; - let trimmed = text_before - .trim_end_matches(|c: char| c.is_whitespace() && c != '\n'); - let word_start = if trimmed.len() < text_before.len() { - trimmed - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - } else { - text_before - .rfind(|c: char| c.is_whitespace() || c == '\n') - .map(|p| p + 1) - .unwrap_or(0) - }; + let word_start = word_start_backward(&self.content, self.cursor); let killed = self.content[word_start..self.cursor].to_string(); if !killed.is_empty() { self.kill_ring = killed; diff --git a/tinyharness-ui/src/tui/widgets/sidebar.rs b/tinyharness-ui/src/tui/widgets/sidebar.rs index 20df1af..70b91d7 100644 --- a/tinyharness-ui/src/tui/widgets/sidebar.rs +++ b/tinyharness-ui/src/tui/widgets/sidebar.rs @@ -8,11 +8,47 @@ use std::fs; use std::path::PathBuf; +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; + use crate::tui::cell::{Cell, Color, Style}; use crate::tui::event::{Event, Key, KeyEvent}; use crate::tui::layout::Rect; use crate::tui::screen::Screen; -use crate::tui::widget::{Action, Widget, styles, truncate_str}; +use crate::tui::widget::{Action, Widget, styles, truncate_str_width}; + +/// Truncate `s` to fit within `max_width` display columns, appending an +/// ellipsis only if truncation occurs. Returns the original string if it fits. +fn truncate_with_ellipsis(s: &str, max_width: usize) -> String { + if s.width() <= max_width { + return s.to_string(); + } + let prefix = "…"; + let avail = max_width.saturating_sub(prefix.width()); + format!("{}{}", prefix, truncate_str_width(s, avail)) +} + +/// Truncate `s` to fit within `max_width` display columns, keeping the rightmost +/// portion and prepending `prefix`. `prefix` is shown only if truncation occurs. +fn truncate_to_width_with_prefix(s: &str, max_width: usize, prefix: &str) -> String { + if s.width() <= max_width { + return s.to_string(); + } + let avail = max_width.saturating_sub(prefix.width()); + if avail == 0 { + return prefix.to_string(); + } + let mut current_width = 0usize; + let mut start = s.len(); + for (idx, ch) in s.char_indices().rev() { + let w = ch.width().unwrap_or(1).max(1); + if current_width + w > avail { + start = idx + ch.len_utf8(); + break; + } + current_width += w; + } + format!("{}{}", prefix, &s[start..]) +} // ── Directory entry for the file browser ────────────────────────────────────── @@ -122,6 +158,8 @@ pub struct SidebarWidget { pub visible: bool, /// Vertical scroll offset in rows (0 = top). scroll_offset: usize, + /// Desired width of the sidebar in columns (can be resized). + pub desired_width: u16, // ── Interactive file browser state ───────────────────────────────────── /// Whether the sidebar is in interactive structure mode. @@ -159,6 +197,7 @@ impl SidebarWidget { active_skills: Vec::new(), visible: true, scroll_offset: 0, + desired_width: 25, structure_mode: false, structure_cwd: cwd.clone(), structure_nav_stack: Vec::new(), @@ -557,11 +596,7 @@ impl Widget for SidebarWidget { SidebarItem::Entry { icon, name, is_dir } => { let suffix = if *is_dir { "/" } else { "" }; let display = format!("{} {}{}", icon, name, suffix); - let truncated = if display.chars().count() > max_width { - format!("{}…", truncate_str(&display, max_width.saturating_sub(1))) - } else { - display - }; + let truncated = truncate_with_ellipsis(&display, max_width); let fg = if *is_dir { Color::BRIGHT_CYAN } else { @@ -577,7 +612,7 @@ impl Widget for SidebarWidget { Style::default(), ); // Clear remaining cells on this row to prevent stale characters - let end_col = area.x + 2 + text.chars().count() as u16; + let end_col = area.x + 2 + text.width() as u16; let right_bound = area.x + area.width.saturating_sub(1); for col in end_col..right_bound { if let Some(cell) = screen.get_mut(screen_row, col) { @@ -597,11 +632,7 @@ impl Widget for SidebarWidget { structure_entry_screen_rows.push(screen_row); let suffix = if *is_dir { "/" } else { "" }; let display = format!("{} {}{}", icon, name, suffix); - let truncated = if display.chars().count() > max_width { - format!("{}…", truncate_str(&display, max_width.saturating_sub(1))) - } else { - display - }; + let truncated = truncate_with_ellipsis(&display, max_width); if *selected { // Highlighted row: inverted or accent background @@ -641,7 +672,7 @@ impl Widget for SidebarWidget { Style::default(), ); // Clear remaining cells on this row to prevent stale characters - let end_col = area.x + 2 + text.chars().count() as u16; + let end_col = area.x + 2 + text.width() as u16; let right_bound = area.x + area.width.saturating_sub(1); for col in end_col..right_bound { if let Some(cell) = screen.get_mut(screen_row, col) { @@ -655,7 +686,7 @@ impl Widget for SidebarWidget { } SidebarItem::Skill(name) => { let display = format!("✦ {}", name); - let end_col = area.x + 2 + display.chars().count() as u16; + let end_col = area.x + 2 + display.width() as u16; screen.write_str( screen_row, area.x + 2, @@ -1087,31 +1118,13 @@ impl SidebarWidget { /// Format the current directory as a breadcrumb for display. fn format_breadcrumb(&self, max_width: usize) -> String { - // Show the path relative to workspace root, or just the last 2 components let path = &self.structure_cwd; let rel = path.strip_prefix(&self.workspace_root).unwrap_or(path); let display = rel.to_string_lossy().to_string(); if display.is_empty() { return ".".to_string(); } - // Truncate if too long - if display.len() > max_width { - let prefix = "…"; - let avail = max_width.saturating_sub(prefix.len()); - if avail > 0 { - let start = display.len().saturating_sub(avail); - // Find char boundary - let mut start = start; - while start < display.len() && !display.is_char_boundary(start) { - start += 1; - } - format!("{}{}", prefix, &display[start..]) - } else { - prefix.to_string() - } - } else { - display - } + truncate_to_width_with_prefix(&display, max_width, "…") } fn draw_section_header( @@ -1123,7 +1136,7 @@ impl SidebarWidget { title: &str, ) -> u16 { let header = format!("┌─ {} ", title); - let header_width = header.chars().count(); + let header_width = header.width(); screen.write_str( row, col, @@ -1157,7 +1170,7 @@ impl SidebarWidget { value: &str, value_color: Color, ) -> u16 { - let label_width = label.chars().count(); + let label_width = label.width(); screen.write_str( row, col, @@ -1168,11 +1181,7 @@ impl SidebarWidget { ); let value_col = col + label_width as u16 + 1; let available = max_width.saturating_sub(label_width + 1); - let display = if value.chars().count() > available { - format!("{}…", truncate_str(value, available.saturating_sub(1))) - } else { - value.to_string() - }; + let display = truncate_with_ellipsis(value, available); screen.write_str( row, value_col, @@ -1182,7 +1191,7 @@ impl SidebarWidget { Style::default(), ); // Clear remaining cells on this row to prevent stale characters - let end_col = value_col + display.chars().count() as u16; + let end_col = value_col + display.width() as u16; let right_bound = col + max_width as u16; for c in end_col..right_bound { if let Some(cell) = screen.get_mut(row, c) { diff --git a/tinyharness-ui/src/tui/widgets/tool_output.rs b/tinyharness-ui/src/tui/widgets/tool_output.rs index 417babe..6d20e2b 100644 --- a/tinyharness-ui/src/tui/widgets/tool_output.rs +++ b/tinyharness-ui/src/tui/widgets/tool_output.rs @@ -3,11 +3,13 @@ // Displays tool call results in a collapsible pane. Tool results start // collapsed (just header line). Click or Enter to expand. +use unicode_width::UnicodeWidthStr; + use crate::tui::cell::{Cell, Color, Style}; use crate::tui::event::{Event, Key, KeyEvent, Modifiers, MouseEvent}; use crate::tui::layout::Rect; use crate::tui::screen::Screen; -use crate::tui::widget::{Action, Widget, truncate_str}; +use crate::tui::widget::{Action, Widget, truncate_str_width}; /// Status of a tool call. #[derive(Clone, Debug, PartialEq)] @@ -130,6 +132,7 @@ impl ToolOutputWidget { }; let header_with_dur = format!("{}{}", header, duration_str); + let header_width = header_with_dur.width(); screen.write_str( row, @@ -142,12 +145,15 @@ impl ToolOutputWidget { // Show content preview (first line, truncated) if !result.content.is_empty() { - let preview_col = (header_with_dur.len() as u16 + 2).min(width.saturating_sub(20)); + let preview_col = (header_width as u16 + 2).min(width.saturating_sub(20)); let available = (width as usize).saturating_sub(preview_col as usize); if available > 10 { let first_line = result.content.lines().next().unwrap_or(""); - let preview = if first_line.len() > available.saturating_sub(3) { - format!("{}…", truncate_str(first_line, available.saturating_sub(3))) + let preview = if first_line.width() > available.saturating_sub(3) { + format!( + "{}…", + truncate_str_width(first_line, available.saturating_sub(3)) + ) } else { first_line.to_string() }; @@ -253,8 +259,8 @@ impl ToolOutputWidget { ); let available = (width as usize).saturating_sub(4); - let display = if line.len() > available { - format!("{}…", truncate_str(line, available.saturating_sub(1))) + let display = if line.width() > available { + format!("{}…", truncate_str_width(line, available.saturating_sub(1))) } else { line.to_string() }; From 4e1a60aea9feee85d1c6474cdfc73ce1c32f6c60 Mon Sep 17 00:00:00 2001 From: PTFOPlayer Date: Sun, 14 Jun 2026 15:51:23 +0200 Subject: [PATCH 2/5] fix(tui): handle wide (CJK/fullwidth) characters with continuation cells Add a field to to track continuation cells for wide characters that occupy 2 columns. When a wide character is written, the following column is marked as a continuation cell so the renderer skips it (the terminal already drew it as part of the wide character). Key changes: - Cell struct gains field, defaulting to false - New constructor for continuation cells - Screen's write_str/write_str_centered/write_str_right all place continuation cells after wide characters - Diff renderer skips continuation cells and tracks cursor position based on actual character display width - All widget code that manually sets cell chars now resets - Input bar handles empty content case to properly clear background - Add unit tests for wide character rendering and diff tracking --- tinyharness-ui/src/tui/app.rs | 10 ++ tinyharness-ui/src/tui/cell.rs | 25 ++++ tinyharness-ui/src/tui/screen.rs | 131 +++++++++++++++++- .../src/tui/widgets/conversation.rs | 4 + tinyharness-ui/src/tui/widgets/input_bar.rs | 41 ++++++ tinyharness-ui/src/tui/widgets/sidebar.rs | 8 ++ tinyharness-ui/src/tui/widgets/status_bar.rs | 1 + 7 files changed, 217 insertions(+), 3 deletions(-) diff --git a/tinyharness-ui/src/tui/app.rs b/tinyharness-ui/src/tui/app.rs index f1ce2b3..960b3c0 100644 --- a/tinyharness-ui/src/tui/app.rs +++ b/tinyharness-ui/src/tui/app.rs @@ -1052,6 +1052,7 @@ impl TuiApp { for col in box_x..box_x + box_width { if let Some(cell) = self.screen.get_mut(row, col) { cell.char = ' '; + cell.wide = false; cell.fg = desc_fg; cell.bg = overlay_bg; cell.style = Style::default(); @@ -1063,30 +1064,35 @@ impl TuiApp { // Top if let Some(cell) = self.screen.get_mut(box_y, box_x) { cell.char = '┌'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } for col in box_x + 1..box_x + box_width - 1 { if let Some(cell) = self.screen.get_mut(box_y, col) { cell.char = '─'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } } if let Some(cell) = self.screen.get_mut(box_y, box_x + box_width - 1) { cell.char = '┐'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } // Bottom if let Some(cell) = self.screen.get_mut(box_y + box_height - 1, box_x) { cell.char = '└'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } for col in box_x + 1..box_x + box_width - 1 { if let Some(cell) = self.screen.get_mut(box_y + box_height - 1, col) { cell.char = '─'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } @@ -1096,6 +1102,7 @@ impl TuiApp { .get_mut(box_y + box_height - 1, box_x + box_width - 1) { cell.char = '┘'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } @@ -1103,11 +1110,13 @@ impl TuiApp { for row in box_y + 1..box_y + box_height - 1 { if let Some(cell) = self.screen.get_mut(row, box_x) { cell.char = '│'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } if let Some(cell) = self.screen.get_mut(row, box_x + box_width - 1) { cell.char = '│'; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; } @@ -1195,6 +1204,7 @@ impl TuiApp { // (one column inside from the right border │) if let Some(cell) = self.screen.get_mut(indicator_y, box_x + box_width - 2) { cell.char = scroll_char; + cell.wide = false; cell.fg = border_fg; cell.bg = overlay_bg; cell.style = Style::default(); diff --git a/tinyharness-ui/src/tui/cell.rs b/tinyharness-ui/src/tui/cell.rs index 917acba..d84e88e 100644 --- a/tinyharness-ui/src/tui/cell.rs +++ b/tinyharness-ui/src/tui/cell.rs @@ -171,12 +171,22 @@ impl Style { /// Each cell stores a character, foreground color, background color, /// and style flags. When the screen is rendered, only cells that changed /// from the previous frame are written to the terminal. +/// +/// For wide (CJK/fullwidth) characters that occupy 2 columns, the first +/// column holds the character and `wide` is false. The second column +/// is a "continuation" cell with `wide = true`, which tells the renderer +/// not to write it separately (the terminal already rendered it as part +/// of the wide character). #[derive(Clone, Debug, PartialEq)] pub struct Cell { pub char: char, pub fg: Color, pub bg: Color, pub style: Style, + /// Whether this cell is the continuation (second column) of a wide + /// character. Continuation cells are skipped during rendering because + /// the terminal already drew them as part of the wide character. + pub wide: bool, } impl Default for Cell { @@ -186,6 +196,7 @@ impl Default for Cell { fg: Color::Default, bg: Color::Default, style: Style::default(), + wide: false, } } } @@ -206,6 +217,20 @@ impl Cell { fg, bg, style, + wide: false, + } + } + + /// Create a continuation cell for a wide character's second column. + /// These cells are skipped during rendering since the terminal + /// already drew them as part of the wide character. + pub fn wide_continuation(fg: Color, bg: Color, style: Style) -> Self { + Cell { + char: ' ', + fg, + bg, + style, + wide: true, } } } diff --git a/tinyharness-ui/src/tui/screen.rs b/tinyharness-ui/src/tui/screen.rs index c4d8fb8..79957b3 100644 --- a/tinyharness-ui/src/tui/screen.rs +++ b/tinyharness-ui/src/tui/screen.rs @@ -112,7 +112,9 @@ impl Screen { /// /// Characters that exceed the screen width are truncated. Each character /// is placed according to its Unicode display width; zero-width chars - /// (e.g. combining marks) overwrite the previous cell. + /// (e.g. combining marks) overwrite the previous cell. Wide (CJK/ + /// fullwidth) characters that occupy 2 columns get a continuation + /// cell marked at `col+1` so the renderer can skip it. pub fn write_str( &mut self, row: u16, @@ -140,8 +142,12 @@ impl Screen { fg, bg, style, + wide: false, }, ); + if width > 1 && c + 1 < self.width { + self.set_cell(row, c + 1, Cell::wide_continuation(fg, bg, style)); + } c += width as u16; } } @@ -192,8 +198,12 @@ impl Screen { fg, bg, style, + wide: false, }, ); + if width > 1 && col + 1 < self.width { + self.set_cell(row, col + 1, Cell::wide_continuation(fg, bg, style)); + } col += width_u16; } @@ -254,8 +264,12 @@ impl Screen { fg, bg, style, + wide: false, }, ); + if width > 1 && col + 1 < self.width { + self.set_cell(row, col + 1, Cell::wide_continuation(fg, bg, style)); + } col += width_u16; } @@ -331,8 +345,12 @@ impl Screen { fg, bg, style, + wide: false, }, ); + if width > 1 && col + 1 < self.width { + self.set_cell(screen_row, col + 1, Cell::wide_continuation(fg, bg, style)); + } } col += width_u16; @@ -369,6 +387,7 @@ impl Screen { fg, bg, style: Style::default(), + wide: false, }, ); } @@ -393,6 +412,7 @@ impl Screen { fg, bg, style: Style::default(), + wide: false, }, ); } @@ -418,6 +438,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); self.set_cell( @@ -428,6 +449,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); self.set_cell( @@ -438,6 +460,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); self.set_cell( @@ -448,6 +471,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); @@ -461,6 +485,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); self.set_cell( @@ -471,6 +496,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); } @@ -485,6 +511,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); self.set_cell( @@ -495,6 +522,7 @@ impl Screen { fg, bg, style, + wide: false, }, ); } @@ -556,7 +584,12 @@ impl Screen { /// /// This is the core of the efficient rendering: we only write cells /// that actually changed, and we batch cursor movements. + /// + /// Handles wide (CJK/fullwidth) characters correctly by skipping + /// continuation cells and tracking display width for cursor position. pub fn render_diff(ops: &[DiffOp], width: u16) -> String { + use unicode_width::UnicodeWidthChar; + if ops.is_empty() { return String::new(); } @@ -568,6 +601,12 @@ impl Screen { for op in ops { match op { DiffOp::SetCell { row, col, cell } => { + // Skip continuation cells — they're rendered as part of + // the wide character in the preceding column + if cell.wide { + continue; + } + // Move cursor if needed let need_move = last_row != Some(*row) || last_col.unwrap_or(0) + 1 != *col; @@ -584,12 +623,14 @@ impl Screen { // Write character output.push(cell.char); + // Track cursor position accounting for display width + let char_width = cell.char.width().unwrap_or(1).max(1) as u16; last_row = Some(*row); - last_col = Some(*col + 1); + last_col = Some(*col + char_width); // If we're at the right edge, the cursor won't advance // further, so we need to move it explicitly next time - if *col + 1 >= width { + if *col + char_width >= width { last_col = None; } } @@ -863,4 +904,88 @@ mod tests { assert_eq!(s.get(0, 0).unwrap().char, 'C'); assert_eq!(s.get(0, 1).unwrap().char, 'D'); } + + #[test] + fn test_screen_write_str_wide_char() { + // Wide (CJK) characters should occupy 2 columns and mark continuation cell + let mut s = Screen::new(10, 3); + // '一' is a CJK character with display width 2 + s.write_str(0, 0, "一x", Color::Default, Color::Default, Style::new()); + + // The wide char should be at col 0 + let cell_0 = s.get(0, 0).unwrap(); + assert_eq!(cell_0.char, '一'); + assert!(!cell_0.wide); + + // The continuation cell should be at col 1 + let cell_1 = s.get(0, 1).unwrap(); + assert!(cell_1.wide); + + // 'x' should be at col 2 (not col 1) + let cell_2 = s.get(0, 2).unwrap(); + assert_eq!(cell_2.char, 'x'); + assert!(!cell_2.wide); + } + + #[test] + fn test_screen_write_str_wide_char_at_edge() { + // Wide char at the right edge should not overflow + let mut s = Screen::new(3, 1); + s.write_str(0, 0, "一", Color::Default, Color::Default, Style::new()); + + // '一' takes cols 0-1, which fits in width 3 + assert_eq!(s.get(0, 0).unwrap().char, '一'); + assert!(s.get(0, 1).unwrap().wide); + assert_eq!(s.get(0, 2).unwrap().char, ' '); // empty + } + + #[test] + fn test_cell_default_not_wide() { + let cell = Cell::default(); + assert!(!cell.wide); + assert_eq!(cell.char, ' '); + } + + #[test] + fn test_cell_wide_continuation() { + let cell = Cell::wide_continuation(Color::RED, Color::BLUE, Style::bold()); + assert!(cell.wide); + assert_eq!(cell.char, ' '); + assert_eq!(cell.fg, Color::RED); + assert_eq!(cell.bg, Color::BLUE); + assert!(cell.style.bold); + } + + #[test] + fn test_screen_diff_wide_char_tracking() { + // When a wide char changes, the continuation cell should also be + // included in the diff so it gets properly updated. + let mut s1 = Screen::new(10, 1); + let mut s2 = Screen::new(10, 1); + s1.write_str(0, 0, "AB", Color::Default, Color::Default, Style::new()); + s2.write_str(0, 0, "一x", Color::Default, Color::Default, Style::new()); + + let diff = s2.diff_from(&s1); + + // Should have diffs for: col 0 (一 replaces A), col 1 (continuation replaces B), col 2 (x replaces nothing) + // At minimum, cols 0 and 1 must differ + assert!(diff.len() >= 2); + + // Check that col 0 has the wide char + let col0_op = diff.iter().find(|op| matches!(op, DiffOp::SetCell { col: 0, .. })); + assert!(col0_op.is_some()); + let DiffOp::SetCell { cell: cell0, .. } = col0_op.unwrap() else { + panic!("expected SetCell"); + }; + assert_eq!(cell0.char, '一'); + assert!(!cell0.wide); + + // Check that col 1 has the continuation marker + let col1_op = diff.iter().find(|op| matches!(op, DiffOp::SetCell { col: 1, .. })); + assert!(col1_op.is_some()); + let DiffOp::SetCell { cell: cell1, .. } = col1_op.unwrap() else { + panic!("expected SetCell"); + }; + assert!(cell1.wide); + } } diff --git a/tinyharness-ui/src/tui/widgets/conversation.rs b/tinyharness-ui/src/tui/widgets/conversation.rs index 0053e46..ca61693 100644 --- a/tinyharness-ui/src/tui/widgets/conversation.rs +++ b/tinyharness-ui/src/tui/widgets/conversation.rs @@ -978,6 +978,7 @@ impl ConversationWidget { if let Some(cell) = screen.get_mut(row, col) { cell.bg = bg; cell.char = ' '; + cell.wide = false; } } @@ -997,6 +998,7 @@ impl ConversationWidget { if let Some(cell) = screen.get_mut(row, col) { cell.bg = styles::STATUS_BAR_BG; cell.char = ' '; + cell.wide = false; } } @@ -1253,6 +1255,7 @@ impl ConversationWidget { for row in area.y..area.y + area.height { if let Some(cell) = screen.get_mut(row, x) { cell.char = '│'; + cell.wide = false; cell.fg = styles::SCROLLBAR_FG; cell.bg = styles::SCROLLBAR_BG; } @@ -1263,6 +1266,7 @@ impl ConversationWidget { if row < area.y + area.height { if let Some(cell) = screen.get_mut(row, x) { cell.char = '█'; + cell.wide = false; cell.fg = styles::SCROLLBAR_FG; } } diff --git a/tinyharness-ui/src/tui/widgets/input_bar.rs b/tinyharness-ui/src/tui/widgets/input_bar.rs index 85ff96b..d72d417 100644 --- a/tinyharness-ui/src/tui/widgets/input_bar.rs +++ b/tinyharness-ui/src/tui/widgets/input_bar.rs @@ -477,6 +477,7 @@ impl Widget for InputBarWidget { if self.focused && col < area.x + area.width { if let Some(cell) = screen.get_mut(input_row, col) { cell.char = '█'; + cell.wide = false; cell.fg = Color::YELLOW; cell.style = Style::blink(); } @@ -700,6 +701,45 @@ fn render_wrapped_input( let prompt_width = first_col.saturating_sub(area_x); let rows = input_rows(content, prompt_width, area_x, available_width); + if rows.is_empty() { + // Empty content: draw the cursor at the prompt end on the first row, + // then fill the rest with background. We must start clearing at + // first_col (after the prompt) on the first row so we don't + // overwrite the prompt label that was already drawn. + if focused { + if let Some(cell) = screen.get_mut(start_row, first_col) { + cell.style.underline = true; + cell.bg = styles::INPUT_BAR_BG; + } + } + + // Fill from first_col on the first row (preserving prompt area), + // then from area_x on subsequent rows. + let prompt_end = first_col + prompt_width.min(available_width); + let content_width = available_width.saturating_sub(prompt_width.min(available_width)); + for c in prompt_end..prompt_end + content_width { + if let Some(cell) = screen.get_mut(start_row, c) { + cell.char = ' '; + cell.wide = false; + cell.fg = Color::Default; + cell.bg = styles::INPUT_BAR_BG; + cell.style = Style::default(); + } + } + for r in (start_row + 1)..=max_row { + for c in area_x..area_x + available_width { + if let Some(cell) = screen.get_mut(r, c) { + cell.char = ' '; + cell.wide = false; + cell.fg = Color::Default; + cell.bg = styles::INPUT_BAR_BG; + cell.style = Style::default(); + } + } + } + return; + } + for (i, row) in rows.iter().enumerate() { let screen_row = start_row + i as u16; if screen_row > max_row { @@ -744,6 +784,7 @@ fn render_wrapped_input( for c in area_x..area_x + available_width { if let Some(cell) = screen.get_mut(r, c) { cell.char = ' '; + cell.wide = false; cell.fg = Color::Default; cell.bg = styles::INPUT_BAR_BG; cell.style = Style::default(); diff --git a/tinyharness-ui/src/tui/widgets/sidebar.rs b/tinyharness-ui/src/tui/widgets/sidebar.rs index 70b91d7..71ccb7c 100644 --- a/tinyharness-ui/src/tui/widgets/sidebar.rs +++ b/tinyharness-ui/src/tui/widgets/sidebar.rs @@ -396,6 +396,7 @@ impl Widget for SidebarWidget { fg: styles::SIDEBAR_FG, bg: styles::SIDEBAR_BG, style: Style::default(), + wide: false, }, ); @@ -617,6 +618,7 @@ impl Widget for SidebarWidget { for col in end_col..right_bound { if let Some(cell) = screen.get_mut(screen_row, col) { cell.char = ' '; + cell.wide = false; cell.fg = styles::SIDEBAR_FG; cell.bg = styles::SIDEBAR_BG; cell.style = Style::default(); @@ -642,6 +644,7 @@ impl Widget for SidebarWidget { for col in 0..area.width.saturating_sub(2) { if let Some(cell) = screen.get_mut(screen_row, area.x + 1 + col) { cell.char = ' '; + cell.wide = false; cell.fg = sel_fg; cell.bg = sel_bg; cell.style = Style::default(); @@ -677,6 +680,7 @@ impl Widget for SidebarWidget { for col in end_col..right_bound { if let Some(cell) = screen.get_mut(screen_row, col) { cell.char = ' '; + cell.wide = false; cell.fg = styles::SIDEBAR_FG; cell.bg = styles::SIDEBAR_BG; cell.style = Style::default(); @@ -700,6 +704,7 @@ impl Widget for SidebarWidget { for col in end_col..right_bound { if let Some(cell) = screen.get_mut(screen_row, col) { cell.char = ' '; + cell.wide = false; cell.fg = styles::SIDEBAR_FG; cell.bg = styles::SIDEBAR_BG; cell.style = Style::default(); @@ -758,6 +763,7 @@ impl Widget for SidebarWidget { for row in sb_top..sb_bottom { if let Some(cell) = screen.get_mut(row, sb_x) { cell.char = '│'; + cell.wide = false; cell.fg = styles::SCROLLBAR_FG; cell.bg = styles::SIDEBAR_BG; } @@ -769,6 +775,7 @@ impl Widget for SidebarWidget { if row < sb_bottom { if let Some(cell) = screen.get_mut(row, sb_x) { cell.char = '█'; + cell.wide = false; cell.fg = styles::SCROLLBAR_FG; } } @@ -1196,6 +1203,7 @@ impl SidebarWidget { for c in end_col..right_bound { if let Some(cell) = screen.get_mut(row, c) { cell.char = ' '; + cell.wide = false; cell.fg = styles::SIDEBAR_FG; cell.bg = styles::SIDEBAR_BG; cell.style = Style::default(); diff --git a/tinyharness-ui/src/tui/widgets/status_bar.rs b/tinyharness-ui/src/tui/widgets/status_bar.rs index dd21b18..020a5f4 100644 --- a/tinyharness-ui/src/tui/widgets/status_bar.rs +++ b/tinyharness-ui/src/tui/widgets/status_bar.rs @@ -131,6 +131,7 @@ impl Widget for StatusBarWidget { fg: styles::STATUS_BAR_FG, bg: styles::STATUS_BAR_BG, style: Style::default(), + wide: false, }, ); From 4fc15eaec2ef7f79c896459474dc399c6c45fa9c Mon Sep 17 00:00:00 2001 From: PTFOPlayer Date: Sun, 14 Jun 2026 16:36:14 +0200 Subject: [PATCH 3/5] fix(tui): stairs/gap bug after confirm prompts and wrapping mismatches Three related bugs fixed: 1. ConfirmPrompt line_height overcounted: each diff line was counted using line_height_for_text (which accounts for wrapping), but the actual rendering truncates each diff line to 1 row. Changed to count each diff line as exactly 1 row, matching the rendering behavior. 2. Question rendering ignored wrapped row tracking: write_str_wrapped_clipped returns the last row written, but the code only did current_row += 1. If a question or answer wrapped to multiple lines, subsequent items would overlap. Now uses the return value to advance current_row correctly. 3. left_margin vs wrap_indent mismatch: User, Assistant, and System text rendering used left_margin=area.x (wrapping to left edge) but line_height_for_text used wrap_indent equal to the prefix width. This caused line_height to overestimate when text wrapped. Fixed all rendering calls to use left_margin = area.x + prefix_width, matching the wrap_indent used in height calculation. Also added 3 new tests: - test_confirm_prompt_line_height_matches_rendering - test_confirm_prompt_does_not_create_stairs - test_question_wrapping_height_matches --- tinyharness-ui/src/tui/screen.rs | 8 +- .../src/tui/widgets/conversation.rs | 694 ++++++++++++++++-- 2 files changed, 644 insertions(+), 58 deletions(-) diff --git a/tinyharness-ui/src/tui/screen.rs b/tinyharness-ui/src/tui/screen.rs index 79957b3..3a51b76 100644 --- a/tinyharness-ui/src/tui/screen.rs +++ b/tinyharness-ui/src/tui/screen.rs @@ -972,7 +972,9 @@ mod tests { assert!(diff.len() >= 2); // Check that col 0 has the wide char - let col0_op = diff.iter().find(|op| matches!(op, DiffOp::SetCell { col: 0, .. })); + let col0_op = diff + .iter() + .find(|op| matches!(op, DiffOp::SetCell { col: 0, .. })); assert!(col0_op.is_some()); let DiffOp::SetCell { cell: cell0, .. } = col0_op.unwrap() else { panic!("expected SetCell"); @@ -981,7 +983,9 @@ mod tests { assert!(!cell0.wide); // Check that col 1 has the continuation marker - let col1_op = diff.iter().find(|op| matches!(op, DiffOp::SetCell { col: 1, .. })); + let col1_op = diff + .iter() + .find(|op| matches!(op, DiffOp::SetCell { col: 1, .. })); assert!(col1_op.is_some()); let DiffOp::SetCell { cell: cell1, .. } = col1_op.unwrap() else { panic!("expected SetCell"); diff --git a/tinyharness-ui/src/tui/widgets/conversation.rs b/tinyharness-ui/src/tui/widgets/conversation.rs index ca61693..6ba52b7 100644 --- a/tinyharness-ui/src/tui/widgets/conversation.rs +++ b/tinyharness-ui/src/tui/widgets/conversation.rs @@ -284,24 +284,34 @@ impl ConversationWidget { ConversationLine::Thinking { text } => (text, 13), ConversationLine::ToolResult { content, .. } => { // For tool results, trim trailing blank lines and limit to - // a reasonable number of visible lines + // 20 visible lines. Each content line is truncated (not wrapped) + // in the rendering, so each takes exactly 1 visual row. let trimmed = content.trim_end_matches('\n'); - let lines: Vec<&str> = trimmed.lines().collect(); - // Show at most 20 lines for a tool result - let visible_lines = lines.iter().copied().take(20).collect::>(); - let joined = visible_lines.join("\n"); - return self.line_height_for_text(&joined, area_width, 4, 4).max(1); + let line_count = trimmed.lines().count(); + return line_count.clamp(1, 20); } ConversationLine::ToolCall { name, args_summary } => { if args_summary.is_empty() { return 1; } let header = format!(" ── {}", name); - let header_len = header.len(); - return self.line_height_for_text(args_summary, area_width, header_len, header_len); + // Use display width (not byte length) for column calculation + let header_display_width = header.width(); + return self.line_height_for_text( + args_summary, + area_width, + header_display_width, + header_display_width, + ); } ConversationLine::Question { question, answers } => { - let question_rows = self.line_height_for_text(question, area_width, 4, 4); + let question_prefix_width = " ❓ ".width(); + let question_rows = self.line_height_for_text( + question, + area_width, + question_prefix_width, + question_prefix_width, + ); let answer_rows: usize = answers .iter() .map(|a| { @@ -317,9 +327,9 @@ impl ConversationWidget { if let Some(diff) = diff_preview && !diff.is_empty() { - for line in diff.lines() { - rows += self.line_height_for_text(line, area_width, 4, 4); - } + // Each diff line is truncated (not wrapped) in rendering, + // so each takes exactly 1 visual row. + rows += diff.lines().count(); } return rows; } @@ -419,6 +429,7 @@ impl ConversationWidget { match line { ConversationLine::User { text } => { let prefix = " You: "; + let prefix_width = prefix.len() as u16; if skip_top == 0 && start_row <= max_row { screen.write_str( start_row, @@ -430,24 +441,24 @@ impl ConversationWidget { ); screen.write_str_wrapped_clipped( start_row, - area.x + prefix.len() as u16, + area.x + prefix_width, text, Color::GREEN, Color::Default, Style::default(), - area.x, + area.x + prefix_width, effective_max_row, wrap_col, ); } else if skip_top > 0 { screen.write_str_wrapped_skip_clipped( start_row, - area.x + prefix.len() as u16, + area.x + prefix_width, text, Color::GREEN, Color::Default, Style::default(), - area.x, + area.x + prefix_width, effective_max_row, wrap_col, skip_top, @@ -456,6 +467,7 @@ impl ConversationWidget { } ConversationLine::Assistant { text } => { let prefix = " AI: "; + let prefix_width = prefix.len() as u16; if skip_top == 0 && start_row <= max_row { screen.write_str( start_row, @@ -467,24 +479,24 @@ impl ConversationWidget { ); screen.write_str_wrapped_clipped( start_row, - area.x + prefix.len() as u16, + area.x + prefix_width, text, Color::WHITE, Color::Default, Style::default(), - area.x, + area.x + prefix_width, effective_max_row, wrap_col, ); } else if skip_top > 0 { screen.write_str_wrapped_skip_clipped( start_row, - area.x + prefix.len() as u16, + area.x + prefix_width, text, Color::WHITE, Color::Default, Style::default(), - area.x, + area.x + prefix_width, effective_max_row, wrap_col, skip_top, @@ -493,6 +505,8 @@ impl ConversationWidget { } ConversationLine::ToolCall { name, args_summary } => { let header = format!(" ── {}", name); + // Use display width (not byte length) for column calculation + let header_display_width = header.width(); if skip_top == 0 && start_row <= max_row { screen.write_str( start_row, @@ -503,7 +517,7 @@ impl ConversationWidget { Style::default(), ); if !args_summary.is_empty() { - let args_indent = area.x + header.len() as u16; + let args_indent = area.x + header_display_width as u16; screen.write_str_wrapped_clipped( start_row, args_indent, @@ -517,8 +531,7 @@ impl ConversationWidget { ); } } else if skip_top > 0 && !args_summary.is_empty() { - let header = format!(" ── {}", name); - let args_indent = area.x + header.len() as u16; + let args_indent = area.x + header_display_width as u16; screen.write_str_wrapped_skip_clipped( start_row, args_indent, @@ -550,33 +563,31 @@ impl ConversationWidget { }; // Trim trailing blank lines and limit to 20 visible lines let trimmed = content.trim_end_matches('\n'); - let content_lines: Vec<&str> = trimmed.lines().take(20).collect(); + let all_lines: Vec<&str> = trimmed.lines().collect(); + let total_lines = all_lines.len(); + let visible_lines: Vec<&str> = all_lines.into_iter().take(20).collect(); let mut current_row = start_row; let max_content_width = (area.width as usize).saturating_sub(5); - for (i, content_line) in content_lines.iter().enumerate() { + // Skip the first `skip_top` content lines when scrolling + for (i, content_line) in visible_lines.iter().enumerate() { if i < skip_top { - current_row += 1; continue; } - if current_row > max_row { + if current_row > effective_max_row { break; } // Detect diff lines and apply appropriate colors with backgrounds let (line_color, line_bg, line_prefix) = if !is_error { if content_line.starts_with("@@") { - // Hunk header: @@ -1,3 +1,3 @@ (Color::CYAN, Color::Default, " │ ") } else if content_line.starts_with("---") || content_line.starts_with("+++") { - // File header: --- a/file.rs or +++ b/file.rs (Color::WHITE, Color::Default, " │ ") } else if content_line.starts_with("-") { - // Removed line — red text with dark red background (Color::RED, Color::Ansi(52), " │ ") } else if content_line.starts_with("+") { - // Added line — green text with dark green background (Color::GREEN, Color::Ansi(22), " │ ") } else { (default_color, bg, " │ ") @@ -624,8 +635,7 @@ impl ConversationWidget { current_row += 1; } // If content was truncated, show a truncation indicator - let total_lines = trimmed.lines().count(); - if total_lines > 20 && skip_top == 0 && current_row <= max_row { + if total_lines > 20 && skip_top <= 20 && current_row <= effective_max_row { let truncation = " │ …"; let trunc_bg = if *is_error { bg } else { Color::Default }; screen.write_str( @@ -647,6 +657,7 @@ impl ConversationWidget { } } ConversationLine::System { text } => { + let prefix_width = 2u16; if skip_top == 0 && start_row <= max_row { screen.write_str( start_row, @@ -658,24 +669,24 @@ impl ConversationWidget { ); screen.write_str_wrapped_clipped( start_row, - area.x + 2, + area.x + prefix_width, text, Color::YELLOW, Color::Default, Style::default(), - area.x, + area.x + prefix_width, effective_max_row, wrap_col, ); } else if skip_top > 0 { screen.write_str_wrapped_skip_clipped( start_row, - area.x + 2, + area.x + prefix_width, text, Color::YELLOW, Color::Default, Style::default(), - area.x, + area.x + prefix_width, effective_max_row, wrap_col, skip_top, @@ -751,9 +762,10 @@ impl ConversationWidget { let mut lines_remaining = rows_available; // Render the prompt line - if skip_top == 0 && current_row <= max_row && lines_remaining > 0 { + if skip_top == 0 && current_row <= effective_max_row && lines_remaining > 0 { let prompt = format!(" ⚠ Confirm {} {}", name, args_summary); let suffix = " [y/n/a]?"; + let prompt_width = prompt.width(); screen.write_str( current_row, area.x, @@ -763,7 +775,7 @@ impl ConversationWidget { Style::bold(), ); let prompt_end = - area.x + prompt.len().min(area.width as usize - suffix.len()) as u16; + area.x + prompt_width.min(area.width as usize - suffix.width()) as u16; screen.write_str( current_row, prompt_end, @@ -782,7 +794,7 @@ impl ConversationWidget { { let max_content_width = (area.width as usize).saturating_sub(5); for line in diff.lines() { - if lines_remaining == 0 || current_row > max_row { + if lines_remaining == 0 || current_row > effective_max_row { break; } @@ -845,6 +857,7 @@ impl ConversationWidget { ConversationLine::Question { question, answers } => { // Question header: " ❓ " let question_prefix = " ❓ "; + let question_prefix_width = question_prefix.width(); let answer_prefix_len = if answers.len() >= 10 { 7 } else { 6 }; let mut current_row = start_row; let mut lines_remaining = rows_available; @@ -859,26 +872,28 @@ impl ConversationWidget { Color::Default, Style::bold(), ); - screen.write_str_wrapped_clipped( + let last_row = screen.write_str_wrapped_clipped( current_row, - area.x + question_prefix.len() as u16, + area.x + question_prefix_width as u16, question, Color::YELLOW, Color::Default, Style::default(), - area.x + question_prefix.len() as u16, + area.x + question_prefix_width as u16, effective_max_row, wrap_col, ); - current_row += 1; - lines_remaining -= 1; + current_row = last_row + 1; + lines_remaining = lines_remaining.saturating_sub( + (last_row - start_row + 1) as usize + ); } else if skip_top > 0 { // Skip question lines let q_height = self.line_height_for_text( question, area.width, - question_prefix.len(), - question_prefix.len(), + question_prefix_width, + question_prefix_width, ); if skip_top >= q_height { // Question entirely skipped; adjust for remaining skip @@ -888,6 +903,7 @@ impl ConversationWidget { // Render answer lines: " N. " for (i, answer) in answers.iter().enumerate() { let answer_label = format!(" {}. ", i + 1); + let answer_label_width = answer_label.width(); let a_height = self.line_height_for_text( answer, area.width, @@ -901,7 +917,7 @@ impl ConversationWidget { // This answer is entirely skipped } else { // Partially visible — render with skip - if current_row <= max_row && lines_remaining > 0 { + if current_row <= effective_max_row && lines_remaining > 0 { screen.write_str( current_row, area.x, @@ -912,19 +928,19 @@ impl ConversationWidget { ); screen.write_str_wrapped_skip_clipped( current_row, - area.x + answer_label.len() as u16, + area.x + answer_label_width as u16, answer, Color::WHITE, Color::Default, Style::default(), - area.x + answer_label.len() as u16, + area.x + answer_label_width as u16, effective_max_row, wrap_col, skip_top, ); } } - } else if current_row <= max_row && lines_remaining > 0 { + } else if current_row <= effective_max_row && lines_remaining > 0 { // Normal rendering (no skipping) screen.write_str( current_row, @@ -934,19 +950,19 @@ impl ConversationWidget { Color::Default, Style::bold(), ); - screen.write_str_wrapped_clipped( + let last_row = screen.write_str_wrapped_clipped( current_row, - area.x + answer_label.len() as u16, + area.x + answer_label_width as u16, answer, Color::WHITE, Color::Default, Style::default(), - area.x + answer_label.len() as u16, + area.x + answer_label_width as u16, effective_max_row, wrap_col, ); - current_row += 1; - lines_remaining = lines_remaining.saturating_sub(1); + current_row = last_row + 1; + lines_remaining = lines_remaining.saturating_sub(a_height); } } } @@ -1881,4 +1897,570 @@ mod tests { let cell = screen.get(search_bar_row, 1).unwrap(); assert_eq!(cell.char, 'S'); // "Search: " starts with S } + + // ── Overflow / clipping tests ───────────────────────────────────────── + + /// Check that no non-default cells are written outside the render area. + fn check_no_overflow(screen: &Screen, area: Rect) -> Vec<(u16, u16, char)> { + let mut overflows = Vec::new(); + for row in 0..screen.height() { + for col in 0..screen.width() { + // Only check cells outside the render area + if row >= area.y + && row < area.y + area.height + && col >= area.x + && col < area.x + area.width + { + continue; + } + let cell = screen.get(row, col).unwrap(); + if cell.char != ' ' && cell.char != '\0' { + overflows.push((row, col, cell.char)); + } + } + } + overflows + } + + #[test] + fn test_tool_call_does_not_overflow_small_area() { + // Create a small screen (40 cols, 10 rows) + // Conversation gets only 5 rows (rows 0-4), rest is "input area" + let mut screen = Screen::new(40, 10); + let mut conv = ConversationWidget::new(); + + // Push a multiline tool call + conv.push(ConversationLine::ToolCall { + name: "run".to_string(), + args_summary: "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8".to_string(), + }); + + // Render into a small 5-row area + let area = Rect::new(0, 0, 40, 5); + conv.render(area, &mut screen); + + // Check for overflow past the bottom of the area (rows 5-9) + let overflows = check_no_overflow(&screen, area); + assert!( + overflows.is_empty(), + "ToolCall content overflows past area: {:?}", + overflows + ); + } + + #[test] + fn test_tool_result_does_not_overflow_small_area() { + // Create a small screen (40 cols, 10 rows) + let mut screen = Screen::new(40, 10); + let mut conv = ConversationWidget::new(); + + // Push a long tool result + let content: String = (0..15) + .map(|i| format!("output line number {}", i)) + .collect::>() + .join("\n"); + + conv.push(ConversationLine::ToolResult { + name: "run".to_string(), + content, + is_error: false, + }); + + // Render into a small 5-row area + let area = Rect::new(0, 0, 40, 5); + conv.render(area, &mut screen); + + // Check for overflow past the bottom of the area + let overflows = check_no_overflow(&screen, area); + assert!( + overflows.is_empty(), + "ToolResult content overflows past area: {:?}", + overflows + ); + } + + #[test] + fn test_tool_call_wrapping_does_not_overflow() { + // Long args_summary that wraps in a narrow area + let mut screen = Screen::new(30, 10); + let mut conv = ConversationWidget::new(); + + conv.push(ConversationLine::ToolCall { + name: "run".to_string(), + args_summary: + "cargo test --all-features --workspace -- --test-threads=1 2>&1 | head -100" + .to_string(), + }); + + // 4-row area — the wrapping should be clipped, not overflow + let area = Rect::new(0, 0, 30, 4); + conv.render(area, &mut screen); + + let overflows = check_no_overflow(&screen, area); + assert!( + overflows.is_empty(), + "ToolCall wrapping overflows past area: {:?}", + overflows + ); + } + + #[test] + fn test_error_tool_result_does_not_overflow() { + // Error tool result with background fill should not overflow + let mut screen = Screen::new(40, 10); + let mut conv = ConversationWidget::new(); + + conv.push(ConversationLine::ToolResult { + name: "run".to_string(), + content: "error line 1\nerror line 2\nerror line 3\nerror line 4\nerror line 5" + .to_string(), + is_error: true, + }); + + let area = Rect::new(0, 0, 40, 3); + conv.render(area, &mut screen); + + let overflows = check_no_overflow(&screen, area); + assert!( + overflows.is_empty(), + "Error ToolResult overflows past area: {:?}", + overflows + ); + } + + #[test] + fn test_tool_call_line_height_matches_rendering() { + // Verify that line_height for ToolCall matches actual rendered height + let conv = ConversationWidget::new(); + + // Test 1: short args that fit on one line + let line = ConversationLine::ToolCall { + name: "read".to_string(), + args_summary: "file.txt".to_string(), + }; + let height = conv.line_height(&line, 40); + // With header " ── read" (9 display cols) and args "file.txt" (8 cols), + // total is 17 cols which fits in 39 cols (40-1 scrollbar). Height = 1. + assert_eq!( + height, 1, + "Short ToolCall should have height 1, got {}", + height + ); + + // Test 2: args with newlines + let line = ConversationLine::ToolCall { + name: "run".to_string(), + args_summary: "cmd1\ncmd2".to_string(), + }; + let height = conv.line_height(&line, 40); + // " ── run" (9 display cols) + "cmd1" fits on line 1, then wrap to "cmd2" on line 2 + assert_eq!( + height, 2, + "ToolCall with newline should have height 2, got {}", + height + ); + + // Test 3: very long args that wrap + let line = ConversationLine::ToolCall { + name: "run".to_string(), + args_summary: "cargo test --all-features --workspace".to_string(), + }; + let height = conv.line_height(&line, 20); + // In a 20-wide area (19 content cols), " ── run" is 9 cols wide. + // But header.len() = 9 (ASCII only) so args start at col 9. + // Wait — " ── run" where ── is U+2500 (3 bytes each) = " " (2) + "──" (6 bytes, 2 display cols) + " run" (4) = 12 bytes, 8 display cols + // header.len() = 12 (bytes), not 8 (display cols) + // So args start at col 12 (should be col 8), causing wrapping earlier than needed. + // With wrap_col = 19, args start at col 12, leaving only 7 cols for text. + // "cargo test" is 10 chars, so it wraps. + // This is a known byte-vs-display-width bug but height calculation should still be consistent. + assert!( + height >= 2, + "Long ToolCall should wrap to >= 2 rows, got {}", + height + ); + } + + #[test] + fn test_tool_result_line_height_consistency() { + // ToolResult line_height uses line_height_for_text which wraps, + // but actual rendering truncates. Verify the height is at least + // as large as the actual number of rendered rows. + let conv = ConversationWidget::new(); + + // Single-line result + let line = ConversationLine::ToolResult { + name: "run".to_string(), + content: "hello world".to_string(), + is_error: false, + }; + let height = conv.line_height(&line, 40); + assert_eq!(height, 1, "Single-line result should have height 1"); + + // Multi-line result (no wrapping needed at width 40) + let line = ConversationLine::ToolResult { + name: "run".to_string(), + content: "line1\nline2\nline3".to_string(), + is_error: false, + }; + let height = conv.line_height(&line, 40); + assert_eq!(height, 3, "3-line result should have height 3"); + + // Multi-line result with long lines (wrapping in height calculation) + // The height should be >= the number of actual rendered lines (which truncate) + let long_line = "a".repeat(100); // Way wider than 40 cols + let line = ConversationLine::ToolResult { + name: "run".to_string(), + content: long_line, + is_error: false, + }; + let height = conv.line_height(&line, 40); + // Since the content is one long line that wraps in height calculation, + // height should be > 1 (it wraps in the height calc even though rendering truncates) + assert!( + height >= 1, + "Long single-line result height should be >= 1, got {}", + height + ); + // Note: This reveals a height overestimation bug — the height calculation + // wraps long lines, but rendering truncates them. This means line_height + // overestimates, causing gaps in the conversation but not overflow. + } + + #[test] + fn test_conversation_tool_call_scroll_clipping() { + // Test that a scrolled conversation with a ToolCall clips correctly + let mut screen = Screen::new(40, 10); + let mut conv = ConversationWidget::new(); + + // Add enough lines to fill the area and require scrolling + for i in 0..20 { + conv.push(ConversationLine::User { + text: format!("Message {}", i), + }); + } + // Add a multiline tool call near the bottom + conv.push(ConversationLine::ToolCall { + name: "run".to_string(), + args_summary: "line1\nline2\nline3\nline4\nline5".to_string(), + }); + conv.push(ConversationLine::ToolResult { + name: "run".to_string(), + content: "output".to_string(), + is_error: false, + }); + + // Render in a 5-row area (rows 0-4) + let area = Rect::new(0, 0, 40, 5); + conv.render(area, &mut screen); + + // Check that rows 5-9 have no non-default content (i.e., no overflow) + let overflows = check_no_overflow(&screen, area); + assert!( + overflows.is_empty(), + "Scrolled conversation with ToolCall overflows: {:?}", + overflows + ); + } + + #[test] + fn test_tool_result_overestimation_does_not_cause_overflow() { + // When line_height overestimates (wrapping in height calc, truncation in render), + // the outer loop should still not cause overflow because screen_row is clamped + // to content_area.bottom(). + let mut screen = Screen::new(40, 10); + let mut conv = ConversationWidget::new(); + + // Add a ToolResult with a very long single line (will be truncated in rendering) + let long_content = "x".repeat(200); + conv.push(ConversationLine::ToolResult { + name: "run".to_string(), + content: long_content, + is_error: false, + }); + + // Render in a 3-row area + let area = Rect::new(0, 0, 40, 3); + conv.render(area, &mut screen); + + // Check that rows 3-9 have no overflow + let overflows = check_no_overflow(&screen, area); + assert!( + overflows.is_empty(), + "ToolResult with long line overflows: {:?}", + overflows + ); + } + + #[test] + fn test_tool_call_with_offset_area_no_overflow() { + // Simulate the actual TUI layout: conversation area starts at row 1 + // (below status bar), with input bar below it. + // Screen: 80x24, status bar at row 0, conversation at rows 1-19, input at 20-23 + let mut screen = Screen::new(80, 24); + + // First, "draw" the input bar area (rows 20-23) with identifiable content + for row in 20u16..24 { + for col in 0..80 { + if let Some(cell) = screen.get_mut(row, col) { + cell.char = 'I'; // I for Input + cell.fg = Color::GREEN; + } + } + } + + let mut conv = ConversationWidget::new(); + + // Add a tool call with multiline args that should fill the conversation area + conv.push(ConversationLine::User { + text: "Please run this command".to_string(), + }); + conv.push(ConversationLine::ToolCall { + name: "run".to_string(), + args_summary: "cargo build --release && cargo test --all-features --workspace 2>&1 | tee build.log".to_string(), + }); + // Add a large tool result + let result_lines: Vec = (0..25) + .map(|i| format!("Compiling crate v{}.{}/{}.0...", i / 10, i % 10, i)) + .collect(); + conv.push(ConversationLine::ToolResult { + name: "run".to_string(), + content: result_lines.join("\n"), + is_error: false, + }); + // Add another tool call after the result + conv.push(ConversationLine::ToolCall { + name: "read".to_string(), + args_summary: "build.log".to_string(), + }); + + // Render conversation in the area rows 1-19 (height 19) + let area = Rect::new(0, 1, 80, 19); + conv.render(area, &mut screen); + + // Check that the input bar area (rows 20-23) is not overwritten + for row in 20u16..24 { + for col in 0..80 { + let cell = screen.get(row, col).unwrap(); + // The input bar cells should still have 'I' character + // (conversation should not have overflowed into them) + assert_eq!( + cell.char, 'I', + "Input bar cell at row={}, col={} was overwritten by conversation (char='{}')", + row, col, cell.char + ); + } + } + } + + #[test] + fn test_tool_result_error_with_bg_fill_no_overflow() { + // Error tool results fill background color across the row width. + // Make sure this doesn't overflow the area. + let mut screen = Screen::new(80, 24); + + // Mark rows below the conversation area + for row in 10u16..24 { + for col in 0..80 { + if let Some(cell) = screen.get_mut(row, col) { + cell.char = 'X'; + } + } + } + + let mut conv = ConversationWidget::new(); + conv.push(ConversationLine::ToolResult { + name: "run".to_string(), + content: "error: could not compile\nerror: build failed\ncargo test failed".to_string(), + is_error: true, + }); + + // Render in a small area (rows 0-9, height 10) + let area = Rect::new(0, 0, 80, 10); + conv.render(area, &mut screen); + + // Check that rows 10-23 still have 'X' (not overwritten) + for row in 10u16..24 { + for col in 0..80 { + let cell = screen.get(row, col).unwrap(); + assert_eq!( + cell.char, 'X', + "Row {} col {} was overwritten (char='{}'), should be 'X'", + row, col, cell.char + ); + } + } + } + + #[test] + fn test_tool_call_multiline_args_render_height() { + // Verify that a ToolCall with multiline args renders correctly + // within the area bounds and that screen_row tracking is accurate. + let mut screen = Screen::new(40, 20); + let mut conv = ConversationWidget::new(); + + // Tool call with newlines in args_summary + conv.push(ConversationLine::ToolCall { + name: "run".to_string(), + args_summary: "cmd1 && cmd2 && cmd3 && cmd4 && cmd5".to_string(), + }); + // Add a marker line after to check screen_row tracking + conv.push(ConversationLine::User { + text: "after tool call".to_string(), + }); + + let area = Rect::new(0, 0, 40, 20); + conv.render(area, &mut screen); + + // Verify something is rendered + assert!( + screen.get(0, 2).unwrap().char != ' ', + "Tool call header should be rendered" + ); + + // The "after tool call" user message should appear after the tool call + // Find where the user message appears + let mut found_after = false; + for row in 1..20 { + for col in 0..40 { + let cell = screen.get(row, col).unwrap(); + if cell.char == 'a' { + // Check if this is "after tool call" + let mut text = String::new(); + for c in col..40 { + if let Some(cell) = screen.get(row, c) { + if cell.char == ' ' { + break; + } + text.push(cell.char); + } + } + if text.starts_with("after") { + found_after = true; + break; + } + } + } + if found_after { + break; + } + } + assert!( + found_after, + "User message 'after tool call' should be rendered" + ); + } + + #[test] + fn test_confirm_prompt_line_height_matches_rendering() { + // Verify that ConfirmPrompt line_height matches actual rendered height. + // This was a bug where line_height used line_height_for_text (which counts wrapping) + // but rendering truncates each diff line to 1 row. + let conv = ConversationWidget::new(); + + // ConfirmPrompt with multi-line diff + let line = ConversationLine::ConfirmPrompt { + name: "edit".to_string(), + args_summary: "src/main.rs".to_string(), + diff_preview: Some( + "@@ -1,3 +1,3 @@\n- old line that is very long and should be wrapped in line_height_for_text\n+ new line\n context line" + .to_string(), + ), + }; + let height = conv.line_height(&line, 40); + // Prompt line (1) + diff lines (4: hunk header, removed, added, context) = 5 + // Each diff line is 1 row because rendering truncates, not wraps. + assert_eq!( + height, 5, + "ConfirmPrompt with 4 diff lines should have height 5 (1 prompt + 4 diff), got {}", + height + ); + } + + #[test] + fn test_confirm_prompt_does_not_create_stairs() { + // The "stairs" bug: ConfirmPrompt line_height overcounts (using line_height_for_text + // which wraps), but rendering truncates. This causes the outer loop's screen_row + // to advance past where content was actually rendered, creating gaps/stairs. + let mut screen = Screen::new(80, 24); + let mut conv = ConversationWidget::new(); + + // Add a ConfirmPrompt with a diff containing long lines + conv.push(ConversationLine::ConfirmPrompt { + name: "edit".to_string(), + args_summary: "src/main.rs".to_string(), + diff_preview: Some( + "@@ -1,3 +1,3 @@\n- old line that is very long and should be truncated in rendering\n+ new line\n context line" + .to_string(), + ), + }); + // Add a user message after — this should appear immediately after the ConfirmPrompt + conv.push(ConversationLine::User { + text: "after confirm".to_string(), + }); + + let area = Rect::new(0, 0, 80, 24); + conv.render(area, &mut screen); + + // Find where the "after confirm" user message starts + let mut found_after_row = None; + for row in 0..24u16 { + for col in 0..80u16 { + let cell = screen.get(row, col).unwrap(); + if cell.char == 'a' { + let mut text = String::new(); + for c in col..80u16 { + if let Some(cell) = screen.get(row, c) { + if cell.char == ' ' { + break; + } + text.push(cell.char); + } + } + if text.starts_with("after") { + found_after_row = Some(row); + break; + } + } + } + if found_after_row.is_some() { + break; + } + } + let after_row = found_after_row.expect("'after confirm' message should be rendered"); + + // ConfirmPrompt should be: 1 prompt line + 4 diff lines = 5 rows + // So "after confirm" should start at row 5 (0-indexed) + assert!( + after_row <= 5, + "After-confirm message should be at row 5 or less (got {}), \ + stairs bug would push it further", + after_row + ); + } + + #[test] + fn test_question_wrapping_height_matches() { + // Verify that Question rendering and line_height are consistent + // when question or answer text wraps. + let mut screen = Screen::new(30, 20); + let mut conv = ConversationWidget::new(); + + // Question with a long question that wraps in narrow area + conv.push(ConversationLine::Question { + question: "What is the best approach for handling very long questions that need to wrap?".to_string(), + answers: vec!["Short answer".to_string()], + }); + + let area = Rect::new(0, 0, 30, 20); + conv.render(area, &mut screen); + + // Check no overflow + let overflows = check_no_overflow(&screen, area); + assert!( + overflows.is_empty(), + "Question with wrapping overflows: {:?}", + overflows + ); + } } From 3f08d76432a53a6c47734cee3183290037972325 Mon Sep 17 00:00:00 2001 From: PTFOPlayer Date: Sun, 14 Jun 2026 18:49:47 +0200 Subject: [PATCH 4/5] Fix ANSI attribute leakage and move thinking indicator to input bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix dim interface and ghost text bugs: add \x1b[0m reset before each cell's style in render_diff() to prevent ANSI attributes (dim, bold, background colors) from leaking into subsequent cells - Move streaming/thinking spinner from conversation area to input bar: - Add streaming state to InputBarWidget with animated spinner frames - Show '⠋ Thinking…' / '⠋ Responding…' in input bar during streaming - Block all keyboard input during streaming except Ctrl+C and Ctrl+D - Remove SpinnerWidget from TuiApp (no longer needed separately) - Clean up unused SpinnerWidget import and field from app.rs --- tinyharness-ui/src/tui/app.rs | 85 +++----------- tinyharness-ui/src/tui/screen.rs | 12 +- .../src/tui/widgets/conversation.rs | 19 +++- tinyharness-ui/src/tui/widgets/input_bar.rs | 105 +++++++++++++++++- 4 files changed, 142 insertions(+), 79 deletions(-) diff --git a/tinyharness-ui/src/tui/app.rs b/tinyharness-ui/src/tui/app.rs index 960b3c0..930dbce 100644 --- a/tinyharness-ui/src/tui/app.rs +++ b/tinyharness-ui/src/tui/app.rs @@ -19,7 +19,6 @@ use super::widget::{Action, Widget}; use super::widgets::conversation::{ContextWarningLevel, ConversationLine, ConversationWidget}; use super::widgets::input_bar::InputBarWidget; use super::widgets::sidebar::SidebarWidget; -use super::widgets::spinner::SpinnerWidget; use super::widgets::status_bar::StatusBarWidget; use super::widgets::tool_output::{ToolOutputWidget, ToolResult, ToolStatus}; @@ -98,7 +97,6 @@ pub struct TuiApp { sidebar: SidebarWidget, input_bar: InputBarWidget, tool_output: ToolOutputWidget, - spinner: SpinnerWidget, // State focus: Focus, @@ -160,7 +158,6 @@ impl TuiApp { sidebar: SidebarWidget::new(), input_bar: InputBarWidget::new("agent", "unknown"), tool_output: ToolOutputWidget::new(), - spinner: SpinnerWidget::new("thinking"), focus: Focus::InputBar, state: TuiState::default(), @@ -324,10 +321,11 @@ impl TuiApp { }); } - /// Set the streaming state (shows/hides spinner). + /// Set the streaming state (shows spinner in input bar, blocks input). pub fn set_streaming(&mut self, streaming: bool) { self.state.streaming = streaming; self.status_bar.set_streaming(streaming); + self.input_bar.set_streaming(streaming); } // ── Layout ─────────────────────────────────────────────────────────── @@ -906,19 +904,6 @@ impl TuiApp { } self.input_bar.render(input_area, &mut self.screen); - // Render spinner if streaming - if self.state.streaming { - // Put spinner in the bottom-right of the conversation area. - // Clip to conv_area bounds so it doesn't overflow into the sidebar. - let spinner_width = 12u16; // frame(1) + space(1) + label up to ~10 chars - let spinner_x = conv_area.x + conv_area.width.saturating_sub(spinner_width); - let spinner_y = conv_area.y + conv_area.height.saturating_sub(1); - // Ensure we don't extend past the conversation area - let actual_width = spinner_width.min(conv_area.right().saturating_sub(spinner_x)); - let spinner_area = Rect::new(spinner_x, spinner_y, actual_width, 1); - self.spinner.render(spinner_area, &mut self.screen); - } - // Render help overlay if visible if self.help_visible { self.render_help_overlay(conv_area); @@ -1213,57 +1198,16 @@ impl TuiApp { } /// Diff the current screen against the previous frame and write changes. + /// + /// Uses `Screen::diff_from` and `Screen::render_diff` which correctly + /// handle wide (CJK/fullwidth) characters and cursor position tracking. fn flush_diff(&mut self) -> io::Result<()> { let width = self.screen.width(); - let height = self.screen.height(); - - let mut last_pos: Option<(u16, u16)> = None; - - for row in 0..height { - for col in 0..width { - let curr = self.screen.get(row, col); - let prev = self.prev_screen.get(row, col); + let ops = self.screen.diff_from(&self.prev_screen); + let output = Screen::render_diff(&ops, width); - if curr != prev { - // Move cursor if not adjacent - if last_pos != Some((row, col.saturating_sub(1))) { - write!(self.terminal, "\x1b[{};{}H", row + 1, col + 1)?; - } - - // Write the cell - let cell = curr.unwrap(); - // Apply foreground color - write!(self.terminal, "{}", cell.fg.fg_escape())?; - // Apply background color - write!(self.terminal, "{}", cell.bg.bg_escape())?; - // Apply style - if cell.style.bold { - write!(self.terminal, "\x1b[1m")?; - } - if cell.style.dim { - write!(self.terminal, "\x1b[2m")?; - } - if cell.style.italic { - write!(self.terminal, "\x1b[3m")?; - } - if cell.style.underline { - write!(self.terminal, "\x1b[4m")?; - } - if cell.style.blink { - write!(self.terminal, "\x1b[5m")?; - } - // Write character - if cell.char != '\0' { - write!(self.terminal, "{}", cell.char)?; - } else { - write!(self.terminal, " ")?; - } - // Reset style - write!(self.terminal, "\x1b[0m")?; - - last_pos = Some((row, col)); - } - } + if !output.is_empty() { + self.terminal.write_all(output.as_bytes())?; } // Swap buffers @@ -1318,7 +1262,7 @@ impl TuiApp { // Update spinner animation if self.state.streaming { - self.spinner.tick(); + self.input_bar.tick_streaming(); } // Render and flush @@ -1441,8 +1385,8 @@ impl TuiApp { self.streaming_text.clear(); self.is_thinking = false; self.thinking_text.clear(); - self.spinner.set_label("Thinking"); - self.spinner.start(); + self.input_bar.set_streaming_label("Thinking"); + self.input_bar.set_streaming(true); self.set_streaming(true); // Don't push a Thinking placeholder here — push it lazily // when the first StreamingThinking event arrives. This avoids @@ -1460,7 +1404,7 @@ impl TuiApp { *t = self.thinking_text.clone(); } self.thinking_text.clear(); - self.spinner.set_label("Responding"); + self.input_bar.set_streaming_label("Responding"); } self.streaming_text.push_str(&text); // Update the last assistant message or add a new one @@ -1513,7 +1457,6 @@ impl TuiApp { self.streaming_text.clear(); } self.is_streaming = false; - self.spinner.stop(); self.set_streaming(false); } TuiAgentEvent::Error(msg) => { @@ -1529,7 +1472,6 @@ impl TuiApp { } self.push_system_message(&format!("⚠ Error: {}", msg)); self.is_streaming = false; - self.spinner.stop(); self.set_streaming(false); } TuiAgentEvent::ToolCall { name, args_summary } => { @@ -1612,7 +1554,6 @@ impl TuiApp { self.thinking_text.clear(); } self.is_streaming = false; - self.spinner.stop(); self.set_streaming(false); } } diff --git a/tinyharness-ui/src/tui/screen.rs b/tinyharness-ui/src/tui/screen.rs index 3a51b76..7875eaa 100644 --- a/tinyharness-ui/src/tui/screen.rs +++ b/tinyharness-ui/src/tui/screen.rs @@ -587,6 +587,10 @@ impl Screen { /// /// Handles wide (CJK/fullwidth) characters correctly by skipping /// continuation cells and tracking display width for cursor position. + /// + /// Each cell's style is applied after a reset to prevent attribute + /// leakage — without this, styles like `dim` or colored backgrounds + /// would "stick" and bleed into subsequent cells. pub fn render_diff(ops: &[DiffOp], width: u16) -> String { use unicode_width::UnicodeWidthChar; @@ -594,7 +598,7 @@ impl Screen { return String::new(); } - let mut output = String::with_capacity(ops.len() * 20); + let mut output = String::with_capacity(ops.len() * 24); let mut last_row: Option = None; let mut last_col: Option = None; @@ -614,6 +618,12 @@ impl Screen { output.push_str(&format!("\x1b[{};{}H", row + 1, col + 1)); } + // Reset terminal attributes before applying this cell's + // style to prevent attribute leakage from previous cells. + // Without this, styles like dim (ESC[2m) or colored + // backgrounds persist and bleed into subsequent cells. + output.push_str("\x1b[0m"); + // Apply style output.push_str(&cell.style.escape()); // Apply foreground color diff --git a/tinyharness-ui/src/tui/widgets/conversation.rs b/tinyharness-ui/src/tui/widgets/conversation.rs index 6ba52b7..008bb14 100644 --- a/tinyharness-ui/src/tui/widgets/conversation.rs +++ b/tinyharness-ui/src/tui/widgets/conversation.rs @@ -286,9 +286,17 @@ impl ConversationWidget { // For tool results, trim trailing blank lines and limit to // 20 visible lines. Each content line is truncated (not wrapped) // in the rendering, so each takes exactly 1 visual row. + // When there are more than 20 lines, a truncation indicator + // row (" │ …") is shown, adding 1 extra visual row. let trimmed = content.trim_end_matches('\n'); let line_count = trimmed.lines().count(); - return line_count.clamp(1, 20); + let visible = line_count.clamp(1, 20); + // Account for the truncation indicator row when content exceeds 20 lines + return if line_count > 20 { + visible + 1 + } else { + visible + }; } ConversationLine::ToolCall { name, args_summary } => { if args_summary.is_empty() { @@ -884,9 +892,8 @@ impl ConversationWidget { wrap_col, ); current_row = last_row + 1; - lines_remaining = lines_remaining.saturating_sub( - (last_row - start_row + 1) as usize - ); + lines_remaining = + lines_remaining.saturating_sub((last_row - start_row + 1) as usize); } else if skip_top > 0 { // Skip question lines let q_height = self.line_height_for_text( @@ -2448,7 +2455,9 @@ mod tests { // Question with a long question that wraps in narrow area conv.push(ConversationLine::Question { - question: "What is the best approach for handling very long questions that need to wrap?".to_string(), + question: + "What is the best approach for handling very long questions that need to wrap?" + .to_string(), answers: vec!["Short answer".to_string()], }); diff --git a/tinyharness-ui/src/tui/widgets/input_bar.rs b/tinyharness-ui/src/tui/widgets/input_bar.rs index d72d417..303ce88 100644 --- a/tinyharness-ui/src/tui/widgets/input_bar.rs +++ b/tinyharness-ui/src/tui/widgets/input_bar.rs @@ -120,6 +120,9 @@ fn input_rows<'a>( rows } +/// Spinner animation frames (Braille patterns). +const STREAMING_FRAMES: &[&str] = &["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; + /// The input bar at the bottom of the screen. /// /// Displays a prompt with mode and model labels, and accepts @@ -128,6 +131,10 @@ fn input_rows<'a>( /// /// In confirmation mode, the input bar shows a `[y/n/a]?` prompt /// and only accepts y (approve), n (deny), or a (approve all) keys. +/// +/// When the model is streaming/thinking, the input bar shows a spinner +/// animation and label, blocking all input except Ctrl+C (interrupt) +/// and Ctrl+D (quit). pub struct InputBarWidget { /// Current input text. content: String, @@ -166,6 +173,12 @@ pub struct InputBarWidget { command_names: Vec, /// Subcommand completions for commands that take arguments. subcommands: HashMap>, + /// Whether the model is currently streaming/thinking. + streaming: bool, + /// The label to show while streaming (e.g., "Thinking", "Responding"). + streaming_label: String, + /// Current spinner frame index (cycles through STREAMING_FRAMES). + streaming_frame: usize, } impl InputBarWidget { @@ -208,6 +221,9 @@ impl InputBarWidget { kill_ring: String::new(), command_names, subcommands, + streaming: false, + streaming_label: String::from("Thinking"), + streaming_frame: 0, } } @@ -254,7 +270,13 @@ impl InputBarWidget { } /// Total visual height of the input content for the given area width. + /// + /// When streaming, always returns 2 (border + spinner row) regardless of + /// content since input is blocked. pub fn content_height(&self, area_width: u16) -> u16 { + if self.streaming { + return 2; // border + spinner row + } let prompt = format!("[{}] ", self.mode_label); let prompt_width = prompt.width() as u16; let available = area_width.saturating_sub(prompt_width).max(1); @@ -303,6 +325,35 @@ impl InputBarWidget { self.questioning } + /// Set the streaming state (model is thinking/responding). + /// + /// When streaming, the input bar shows a spinner animation and label, + /// and blocks all text input. Only Ctrl+C (interrupt) and Ctrl+D (quit) + /// are accepted. + pub fn set_streaming(&mut self, streaming: bool) { + self.streaming = streaming; + if streaming { + self.streaming_frame = 0; + } + } + + /// Check if the input bar is in streaming mode. + pub fn is_streaming(&self) -> bool { + self.streaming + } + + /// Set the streaming label (e.g., "Thinking", "Responding"). + pub fn set_streaming_label(&mut self, label: &str) { + self.streaming_label = label.to_string(); + } + + /// Advance the spinner animation to the next frame. + pub fn tick_streaming(&mut self) { + if self.streaming { + self.streaming_frame = (self.streaming_frame + 1) % STREAMING_FRAMES.len(); + } + } + /// Handle a mouse click on the input bar to position the cursor. /// /// Computes where the user clicked relative to the prompt and text @@ -459,7 +510,37 @@ impl Widget for InputBarWidget { // Draw prompt and input on the next rows let input_row = row + 1; - if self.confirming { + if self.streaming { + // ── Streaming mode: show spinner + label, block input ── + let frame = STREAMING_FRAMES.get(self.streaming_frame).unwrap_or(&"⠋"); + let label = format!("{} {}…", frame, self.streaming_label); + let mut col = area.x; + screen.write_str( + input_row, + col, + &label, + Color::ORANGE, + styles::INPUT_BAR_BG, + Style::dim(), + ); + col += label.width() as u16; + + // Fill the rest with background + for c in col..area.x + area.width { + if let Some(cell) = screen.get_mut(input_row, c) { + cell.bg = styles::INPUT_BAR_BG; + } + } + + // Fill any additional rows with background + for r in (input_row + 1)..area.y + area.height { + for c in area.x..area.x + area.width { + if let Some(cell) = screen.get_mut(r, c) { + cell.bg = styles::INPUT_BAR_BG; + } + } + } + } else if self.confirming { // In confirmation mode, show a yellow prompt asking for y/n/a let confirm_prompt = "[y/n/a]? "; let mut col = area.x; @@ -564,6 +645,28 @@ impl Widget for InputBarWidget { } fn handle_event(&mut self, event: &Event) -> Action { + // In streaming mode, block all input except Ctrl+C and Ctrl+D + if self.streaming { + if let Event::Key(key) = event { + match key { + KeyEvent { + key: Key::Char('c'), + modifiers, + } if modifiers.ctrl => { + return Action::Interrupt; + } + KeyEvent { + key: Key::Char('d'), + modifiers, + } if modifiers.ctrl => { + return Action::Quit; + } + _ => return Action::None, + } + } + return Action::None; + } + // Handle paste events (bracketed paste mode) if let Event::Paste(text) = event { if !self.confirming && !self.questioning { From dd7d24f6b53b484145232f063d7288c30798e39a Mon Sep 17 00:00:00 2001 From: PTFOPlayer Date: Tue, 16 Jun 2026 09:24:43 +0200 Subject: [PATCH 5/5] refactor(tui): unify text wrapping into write_wrapped and simplify help dismiss - Extract WrapConfig struct to consolidate the three duplicated wrapping implementations (write_str_wrapped, write_str_wrapped_clipped, write_str_wrapped_skip_clipped) into a single write_wrapped method. The original methods are now thin convenience wrappers. - Add dismiss_help() helper to replace repeated help_visible/help_scroll reset pairs in the help overlay event handler. - Improve ANSI render_diff to deduplicate style attributes and emit fewer escape sequences by tracking active fg/bg/style state. - Clean up else-if chains in help key handling for clarity. - Add comprehensive unit tests for wrapping, clipping, scrolling, style deduplication, and space preservation. --- tinyharness-ui/src/tui/app.rs | 221 ++++-------- tinyharness-ui/src/tui/screen.rs | 569 ++++++++++++++++++++++++------- 2 files changed, 512 insertions(+), 278 deletions(-) diff --git a/tinyharness-ui/src/tui/app.rs b/tinyharness-ui/src/tui/app.rs index 930dbce..b629181 100644 --- a/tinyharness-ui/src/tui/app.rs +++ b/tinyharness-ui/src/tui/app.rs @@ -10,7 +10,7 @@ use std::time::Duration; use super::TuiAgentEvent; use super::backend::Backend; -use super::cell::Style; +use super::cell::{Cell, Style}; use super::event::{Event, EventParser, Key, KeyEvent, MouseButton, MouseEvent}; use super::layout::{Constraint, Direction, Layout, Rect}; use super::screen::Screen; @@ -405,6 +405,12 @@ impl TuiApp { // ── Event handling ──────────────────────────────────────────────────── + /// Dismiss the help overlay (close and reset scroll). + fn dismiss_help(&mut self) { + self.help_visible = false; + self.help_scroll = 0; + } + /// Handle a single event and return any action. fn handle_event(&mut self, event: &Event) -> Action { // If help overlay is visible, handle scrolling and dismiss keys @@ -416,45 +422,39 @@ impl TuiApp { key: Key::Char('h'), modifiers, } if modifiers.ctrl => { - self.help_visible = false; - self.help_scroll = 0; + self.dismiss_help(); return Action::None; } KeyEvent { key: Key::F(1), modifiers, } if !modifiers.ctrl && !modifiers.alt => { - self.help_visible = false; - self.help_scroll = 0; + self.dismiss_help(); return Action::None; } - // Escape closes the overlay (explicit, not via catch-all) + // Escape closes the overlay KeyEvent { key: Key::Escape, .. } => { - self.help_visible = false; - self.help_scroll = 0; + self.dismiss_help(); return Action::None; } - // Ctrl+C passes through as quit/interrupt even when help is open + // Ctrl+C and Ctrl+D pass through (close help first, then fall through) KeyEvent { key: Key::Char('c'), modifiers, } if modifiers.ctrl => { - self.help_visible = false; - self.help_scroll = 0; + self.dismiss_help(); // Fall through to global handler below } - // Ctrl+D passes through as quit even when help is open KeyEvent { key: Key::Char('d'), modifiers, } if modifiers.ctrl => { - self.help_visible = false; - self.help_scroll = 0; + self.dismiss_help(); // Fall through to global handler below } - // Scroll up + // Scroll up (Up arrow or 'k') KeyEvent { key: Key::Up, modifiers, @@ -469,7 +469,7 @@ impl TuiApp { self.help_scroll = self.help_scroll.saturating_sub(1); return Action::None; } - // Scroll down + // Scroll down (Down arrow or 'j') KeyEvent { key: Key::Down, modifiers, @@ -484,26 +484,23 @@ impl TuiApp { self.help_scroll = self.help_scroll.saturating_add(1); return Action::None; } - // Page up + // Page up / Page down / Home / End KeyEvent { key: Key::PageUp, .. } => { self.help_scroll = self.help_scroll.saturating_sub(10); return Action::None; } - // Page down KeyEvent { key: Key::PageDown, .. } => { self.help_scroll = self.help_scroll.saturating_add(10); return Action::None; } - // Home — scroll to top KeyEvent { key: Key::Home, .. } => { self.help_scroll = 0; return Action::None; } - // End — scroll to bottom KeyEvent { key: Key::End, .. } => { let content_height = self.help_content_height(); let max_scroll = self.help_line_count.saturating_sub(content_height); @@ -512,8 +509,7 @@ impl TuiApp { } // Any other key dismisses the overlay _ => { - self.help_visible = false; - self.help_scroll = 0; + self.dismiss_help(); return Action::None; } } @@ -530,7 +526,6 @@ impl TuiApp { modifiers, } if modifiers.ctrl => { if self.state.streaming { - // Interrupt streaming — notify the agent loop self.set_streaming(false); return Action::Interrupt; } @@ -567,13 +562,11 @@ impl TuiApp { modifiers, } if modifiers.ctrl => { self.conversation.toggle_search(); - if self.conversation.is_search_active() { - // Enter conversation focus for search input - self.set_focus(Focus::Conversation); + self.set_focus(if self.conversation.is_search_active() { + Focus::Conversation } else { - // Search closed — return to input bar - self.set_focus(Focus::InputBar); - } + Focus::InputBar + }); return Action::None; } // F1 or Ctrl+H: toggle help overlay @@ -651,36 +644,39 @@ impl TuiApp { // Scroll-related key events go to the focused scrollable widget if let Event::Key(key) = event { + let sidebar_focused = matches!(self.focus, Focus::Sidebar | Focus::Structure); match key { KeyEvent { key: Key::PageUp, .. } => { - match self.focus { - Focus::Sidebar | Focus::Structure => self.sidebar.scroll_up(10), - _ => self.conversation.scroll_up(20), + if sidebar_focused { + self.sidebar.scroll_up(10) + } else { + self.conversation.scroll_up(20) } return Action::None; } KeyEvent { key: Key::PageDown, .. } => { - match self.focus { - Focus::Sidebar | Focus::Structure => self.sidebar.scroll_down(10), - _ => self.conversation.scroll_down(20), + if sidebar_focused { + self.sidebar.scroll_down(10) + } else { + self.conversation.scroll_down(20) } return Action::None; } KeyEvent { key: Key::Home, .. } => { - match self.focus { - Focus::Sidebar | Focus::Structure => self.sidebar.scroll_home(), - _ => self.conversation.scroll_home(), + if sidebar_focused { + self.sidebar.scroll_home() + } else { + self.conversation.scroll_home() } return Action::None; } KeyEvent { key: Key::End, .. } => { - match self.focus { - Focus::Sidebar => { /* sidebar has no scroll-to-bottom */ } - _ => self.conversation.scroll_to_bottom(), + if !sidebar_focused { + self.conversation.scroll_to_bottom() } return Action::None; } @@ -1033,79 +1029,21 @@ impl TuiApp { let dim_fg = Color::Ansi(244); // Draw background fill - for row in box_y..box_y + box_height { - for col in box_x..box_x + box_width { - if let Some(cell) = self.screen.get_mut(row, col) { - cell.char = ' '; - cell.wide = false; - cell.fg = desc_fg; - cell.bg = overlay_bg; - cell.style = Style::default(); - } - } - } + let box_rect = Rect::new(box_x, box_y, box_width, box_height); + self.screen.fill_rect( + box_rect, + Cell { + char: ' ', + fg: desc_fg, + bg: overlay_bg, + style: Style::default(), + wide: false, + }, + ); - // Draw border - // Top - if let Some(cell) = self.screen.get_mut(box_y, box_x) { - cell.char = '┌'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - for col in box_x + 1..box_x + box_width - 1 { - if let Some(cell) = self.screen.get_mut(box_y, col) { - cell.char = '─'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - } - if let Some(cell) = self.screen.get_mut(box_y, box_x + box_width - 1) { - cell.char = '┐'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - // Bottom - if let Some(cell) = self.screen.get_mut(box_y + box_height - 1, box_x) { - cell.char = '└'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - for col in box_x + 1..box_x + box_width - 1 { - if let Some(cell) = self.screen.get_mut(box_y + box_height - 1, col) { - cell.char = '─'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - } - if let Some(cell) = self - .screen - .get_mut(box_y + box_height - 1, box_x + box_width - 1) - { - cell.char = '┘'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - // Sides - for row in box_y + 1..box_y + box_height - 1 { - if let Some(cell) = self.screen.get_mut(row, box_x) { - cell.char = '│'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - if let Some(cell) = self.screen.get_mut(row, box_x + box_width - 1) { - cell.char = '│'; - cell.wide = false; - cell.fg = border_fg; - cell.bg = overlay_bg; - } - } + // Draw border using the screen's draw_box method + self.screen + .draw_box(box_rect, border_fg, overlay_bg, Style::default()); // Draw text lines (scrolled) let content_x = box_x + 1; @@ -1377,6 +1315,18 @@ impl TuiApp { } } + /// Finalize any in-progress thinking block, updating the conversation + /// with the accumulated thinking text and clearing internal state. + fn finalize_thinking(&mut self) { + if self.is_thinking { + self.is_thinking = false; + if let Some(ConversationLine::Thinking { text: t }) = self.conversation.last_mut() { + *t = self.thinking_text.clone(); + } + self.thinking_text.clear(); + } + } + /// Process an agent event received from the background task. fn handle_agent_event(&mut self, event: TuiAgentEvent) { match event { @@ -1395,15 +1345,9 @@ impl TuiApp { } TuiAgentEvent::StreamingText(text) => { // If we were thinking, finalize the thinking block first - if self.is_thinking { - self.is_thinking = false; - // Update the thinking line with accumulated text - if let Some(ConversationLine::Thinking { text: t }) = - self.conversation.last_mut() - { - *t = self.thinking_text.clone(); - } - self.thinking_text.clear(); + let was_thinking = self.is_thinking; + self.finalize_thinking(); + if was_thinking { self.input_bar.set_streaming_label("Responding"); } self.streaming_text.push_str(&text); @@ -1440,16 +1384,7 @@ impl TuiApp { self.conversation.scroll_to_bottom(); } TuiAgentEvent::StreamingDone => { - // Finalize thinking if still active - if self.is_thinking { - self.is_thinking = false; - if let Some(ConversationLine::Thinking { text: t }) = - self.conversation.last_mut() - { - *t = self.thinking_text.clone(); - } - self.thinking_text.clear(); - } + self.finalize_thinking(); // Finalize the streaming text as a complete assistant message if !self.streaming_text.is_empty() { // The streaming text was already being displayed incrementally @@ -1460,16 +1395,7 @@ impl TuiApp { self.set_streaming(false); } TuiAgentEvent::Error(msg) => { - // Finalize thinking if still active - if self.is_thinking { - self.is_thinking = false; - if let Some(ConversationLine::Thinking { text: t }) = - self.conversation.last_mut() - { - *t = self.thinking_text.clone(); - } - self.thinking_text.clear(); - } + self.finalize_thinking(); self.push_system_message(&format!("⚠ Error: {}", msg)); self.is_streaming = false; self.set_streaming(false); @@ -1543,16 +1469,7 @@ impl TuiApp { self.pending_question_answers = answers; } TuiAgentEvent::Done => { - // Finalize thinking if still active - if self.is_thinking { - self.is_thinking = false; - if let Some(ConversationLine::Thinking { text: t }) = - self.conversation.last_mut() - { - *t = self.thinking_text.clone(); - } - self.thinking_text.clear(); - } + self.finalize_thinking(); self.is_streaming = false; self.set_streaming(false); } diff --git a/tinyharness-ui/src/tui/screen.rs b/tinyharness-ui/src/tui/screen.rs index 7875eaa..2391220 100644 --- a/tinyharness-ui/src/tui/screen.rs +++ b/tinyharness-ui/src/tui/screen.rs @@ -11,6 +11,28 @@ use unicode_width::UnicodeWidthChar; use super::cell::{Cell, Color, Style}; use super::layout::Rect; +// ── Wrap configuration ──────────────────────────────────────────────────────── + +/// Configuration for wrapped text rendering. +/// +/// Controls wrapping, clipping, and skipping behavior for the +/// [`Screen::write_wrapped`] method. +struct WrapConfig { + /// Maximum column number; text wraps when `col + char_width > wrap_col`. + wrap_col: u16, + /// Column where wrapped lines start (left margin for continuation lines). + left_margin: u16, + /// Maximum screen row; text stops when `screen_row > max_row`. + max_row: u16, + /// Number of visual rows to skip before rendering (for scroll offset). + skip_rows: usize, + /// If true, don't wrap — truncate at `wrap_col` instead. + no_wrap: bool, + /// If true, use screen-bounded wrapping (old `write_str_wrapped` semantics): + /// wraps at screen width and clips at screen height. + screen_bounded: bool, +} + // ── Screen ────────────────────────────────────────────────────────────────── /// A double-buffered screen of cells. @@ -156,6 +178,9 @@ impl Screen { /// /// If `wrap` is true, text wraps to the next line. If false, text is /// truncated at the right edge. Uses Unicode display widths. + /// + /// This is a convenience wrapper around [`Self::write_wrapped`] with + /// simple wrapping bounded by the screen dimensions. pub fn write_str_wrapped( &mut self, start_row: u16, @@ -166,48 +191,22 @@ impl Screen { style: Style, wrap: bool, ) -> u16 { - let mut row = start_row; - let mut col = start_col; - - for ch in text.chars() { - let width = ch.width().unwrap_or(1); - if width == 0 { - let in_view = col < self.width && row < self.height; - self.merge_combining_mark(row, col, start_col, ch, fg, bg, style, in_view); - continue; - } - let width_u16 = width as u16; - if col + width_u16 > self.width { - if wrap && row + 1 < self.height { - row += 1; - col = 0; - } else { - break; - } - } - if ch == '\n' { - row += 1; - col = 0; - continue; - } - self.set_cell( - row, - col, - Cell { - char: ch, - fg, - bg, - style, - wide: false, - }, - ); - if width > 1 && col + 1 < self.width { - self.set_cell(row, col + 1, Cell::wide_continuation(fg, bg, style)); - } - col += width_u16; - } - - row + self.write_wrapped( + start_row, + start_col, + text, + fg, + bg, + style, + WrapConfig { + wrap_col: self.width, + left_margin: 0, + max_row: self.height.saturating_sub(1), + skip_rows: 0, + no_wrap: !wrap, + screen_bounded: true, + }, + ) } /// Write a string with wrapping, but clip rendering at the given maximum row @@ -216,6 +215,9 @@ impl Screen { /// `wrap_col` is the maximum column number; text wraps when `col >= wrap_col`. /// `max_row` is the maximum row; text stops when `row > max_row`. /// `left_margin` is the column where wrapped lines start. Uses Unicode display widths. + /// + /// This is a convenience wrapper around [`Self::write_wrapped`] with + /// `skip_rows = 0` and wrapping enabled. pub fn write_str_wrapped_clipped( &mut self, start_row: u16, @@ -228,52 +230,22 @@ impl Screen { max_row: u16, wrap_col: u16, ) -> u16 { - let mut row = start_row; - let mut col = start_col; - - for ch in text.chars() { - let width = ch.width().unwrap_or(1); - if width == 0 { - let in_view = row <= max_row; - self.merge_combining_mark(row, col, start_col, ch, fg, bg, style, in_view); - continue; - } - let width_u16 = width as u16; - if col + width_u16 > wrap_col { - // Wrap to next line - row += 1; - col = left_margin; - } - // Stop if we've exceeded the max row - if row > max_row { - break; - } - if ch == '\n' { - row += 1; - col = left_margin; - if row > max_row { - break; - } - continue; - } - self.set_cell( - row, - col, - Cell { - char: ch, - fg, - bg, - style, - wide: false, - }, - ); - if width > 1 && col + 1 < self.width { - self.set_cell(row, col + 1, Cell::wide_continuation(fg, bg, style)); - } - col += width_u16; - } - - row + self.write_wrapped( + start_row, + start_col, + text, + fg, + bg, + style, + WrapConfig { + wrap_col, + left_margin, + max_row, + skip_rows: 0, + no_wrap: false, + screen_bounded: false, + }, + ) } /// Write a string with wrapping, skip the first `skip_rows` visual rows, @@ -283,6 +255,9 @@ impl Screen { /// `skip_rows` is the number of visual rows to skip before rendering. /// `max_row` is the maximum row; text stops when `row > max_row`. /// `left_margin` is the column where wrapped lines start. Uses Unicode display widths. + /// + /// This is a convenience wrapper around [`Self::write_wrapped`] with + /// `skip_rows > 0`. pub fn write_str_wrapped_skip_clipped( &mut self, start_row: u16, @@ -296,6 +271,41 @@ impl Screen { wrap_col: u16, skip_rows: usize, ) { + self.write_wrapped( + start_row, + start_col, + text, + fg, + bg, + style, + WrapConfig { + wrap_col, + left_margin, + max_row, + skip_rows, + no_wrap: false, + screen_bounded: false, + }, + ); + } + + /// Unified wrapped text writing method. + /// + /// Handles all wrapping scenarios through a single [`WrapConfig`] struct. + /// The method tracks both a visual row counter (for skip/clipping) and a + /// screen row counter (for actual cell placement). + /// + /// Returns the final screen row where text ended. + fn write_wrapped( + &mut self, + start_row: u16, + start_col: u16, + text: &str, + fg: Color, + bg: Color, + style: Style, + cfg: WrapConfig, + ) -> u16 { let mut visual_row: usize = 0; let mut col = start_col; let mut screen_row = start_row; @@ -303,40 +313,53 @@ impl Screen { for ch in text.chars() { let width = ch.width().unwrap_or(1); if width == 0 { - let in_view = visual_row >= skip_rows && screen_row <= max_row; + let in_view = if cfg.skip_rows > 0 { + visual_row >= cfg.skip_rows && screen_row <= cfg.max_row + } else if cfg.screen_bounded { + col < self.width && screen_row < self.height + } else { + screen_row <= cfg.max_row + }; self.merge_combining_mark(screen_row, col, start_col, ch, fg, bg, style, in_view); continue; } let width_u16 = width as u16; - // Check if we need to wrap before placing this character - if ch != '\n' && col + width_u16 > wrap_col { - // Wrap to next visual line + + // Handle newline + if ch == '\n' { visual_row += 1; - col = left_margin; - // Only advance screen_row if we're past the renderable zone - if visual_row > skip_rows { + col = cfg.left_margin; + if cfg.skip_rows == 0 || visual_row > cfg.skip_rows { screen_row += 1; } - if screen_row > max_row { + if screen_row > cfg.max_row { break; } + continue; } - if ch == '\n' { - // Newline — advance to next visual row + // Handle wrap + if col + width_u16 > cfg.wrap_col { + if cfg.no_wrap { + break; + } visual_row += 1; - col = left_margin; - if visual_row > skip_rows { + col = cfg.left_margin; + if cfg.skip_rows == 0 || visual_row > cfg.skip_rows { screen_row += 1; } - if screen_row > max_row { + if screen_row > cfg.max_row { + break; + } + // For screen_bounded mode (old write_str_wrapped), check screen height + if cfg.screen_bounded && screen_row >= self.height { break; } - continue; } - // Only write the cell if we're past the skip zone - if visual_row >= skip_rows && screen_row <= max_row { + // Only write the cell if we're past the skip zone and within bounds + let past_skip = cfg.skip_rows == 0 || visual_row >= cfg.skip_rows; + if past_skip && screen_row <= cfg.max_row { self.set_cell( screen_row, col, @@ -355,6 +378,8 @@ impl Screen { col += width_u16; } + + screen_row } /// Fill a rectangular area with the given cell. @@ -588,9 +613,13 @@ impl Screen { /// Handles wide (CJK/fullwidth) characters correctly by skipping /// continuation cells and tracking display width for cursor position. /// - /// Each cell's style is applied after a reset to prevent attribute - /// leakage — without this, styles like `dim` or colored backgrounds - /// would "stick" and bleed into subsequent cells. + /// Optimizations for terminal throughput: + /// - Consecutive cells on the same row skip the cursor move (cursor + /// naturally advances after writing a character). + /// - Style reset (`\x1b[0m`) + re-application is only emitted when the + /// style actually changes from the previous cell, not per-cell. + /// - Cells with default style and colors (typically spaces) skip the + /// style/fg/bg escape sequences entirely. pub fn render_diff(ops: &[DiffOp], width: u16) -> String { use unicode_width::UnicodeWidthChar; @@ -601,6 +630,11 @@ impl Screen { let mut output = String::with_capacity(ops.len() * 24); let mut last_row: Option = None; let mut last_col: Option = None; + // Track the currently active style on the terminal so we only emit + // changes, not a full reset+reapply for every cell. + let mut active_fg: Option = None; + let mut active_bg: Option = None; + let mut active_style: Option