mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
fix(tui): handle zellij redraw and composer rendering (#16578)
## TL;DR Fixes the issues when using Codex CLI with Zellij multiplexer. Before this PR there would be no scrollback when using it inside a zellij terminal. ## Problem Addresses #2558 Zellij does not support ANSI scroll-region manipulation (`DECSTBM` / Reverse Index) or the alternate screen buffer in the way traditional terminals do. When codex's TUI runs inside Zellij, two things break: (1) inline history insertion corrupts the display because the scroll-region escape sequences are silently dropped or mishandled, and (2) the composer textarea renders with inherited background/foreground styles that produce unreadable text against Zellij's pane chrome. ## Mental model The fix introduces a **Zellij mode** — a runtime boolean detected once at startup via `codex_terminal_detection::terminal_info().is_zellij()` — that gates two subsystems onto Zellij-safe terminal strategies: - **History insertion** (`insert_history.rs`): Instead of using `DECSTBM` scroll regions and Reverse Index (`ESC M`) to slide content above the viewport, Zellij mode scrolls the screen by emitting `\n` at the bottom row and then writes history lines at absolute positions. This avoids every escape sequence Zellij mishandles. - **Viewport expansion** (`tui.rs`): When the viewport grows taller than available space, the standard path uses `scroll_region_up` on the backend. Zellij mode instead emits newlines at the screen bottom to push content up, then invalidates the ratatui diff buffer so the next draw is a full repaint. - **Composer rendering** (`chat_composer.rs`, `textarea.rs`): All text rendering in the input area uses an explicit `base_style` with `Color::Reset` foreground, preventing Zellij's pane styling from bleeding into the textarea. The prompt chevron (`›`) and placeholder text use explicit color constants instead of relying on `.bold()` / `.dim()` modifiers that render inconsistently under Zellij. ## Non-goals - This change does not fix or improve Zellij's terminal emulation itself. - It does not rearchitect the inline viewport model; it adds a parallel code path gated on detection. - It does not touch the alternate-screen disable logic (that already existed and continues to use `is_zellij` via the same detection). ## Tradeoffs - **Code duplication in `insert_history.rs`**: The Zellij and Standard branches share the line-rendering loop (color setup, span merging, `write_spans`) but differ in the scrolling preamble. The duplication is intentional — merging them would force a complex conditional state machine that's harder to reason about than two flat sequences. - **`invalidate_viewport` after every Zellij history flush or viewport expansion**: This forces a full repaint on every draw cycle in Zellij, which is more expensive than ratatui's normal diff-based rendering. This is necessary because Zellij's lack of scroll-region support means the diff buffer's assumptions about what's on screen are invalid after we manually move content. - **Explicit colors vs semantic modifiers**: Replacing `.bold()` / `.dim()` with `Color::Cyan` / `Color::DarkGray` / `Color::White` in the Zellij branch sacrifices theme-awareness for correctness. If the project ever adopts a theming system, Zellij styling will need to participate. ## Architecture The Zellij detection flag flows through three layers: 1. **`codex_terminal_detection`** — `TerminalInfo::is_zellij()` (new convenience method) reads the already-detected `Multiplexer` variant. 2. **`Tui` struct** — caches `is_zellij` at construction; passes it into `update_inline_viewport`, `flush_pending_history_lines`, and `insert_history_lines_with_mode`. 3. **`ChatComposer` struct** — independently caches `is_zellij` at construction; uses it in `render_textarea` for style decisions. The two caches (`Tui.is_zellij` and `ChatComposer.is_zellij`) are read from the same global `OnceLock<TerminalInfo>`, so they always agree. ## Observability No new logging, metrics, or tracing is introduced. Diagnosis depends on: - Whether `ZELLIJ` or `ZELLIJ_SESSION_NAME` env vars are set (the detection heuristic). - Visual inspection of the rendered TUI inside Zellij vs a standard terminal. - The insta snapshot `zellij_empty_composer` captures the Zellij-mode render path. ## Tests - `terminal_info_reports_is_zellij` — unit test in `terminal-detection` confirming the convenience method. - `zellij_empty_composer_snapshot` — insta snapshot in `chat_composer` validating the Zellij render path for an empty composer. - `vt100_zellij_mode_inserts_history_and_updates_viewport` — integration test in `insert_history` verifying that Zellij-mode history insertion writes content and shifts the viewport. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9bb7f0a694
commit
0bd31dc382
9 files changed
+444
-125
No files matched your search
@@ -19,7 +19,6 @@ use crossterm::event::KeyEventKind;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::style::Color;
|
||||
use ratatui::style::Style;
|
||||
use ratatui::widgets::StatefulWidgetRef;
|
||||
use ratatui::widgets::WidgetRef;
|
||||
@@ -1322,7 +1321,7 @@ impl TextArea {
|
||||
impl WidgetRef for &TextArea {
|
||||
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
|
||||
let lines = self.wrapped_lines(area.width);
|
||||
self.render_lines(area, buf, &lines, 0..lines.len());
|
||||
self.render_lines(area, buf, &lines, 0..lines.len(), Style::default());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1336,7 +1335,7 @@ impl StatefulWidgetRef for &TextArea {
|
||||
|
||||
let start = scroll as usize;
|
||||
let end = (scroll + area.height).min(lines.len() as u16) as usize;
|
||||
self.render_lines(area, buf, &lines, start..end);
|
||||
self.render_lines(area, buf, &lines, start..end, Style::default());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1347,6 +1346,7 @@ impl TextArea {
|
||||
buf: &mut Buffer,
|
||||
state: &mut TextAreaState,
|
||||
mask_char: char,
|
||||
base_style: Style,
|
||||
) {
|
||||
let lines = self.wrapped_lines(area.width);
|
||||
let scroll = self.effective_scroll(area.height, &lines, state.scroll);
|
||||
@@ -1354,7 +1354,25 @@ impl TextArea {
|
||||
|
||||
let start = scroll as usize;
|
||||
let end = (scroll + area.height).min(lines.len() as u16) as usize;
|
||||
self.render_lines_masked(area, buf, &lines, start..end, mask_char);
|
||||
self.render_lines_masked(area, buf, &lines, start..end, mask_char, base_style);
|
||||
}
|
||||
|
||||
/// Render the textarea with an explicit `base_style` applied to every cell,
|
||||
/// used by the Zellij code path to override inherited terminal styles.
|
||||
pub(crate) fn render_ref_styled(
|
||||
&self,
|
||||
area: Rect,
|
||||
buf: &mut Buffer,
|
||||
state: &mut TextAreaState,
|
||||
base_style: Style,
|
||||
) {
|
||||
let lines = self.wrapped_lines(area.width);
|
||||
let scroll = self.effective_scroll(area.height, &lines, state.scroll);
|
||||
state.scroll = scroll;
|
||||
|
||||
let start = scroll as usize;
|
||||
let end = (scroll + area.height).min(lines.len() as u16) as usize;
|
||||
self.render_lines(area, buf, &lines, start..end, base_style);
|
||||
}
|
||||
|
||||
fn render_lines(
|
||||
@@ -1363,13 +1381,15 @@ impl TextArea {
|
||||
buf: &mut Buffer,
|
||||
lines: &[Range<usize>],
|
||||
range: std::ops::Range<usize>,
|
||||
base_style: Style,
|
||||
) {
|
||||
for (row, idx) in range.enumerate() {
|
||||
let r = &lines[idx];
|
||||
let y = area.y + row as u16;
|
||||
let line_range = r.start..r.end - 1;
|
||||
// Draw base line with default style.
|
||||
buf.set_string(area.x, y, &self.text[line_range.clone()], Style::default());
|
||||
buf.set_style(Rect::new(area.x, y, area.width, 1), base_style);
|
||||
// Draw base line with the provided style.
|
||||
buf.set_string(area.x, y, &self.text[line_range.clone()], base_style);
|
||||
|
||||
// Overlay styled segments for elements that intersect this line.
|
||||
for elem in &self.elements {
|
||||
@@ -1381,7 +1401,7 @@ impl TextArea {
|
||||
}
|
||||
let styled = &self.text[overlap_start..overlap_end];
|
||||
let x_off = self.text[line_range.start..overlap_start].width() as u16;
|
||||
let style = Style::default().fg(Color::Cyan);
|
||||
let style = base_style.fg(ratatui::style::Color::Cyan);
|
||||
buf.set_string(area.x + x_off, y, styled, style);
|
||||
}
|
||||
}
|
||||
@@ -1394,16 +1414,18 @@ impl TextArea {
|
||||
lines: &[Range<usize>],
|
||||
range: std::ops::Range<usize>,
|
||||
mask_char: char,
|
||||
base_style: Style,
|
||||
) {
|
||||
for (row, idx) in range.enumerate() {
|
||||
let r = &lines[idx];
|
||||
let y = area.y + row as u16;
|
||||
let line_range = r.start..r.end - 1;
|
||||
buf.set_style(Rect::new(area.x, y, area.width, 1), base_style);
|
||||
let masked = self.text[line_range.clone()]
|
||||
.chars()
|
||||
.map(|_| mask_char)
|
||||
.collect::<String>();
|
||||
buf.set_string(area.x, y, &masked, Style::default());
|
||||
buf.set_string(area.x, y, &masked, base_style);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user