From 6f77b70ff3d9995fa7e0dd7b88f11af3bef05d0d Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Wed, 13 May 2026 12:11:15 -0300 Subject: [PATCH] feat(tui): remove Zellij TUI workarounds (#22214) ## Why We added Zellij-specific TUI workarounds because older Zellij behavior did not work with Codex's normal terminal model: - #8555 made `tui.alternate_screen = "auto"` disable alternate screen in Zellij so transcript history stayed available. - #16578 avoided scroll-region operations in Zellij by emitting raw newlines and using a separate composer styling path. This PR removes both workarounds because the latest Zellij release tested locally (`zellij 0.44.1`) works correctly with Codex's standard TUI behavior: normal alternate-screen handling, redraw, and history insertion. ## What Changed - Removed the `InsertHistoryMode::Zellij` path and the Zellij-only newline scrollback insertion behavior. - Removed cached `is_zellij` state from the TUI and composer. - Removed Zellij-specific composer styling, the helper snapshot, and the `TerminalInfo::is_zellij()` convenience method that only served this workaround. - Changed `tui.alternate_screen = "auto"` to use alternate screen for Zellij too; `--no-alt-screen` and `tui.alternate_screen = "never"` still preserve the inline mode escape hatch. - Updated the generated config schema description for `tui.alternate_screen`. ## How to Test Manual smoke path used with `zellij 0.44.1`: 1. Build and run this branch inside a Zellij `0.44.1` session with default config. 2. Start Codex normally and produce enough assistant/tool output to create scrollback. 3. Confirm the transcript remains readable, the composer renders normally, and scrolling through terminal history works. 4. Resize the Zellij pane while output exists and confirm the TUI redraws without duplicated, missing, or stale rows. 5. Compare with `--no-alt-screen` or `-c tui.alternate_screen=never` if you want to verify the inline fallback still works. Targeted tests: - `just write-config-schema` - `just fmt` - `just fix -p codex-tui` - `cargo test -p codex-terminal-detection` - `cargo test -p codex-tui alternate_screen_auto_uses_alt_screen` Attempted but did not complete locally: - `cargo test -p codex-tui` built and ran the new test successfully, then failed later on unrelated local failures in `status_permissions_full_disk_managed_*` and a stack overflow in `tests::fork_last_filters_latest_session_by_cwd_unless_show_all`. ## Documentation No developers.openai.com Codex documentation update is needed for this revert. --- codex-rs/config/src/types.rs | 7 +- codex-rs/core/config.schema.json | 8 +- codex-rs/core/src/config/mod.rs | 4 +- codex-rs/protocol/src/config_types.rs | 23 +- codex-rs/tui/src/bottom_pane/chat_composer.rs | 127 +---------- ...omposer__tests__zellij_empty_composer.snap | 14 -- codex-rs/tui/src/bottom_pane/textarea.rs | 28 +-- codex-rs/tui/src/cli.rs | 4 +- codex-rs/tui/src/custom_terminal.rs | 5 +- codex-rs/tui/src/insert_history.rs | 204 ++++-------------- codex-rs/tui/src/lib.rs | 50 ++--- codex-rs/tui/src/tui.rs | 126 +++-------- 12 files changed, 126 insertions(+), 474 deletions(-) delete mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__zellij_empty_composer.snap diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index dfc81852ea..21d9b44752 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -668,12 +668,9 @@ pub struct Tui { /// Controls whether the TUI uses the terminal's alternate screen buffer. /// - /// - `auto` (default): Disable alternate screen in Zellij, enable elsewhere. - /// - `always`: Always use alternate screen (original behavior). + /// - `auto` (default): Use alternate screen. + /// - `always`: Always use alternate screen. /// - `never`: Never use alternate screen (inline mode only, preserves scrollback). - /// - /// Using alternate screen provides a cleaner fullscreen experience but prevents - /// scrollback in terminal multiplexers like Zellij that follow the xterm spec. #[serde(default)] pub alternate_screen: AltScreenMode, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 7601f01377..1d3ca3210b 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -62,17 +62,17 @@ "type": "object" }, "AltScreenMode": { - "description": "Controls whether the TUI uses the terminal's alternate screen buffer.\n\n**Background:** The alternate screen buffer provides a cleaner fullscreen experience without polluting the terminal's scrollback history. However, it conflicts with terminal multiplexers like Zellij that strictly follow the xterm specification, which defines that alternate screen buffers should not have scrollback.\n\n**Zellij's behavior:** Zellij intentionally disables scrollback in alternate screen mode (see https://github.com/zellij-org/zellij/pull/1032) to comply with the xterm spec. This is by design and not configurable in Zellij—there is no option to enable scrollback in alternate screen mode.\n\n**Solution:** This setting provides a pragmatic workaround: - `auto` (default): Automatically detect the terminal multiplexer. If running in Zellij, disable alternate screen to preserve scrollback. Enable it everywhere else. - `always`: Always use alternate screen mode (original behavior before this fix). - `never`: Never use alternate screen mode. Runs in inline mode, preserving scrollback in all multiplexers.\n\nThe CLI flag `--no-alt-screen` can override this setting at runtime.", + "description": "Controls whether the TUI uses the terminal's alternate screen buffer.\n\n- `auto` (default): Use alternate screen mode. - `always`: Always use alternate screen mode. - `never`: Never use alternate screen mode. Runs in inline mode, preserving scrollback.\n\nThe CLI flag `--no-alt-screen` can override this setting at runtime.", "oneOf": [ { - "description": "Auto-detect: disable alternate screen in Zellij, enable elsewhere.", + "description": "Use alternate screen mode.", "enum": [ "auto" ], "type": "string" }, { - "description": "Always use alternate screen (original behavior).", + "description": "Always use alternate screen mode.", "enum": [ "always" ], @@ -2536,7 +2536,7 @@ } ], "default": "auto", - "description": "Controls whether the TUI uses the terminal's alternate screen buffer.\n\n- `auto` (default): Disable alternate screen in Zellij, enable elsewhere. - `always`: Always use alternate screen (original behavior). - `never`: Never use alternate screen (inline mode only, preserves scrollback).\n\nUsing alternate screen provides a cleaner fullscreen experience but prevents scrollback in terminal multiplexers like Zellij that follow the xterm spec." + "description": "Controls whether the TUI uses the terminal's alternate screen buffer.\n\n- `auto` (default): Use alternate screen. - `always`: Always use alternate screen. - `never`: Never use alternate screen (inline mode only, preserves scrollback)." }, "animations": { "default": true, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index f5f95f9e87..d74f83102c 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -531,8 +531,8 @@ pub struct Config { /// Controls whether the TUI uses the terminal's alternate screen buffer. /// /// This is the same `tui.alternate_screen` value from `config.toml`. - /// - `auto` (default): Disable alternate screen in Zellij, enable elsewhere. - /// - `always`: Always use alternate screen (original behavior). + /// - `auto` (default): Use alternate screen. + /// - `always`: Always use alternate screen. /// - `never`: Never use alternate screen (inline mode, preserves scrollback). pub tui_alternate_screen: AltScreenMode, /// Ordered list of status line item identifiers for the TUI. diff --git a/codex-rs/protocol/src/config_types.rs b/codex-rs/protocol/src/config_types.rs index 47dc15f183..37e8d95e25 100644 --- a/codex-rs/protocol/src/config_types.rs +++ b/codex-rs/protocol/src/config_types.rs @@ -465,22 +465,9 @@ pub enum TrustLevel { /// Controls whether the TUI uses the terminal's alternate screen buffer. /// -/// **Background:** The alternate screen buffer provides a cleaner fullscreen experience -/// without polluting the terminal's scrollback history. However, it conflicts with terminal -/// multiplexers like Zellij that strictly follow the xterm specification, which defines -/// that alternate screen buffers should not have scrollback. -/// -/// **Zellij's behavior:** Zellij intentionally disables scrollback in alternate screen mode -/// (see https://github.com/zellij-org/zellij/pull/1032) to comply with the xterm spec. This -/// is by design and not configurable in Zellij—there is no option to enable scrollback in -/// alternate screen mode. -/// -/// **Solution:** This setting provides a pragmatic workaround: -/// - `auto` (default): Automatically detect the terminal multiplexer. If running in Zellij, -/// disable alternate screen to preserve scrollback. Enable it everywhere else. -/// - `always`: Always use alternate screen mode (original behavior before this fix). -/// - `never`: Never use alternate screen mode. Runs in inline mode, preserving scrollback -/// in all multiplexers. +/// - `auto` (default): Use alternate screen mode. +/// - `always`: Always use alternate screen mode. +/// - `never`: Never use alternate screen mode. Runs in inline mode, preserving scrollback. /// /// The CLI flag `--no-alt-screen` can override this setting at runtime. #[derive( @@ -489,10 +476,10 @@ pub enum TrustLevel { #[serde(rename_all = "lowercase")] #[strum(serialize_all = "lowercase")] pub enum AltScreenMode { - /// Auto-detect: disable alternate screen in Zellij, enable elsewhere. + /// Use alternate screen mode. #[default] Auto, - /// Always use alternate screen (original behavior). + /// Always use alternate screen mode. Always, /// Never use alternate screen (inline mode only). Never, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 5f93febfa7..0f97b7f6c9 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -415,7 +415,6 @@ pub(crate) struct ChatComposer { audio_device_selection_enabled: bool, windows_degraded_sandbox_active: bool, side_conversation_active: bool, - is_zellij: bool, status_line_value: Option>, status_line_hyperlink_url: Option, status_line_enabled: bool, @@ -599,10 +598,6 @@ impl ChatComposer { audio_device_selection_enabled: false, windows_degraded_sandbox_active: false, side_conversation_active: false, - is_zellij: matches!( - codex_terminal_detection::terminal_info().multiplexer, - Some(codex_terminal_detection::Multiplexer::Zellij {}) - ), status_line_value: None, status_line_hyperlink_url: None, status_line_enabled: false, @@ -4788,56 +4783,20 @@ impl ChatComposer { } } } - self.render_textarea( - composer_rect, - remote_images_rect, - textarea_rect, - buf, - mask_char, - ); - } - - /// Paint the composer's text input area, prompt chevron, and placeholder text. - /// - /// In Zellij sessions the textarea uses explicit `Color::Reset` foreground styling - /// to prevent the multiplexer's pane chrome from bleeding into cell styles, and - /// substitutes hardcoded colors for `.bold()` / `.dim()` modifiers that Zellij - /// renders inconsistently. The standard path is unchanged. - fn render_textarea( - &self, - composer_rect: Rect, - remote_images_rect: Rect, - textarea_rect: Rect, - buf: &mut Buffer, - mask_char: Option, - ) { - let is_zellij = self.is_zellij; let style = user_message_style(); - let textarea_style = style.fg(ratatui::style::Color::Reset); Block::default().style(style).render_ref(composer_rect, buf); if !remote_images_rect.is_empty() { Paragraph::new(self.remote_images_lines(remote_images_rect.width)) .style(style) .render_ref(remote_images_rect, buf); } - if is_zellij && !textarea_rect.is_empty() { - buf.set_style(textarea_rect, textarea_style); - } if !textarea_rect.is_empty() { let prompt = if self.input_enabled { if self.is_bash_mode { - if is_zellij { - Span::from("!").light_red() - } else { - Span::from("!").light_red().bold() - } - } else if is_zellij { - Span::styled("›", style.fg(ratatui::style::Color::Cyan)) + Span::from("!").light_red().bold() } else { "›".bold() } - } else if is_zellij { - Span::styled("›", style.fg(ratatui::style::Color::DarkGray)) } else { "›".dim() }; @@ -4852,39 +4811,8 @@ impl ChatComposer { let mut state = self.textarea_state.borrow_mut(); let textarea_is_empty = self.textarea.text().is_empty() && !self.is_bash_mode; if let Some(mask_char) = mask_char { - self.textarea.render_ref_masked( - textarea_rect, - buf, - &mut state, - mask_char, - if is_zellij { - textarea_style - } else { - ratatui::style::Style::default() - }, - ); - } else if is_zellij && textarea_is_empty { - buf.set_style(textarea_rect, textarea_style); - } else if is_zellij { - let highlight_ranges = self.history_search_highlight_ranges(); - if highlight_ranges.is_empty() { - self.textarea - .render_ref_styled(textarea_rect, buf, &mut state, textarea_style); - } else { - let highlight_style = - textarea_style.add_modifier(Modifier::REVERSED | Modifier::BOLD); - let highlights = highlight_ranges - .into_iter() - .map(|range| (range, highlight_style)) - .collect::>(); - self.textarea.render_ref_styled_with_highlights( - textarea_rect, - buf, - &mut state, - textarea_style, - &highlights, - ); - } + self.textarea + .render_ref_masked(textarea_rect, buf, &mut state, mask_char); } else { let highlight_ranges = self.history_search_highlight_ranges(); if highlight_ranges.is_empty() { @@ -4915,18 +4843,9 @@ impl ChatComposer { .to_string() }; if !textarea_rect.is_empty() { - if is_zellij { - buf.set_string( - textarea_rect.x, - textarea_rect.y, - text, - textarea_style.fg(ratatui::style::Color::White).italic(), - ); - } else { - let placeholder = Span::from(text).dim(); - let line = Line::from(vec![placeholder]); - line.render_ref(textarea_rect.inner(Margin::new(0, 0)), buf); - } + let placeholder = Span::from(text).dim(); + Line::from(vec![placeholder]) + .render_ref(textarea_rect.inner(Margin::new(0, 0)), buf); } } } @@ -5149,35 +5068,6 @@ mod tests { ); } - fn snapshot_zellij_composer_state(name: &str, setup: F) - where - F: FnOnce(&mut ChatComposer), - { - use ratatui::Terminal; - use ratatui::backend::TestBackend; - - let (tx, _rx) = unbounded_channel::(); - let sender = AppEventSender::new(tx); - let mut composer = ChatComposer::new( - /*has_input_focus*/ true, - sender, - /*enhanced_keys_supported*/ true, - "Ask Codex to do anything".to_string(), - /*disable_paste_burst*/ false, - ); - composer.is_zellij = true; - setup(&mut composer); - let footer_props = composer.footer_props(); - let footer_lines = footer_height(&footer_props); - let footer_spacing = ChatComposer::footer_spacing(footer_lines); - let height = footer_lines + footer_spacing + 8; - let mut terminal = Terminal::new(TestBackend::new(100, height)).unwrap(); - terminal - .draw(|f| composer.render(f.area(), f.buffer_mut())) - .unwrap(); - insta::assert_snapshot!(name, terminal.backend()); - } - #[test] fn footer_mode_snapshots() { use crossterm::event::KeyCode; @@ -5697,11 +5587,6 @@ mod tests { ); } - #[test] - fn zellij_empty_composer_snapshot() { - snapshot_zellij_composer_state("zellij_empty_composer", |_composer| {}); - } - #[test] fn esc_hint_stays_hidden_with_draft_content() { use crossterm::event::KeyCode; diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__zellij_empty_composer.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__zellij_empty_composer.snap deleted file mode 100644 index 15496a4539..0000000000 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__zellij_empty_composer.snap +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: tui/src/bottom_pane/chat_composer.rs -assertion_line: 4001 -expression: terminal.backend() ---- -" " -"› Ask Codex to do anything " -" " -" " -" " -" " -" " -" " -" ? for shortcuts 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 0638412110..96f6cf8cbb 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -24,6 +24,7 @@ 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; @@ -1893,7 +1894,6 @@ 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); @@ -1901,25 +1901,7 @@ 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, 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, &[]); + self.render_lines_masked(area, buf, &lines, start..end, mask_char); } /// Render the textarea with `base_style` plus additional render-only highlight ranges. @@ -1970,7 +1952,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 = base_style.fg(ratatui::style::Color::Cyan); + let style = base_style.fg(Color::Cyan); buf.set_string(area.x + x_off, y, styled, style); } @@ -1996,18 +1978,16 @@ impl TextArea { lines: &[Range], range: std::ops::Range, 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::(); - buf.set_string(area.x, y, &masked, base_style); + buf.set_string(area.x, y, &masked, Style::default()); } } } diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index 3d47442c90..d6001c60d8 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -63,9 +63,7 @@ pub struct Cli { /// Disable alternate screen mode /// - /// Runs the TUI in inline mode, preserving terminal scrollback history. This is useful - /// in terminal multiplexers like Zellij that follow the xterm spec strictly and disable - /// scrollback in alternate screen buffers. + /// Runs the TUI in inline mode, preserving terminal scrollback history. #[arg(long = "no-alt-screen", default_value_t = false)] pub no_alt_screen: bool, diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index 3d0519080e..995a0358f6 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -486,9 +486,8 @@ where } /// Force the next draw pass to repaint the entire viewport by resetting the - /// diff buffer. Call this after operations that move screen content outside of - /// ratatui's knowledge (e.g., Zellij-mode scrolling via raw newlines), since - /// the diff buffer's assumptions about what is currently displayed are invalid. + /// diff buffer. Call this after raw terminal operations that move screen + /// content outside ratatui's knowledge. pub fn invalidate_viewport(&mut self) { self.previous_buffer_mut().reset(); } diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 6a83594e1f..f2c64a1098 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -1,8 +1,7 @@ //! Inserts finalized history rows into terminal scrollback. //! //! Codex uses the terminal scrollback itself for finalized chat history, so inserting a history -//! cell is an escape-sequence operation rather than a normal ratatui render. The mode determines -//! how to create room for new history above the inline viewport. +//! cell is an escape-sequence operation rather than a normal ratatui render. use std::fmt; use std::io; @@ -35,28 +34,6 @@ use ratatui::style::Modifier; use ratatui::text::Line; use ratatui::text::Span; -/// Selects the terminal escape strategy for inserting history lines above the viewport. -/// -/// Standard terminals support `DECSTBM` scroll regions and Reverse Index (`ESC M`), -/// which let us slide existing content down without redrawing it. Zellij silently -/// drops or mishandles those sequences, so `Zellij` mode falls back to emitting -/// newlines at the bottom of the screen and writing lines at absolute positions. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum InsertHistoryMode { - Standard, - Zellij, -} - -impl InsertHistoryMode { - pub fn new(is_zellij: bool) -> Self { - if is_zellij { - Self::Zellij - } else { - Self::Standard - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum HistoryLineWrapPolicy { PreWrap, @@ -72,38 +49,12 @@ pub fn insert_history_lines( where B: Backend + Write, { - insert_history_lines_with_mode(terminal, lines, InsertHistoryMode::Standard) + insert_history_lines_with_wrap_policy(terminal, lines, HistoryLineWrapPolicy::PreWrap) } -/// Insert `lines` above the viewport, using the escape strategy selected by `mode`. -/// -/// In `Standard` mode this manipulates DECSTBM scroll regions to slide existing -/// scrollback down and writes new lines into the freed space. In `Zellij` mode it -/// emits newlines at the screen bottom to create space (since Zellij ignores scroll -/// region escapes) and writes lines at computed absolute positions. Both modes -/// update `terminal.viewport_area` so subsequent draw passes know where the -/// viewport moved to. Resize reflow uses the same viewport-aware path after -/// clearing old scrollback. -pub fn insert_history_lines_with_mode( +pub fn insert_history_lines_with_wrap_policy( terminal: &mut crate::custom_terminal::Terminal, lines: Vec, - mode: InsertHistoryMode, -) -> io::Result<()> -where - B: Backend + Write, -{ - insert_history_lines_with_mode_and_wrap_policy( - terminal, - lines, - mode, - HistoryLineWrapPolicy::PreWrap, - ) -} - -pub fn insert_history_lines_with_mode_and_wrap_policy( - terminal: &mut crate::custom_terminal::Terminal, - lines: Vec, - mode: InsertHistoryMode, wrap_policy: HistoryLineWrapPolicy, ) -> io::Result<()> where @@ -151,90 +102,56 @@ where wrapped.extend(line_wrapped); } let wrapped_lines = wrapped_rows 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. + // Don't scroll it past the bottom of the screen. + let scroll_amount = wrapped_lines.min(screen_size.height - area.bottom()); - match mode { - InsertHistoryMode::Zellij => { - let space_below = screen_size.height.saturating_sub(area.bottom()); - let shift_down = wrapped_lines.min(space_below); - let scroll_up_amount = wrapped_lines.saturating_sub(shift_down); - - if scroll_up_amount > 0 { - // Scroll the entire screen up by emitting \n at the bottom - queue!( - writer, - MoveTo(/*x*/ 0, screen_size.height.saturating_sub(1)) - )?; - for _ in 0..scroll_up_amount { - queue!(writer, Print("\n"))?; - } - } - - if shift_down > 0 { - area.y += shift_down; - should_update_area = true; - } - - let cursor_top = area.top().saturating_sub(scroll_up_amount + shift_down); - queue!(writer, MoveTo(/*x*/ 0, cursor_top))?; - - for (i, line) in wrapped.iter().enumerate() { - if i > 0 { - queue!(writer, Print("\r\n"))?; - } - write_history_line(writer, line, wrap_width)?; - } + let top_1based = area.top() + 1; + queue!(writer, SetScrollRegion(top_1based..screen_size.height))?; + queue!(writer, MoveTo(/*x*/ 0, area.top()))?; + for _ in 0..scroll_amount { + queue!(writer, Print("\x1bM"))?; } - InsertHistoryMode::Standard => { - let cursor_top = if area.bottom() < screen_size.height { - let scroll_amount = wrapped_lines.min(screen_size.height - area.bottom()); + queue!(writer, ResetScrollRegion)?; - let top_1based = area.top() + 1; - queue!(writer, SetScrollRegion(top_1based..screen_size.height))?; - queue!(writer, MoveTo(/*x*/ 0, area.top()))?; - for _ in 0..scroll_amount { - queue!(writer, Print("\x1bM"))?; - } - queue!(writer, ResetScrollRegion)?; + let cursor_top = area.top().saturating_sub(1); + area.y += scroll_amount; + should_update_area = true; + cursor_top + } else { + area.top().saturating_sub(1) + }; - let cursor_top = area.top().saturating_sub(1); - area.y += scroll_amount; - should_update_area = true; - cursor_top - } else { - area.top().saturating_sub(1) - }; + // 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 + // at the end of the scroll region, and add lines starting there. + // + // ┌─Screen───────────────────────┐ + // │┌╌Scroll region╌╌╌╌╌╌╌╌╌╌╌╌╌╌┐│ + // │┆ ┆│ + // │┆ ┆│ + // │┆ ┆│ + // │█╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┘│ + // │╭─Viewport───────────────────╮│ + // ││ ││ + // │╰────────────────────────────╯│ + // └──────────────────────────────┘ + queue!(writer, SetScrollRegion(1..area.top()))?; - // 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 - // at the end of the scroll region, and add lines starting there. - // - // ┌─Screen───────────────────────┐ - // │┌╌Scroll region╌╌╌╌╌╌╌╌╌╌╌╌╌╌┐│ - // │┆ ┆│ - // │┆ ┆│ - // │┆ ┆│ - // │█╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┘│ - // │╭─Viewport───────────────────╮│ - // ││ ││ - // │╰────────────────────────────╯│ - // └──────────────────────────────┘ - queue!(writer, SetScrollRegion(1..area.top()))?; + // NB: we are using MoveTo instead of set_cursor_position here to avoid messing with the + // terminal's last_known_cursor_position, which hopefully will still be accurate after we + // fetch/restore the cursor position. insert_history_lines should be cursor-position-neutral :) + queue!(writer, MoveTo(/*x*/ 0, cursor_top))?; - // NB: we are using MoveTo instead of set_cursor_position here to avoid messing with the - // terminal's last_known_cursor_position, which hopefully will still be accurate after we - // fetch/restore the cursor position. insert_history_lines should be cursor-position-neutral :) - queue!(writer, MoveTo(/*x*/ 0, cursor_top))?; - - for line in &wrapped { - queue!(writer, Print("\r\n"))?; - write_history_line(writer, line, wrap_width)?; - } - - queue!(writer, ResetScrollRegion)?; - } + for line in &wrapped { + queue!(writer, Print("\r\n"))?; + write_history_line(writer, line, wrap_width)?; } + queue!(writer, ResetScrollRegion)?; + // Restore the cursor position to where it was before we started. queue!(writer, MoveTo(last_cursor_pos.x, last_cursor_pos.y))?; @@ -864,10 +781,9 @@ mod tests { let line = Line::from("alpha beta gamma delta epsilon zeta"); - insert_history_lines_with_mode_and_wrap_policy( + insert_history_lines_with_wrap_policy( &mut term, vec![line], - InsertHistoryMode::Standard, HistoryLineWrapPolicy::Terminal, ) .expect("insert raw history"); @@ -954,32 +870,4 @@ mod tests { "expected URL content to appear immediately after prompt (allowing at most one spacer row), got prompt_row={prompt_row}, url_row={url_row}, rows={rows:?}", ); } - - #[test] - fn vt100_zellij_mode_inserts_history_and_updates_viewport() { - let width: u16 = 32; - let height: u16 = 8; - let backend = VT100Backend::new(width, height); - let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); - let viewport = Rect::new(/*x*/ 0, /*y*/ 4, width, /*height*/ 2); - term.set_viewport_area(viewport); - - let line: Line<'static> = Line::from("zellij history"); - insert_history_lines_with_mode(&mut term, vec![line], InsertHistoryMode::Zellij) - .expect("insert zellij history"); - - let start_row = 0; - let rows: Vec = term - .backend() - .vt100() - .screen() - .rows(start_row, width) - .collect(); - assert!( - rows.iter().any(|row| row.contains("zellij history")), - "expected zellij history row in screen output, rows: {rows:?}" - ); - assert_eq!(term.viewport_area, Rect::new(0, 5, width, 2)); - assert_eq!(term.visible_history_rows(), 1); - } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index b86c92dfa4..97c7d023f5 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -53,7 +53,6 @@ use codex_protocol::config_types::WindowsSandboxLevel; use codex_rollout::StateDbHandle; use codex_rollout::state_db; use codex_state::log_db; -use codex_terminal_detection::terminal_info; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_existing_preserving_symlinks; use codex_utils_oss::ensure_oss_provider_ready; @@ -1655,36 +1654,17 @@ impl Drop for TerminalRestoreGuard { /// Determine whether to use the terminal's alternate screen buffer. /// -/// The alternate screen buffer provides a cleaner fullscreen experience without polluting -/// the terminal's scrollback history. However, it conflicts with terminal multiplexers like -/// Zellij that strictly follow the xterm spec, which disallows scrollback in alternate screen -/// buffers. Zellij intentionally disables scrollback in alternate screen mode (see -/// https://github.com/zellij-org/zellij/pull/1032) and offers no configuration option to -/// change this behavior. -/// -/// This function implements a pragmatic workaround: /// - If `--no-alt-screen` is explicitly passed, always disable alternate screen /// - Otherwise, respect the `tui.alternate_screen` config setting: -/// - `always`: Use alternate screen everywhere (original behavior) +/// - `always`: Use alternate screen /// - `never`: Inline mode only, preserves scrollback -/// - `auto` (default): Auto-detect the terminal multiplexer and disable alternate screen -/// only in Zellij, enabling it everywhere else +/// - `auto` (default): Use alternate screen fn determine_alt_screen_mode(no_alt_screen: bool, tui_alternate_screen: AltScreenMode) -> bool { if no_alt_screen { - false - } else { - match tui_alternate_screen { - AltScreenMode::Always => true, - AltScreenMode::Never => false, - AltScreenMode::Auto => { - let terminal_info = terminal_info(); - !matches!( - terminal_info.multiplexer, - Some(codex_terminal_detection::Multiplexer::Zellij {}) - ) - } - } + return false; } + + tui_alternate_screen != AltScreenMode::Never } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -1817,6 +1797,26 @@ mod tests { .await } + #[test] + fn alternate_screen_auto_uses_alt_screen() { + assert!(determine_alt_screen_mode( + /*no_alt_screen*/ false, + AltScreenMode::Auto, + )); + assert!(determine_alt_screen_mode( + /*no_alt_screen*/ false, + AltScreenMode::Always, + )); + assert!(!determine_alt_screen_mode( + /*no_alt_screen*/ false, + AltScreenMode::Never, + )); + assert!(!determine_alt_screen_mode( + /*no_alt_screen*/ true, + AltScreenMode::Auto, + )); + } + #[test] fn session_target_display_label_falls_back_to_thread_id() { let thread_id = ThreadId::new(); diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 9255d188c4..2d22e161fa 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -33,7 +33,6 @@ use ratatui::crossterm::terminal::enable_raw_mode; use ratatui::layout::Offset; use ratatui::layout::Position; use ratatui::layout::Rect; -use ratatui::layout::Size; use ratatui::text::Line; use tokio::sync::broadcast; use tokio_stream::Stream; @@ -440,8 +439,7 @@ pub struct Tui { enhanced_keys_supported: bool, notification_backend: Option, notification_condition: NotificationCondition, - is_zellij: bool, - // When false, enter_alt_screen() becomes a no-op (for Zellij scrollback support) + // When false, enter_alt_screen() becomes a no-op. alt_screen_enabled: bool, } @@ -474,10 +472,6 @@ impl Tui { // Cache this to avoid contention with the event reader. supports_color::on_cached(supports_color::Stream::Stdout); let _ = crate::terminal_palette::default_colors(); - let is_zellij = matches!( - codex_terminal_detection::terminal_info().multiplexer, - Some(codex_terminal_detection::Multiplexer::Zellij {}) - ); Self { frame_requester, @@ -495,7 +489,6 @@ impl Tui { enhanced_keys_supported, notification_backend: Some(detect_backend(NotificationMethod::default())), notification_condition: NotificationCondition::default(), - is_zellij, alt_screen_enabled: true, } } @@ -687,62 +680,6 @@ impl Tui { self.pending_history_lines.clear(); } - /// Resize the inline viewport to `height` rows, scrolling content above it if - /// the viewport would extend past the bottom of the screen. Returns `true` when - /// the caller must invalidate the diff buffer (Zellij mode), because the scroll - /// was performed with raw newlines that ratatui cannot track. - fn update_inline_viewport( - terminal: &mut Terminal, - height: u16, - is_zellij: bool, - ) -> Result { - let size = terminal.size()?; - let mut needs_full_repaint = false; - - let mut area = terminal.viewport_area; - area.height = height.min(size.height); - area.width = size.width; - if area.bottom() > size.height { - let scroll_by = area.bottom() - size.height; - if is_zellij { - Self::scroll_zellij_expanded_viewport(terminal, size, scroll_by)?; - needs_full_repaint = true; - } else { - terminal - .backend_mut() - .scroll_region_up(0..area.top(), scroll_by)?; - } - area.y = size.height - area.height; - } - if area != terminal.viewport_area { - // On startup, the old viewport can still be empty. Clear from the - // new viewport top so stale shell cells do not show through spaces. - clear_for_viewport_change(terminal, area)?; - terminal.set_viewport_area(area); - } - - Ok(needs_full_repaint) - } - - /// Push content above the viewport upward by `scroll_by` rows using raw - /// newlines at the screen bottom. This is the Zellij-safe alternative to - /// `scroll_region_up`, which relies on DECSTBM sequences Zellij does not - /// support. - fn scroll_zellij_expanded_viewport( - terminal: &mut Terminal, - size: Size, - scroll_by: u16, - ) -> Result<()> { - crossterm::queue!( - terminal.backend_mut(), - crossterm::cursor::MoveTo(0, size.height.saturating_sub(1)) - )?; - for _ in 0..scroll_by { - crossterm::queue!(terminal.backend_mut(), crossterm::style::Print("\n"))?; - } - Ok(()) - } - /// Resize the inline viewport for the resize-reflow path. /// /// Unlike the legacy draw path, this path does not scroll rows above the viewport when the @@ -751,7 +688,6 @@ impl Tui { fn update_inline_viewport_for_resize_reflow( terminal: &mut Terminal, height: u16, - is_zellij: bool, ) -> Result { let size = terminal.size()?; let terminal_height_shrank = size.height < terminal.last_known_screen_size.height; @@ -768,13 +704,9 @@ impl Tui { if area.bottom() > size.height { let scroll_by = area.bottom() - size.height; if !terminal_height_shrank { - if is_zellij { - Self::scroll_zellij_expanded_viewport(terminal, size, scroll_by)?; - } else { - terminal - .backend_mut() - .scroll_region_up(0..area.top(), scroll_by)?; - } + terminal + .backend_mut() + .scroll_region_up(0..area.top(), scroll_by)?; } area.y = size.height - area.height; } else if terminal_height_grew && viewport_was_bottom_aligned { @@ -792,27 +724,23 @@ impl Tui { } /// Write any buffered history lines above the viewport and clear the buffer. - /// Returns `true` when Zellij mode was used, signaling that the caller must - /// invalidate the diff buffer for a full repaint. fn flush_pending_history_lines( terminal: &mut Terminal, pending_history_lines: &mut Vec, - is_zellij: bool, - ) -> Result { + ) -> Result<()> { if pending_history_lines.is_empty() { - return Ok(false); + return Ok(()); } for batch in pending_history_lines.iter() { - crate::insert_history::insert_history_lines_with_mode_and_wrap_policy( + crate::insert_history::insert_history_lines_with_wrap_policy( terminal, batch.lines.clone(), - crate::insert_history::InsertHistoryMode::new(is_zellij), batch.wrap_policy, )?; } pending_history_lines.clear(); - Ok(is_zellij) + Ok(()) } pub fn draw( @@ -843,17 +771,26 @@ impl Tui { terminal.clear()?; } - let mut needs_full_repaint = - Self::update_inline_viewport(terminal, height, self.is_zellij)?; - needs_full_repaint |= Self::flush_pending_history_lines( - terminal, - &mut self.pending_history_lines, - self.is_zellij, - )?; + let size = terminal.size()?; - if needs_full_repaint { - terminal.invalidate_viewport(); + let mut area = terminal.viewport_area; + area.height = height.min(size.height); + area.width = size.width; + // If the viewport has expanded, scroll everything else up to make room. + if area.bottom() > size.height { + terminal + .backend_mut() + .scroll_region_up(0..area.top(), area.bottom() - size.height)?; + area.y = size.height - area.height; } + if area != terminal.viewport_area { + // On startup, the old viewport can still be empty. Clear from the + // new viewport top so stale shell cells do not show through spaces. + clear_for_viewport_change(terminal, area)?; + terminal.set_viewport_area(area); + } + + Self::flush_pending_history_lines(terminal, &mut self.pending_history_lines)?; // Update the y position for suspending so Ctrl-Z can place the cursor correctly. #[cfg(unix)] @@ -943,14 +880,9 @@ impl Tui { } let terminal = &mut self.terminal; - let mut needs_full_repaint = - Self::update_inline_viewport_for_resize_reflow(terminal, height, self.is_zellij)?; - let flushed_history = Self::flush_pending_history_lines( - terminal, - &mut self.pending_history_lines, - self.is_zellij, - )?; - needs_full_repaint |= flushed_history; + let needs_full_repaint = + Self::update_inline_viewport_for_resize_reflow(terminal, height)?; + Self::flush_pending_history_lines(terminal, &mut self.pending_history_lines)?; if needs_full_repaint { terminal.invalidate_viewport();