test feedback

codex-rs/Cargo.lock

@@ -3393,7 +3393,7 @@ dependencies = [
 [[package]]
 name = "ratatui"
 version = "0.29.0"
-source = "git+https://github.com/easong-openai/ratatui?branch=nornagon-v0.29.0-patch#159c1978c2f829cd322ec778df4168815ed9af96"
+source = "git+https://github.com/nornagon/ratatui?branch=nornagon-v0.29.0-patch#9b2ad1298408c45918ee9f8241a6f95498cdbed2"
 dependencies = [
  "bitflags 2.9.1",
  "cassowary",

codex-rs/Cargo.toml

@@ -44,4 +44,4 @@ codegen-units = 1
 
 [patch.crates-io]
 # ratatui = { path = "../../ratatui" }
-ratatui = { git = "https://github.com/easong-openai/ratatui", branch = "nornagon-v0.29.0-patch" }
+ratatui = { git = "https://github.com/nornagon/ratatui", branch = "nornagon-v0.29.0-patch" }

codex-rs/tui/tests/vt100_history.rs

@@ -1,4 +1,5 @@
 #![cfg(feature = "vt100-tests")]
+#![expect(clippy::expect_used)]
 
 use ratatui::backend::TestBackend;
 use ratatui::layout::Rect;
@@ -7,85 +8,102 @@ use ratatui::style::Style;
 use ratatui::text::Line;
 use ratatui::text::Span;
 
-/// HIST-001: Basic insertion at bottom, no wrap.
-///
-/// This test captures the ANSI bytes produced by `insert_history_lines_to_writer`
-/// when the viewport is at the bottom of the screen (so no pre-scroll is
-/// required). It feeds the bytes into a vt100 parser and asserts that the
-/// inserted lines are visible near the bottom of the screen.
-#[test]
-fn hist_001_basic_insertion_no_wrap() {
-    // Screen of 20x6; viewport is the last row (height=1 at y=5)
-    let backend = TestBackend::new(20, 6);
-    let mut term = match codex_tui::custom_terminal::Terminal::with_options(backend) {
-        Ok(t) => t,
-        Err(e) => panic!("failed to construct terminal: {e}"),
+// Small helper macro to assert a collection contains an item with a clearer
+// failure message.
+macro_rules! assert_contains {
+    ($collection:expr, $item:expr $(,)?) => {
+        assert!(
+            $collection.contains(&$item),
+            "Expected {:?} to contain {:?}",
+            $collection,
+            $item
+        );
     };
+    ($collection:expr, $item:expr, $($arg:tt)+) => {
+        assert!($collection.contains(&$item), $($arg)+);
+    };
+}
 
-    // Place the viewport at the bottom row
-    let area = Rect::new(0, 5, 20, 1);
-    term.set_viewport_area(area);
+struct TestScenario {
+    width: u16,
+    height: u16,
+    term: codex_tui::custom_terminal::Terminal<TestBackend>,
+}
 
-    let lines = vec![Line::from("first"), Line::from("second")];
-    let mut buf: Vec<u8> = Vec::new();
+impl TestScenario {
+    fn new(width: u16, height: u16, viewport: Rect) -> Self {
+        let backend = TestBackend::new(width, height);
+        let mut term = codex_tui::custom_terminal::Terminal::with_options(backend)
+            .expect("failed to construct terminal");
+        term.set_viewport_area(viewport);
+        Self {
+            width,
+            height,
+            term,
+        }
+    }
 
-    codex_tui::insert_history::insert_history_lines_to_writer(&mut term, &mut buf, lines);
+    fn run_insert(&mut self, lines: Vec<Line<'static>>) -> Vec<u8> {
+        let mut buf: Vec<u8> = Vec::new();
+        codex_tui::insert_history::insert_history_lines_to_writer(&mut self.term, &mut buf, lines);
+        buf
+    }
 
-    // Feed captured bytes into vt100 emulator
-    let mut parser = vt100::Parser::new(6, 20, 0);
-    parser.process(&buf);
-    let screen = parser.screen();
+    fn screen_rows_from_bytes(&self, bytes: &[u8]) -> Vec<String> {
+        let mut parser = vt100::Parser::new(self.height, self.width, 0);
+        parser.process(bytes);
+        let screen = parser.screen();
 
-    // Gather visible rows as strings
-    let mut rows: Vec<String> = Vec::new();
-    for row in 0..6 {
-        let mut s = String::new();
-        for col in 0..20 {
-            if let Some(cell) = screen.cell(row, col) {
-                let cont = cell.contents();
-                if let Some(ch) = cont.chars().next() {
-                    s.push(ch);
+        let mut rows: Vec<String> = Vec::with_capacity(self.height as usize);
+        for row in 0..self.height {
+            let mut s = String::with_capacity(self.width as usize);
+            for col in 0..self.width {
+                if let Some(cell) = screen.cell(row, col) {
+                    if let Some(ch) = cell.contents().chars().next() {
+                        s.push(ch);
+                    } else {
+                        s.push(' ');
+                    }
                 } else {
                     s.push(' ');
                 }
-            } else {
-                s.push(' ');
             }
+            rows.push(s.trim_end().to_string());
         }
-        rows.push(s);
+        rows
     }
-
-    // The inserted lines should appear somewhere above the viewport; in this
-    // simple case, they will occupy the two rows immediately above the final
-    // row of the scroll region.
-    let joined = rows.join("\n");
-    assert!(
-        joined.contains("first"),
-        "screen did not contain 'first'\n{joined}"
-    );
-    assert!(
-        joined.contains("second"),
-        "screen did not contain 'second'\n{joined}"
-    );
 }
 
-/// HIST-002: Long token wraps across rows within the scroll region.
+#[test]
+fn hist_001_basic_insertion_no_wrap() {
+    // Screen of 20x6; viewport is the last row (height=1 at y=5)
+    let area = Rect::new(0, 5, 20, 1);
+    let mut scenario = TestScenario::new(20, 6, area);
+
+    let lines = vec![Line::from("first"), Line::from("second")];
+    let buf = scenario.run_insert(lines);
+    let rows = scenario.screen_rows_from_bytes(&buf);
+    assert_contains!(rows, String::from("first"));
+    assert_contains!(rows, String::from("second"));
+    let first_idx = rows
+        .iter()
+        .position(|r| r == "first")
+        .expect("expected 'first' row to be present");
+    let second_idx = rows
+        .iter()
+        .position(|r| r == "second")
+        .expect("expected 'second' row to be present");
+    assert_eq!(second_idx, first_idx + 1, "rows should be adjacent");
+}
+
 #[test]
 fn hist_002_long_token_wraps() {
-    let backend = TestBackend::new(20, 6);
-    let mut term = match codex_tui::custom_terminal::Terminal::with_options(backend) {
-        Ok(t) => t,
-        Err(e) => panic!("failed to construct terminal: {e}"),
-    };
     let area = Rect::new(0, 5, 20, 1);
-    term.set_viewport_area(area);
+    let mut scenario = TestScenario::new(20, 6, area);
 
     let long = "A".repeat(45); // > 2 lines at width 20
     let lines = vec![Line::from(long.clone())];
-    let mut buf: Vec<u8> = Vec::new();
-
-    codex_tui::insert_history::insert_history_lines_to_writer(&mut term, &mut buf, lines);
-
+    let buf = scenario.run_insert(lines);
     let mut parser = vt100::Parser::new(6, 20, 0);
     parser.process(&buf);
     let screen = parser.screen();
@@ -111,23 +129,14 @@ fn hist_002_long_token_wraps() {
     );
 }
 
-/// HIST-003: Emoji/CJK content renders fully (no broken graphemes).
 #[test]
 fn hist_003_emoji_and_cjk() {
-    let backend = TestBackend::new(20, 6);
-    let mut term = match codex_tui::custom_terminal::Terminal::with_options(backend) {
-        Ok(t) => t,
-        Err(e) => panic!("failed to construct terminal: {e}"),
-    };
     let area = Rect::new(0, 5, 20, 1);
-    term.set_viewport_area(area);
+    let mut scenario = TestScenario::new(20, 6, area);
 
     let text = String::from("😀😀😀😀😀 你好世界");
     let lines = vec![Line::from(text.clone())];
-    let mut buf: Vec<u8> = Vec::new();
-
-    codex_tui::insert_history::insert_history_lines_to_writer(&mut term, &mut buf, lines);
-
+    let buf = scenario.run_insert(lines);
     let mut parser = vt100::Parser::new(6, 20, 0);
     parser.process(&buf);
     let screen = parser.screen();
@@ -155,69 +164,27 @@ fn hist_003_emoji_and_cjk() {
     }
 }
 
-/// HIST-004: Mixed ANSI spans render textual content correctly (styles stripped in emulator).
 #[test]
 fn hist_004_mixed_ansi_spans() {
-    let backend = TestBackend::new(20, 6);
-    let mut term = match codex_tui::custom_terminal::Terminal::with_options(backend) {
-        Ok(t) => t,
-        Err(e) => panic!("failed to construct terminal: {e}"),
-    };
     let area = Rect::new(0, 5, 20, 1);
-    term.set_viewport_area(area);
+    let mut scenario = TestScenario::new(20, 6, area);
 
     let line = Line::from(vec![
         Span::styled("red", Style::default().fg(Color::Red)),
         Span::raw("+plain"),
     ]);
-    let mut buf: Vec<u8> = Vec::new();
-
-    codex_tui::insert_history::insert_history_lines_to_writer(&mut term, &mut buf, vec![line]);
-
-    let mut parser = vt100::Parser::new(6, 20, 0);
-    parser.process(&buf);
-    let screen = parser.screen();
-
-    let mut rows: Vec<String> = Vec::new();
-    for row in 0..6 {
-        let mut s = String::new();
-        for col in 0..20 {
-            if let Some(cell) = screen.cell(row, col) {
-                let cont = cell.contents();
-                if let Some(ch) = cont.chars().next() {
-                    s.push(ch);
-                } else {
-                    s.push(' ');
-                }
-            } else {
-                s.push(' ');
-            }
-        }
-        rows.push(s);
-    }
-    let joined = rows.join("\n");
-    assert!(
-        joined.contains("red+plain"),
-        "styled text did not render as expected\n{joined}"
-    );
+    let buf = scenario.run_insert(vec![line]);
+    let rows = scenario.screen_rows_from_bytes(&buf);
+    assert_contains!(rows, String::from("red+plain"));
 }
 
-/// HIST-006: Cursor is restored after insertion (CUP to 1;1 when backend reports 0,0).
 #[test]
 fn hist_006_cursor_restoration() {
-    let backend = TestBackend::new(20, 6);
-    let mut term = match codex_tui::custom_terminal::Terminal::with_options(backend) {
-        Ok(t) => t,
-        Err(e) => panic!("failed to construct terminal: {e}"),
-    };
     let area = Rect::new(0, 5, 20, 1);
-    term.set_viewport_area(area);
+    let mut scenario = TestScenario::new(20, 6, area);
 
     let lines = vec![Line::from("x")];
-    let mut buf: Vec<u8> = Vec::new();
-
-    codex_tui::insert_history::insert_history_lines_to_writer(&mut term, &mut buf, lines);
-
+    let buf = scenario.run_insert(lines);
     let s = String::from_utf8_lossy(&buf);
     // CUP to 1;1 (ANSI: ESC[1;1H)
     assert!(
@@ -231,22 +198,14 @@ fn hist_006_cursor_restoration() {
     );
 }
 
-/// HIST-005: Pre-scroll region is emitted via ANSI when viewport is not at bottom.
 #[test]
 fn hist_005_pre_scroll_region_down() {
-    let backend = TestBackend::new(20, 6);
-    let mut term = match codex_tui::custom_terminal::Terminal::with_options(backend) {
-        Ok(t) => t,
-        Err(e) => panic!("failed to construct terminal: {e}"),
-    };
     // Viewport not at bottom: y=3 (0-based), height=1
     let area = Rect::new(0, 3, 20, 1);
-    term.set_viewport_area(area);
+    let mut scenario = TestScenario::new(20, 6, area);
 
     let lines = vec![Line::from("first"), Line::from("second")];
-    let mut buf: Vec<u8> = Vec::new();
-    codex_tui::insert_history::insert_history_lines_to_writer(&mut term, &mut buf, lines);
-
+    let buf = scenario.run_insert(lines);
     let s = String::from_utf8_lossy(&buf);
     // Expect we limited scroll region to [top+1 .. screen_height] => [4 .. 6] (1-based)
     assert!(