Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions codex-rs/tui/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,28 @@ State fields on `ChatWidget`: `loop_remaining: Option<i32>` and `loop_total: Opt

The loop is cancelled (both fields set to `None`) when an error occurs (`on_error()`) or the user interrupts (`on_interrupted_turn()`). The `/config` sub-picker is a custom `BottomPaneView` implemented by `LoopCountPickerView` in `@/codex-rs/tui/src/nori/loop_count_picker.rs`. It offers preset options (Disabled, 2, 3, 5, 10) plus a "Custom..." option that enters an input mode where the user can type an arbitrary number (2-1000). Values <= 1 are treated as disabled, values > 1000 are capped. This follows the same `BottomPaneView` pattern used by `HotkeyPickerView`. The setting persists to `[tui]` in `config.toml` via `persist_loop_count_setting()`.

**History Insertion and Scrollback (`insert_history.rs`):**

`insert_history_lines()` pushes content into the terminal's native scrollback buffer above the ratatui viewport without disturbing ratatui's diff-based renderer. It works by manipulating ANSI scroll regions (DECSTBM, `\x1b[Pt;Pbr`) directly against the crossterm backend writer, bypassing the normal ratatui render pass.

The insertion algorithm:

```
1. If viewport is not at screen bottom: scroll viewport downward using RI (ESC M) inside
a temporary scroll region covering [viewport.top()+1 .. screen_height].
2. Early return if area.top() == 0 (viewport fills the whole screen; no space above it).
3. Set scroll region to [1 .. area.top()] (only the history area above the viewport).
4. Write lines into that region with \r\n advancement.
5. Reset scroll region to full screen.
6. Restore cursor to its pre-call position.
```

The critical invariant: **DECSTBM `Pb=0` means "bottom of screen"**, not row 0. Calling `SetScrollRegion(1..0)` when `area.top() == 0` produces `\x1b[1;0r`, which sets the scroll region to the entire terminal rather than an empty region. Any subsequent writes then scroll through the viewport, corrupting ratatui's content in ways the diff-based renderer cannot detect. The `area.top() == 0` early return guards against this.

Two crossterm `Command` implementations support the function:
- `SetScrollRegion(Range<u16>)` — emits `\x1b[{start};{end}r`
- `ResetScrollRegion` — emits `\x1b[r` (restores full-screen scrolling)

### Things to Know

**Module Structure Convention:**
Expand Down
111 changes: 111 additions & 0 deletions codex-rs/tui/src/insert_history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ where
area.top().saturating_sub(1)
};

// No room above the viewport for history lines.
if area.top() == 0 {
let _ = writer;
return Ok(());
}

// Limit the scroll region to the lines from the top of the screen to the
// top of the viewport. With this in place, when we add lines inside this
// area, only the lines in this area will be scrolled. We place the cursor
Expand Down Expand Up @@ -527,4 +533,109 @@ mod tests {
);
}
}

/// When the viewport occupies the entire screen (area.top() == 0), there is
/// no room above the viewport. insert_history_lines must not corrupt the
/// viewport content by writing through a degenerate scroll region.
#[test]
fn full_screen_viewport_does_not_corrupt_display() {
let width: u16 = 40;
let height: u16 = 10;
let backend = VT100Backend::new(width, height);
let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal");

// Viewport fills the entire screen: y=0, height=10
let viewport = Rect::new(0, 0, width, height);
term.set_viewport_area(viewport);

// Draw some known content into the viewport first
term.draw(|frame| {
let buf = frame.buffer_mut();
for y in 0..height {
let text = format!("Row {y}");
buf.set_string(0, y, &text, ratatui::style::Style::default());
}
})
.expect("draw");
// Flush the draw output so vt100 sees it
Backend::flush(term.backend_mut()).expect("flush");

// Capture the screen contents before insert_history_lines
let before: Vec<String> = term.backend().vt100().screen().rows(0, width).collect();

// Now try to insert history lines — there's no room above the viewport
let line = Line::from("This should not corrupt the display");
insert_history_lines(&mut term, vec![line]).expect("insert");
Backend::flush(term.backend_mut()).expect("flush");

// The viewport content must be unchanged
let after: Vec<String> = term.backend().vt100().screen().rows(0, width).collect();

pretty_assertions::assert_eq!(
before,
after,
"viewport content was corrupted by insert_history_lines when area.top()==0"
);
}

/// When there IS room above the viewport, history lines should appear
/// above the viewport and the viewport content should be preserved.
#[test]
fn history_lines_inserted_above_viewport_with_room() {
let width: u16 = 40;
let height: u16 = 10;
let viewport_y: usize = 5;
let viewport_h: usize = 5;
let backend = VT100Backend::new(width, height);
let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal");

// Viewport at y=5 with height=5, leaving 5 rows above for history
let viewport = Rect::new(0, viewport_y as u16, width, viewport_h as u16);
term.set_viewport_area(viewport);

// Draw known viewport content
term.draw(|frame| {
let area = frame.area();
let buf = frame.buffer_mut();
for i in 0..area.height {
let text = format!("Viewport row {i}");
buf.set_string(area.x, area.y + i, &text, ratatui::style::Style::default());
}
})
.expect("draw");
Backend::flush(term.backend_mut()).expect("flush");

// Insert a history line
let line = Line::from("History entry");
insert_history_lines(&mut term, vec![line]).expect("insert");
Backend::flush(term.backend_mut()).expect("flush");

let rows: Vec<String> = term.backend().vt100().screen().rows(0, width).collect();

// The history line should appear somewhere above the viewport
let history_row = rows
.iter()
.position(|r| r.contains("History entry"))
.unwrap_or_else(|| {
panic!("expected 'History entry' above viewport, got rows: {rows:?}")
});
assert!(
history_row < viewport_y,
"history line at row {history_row} should be above viewport at y={viewport_y}",
);

// Viewport content should still be intact — find it by searching for
// "Viewport row 0" and checking consecutive rows from there.
let vp_start = rows
.iter()
.position(|r| r.contains("Viewport row 0"))
.expect("could not find 'Viewport row 0' on screen");
for i in 0..viewport_h {
let row_text = &rows[vp_start + i];
assert!(
row_text.contains(&format!("Viewport row {i}")),
"viewport row {i} should contain 'Viewport row {i}', got: {row_text:?}"
);
}
}
}
Loading