diff --git a/AGENTS.md b/AGENTS.md index 83fd2a3e..697de700 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,15 @@ See `codex-rs/tui/styles.md`. - Avoid churn: don’t refactor between equivalent forms (Span::styled ↔ set_style, Line::from ↔ .into()) without a clear readability or functional gain; follow file‑local conventions and do not introduce type annotations solely to satisfy .into(). - Compactness: prefer the form that stays on one line after rustfmt; if only one of Line::from(vec![…]) or vec![…].into() avoids wrapping, choose that. If both wrap, pick the one with fewer wrapped lines. -## Snapshot tests +### Text wrapping +- Always use textwrap::wrap to wrap plain strings. +- If you have a ratatui Line and you want to wrap it, use the helpers in tui/src/wrapping.rs, e.g. word_wrap_lines / word_wrap_line. +- If you need to indent wrapped lines, use the initial_indent / subsequent_indent options from RtOptions if you can, rather than writing custom logic. +- If you have a list of lines and you need to prefix them all with some prefix (optionally different on the first vs subsequent lines), use the `prefix_lines` helper from line_utils. + +## Tests + +### Snapshot tests This repo uses snapshot tests (via `insta`), especially in `codex-rs/tui`, to validate rendered output. When UI or text output changes intentionally, update the snapshots as follows: @@ -52,3 +60,7 @@ This repo uses snapshot tests (via `insta`), especially in `codex-rs/tui`, to va If you don’t have the tool: - `cargo install cargo-insta` + +### Test assertions + +- Tests should use pretty_assertions::assert_eq for clearer diffs. Import this at the top of the test module if it isn't already. diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 358c58de..6d26ad2e 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -7,7 +7,7 @@ mod apply_patch; pub mod auth; -mod bash; +pub mod bash; mod chat_completions; mod client; mod client_common; diff --git a/codex-rs/tui/src/backtrack_helpers.rs b/codex-rs/tui/src/backtrack_helpers.rs index b006ae38..e2306b80 100644 --- a/codex-rs/tui/src/backtrack_helpers.rs +++ b/codex-rs/tui/src/backtrack_helpers.rs @@ -12,7 +12,7 @@ pub(crate) fn highlight_range_for_nth_last_user( /// Compute the wrapped display-line offset before `header_idx`, for a given width. pub(crate) fn wrapped_offset_before(lines: &[Line<'_>], header_idx: usize, width: u16) -> usize { let before = &lines[0..header_idx]; - crate::insert_history::word_wrap_lines(before, width).len() + crate::wrapping::word_wrap_lines(before, width as usize).len() } /// Find the header index for the Nth last user message in the transcript. diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index cb6ef501..37aa85ff 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -832,25 +832,10 @@ impl TextArea { None => true, }; if needs_recalc { - let mut lines: Vec> = Vec::new(); - for line in textwrap::wrap( + let lines = crate::wrapping::wrap_ranges( &self.text, Options::new(width as usize).wrap_algorithm(textwrap::WrapAlgorithm::FirstFit), - ) - .iter() - { - match line { - std::borrow::Cow::Borrowed(slice) => { - let start = - unsafe { slice.as_ptr().offset_from(self.text.as_ptr()) as usize }; - let end = start + slice.len(); - let trailing_spaces = - self.text[end..].chars().take_while(|c| *c == ' ').count(); - lines.push(start..end + trailing_spaces + 1); - } - std::borrow::Cow::Owned(_) => unreachable!(), - } - } + ); *cache = Some(WrapCache { width, lines }); } } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap index 5d6d5cb4..0b6ec3b3 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap @@ -1,5 +1,6 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 648 expression: visible_after --- > I’m going to scan the workspace and Cargo manifests to see build profiles and @@ -22,9 +23,9 @@ expression: visible_after Main Causes - - Static linking style: Each bin (codex, codex-tui, codex-exec, - codex-mcp-server, etc.) statically links its full dependency graph, so common - code isn’t shared at runtime across executables. + - Static linking style: Each bin (codex, codex-tui, codex-exec, codex-mcp- + server, etc.) statically links its full dependency graph, so common code isn’t + shared at runtime across executables. - Heavy deps (HTTP/TLS): reqwest brings in Hyper, HTTP/2, compressors, and a TLS stack (rustls by default; OpenSSL on musl). In core, login, tui, and ollama you enable reqwest with json/stream, which still pulls a large @@ -39,9 +40,9 @@ expression: visible_after per bin. - Panic + backtraces: Default panic = unwind and backtrace support keep unwinding tables and symbols that add weight. - - Per-target OpenSSL (musl): For *-unknown-linux-musl, core enables - openssl-sys with vendored, compiling OpenSSL into the binary—this adds - multiple megabytes per executable. + - Per-target OpenSSL (musl): For *-unknown-linux-musl, core enables openssl- + sys with vendored, compiling OpenSSL into the binary—this adds multiple + megabytes per executable. Build-Mode Notes @@ -52,6 +53,6 @@ expression: visible_after - Debug builds: cargo build (dev profile) includes full debuginfo, no LTO, and assertions—outputs are much larger than cargo build --release. - If you want, I can outline targeted trims (e.g., strip = "debuginfo", - opt-level = "z", panic abort, tighter tokio/reqwest features) and estimate - impact per binary. + If you want, I can outline targeted trims (e.g., strip = "debuginfo", opt- + level = "z", panic abort, tighter tokio/reqwest features) and estimate impact + per binary. diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index ce048f48..01583958 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -2,9 +2,14 @@ use crate::diff_render::create_diff_summary; use crate::exec_command::relativize_to_home; use crate::exec_command::strip_bash_lc_and_escape; use crate::markdown::append_markdown; +use crate::render::line_utils::line_to_static; use crate::render::line_utils::prefix_lines; +use crate::render::line_utils::push_owned_lines; use crate::slash_command::SlashCommand; use crate::text_formatting::format_and_truncate_tool_result; +use crate::wrapping::RtOptions; +use crate::wrapping::word_wrap_line; +use crate::wrapping::word_wrap_lines; use base64::Engine; use codex_ansi_escape::ansi_escape_line; use codex_common::create_config_summary_entries; @@ -97,8 +102,7 @@ impl HistoryCell for UserHistoryCell { let wrapped = textwrap::wrap( &self.message, textwrap::Options::new(wrap_width as usize) - .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit) // Match textarea wrap - .word_splitter(textwrap::WordSplitter::NoHyphenation), + .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit), // Match textarea wrap ); for line in wrapped { @@ -132,28 +136,16 @@ impl AgentMessageCell { impl HistoryCell for AgentMessageCell { fn display_lines(&self, width: u16) -> Vec> { - let mut out: Vec> = Vec::new(); - // We want: - // - First visual line: "> " prefix (collapse with header logic) - // - All subsequent visual lines: two-space prefix - let mut is_first_visual = true; - let wrap_width = width.saturating_sub(2); // account for prefix - for line in &self.lines { - let wrapped = - crate::insert_history::word_wrap_lines(std::slice::from_ref(line), wrap_width); - for (i, piece) in wrapped.into_iter().enumerate() { - let mut spans = Vec::with_capacity(piece.spans.len() + 1); - spans.push(if is_first_visual && i == 0 && self.is_first_line { + word_wrap_lines( + &self.lines, + RtOptions::new(width as usize) + .initial_indent(if self.is_first_line { "> ".into() } else { " ".into() - }); - spans.extend(piece.spans.into_iter()); - out.push(spans.into()); - } - is_first_visual = false; - } - out + }) + .subsequent_indent(" ".into()), + ) } fn transcript_lines(&self) -> Vec> { @@ -278,13 +270,13 @@ impl ExecCell { } fn exploring_display_lines(&self, width: u16) -> Vec> { - let mut lines: Vec> = Vec::new(); + let mut out: Vec> = Vec::new(); let active_start_time = self .calls .iter() .find(|c| c.output.is_none()) .and_then(|c| c.start_time); - lines.push(Line::from(vec![ + out.push(Line::from(vec![ if self.is_active() { // Show an animated spinner while exploring spinner(active_start_time) @@ -299,7 +291,7 @@ impl ExecCell { }, ])); let mut calls = self.calls.clone(); - let mut first = true; + let mut out_indented = Vec::new(); while !calls.is_empty() { let mut call = calls.remove(0); if call @@ -372,39 +364,24 @@ impl ExecCell { lines }; for (title, line) in call_lines { - let prefix_len = 4 + title.len() + 1; // " └ " + title + " " - let wrapped = crate::insert_history::word_wrap_lines( - &[line.into()], - width.saturating_sub(prefix_len as u16), + let line = Line::from(line); + let initial_indent = Line::from(vec![title.cyan(), " ".into()]); + let subsequent_indent = " ".repeat(initial_indent.width()).into(); + let wrapped = word_wrap_line( + &line, + RtOptions::new(width as usize) + .initial_indent(initial_indent) + .subsequent_indent(subsequent_indent), ); - let mut first_sub = true; - for mut line in wrapped { - let mut spans = Vec::with_capacity(line.spans.len() + 1); - spans.push(if first { - first = false; - " └ ".dim() - } else { - " ".into() - }); - if first_sub { - first_sub = false; - spans.push(title.cyan()); - spans.push(" ".into()); - } else { - spans.push(" ".repeat(title.width() + 1).into()); - } - spans.extend(line.spans.into_iter()); - line.spans = spans; - lines.push(line); - } + push_owned_lines(&wrapped, &mut out_indented); } } - lines + out.extend(prefix_lines(out_indented, " └ ".dim(), " ".into())); + out } fn command_display_lines(&self, width: u16) -> Vec> { use textwrap::Options as TwOptions; - use textwrap::WordSplitter; let mut lines: Vec> = Vec::new(); let [call] = &self.calls.as_slice() else { @@ -424,38 +401,28 @@ impl ExecCell { // "• Running " (including trailing space) as the reserved prefix width. // If the command contains newlines, always use the multi-line variant. let reserved = "• Running ".width(); - let mut branch_consumed = false; - if !cmd_display.contains('\n') - && cmd_display.width() < (width as usize).saturating_sub(reserved) + let mut body_lines: Vec> = Vec::new(); + + let highlighted_lines = crate::render::highlight::highlight_bash_to_lines(&cmd_display); + + if highlighted_lines.len() == 1 + && highlighted_lines[0].width() < (width as usize).saturating_sub(reserved) { - lines.push(Line::from(vec![ - bullet, - " ".into(), - title.bold(), - " ".into(), - cmd_display.clone().into(), - ])); + let mut line = Line::from(vec![bullet, " ".into(), title.bold(), " ".into()]); + line.extend(highlighted_lines[0].clone()); + lines.push(line); } else { - branch_consumed = true; lines.push(vec![bullet, " ".into(), title.bold()].into()); - // Wrap the command line. - for (i, line) in cmd_display.lines().enumerate() { - let wrapped = textwrap::wrap( - line, - TwOptions::new(width as usize) - .initial_indent(" ") - .subsequent_indent(" ") - .word_splitter(WordSplitter::NoHyphenation), - ); - lines.extend(wrapped.into_iter().enumerate().map(|(j, l)| { - if i == 0 && j == 0 { - vec![" └ ".dim(), l[4..].to_string().into()].into() - } else { - l.to_string().into() - } - })); + for hl_line in highlighted_lines.iter() { + let opts = crate::wrapping::RtOptions::new((width as usize).saturating_sub(4)) + .initial_indent("".into()) + .subsequent_indent(" ".into()) + // Hyphenation likes to break words on hyphens, which is bad for bash scripts --because-of-flags. + .word_splitter(textwrap::WordSplitter::NoHyphenation); + let wrapped_borrowed = crate::wrapping::word_wrap_line(hl_line, opts); + body_lines.extend(wrapped_borrowed.iter().map(|l| line_to_static(l))); } } if let Some(output) = call.output.as_ref() @@ -466,25 +433,13 @@ impl ExecCell { .join("\n"); if !out.trim().is_empty() { // Wrap the output. - for (i, line) in out.lines().enumerate() { - let wrapped = textwrap::wrap( - line, - TwOptions::new(width as usize - 4) - .word_splitter(WordSplitter::NoHyphenation), - ); - lines.extend(wrapped.into_iter().map(|l| { - Line::from(vec![ - if i == 0 && !branch_consumed { - " └ ".dim() - } else { - " ".dim() - }, - l.to_string().dim(), - ]) - })); + for line in out.lines() { + let wrapped = textwrap::wrap(line, TwOptions::new(width as usize - 4)); + body_lines.extend(wrapped.into_iter().map(|l| Line::from(l.to_string().dim()))); } } } + lines.extend(prefix_lines(body_lines, " └ ".dim(), " ".into())); lines } } diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 1ecbd344..bffd3f2b 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -3,6 +3,7 @@ use std::io; use std::io::Write; use crate::tui; +use crate::wrapping::word_wrap_lines_borrowed; use crossterm::Command; use crossterm::cursor::MoveTo; use crossterm::queue; @@ -18,8 +19,6 @@ use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::text::Line; use ratatui::text::Span; -use textwrap::Options as TwOptions; -use textwrap::WordSplitter; /// Insert `lines` above the viewport using the terminal's backend writer /// (avoids direct stdout references). @@ -44,7 +43,7 @@ pub fn insert_history_lines_to_writer( // Pre-wrap lines using word-aware wrapping so terminal scrollback sees the same // formatting as the TUI. This avoids character-level hard wrapping by the terminal. - let wrapped = word_wrap_lines(&lines, area.width.max(1)); + let wrapped = word_wrap_lines_borrowed(&lines, area.width.max(1) as usize); let wrapped_lines = wrapped.len() as u16; let cursor_top = if area.bottom() < screen_size.height { // If the viewport is not at the bottom of the screen, scroll it down to make room. @@ -98,7 +97,7 @@ pub fn insert_history_lines_to_writer( for line in wrapped { queue!(writer, Print("\r\n")).ok(); - write_spans(writer, line.iter()).ok(); + write_spans(writer, &line).ok(); } queue!(writer, ResetScrollRegion).ok(); @@ -223,7 +222,7 @@ impl ModifierDiff { fn write_spans<'a, I>(mut writer: &mut impl Write, content: I) -> io::Result<()> where - I: Iterator>, + I: IntoIterator>, { let mut fg = Color::Reset; let mut bg = Color::Reset; @@ -262,129 +261,6 @@ where ) } -/// Word-aware wrapping for a list of `Line`s preserving styles. -pub(crate) fn word_wrap_lines<'a, I>(lines: I, width: u16) -> Vec> -where - I: IntoIterator>, -{ - let mut out = Vec::new(); - let w = width.max(1) as usize; - for line in lines { - out.extend(word_wrap_line(line, w)); - } - out -} - -fn word_wrap_line(line: &Line, width: usize) -> Vec> { - if width == 0 { - return vec![to_owned_line(line)]; - } - // Concatenate content and keep span boundaries for later re-slicing. - let mut flat = String::new(); - let mut span_bounds = Vec::new(); // (start_byte, end_byte, style) - let mut cursor = 0usize; - for s in &line.spans { - let text = s.content.as_ref(); - let start = cursor; - flat.push_str(text); - cursor += text.len(); - span_bounds.push((start, cursor, s.style)); - } - - // Use textwrap for robust word-aware wrapping; no hyphenation, no breaking words. - let opts = TwOptions::new(width) - .break_words(false) - .word_splitter(WordSplitter::NoHyphenation); - let wrapped = textwrap::wrap(&flat, &opts); - - if wrapped.len() <= 1 { - return vec![to_owned_line(line)]; - } - - // Map wrapped pieces back to byte ranges in `flat` sequentially. - let mut start_cursor = 0usize; - let mut out: Vec> = Vec::with_capacity(wrapped.len()); - for piece in wrapped { - let piece_str: &str = &piece; - if piece_str.is_empty() { - out.push(Line { - style: line.style, - alignment: line.alignment, - spans: Vec::new(), - }); - continue; - } - // Find the next occurrence of piece_str at or after start_cursor. - // textwrap preserves order, so a linear scan is sufficient. - if let Some(rel) = flat[start_cursor..].find(piece_str) { - let s = start_cursor + rel; - let e = s + piece_str.len(); - out.push(slice_line_spans(line, &span_bounds, s, e)); - start_cursor = e; - } else { - // Fallback: slice by length from cursor. - let s = start_cursor; - let e = (start_cursor + piece_str.len()).min(flat.len()); - out.push(slice_line_spans(line, &span_bounds, s, e)); - start_cursor = e; - } - } - - out -} - -fn to_owned_line(l: &Line<'_>) -> Line<'static> { - Line { - style: l.style, - alignment: l.alignment, - spans: l - .spans - .iter() - .map(|s| Span { - style: s.style, - content: std::borrow::Cow::Owned(s.content.to_string()), - }) - .collect(), - } -} - -fn slice_line_spans( - original: &Line<'_>, - span_bounds: &[(usize, usize, ratatui::style::Style)], - start_byte: usize, - end_byte: usize, -) -> Line<'static> { - let mut acc: Vec> = Vec::new(); - for (i, (s, e, style)) in span_bounds.iter().enumerate() { - if *e <= start_byte { - continue; - } - if *s >= end_byte { - break; - } - let seg_start = start_byte.max(*s); - let seg_end = end_byte.min(*e); - if seg_end > seg_start { - let local_start = seg_start - *s; - let local_end = seg_end - *s; - let content = original.spans[i].content.as_ref(); - let slice = &content[local_start..local_end]; - acc.push(Span { - style: *style, - content: std::borrow::Cow::Owned(slice.to_string()), - }); - } - if *e >= end_byte { - break; - } - } - Line { - style: original.style, - alignment: original.alignment, - spans: acc, - } -} - #[cfg(test)] mod tests { use super::*; @@ -416,38 +292,4 @@ mod tests { String::from_utf8(expected).unwrap() ); } - - #[test] - fn line_height_counts_double_width_emoji() { - let line = "😀😀😀".into(); // each emoji ~ width 2 - assert_eq!(word_wrap_line(&line, 4).len(), 2); - assert_eq!(word_wrap_line(&line, 2).len(), 3); - assert_eq!(word_wrap_line(&line, 6).len(), 1); - } - - #[test] - fn word_wrap_does_not_split_words_simple_english() { - let sample = "Years passed, and Willowmere thrived in peace and friendship. Mira’s herb garden flourished with both ordinary and enchanted plants, and travelers spoke of the kindness of the woman who tended them."; - let line = sample.into(); - // Force small width to exercise wrapping at spaces. - let wrapped = word_wrap_lines(&[line], 40); - let joined: String = wrapped - .iter() - .map(|l| { - l.spans - .iter() - .map(|s| s.content.clone()) - .collect::() - }) - .collect::>() - .join("\n"); - assert!( - !joined.contains("bo\nth"), - "word 'both' should not be split across lines:\n{joined}" - ); - assert!( - !joined.contains("Willowm\nere"), - "should not split inside words:\n{joined}" - ); - } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index c52f48e6..c43572dd 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -58,6 +58,7 @@ mod streaming; mod text_formatting; mod tui; mod user_approval_widget; +mod wrapping; // Internal vt100-based replay tests live as a separate source file to keep them // close to the widget code. Include them in unit tests. diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 70365aed..0a7e54f0 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -1,7 +1,7 @@ use std::io::Result; use std::time::Duration; -use crate::insert_history; +use crate::render::line_utils::push_owned_lines; use crate::tui; use crate::tui::TuiEvent; use crossterm::event::KeyCode; @@ -287,9 +287,9 @@ impl PagerView { let mut wrapped: Vec> = Vec::new(); let mut src_idx: Vec = Vec::new(); for (i, line) in self.lines.iter().enumerate() { - let ws = insert_history::word_wrap_lines(std::slice::from_ref(line), width); + let ws = crate::wrapping::word_wrap_line(line, width as usize); src_idx.extend(std::iter::repeat_n(i, ws.len())); - wrapped.extend(ws); + push_owned_lines(&ws, &mut wrapped); } self.wrap_cache = Some(WrapCache { width, diff --git a/codex-rs/tui/src/render/highlight.rs b/codex-rs/tui/src/render/highlight.rs new file mode 100644 index 00000000..393aa337 --- /dev/null +++ b/codex-rs/tui/src/render/highlight.rs @@ -0,0 +1,145 @@ +use codex_core::bash::try_parse_bash; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::text::Span; + +/// Convert the full bash script into per-line styled content by first +/// computing operator-dimmed spans across the entire script, then splitting +/// by newlines and dimming heredoc body lines. Performs a single parse and +/// reuses it for both highlighting and heredoc detection. +pub(crate) fn highlight_bash_to_lines(script: &str) -> Vec> { + // Parse once; use the tree for both highlighting and heredoc body detection. + let spans: Vec> = if let Some(tree) = try_parse_bash(script) { + // Single walk: collect operator ranges and heredoc rows. + let root = tree.root_node(); + let mut cursor = root.walk(); + let mut stack = vec![root]; + let mut ranges: Vec<(usize, usize)> = Vec::new(); + while let Some(node) = stack.pop() { + if !node.is_named() && !node.is_extra() { + let kind = node.kind(); + let is_quote = matches!(kind, "\"" | "'" | "`"); + let is_whitespace = kind.trim().is_empty(); + if !is_quote && !is_whitespace { + ranges.push((node.start_byte(), node.end_byte())); + } + } else if node.kind() == "heredoc_body" { + ranges.push((node.start_byte(), node.end_byte())); + } + for child in node.children(&mut cursor) { + stack.push(child); + } + } + if ranges.is_empty() { + ranges.push((script.len(), script.len())); + } + ranges.sort_by_key(|(st, _)| *st); + let mut spans: Vec> = Vec::new(); + let mut i = 0usize; + for (start, end) in ranges.into_iter() { + let dim_start = start.max(i); + let dim_end = end; + if dim_start < dim_end { + if dim_start > i { + spans.push(script[i..dim_start].to_string().into()); + } + spans.push(script[dim_start..dim_end].to_string().dim()); + i = dim_end; + } + } + if i < script.len() { + spans.push(script[i..].to_string().into()); + } + spans + } else { + vec![script.to_string().into()] + }; + // Split spans into lines preserving style boundaries and highlights across newlines. + let mut lines: Vec> = vec![Line::from("")]; + for sp in spans { + let style = sp.style; + let text = sp.content.into_owned(); + for (i, part) in text.split('\n').enumerate() { + if i > 0 { + lines.push(Line::from("")); + } + if part.is_empty() { + continue; + } + let span = Span { + style, + content: std::borrow::Cow::Owned(part.to_string()), + }; + if let Some(last) = lines.last_mut() { + last.spans.push(span); + } + } + } + lines +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use ratatui::style::Modifier; + + #[test] + fn dims_expected_bash_operators() { + let s = "echo foo && bar || baz | qux & (echo hi)"; + let lines = highlight_bash_to_lines(s); + let reconstructed: String = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|sp| sp.content.clone()) + .collect::() + }) + .collect::>() + .join("\n"); + assert_eq!(reconstructed, s); + + fn is_dim(span: &Span<'_>) -> bool { + span.style.add_modifier.contains(Modifier::DIM) + } + let dimmed: Vec = lines + .iter() + .flat_map(|l| l.spans.iter()) + .filter(|sp| is_dim(sp)) + .map(|sp| sp.content.clone().into_owned()) + .collect(); + assert_eq!(dimmed, vec!["&&", "||", "|", "&", "(", ")"]); + } + + #[test] + fn does_not_dim_quotes_but_dims_other_punct() { + let s = "echo \"hi\" > out.txt; echo 'ok'"; + let lines = highlight_bash_to_lines(s); + let reconstructed: String = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|sp| sp.content.clone()) + .collect::() + }) + .collect::>() + .join("\n"); + assert_eq!(reconstructed, s); + + fn is_dim(span: &Span<'_>) -> bool { + span.style.add_modifier.contains(Modifier::DIM) + } + let dimmed: Vec = lines + .iter() + .flat_map(|l| l.spans.iter()) + .filter(|sp| is_dim(sp)) + .map(|sp| sp.content.clone().into_owned()) + .collect(); + assert!(dimmed.contains(&">".to_string())); + assert!(dimmed.contains(&";".to_string())); + assert!(!dimmed.contains(&"\"".to_string())); + assert!(!dimmed.contains(&"'".to_string())); + } +} diff --git a/codex-rs/tui/src/render/mod.rs b/codex-rs/tui/src/render/mod.rs index 71ae62db..ed1e215e 100644 --- a/codex-rs/tui/src/render/mod.rs +++ b/codex-rs/tui/src/render/mod.rs @@ -1,2 +1,3 @@ +pub mod highlight; pub mod line_utils; pub mod markdown_utils; diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_both_lines_wrap_with_correct_prefixes.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_both_lines_wrap_with_correct_prefixes.snap index 99d6579f..0964b64a 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_both_lines_wrap_with_correct_prefixes.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_both_lines_wrap_with_correct_prefixes.snap @@ -1,9 +1,10 @@ --- source: tui/src/history_cell.rs +assertion_line: 1942 expression: rendered --- • Ran - └ first_token_is_long_ - enough_to_wrap - second_token_is_also - _long_enough_to_wrap + └ first_token_is_long_enou + gh_to_wrap + second_token_is_also_lon + g_enough_to_wrap diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_wraps_with_extra_indent_on_subsequent_lines.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_wraps_with_extra_indent_on_subsequent_lines.snap index 4e383a5a..de36cd92 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_wraps_with_extra_indent_on_subsequent_lines.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_wraps_with_extra_indent_on_subsequent_lines.snap @@ -1,5 +1,6 @@ --- source: tui/src/history_cell.rs +assertion_line: 1797 expression: rendered --- • Ran diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_wraps_with_four_space_continuation.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_wraps_with_four_space_continuation.snap index 548d50ce..5ca13307 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_wraps_with_four_space_continuation.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_wraps_with_four_space_continuation.snap @@ -1,9 +1,9 @@ --- source: tui/src/history_cell.rs +assertion_line: 1869 expression: rendered --- • Ran - └ a_very_long_toke - n_without_spaces - _to_force_wrappi - ng + └ a_very_long_token_wi + thout_spaces_to_ + force_wrapping diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index 07ecba0c..8f492c06 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -17,8 +17,6 @@ use crate::app_event_sender::AppEventSender; use crate::key_hint; use crate::shimmer::shimmer_spans; use crate::tui::FrameRequester; -use textwrap::Options as TwOptions; -use textwrap::WordSplitter; pub(crate) struct StatusIndicatorWidget { /// Animated header text (defaults to "Working"). @@ -54,11 +52,8 @@ impl StatusIndicatorWidget { let mut total: u16 = 1; // status line let text_width = inner_width.saturating_sub(3); // account for " ↳ " prefix if text_width > 0 { - let opts = TwOptions::new(text_width) - .break_words(false) - .word_splitter(WordSplitter::NoHyphenation); for q in &self.queued_messages { - let wrapped = textwrap::wrap(q, &opts); + let wrapped = textwrap::wrap(q, text_width); let lines = wrapped.len().min(3) as u16; total = total.saturating_add(lines); if wrapped.len() > 3 { @@ -157,11 +152,8 @@ impl WidgetRef for StatusIndicatorWidget { lines.push(Line::from(spans)); // Wrap queued messages using textwrap and show up to the first 3 lines per message. let text_width = area.width.saturating_sub(3); // " ↳ " prefix - let opts = TwOptions::new(text_width as usize) - .break_words(false) - .word_splitter(WordSplitter::NoHyphenation); for q in &self.queued_messages { - let wrapped = textwrap::wrap(q, &opts); + let wrapped = textwrap::wrap(q, text_width as usize); for (i, piece) in wrapped.iter().take(3).enumerate() { let prefix = if i == 0 { " ↳ " } else { " " }; let content = format!("{prefix}{piece}"); diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs new file mode 100644 index 00000000..508c6a45 --- /dev/null +++ b/codex-rs/tui/src/wrapping.rs @@ -0,0 +1,545 @@ +use ratatui::text::Line; +use ratatui::text::Span; +use std::ops::Range; +use textwrap::Options; + +use crate::render::line_utils::push_owned_lines; + +pub(crate) fn wrap_ranges<'a, O>(text: &str, width_or_options: O) -> Vec> +where + O: Into>, +{ + let opts = width_or_options.into(); + let mut lines: Vec> = Vec::new(); + for line in textwrap::wrap(text, opts).iter() { + match line { + std::borrow::Cow::Borrowed(slice) => { + let start = unsafe { slice.as_ptr().offset_from(text.as_ptr()) as usize }; + let end = start + slice.len(); + let trailing_spaces = text[end..].chars().take_while(|c| *c == ' ').count(); + lines.push(start..end + trailing_spaces + 1); + } + std::borrow::Cow::Owned(_) => panic!("wrap_ranges: unexpected owned string"), + } + } + lines +} + +/// Like `wrap_ranges` but returns ranges without trailing whitespace and +/// without the sentinel extra byte. Suitable for general wrapping where +/// trailing spaces should not be preserved. +pub(crate) fn wrap_ranges_trim<'a, O>(text: &str, width_or_options: O) -> Vec> +where + O: Into>, +{ + let opts = width_or_options.into(); + let mut lines: Vec> = Vec::new(); + for line in textwrap::wrap(text, opts).iter() { + match line { + std::borrow::Cow::Borrowed(slice) => { + let start = unsafe { slice.as_ptr().offset_from(text.as_ptr()) as usize }; + let end = start + slice.len(); + lines.push(start..end); + } + std::borrow::Cow::Owned(_) => panic!("wrap_ranges_trim: unexpected owned string"), + } + } + lines +} + +#[derive(Debug, Clone)] +pub struct RtOptions<'a> { + /// The width in columns at which the text will be wrapped. + pub width: usize, + /// Line ending used for breaking lines. + pub line_ending: textwrap::LineEnding, + /// Indentation used for the first line of output. See the + /// [`Options::initial_indent`] method. + pub initial_indent: Line<'a>, + /// Indentation used for subsequent lines of output. See the + /// [`Options::subsequent_indent`] method. + pub subsequent_indent: Line<'a>, + /// Allow long words to be broken if they cannot fit on a line. + /// When set to `false`, some lines may be longer than + /// `self.width`. See the [`Options::break_words`] method. + pub break_words: bool, + /// Wrapping algorithm to use, see the implementations of the + /// [`WrapAlgorithm`] trait for details. + pub wrap_algorithm: textwrap::WrapAlgorithm, + /// The line breaking algorithm to use, see the [`WordSeparator`] + /// trait for an overview and possible implementations. + pub word_separator: textwrap::WordSeparator, + /// The method for splitting words. This can be used to prohibit + /// splitting words on hyphens, or it can be used to implement + /// language-aware machine hyphenation. + pub word_splitter: textwrap::WordSplitter, +} +impl From for RtOptions<'_> { + fn from(width: usize) -> Self { + RtOptions::new(width) + } +} + +#[allow(dead_code)] +impl<'a> RtOptions<'a> { + pub fn new(width: usize) -> Self { + RtOptions { + width, + line_ending: textwrap::LineEnding::LF, + initial_indent: Line::default(), + subsequent_indent: Line::default(), + break_words: true, + word_separator: textwrap::WordSeparator::new(), + wrap_algorithm: textwrap::WrapAlgorithm::new(), + word_splitter: textwrap::WordSplitter::HyphenSplitter, + } + } + + pub fn line_ending(self, line_ending: textwrap::LineEnding) -> Self { + RtOptions { + line_ending, + ..self + } + } + + pub fn width(self, width: usize) -> Self { + RtOptions { width, ..self } + } + + pub fn initial_indent(self, initial_indent: Line<'a>) -> Self { + RtOptions { + initial_indent, + ..self + } + } + + pub fn subsequent_indent(self, subsequent_indent: Line<'a>) -> Self { + RtOptions { + subsequent_indent, + ..self + } + } + + pub fn break_words(self, break_words: bool) -> Self { + RtOptions { + break_words, + ..self + } + } + + pub fn word_separator(self, word_separator: textwrap::WordSeparator) -> RtOptions<'a> { + RtOptions { + word_separator, + ..self + } + } + + pub fn wrap_algorithm(self, wrap_algorithm: textwrap::WrapAlgorithm) -> RtOptions<'a> { + RtOptions { + wrap_algorithm, + ..self + } + } + + pub fn word_splitter(self, word_splitter: textwrap::WordSplitter) -> RtOptions<'a> { + RtOptions { + word_splitter, + ..self + } + } +} + +pub(crate) fn word_wrap_line<'a, O>(line: &'a Line<'a>, width_or_options: O) -> Vec> +where + O: Into>, +{ + // Flatten the line and record span byte ranges. + let mut flat = String::new(); + let mut span_bounds = Vec::new(); + let mut acc = 0usize; + for s in &line.spans { + let text = s.content.as_ref(); + let start = acc; + flat.push_str(text); + acc += text.len(); + span_bounds.push((start..acc, s.style)); + } + + let rt_opts: RtOptions<'a> = width_or_options.into(); + let opts = Options::new(rt_opts.width) + .line_ending(rt_opts.line_ending) + .break_words(rt_opts.break_words) + .wrap_algorithm(rt_opts.wrap_algorithm) + .word_separator(rt_opts.word_separator) + .word_splitter(rt_opts.word_splitter); + + let mut out: Vec> = Vec::new(); + + // Compute first line range with reduced width due to initial indent. + let initial_width_available = opts + .width + .saturating_sub(rt_opts.initial_indent.width()) + .max(1); + let initial_wrapped = wrap_ranges_trim(&flat, opts.clone().width(initial_width_available)); + let Some(first_line_range) = initial_wrapped.first() else { + return vec![rt_opts.initial_indent.clone()]; + }; + + // Build first wrapped line with initial indent. + let mut first_line = rt_opts.initial_indent.clone(); + { + let mut sliced = slice_line_spans(line, &span_bounds, first_line_range); + let mut spans = first_line.spans; + spans.append(&mut sliced.spans); + first_line.spans = spans; + out.push(first_line); + } + + // Wrap the remainder using subsequent indent width and map back to original indices. + let base = first_line_range.end; + let skip_leading_spaces = flat[base..].chars().take_while(|c| *c == ' ').count(); + let base = base + skip_leading_spaces; + let subsequent_width_available = opts + .width + .saturating_sub(rt_opts.subsequent_indent.width()) + .max(1); + let remaining_wrapped = wrap_ranges_trim(&flat[base..], opts.width(subsequent_width_available)); + for r in &remaining_wrapped { + if r.is_empty() { + continue; + } + let mut subsequent_line = rt_opts.subsequent_indent.clone(); + let offset_range = (r.start + base)..(r.end + base); + let mut sliced = slice_line_spans(line, &span_bounds, &offset_range); + let mut spans = subsequent_line.spans; + spans.append(&mut sliced.spans); + subsequent_line.spans = spans; + out.push(subsequent_line); + } + + out +} + +/// Wrap a sequence of lines, applying the initial indent only to the very first +/// output line, and using the subsequent indent for all later wrapped pieces. +#[allow(dead_code)] +pub(crate) fn word_wrap_lines<'a, I, O>(lines: I, width_or_options: O) -> Vec> +where + I: IntoIterator>, + O: Into>, +{ + let base_opts: RtOptions<'a> = width_or_options.into(); + let mut out: Vec> = Vec::new(); + + for (idx, line) in lines.into_iter().enumerate() { + let opts = if idx == 0 { + base_opts.clone() + } else { + let mut o = base_opts.clone(); + let sub = o.subsequent_indent.clone(); + o = o.initial_indent(sub); + o + }; + let wrapped = word_wrap_line(line, opts); + push_owned_lines(&wrapped, &mut out); + } + + out +} + +#[allow(dead_code)] +pub(crate) fn word_wrap_lines_borrowed<'a, I, O>(lines: I, width_or_options: O) -> Vec> +where + I: IntoIterator>, + O: Into>, +{ + let base_opts: RtOptions<'a> = width_or_options.into(); + let mut out: Vec> = Vec::new(); + let mut first = true; + for line in lines.into_iter() { + let opts = if first { + base_opts.clone() + } else { + base_opts + .clone() + .initial_indent(base_opts.subsequent_indent.clone()) + }; + out.extend(word_wrap_line(line, opts)); + first = false; + } + out +} + +fn slice_line_spans<'a>( + original: &'a Line<'a>, + span_bounds: &[(Range, ratatui::style::Style)], + range: &Range, +) -> Line<'a> { + let start_byte = range.start; + let end_byte = range.end; + let mut acc: Vec> = Vec::new(); + for (i, (range, style)) in span_bounds.iter().enumerate() { + let s = range.start; + let e = range.end; + if e <= start_byte { + continue; + } + if s >= end_byte { + break; + } + let seg_start = start_byte.max(s); + let seg_end = end_byte.min(e); + if seg_end > seg_start { + let local_start = seg_start - s; + let local_end = seg_end - s; + let content = original.spans[i].content.as_ref(); + let slice = &content[local_start..local_end]; + acc.push(Span { + style: *style, + content: std::borrow::Cow::Borrowed(slice), + }); + } + if e >= end_byte { + break; + } + } + Line { + style: original.style, + alignment: original.alignment, + spans: acc, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use itertools::Itertools as _; + use pretty_assertions::assert_eq; + use ratatui::style::Color; + use ratatui::style::Stylize; + + fn concat_line(line: &Line) -> String { + line.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + } + + #[test] + fn trivial_unstyled_no_indents_wide_width() { + let line = Line::from("hello"); + let out = word_wrap_line(&line, 10); + assert_eq!(out.len(), 1); + assert_eq!(concat_line(&out[0]), "hello"); + } + + #[test] + fn simple_unstyled_wrap_narrow_width() { + let line = Line::from("hello world"); + let out = word_wrap_line(&line, 5); + assert_eq!(out.len(), 2); + assert_eq!(concat_line(&out[0]), "hello"); + assert_eq!(concat_line(&out[1]), "world"); + } + + #[test] + fn simple_styled_wrap_preserves_styles() { + let line = Line::from(vec!["hello ".red(), "world".into()]); + let out = word_wrap_line(&line, 6); + assert_eq!(out.len(), 2); + // First line should carry the red style + assert_eq!(concat_line(&out[0]), "hello"); + assert_eq!(out[0].spans.len(), 1); + assert_eq!(out[0].spans[0].style.fg, Some(Color::Red)); + // Second line is unstyled + assert_eq!(concat_line(&out[1]), "world"); + assert_eq!(out[1].spans.len(), 1); + assert_eq!(out[1].spans[0].style.fg, None); + } + + #[test] + fn with_initial_and_subsequent_indents() { + let opts = RtOptions::new(8) + .initial_indent(Line::from("- ")) + .subsequent_indent(Line::from(" ")); + let line = Line::from("hello world foo"); + let out = word_wrap_line(&line, opts); + // Expect three lines with proper prefixes + assert!(concat_line(&out[0]).starts_with("- ")); + assert!(concat_line(&out[1]).starts_with(" ")); + assert!(concat_line(&out[2]).starts_with(" ")); + // And content roughly segmented + assert_eq!(concat_line(&out[0]), "- hello"); + assert_eq!(concat_line(&out[1]), " world"); + assert_eq!(concat_line(&out[2]), " foo"); + } + + #[test] + fn empty_initial_indent_subsequent_spaces() { + let opts = RtOptions::new(8) + .initial_indent(Line::from("")) + .subsequent_indent(Line::from(" ")); + let line = Line::from("hello world foobar"); + let out = word_wrap_line(&line, opts); + assert!(concat_line(&out[0]).starts_with("hello")); + for l in &out[1..] { + assert!(concat_line(l).starts_with(" ")); + } + } + + #[test] + fn empty_input_yields_single_empty_line() { + let line = Line::from(""); + let out = word_wrap_line(&line, 10); + assert_eq!(out.len(), 1); + assert_eq!(concat_line(&out[0]), ""); + } + + #[test] + fn leading_spaces_preserved_on_first_line() { + let line = Line::from(" hello"); + let out = word_wrap_line(&line, 8); + assert_eq!(out.len(), 1); + assert_eq!(concat_line(&out[0]), " hello"); + } + + #[test] + fn multiple_spaces_between_words_dont_start_next_line_with_spaces() { + let line = Line::from("hello world"); + let out = word_wrap_line(&line, 8); + assert_eq!(out.len(), 2); + assert_eq!(concat_line(&out[0]), "hello"); + assert_eq!(concat_line(&out[1]), "world"); + } + + #[test] + fn break_words_false_allows_overflow_for_long_word() { + let opts = RtOptions::new(5).break_words(false); + let line = Line::from("supercalifragilistic"); + let out = word_wrap_line(&line, opts); + assert_eq!(out.len(), 1); + assert_eq!(concat_line(&out[0]), "supercalifragilistic"); + } + + #[test] + fn hyphen_splitter_breaks_at_hyphen() { + let line = Line::from("hello-world"); + let out = word_wrap_line(&line, 7); + assert_eq!(out.len(), 2); + assert_eq!(concat_line(&out[0]), "hello-"); + assert_eq!(concat_line(&out[1]), "world"); + } + + #[test] + fn indent_consumes_width_leaving_one_char_space() { + let opts = RtOptions::new(4) + .initial_indent(Line::from(">>>>")) + .subsequent_indent(Line::from("--")); + let line = Line::from("hello"); + let out = word_wrap_line(&line, opts); + assert_eq!(out.len(), 3); + assert_eq!(concat_line(&out[0]), ">>>>h"); + assert_eq!(concat_line(&out[1]), "--el"); + assert_eq!(concat_line(&out[2]), "--lo"); + } + + #[test] + fn wide_unicode_wraps_by_display_width() { + let line = Line::from("😀😀😀"); + let out = word_wrap_line(&line, 4); + assert_eq!(out.len(), 2); + assert_eq!(concat_line(&out[0]), "😀😀"); + assert_eq!(concat_line(&out[1]), "😀"); + } + + #[test] + fn styled_split_within_span_preserves_style() { + use ratatui::style::Stylize; + let line = Line::from(vec!["abcd".red()]); + let out = word_wrap_line(&line, 2); + assert_eq!(out.len(), 2); + assert_eq!(out[0].spans.len(), 1); + assert_eq!(out[1].spans.len(), 1); + assert_eq!(out[0].spans[0].style.fg, Some(Color::Red)); + assert_eq!(out[1].spans[0].style.fg, Some(Color::Red)); + assert_eq!(concat_line(&out[0]), "ab"); + assert_eq!(concat_line(&out[1]), "cd"); + } + + #[test] + fn wrap_lines_applies_initial_indent_only_once() { + let opts = RtOptions::new(8) + .initial_indent(Line::from("- ")) + .subsequent_indent(Line::from(" ")); + + let lines = vec![Line::from("hello world"), Line::from("foo bar baz")]; + let out = word_wrap_lines(&lines, opts); + + // Expect: first line prefixed with "- ", subsequent wrapped pieces with " " + // and for the second input line, there should be no "- " prefix on its first piece + let rendered: Vec = out.iter().map(concat_line).collect(); + assert!(rendered[0].starts_with("- ")); + for r in rendered.iter().skip(1) { + assert!(r.starts_with(" ")); + } + } + + #[test] + fn wrap_lines_without_indents_is_concat_of_single_wraps() { + let lines = vec![Line::from("hello"), Line::from("world!")]; + let out = word_wrap_lines(&lines, 10); + let rendered: Vec = out.iter().map(concat_line).collect(); + assert_eq!(rendered, vec!["hello", "world!"]); + } + + #[test] + fn wrap_lines_borrowed_applies_initial_indent_only_once() { + let opts = RtOptions::new(8) + .initial_indent(Line::from("- ")) + .subsequent_indent(Line::from(" ")); + + let lines = [Line::from("hello world"), Line::from("foo bar baz")]; + let out = word_wrap_lines_borrowed(lines.iter().collect::>(), opts); + + let rendered: Vec = out.iter().map(concat_line).collect(); + assert!(rendered.first().unwrap().starts_with("- ")); + for r in rendered.iter().skip(1) { + assert!(r.starts_with(" ")); + } + } + + #[test] + fn wrap_lines_borrowed_without_indents_is_concat_of_single_wraps() { + let lines = [Line::from("hello"), Line::from("world!")]; + let out = word_wrap_lines_borrowed(lines.iter().collect::>(), 10); + let rendered: Vec = out.iter().map(concat_line).collect(); + assert_eq!(rendered, vec!["hello", "world!"]); + } + + #[test] + fn line_height_counts_double_width_emoji() { + let line = "😀😀😀".into(); // each emoji ~ width 2 + assert_eq!(word_wrap_line(&line, 4).len(), 2); + assert_eq!(word_wrap_line(&line, 2).len(), 3); + assert_eq!(word_wrap_line(&line, 6).len(), 1); + } + + #[test] + fn word_wrap_does_not_split_words_simple_english() { + let sample = "Years passed, and Willowmere thrived in peace and friendship. Mira’s herb garden flourished with both ordinary and enchanted plants, and travelers spoke of the kindness of the woman who tended them."; + let line = Line::from(sample); + let lines = [line]; + // Force small width to exercise wrapping at spaces. + let wrapped = word_wrap_lines_borrowed(&lines, 40); + let joined: String = wrapped.iter().map(|l| l.to_string()).join("\n"); + assert_eq!( + joined, + r#"Years passed, and Willowmere thrived +in peace and friendship. Mira’s herb +garden flourished with both ordinary and +enchanted plants, and travelers spoke +of the kindness of the woman who tended +them."# + ); + } +}