diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 36ef47da5e4..17750a73515 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -2,7 +2,8 @@ use std::fmt; use std::io; use std::io::Write; -use crate::wrapping::word_wrap_lines_borrowed; +use crate::wrapping::RtOptions; +use crate::wrapping::word_wrap_line_url_aware; use crossterm::Command; use crossterm::cursor::MoveTo; use crossterm::queue; @@ -22,6 +23,37 @@ use ratatui::style::Modifier; use ratatui::text::Line; use ratatui::text::Span; +fn wrap_history_lines<'a>(lines: &'a [Line<'a>], width: u16) -> Vec> { + let base_opts = RtOptions::new(width.max(1) as usize); + let mut wrapped: Vec> = Vec::new(); + let mut first = true; + for line in lines { + let opts = if first { + base_opts.clone() + } else { + base_opts + .clone() + .initial_indent(base_opts.subsequent_indent.clone()) + }; + wrapped.extend(word_wrap_line_url_aware(line, opts)); + first = false; + } + wrapped +} + +/// Counts the number of terminal rows needed after wrapping `lines` at `width`. +/// This accounts for any long tokens (like URLs) that are still forced to +/// hard-wrap by the terminal even after word-aware wrapping. +fn wrapped_row_count(lines: &[Line<'_>], width: u16) -> u16 { + let width = width.max(1) as usize; + let mut rows = 0usize; + for line in lines { + let line_width = line.width().max(1); + rows = rows.saturating_add(line_width.div_ceil(width)); + } + u16::try_from(rows).unwrap_or(u16::MAX) +} + /// Insert `lines` above the viewport using the terminal's backend writer /// (avoids direct stdout references). pub fn insert_history_lines( @@ -40,8 +72,8 @@ where // 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_borrowed(&lines, area.width.max(1) as usize); - let wrapped_lines = wrapped.len() as u16; + let wrapped = wrap_history_lines(&lines, area.width); + let wrapped_lines = wrapped_row_count(&wrapped, area.width); 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. // Don't scroll it past the bottom of the screen. @@ -287,6 +319,7 @@ mod tests { use super::*; use crate::markdown_render::render_markdown_text; use crate::test_backend::VT100Backend; + use pretty_assertions::assert_eq; use ratatui::layout::Rect; use ratatui::style::Color; @@ -318,6 +351,55 @@ mod tests { ); } + #[test] + fn wrap_history_lines_wraps_before_url() { + let url = "http://foobar"; + let line: Line<'static> = + Line::from("Here is some text and a http://foobar url in the middle"); + + let lines = [line]; + let wrapped = wrap_history_lines(&lines, 24); + let rendered: Vec = wrapped + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect() + }) + .collect(); + + let Some(url_line) = rendered.iter().find(|line| line.contains(url)) else { + panic!("expected wrapped output to contain url"); + }; + + assert!(url_line.starts_with(url)); + } + + #[test] + fn wrap_history_lines_breaks_long_url_when_too_long() { + let url = "https://github.com/openai/codex/issues/new?template=2-bug-report.yml&steps=Uploaded%20thread:thread-1"; + let line: Line<'static> = Line::from(url); + let width = 20u16; + + let lines = [line]; + let wrapped = wrap_history_lines(&lines, width); + let rows = wrapped_row_count(&wrapped, width); + let rendered: Vec = wrapped + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect() + }) + .collect(); + + assert!(wrapped.len() > 1); + assert!(rendered.iter().all(|line| !line.contains(url))); + assert_eq!(rows as usize, wrapped.len()); + } + #[test] fn vt100_blockquote_line_emits_green_fg() { // Set up a small off-screen terminal diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index c29106651d4..22d86081a59 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -234,6 +234,80 @@ where out } +#[must_use] +pub(crate) fn word_wrap_line_url_aware<'a, O>( + line: &'a Line<'a>, + width_or_options: O, +) -> Vec> +where + O: Into>, +{ + let mut rt_opts: RtOptions<'a> = width_or_options.into(); + rt_opts.word_separator = textwrap::WordSeparator::Custom(url_aware_words); + word_wrap_line(line, rt_opts) +} + +fn url_aware_words<'a>(line: &'a str) -> Box> + 'a> { + if !line.contains("http://") && !line.contains("https://") { + return textwrap::WordSeparator::new().find_words(line); + } + + let mut words: Vec> = Vec::new(); + let mut cursor = 0usize; + let separator = textwrap::WordSeparator::new(); + + while let Some((url_start, url_end)) = find_next_url(line, cursor) { + if cursor < url_start { + words.extend(separator.find_words(&line[cursor..url_start])); + } + let url_with_spaces_end = consume_trailing_spaces(line, url_end); + words.push(textwrap::core::Word::from( + &line[url_start..url_with_spaces_end], + )); + cursor = url_with_spaces_end; + } + + if cursor < line.len() { + words.extend(separator.find_words(&line[cursor..])); + } + + Box::new(words.into_iter()) +} + +fn find_next_url(line: &str, start: usize) -> Option<(usize, usize)> { + let http = line[start..].find("http://"); + let https = line[start..].find("https://"); + let (rel_start, scheme_len) = match (http, https) { + (Some(http_pos), Some(https_pos)) => { + if http_pos <= https_pos { + (http_pos, "http://".len()) + } else { + (https_pos, "https://".len()) + } + } + (Some(http_pos), None) => (http_pos, "http://".len()), + (None, Some(https_pos)) => (https_pos, "https://".len()), + (None, None) => return None, + }; + let url_start = start + rel_start; + let mut url_end = line.len(); + for (offset, ch) in line[url_start + scheme_len..].char_indices() { + if ch.is_whitespace() { + url_end = url_start + scheme_len + offset; + break; + } + } + Some((url_start, url_end)) +} + +fn consume_trailing_spaces(line: &str, mut idx: usize) -> usize { + let bytes = line.as_bytes(); + while matches!(bytes.get(idx), Some(b' ')) { + idx += 1; + } + idx +} + /// Utilities to allow wrapping either borrowed or owned lines. #[derive(Debug)] enum LineInput<'a> { diff --git a/codex-rs/tui2/src/insert_history.rs b/codex-rs/tui2/src/insert_history.rs index 1b236e8eec3..87b5ba53ded 100644 --- a/codex-rs/tui2/src/insert_history.rs +++ b/codex-rs/tui2/src/insert_history.rs @@ -25,7 +25,8 @@ use std::fmt; use std::io; use std::io::Write; -use crate::wrapping::word_wrap_lines_borrowed; +use crate::wrapping::RtOptions; +use crate::wrapping::word_wrap_line; use crossterm::Command; use crossterm::cursor::MoveTo; use crossterm::queue; @@ -42,6 +43,46 @@ use ratatui::style::Modifier; use ratatui::text::Line; use ratatui::text::Span; +fn line_contains_url(line: &Line<'_>) -> bool { + let mut text = String::new(); + for span in &line.spans { + text.push_str(span.content.as_ref()); + } + text.contains("https://") || text.contains("http://") +} + +fn wrap_history_lines<'a>(lines: &'a [Line<'a>], width: u16) -> Vec> { + let base_opts = RtOptions::new(width.max(1) as usize); + let mut wrapped: Vec> = Vec::new(); + let mut first = true; + for line in lines { + if line_contains_url(line) { + wrapped.push(line.clone()); + } else { + let opts = if first { + base_opts.clone() + } else { + base_opts + .clone() + .initial_indent(base_opts.subsequent_indent.clone()) + }; + wrapped.extend(word_wrap_line(line, opts)); + } + first = false; + } + wrapped +} + +fn wrapped_row_count(lines: &[Line<'_>], width: u16) -> u16 { + let width = width.max(1) as usize; + let mut rows = 0usize; + for line in lines { + let line_width = line.width().max(1); + rows = rows.saturating_add(line_width.div_ceil(width)); + } + u16::try_from(rows).unwrap_or(u16::MAX) +} + /// Insert `lines` above the viewport using the terminal's backend writer /// (avoids direct stdout references). pub fn insert_history_lines( @@ -60,8 +101,8 @@ where // 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_borrowed(&lines, area.width.max(1) as usize); - let wrapped_lines = wrapped.len() as u16; + let wrapped = wrap_history_lines(&lines, area.width); + let wrapped_lines = wrapped_row_count(&wrapped, area.width); 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. // Don't scroll it past the bottom of the screen. @@ -324,6 +365,41 @@ mod tests { ); } + #[test] + fn wrap_history_lines_preserves_long_url() { + let url = "https://github.com/openai/codex/issues/new?template=2-bug-report.yml&steps=Uploaded%20thread:thread-1"; + let line: Line<'static> = Line::from(vec![" ".into(), url.into()]); + + let lines = [line]; + let wrapped = wrap_history_lines(&lines, 20); + let rendered: Vec = wrapped + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect() + }) + .collect(); + + assert_eq!(rendered, vec![format!(" {url}")]); + } + + #[test] + fn wrapped_row_count_accounts_for_long_url() { + let url = "https://github.com/openai/codex/issues/new?template=2-bug-report.yml&steps=Uploaded%20thread:thread-1"; + let line: Line<'static> = Line::from(vec![" ".into(), url.into()]); + let width = 20u16; + + let lines = [line]; + let wrapped = wrap_history_lines(&lines, width); + let rows = wrapped_row_count(&wrapped, width); + let line_width = 2 + url.len(); + let expected_rows = line_width.div_ceil(width as usize); + + assert_eq!(rows, expected_rows as u16); + } + #[test] fn write_spans_emits_truecolor_and_indexed_sgr() { // This test asserts that `write_spans` emits the correct SGR sequences for colors that