From 85f780f2a82a9468c006c88dd723cea9136b7fae Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 4 Jul 2026 00:59:15 -0300 Subject: [PATCH] refactor(tui): centralize alternate screen ownership --- codex-rs/tui/src/tui.rs | 77 ++--- codex-rs/tui/src/tui/event_stream.rs | 12 +- codex-rs/tui/src/tui/job_control.rs | 51 ++- codex-rs/tui/src/tui/screen_session.rs | 329 +++++++++++++++++++ codex-rs/tui/src/tui/screen_session_tests.rs | 184 +++++++++++ 5 files changed, 566 insertions(+), 87 deletions(-) create mode 100644 codex-rs/tui/src/tui/screen_session.rs create mode 100644 codex-rs/tui/src/tui/screen_session_tests.rs diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 7bf72f7cdf..2bfe8b3e39 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -21,8 +21,6 @@ use crossterm::event::DisableFocusChange; use crossterm::event::EnableBracketedPaste; use crossterm::event::EnableFocusChange; use crossterm::event::KeyEvent; -use crossterm::terminal::EnterAlternateScreen; -use crossterm::terminal::LeaveAlternateScreen; #[cfg(not(unix))] use crossterm::terminal::supports_keyboard_enhancement; use ratatui::backend::Backend; @@ -50,6 +48,7 @@ use crate::tui::event_stream::EventBroker; use crate::tui::event_stream::TuiEventStream; #[cfg(unix)] use crate::tui::job_control::SuspendContext; +use crate::tui::screen_session::ScreenSession; use codex_config::types::NotificationCondition; use codex_config::types::NotificationMethod; @@ -59,6 +58,7 @@ mod frame_requester; #[cfg(unix)] mod job_control; mod keyboard_modes; +mod screen_session; mod terminal_stderr; #[cfg(test)] pub(crate) mod test_support; @@ -532,11 +532,9 @@ pub struct Tui { pending_history_lines: Vec, ambient_pet_image_state: crate::pets::PetImageRenderState, pet_picker_preview_image_state: crate::pets::PetImageRenderState, - alt_saved_viewport: Option, + screen_session: ScreenSession, #[cfg(unix)] suspend_context: SuspendContext, - // True when overlay alt-screen UI is active - alt_screen_active: Arc, // True when terminal/tab is focused; updated internally from crossterm events terminal_focused: Arc, enhanced_keys_supported: bool, @@ -544,8 +542,6 @@ pub struct Tui { notification_condition: NotificationCondition, // Raw terminal-wrapped history needs a non-scroll-region insertion path in Zellij. is_zellij: bool, - // When false, enter_alt_screen() becomes a no-op. - alt_screen_enabled: bool, // Keeps unmanaged process stderr writes out of the inline viewport. _stderr_guard: terminal_stderr::TerminalStderrGuard, } @@ -589,23 +585,21 @@ impl Tui { pending_history_lines: vec![], ambient_pet_image_state: crate::pets::PetImageRenderState::default(), pet_picker_preview_image_state: crate::pets::PetImageRenderState::default(), - alt_saved_viewport: None, + screen_session: ScreenSession::new(), #[cfg(unix)] suspend_context: SuspendContext::new(), - alt_screen_active: Arc::new(AtomicBool::new(false)), terminal_focused: Arc::new(AtomicBool::new(true)), enhanced_keys_supported, notification_backend: Some(detect_backend(NotificationMethod::default())), notification_condition: NotificationCondition::default(), is_zellij, - alt_screen_enabled: true, _stderr_guard: stderr_guard, } } /// Set whether alternate screen is enabled. When false, enter_alt_screen() becomes a no-op. pub fn set_alt_screen_enabled(&mut self, enabled: bool) { - self.alt_screen_enabled = enabled; + self.screen_session.set_enabled(enabled); } pub fn set_notification_settings( @@ -626,7 +620,7 @@ impl Tui { } pub fn is_alt_screen_active(&self) -> bool { - self.alt_screen_active.load(Ordering::Relaxed) + self.screen_session.is_active() } // Drop crossterm EventStream to avoid stdin conflicts with other processes. @@ -655,8 +649,8 @@ impl Tui { // Leave alt screen if active to avoid conflicts with external program `f`. let was_alt_screen = self.is_alt_screen_active(); - if was_alt_screen { - let _ = self.leave_alt_screen(); + if was_alt_screen && let Err(err) = self.screen_session.suspend(&mut self.terminal) { + tracing::warn!("failed to suspend alternate screen before external program: {err}"); } if let Err(err) = mode.restore() { @@ -677,8 +671,8 @@ impl Tui { // After the external program `f` finishes, reset terminal state and flush any buffered keypresses. flush_terminal_input_buffer(); - if was_alt_screen { - let _ = self.enter_alt_screen(); + if was_alt_screen && let Err(err) = self.screen_session.resume(&mut self.terminal) { + tracing::warn!("failed to resume alternate screen after external program: {err}"); } self.resume_events(); @@ -720,7 +714,7 @@ impl Tui { self.draw_tx.subscribe(), self.terminal_focused.clone(), self.suspend_context.clone(), - self.alt_screen_active.clone(), + self.screen_session.clone(), ); #[cfg(not(unix))] let stream = TuiEventStream::new( @@ -734,39 +728,12 @@ impl Tui { /// Enter alternate screen and expand the viewport to full terminal size, saving the current /// inline viewport for restoration when leaving. pub fn enter_alt_screen(&mut self) -> Result<()> { - if !self.alt_screen_enabled { - return Ok(()); - } - let _ = execute!(self.terminal.backend_mut(), EnterAlternateScreen); - // Enable "alternate scroll" so terminals may translate wheel to arrows - let _ = execute!(self.terminal.backend_mut(), EnableAlternateScroll); - if let Ok(size) = self.terminal.size() { - self.alt_saved_viewport = Some(self.terminal.viewport_area); - self.terminal.set_viewport_area(ratatui::layout::Rect::new( - 0, - 0, - size.width, - size.height, - )); - let _ = self.terminal.clear(); - } - self.alt_screen_active.store(true, Ordering::Relaxed); - Ok(()) + self.screen_session.enter(&mut self.terminal) } /// Leave alternate screen and restore the previously saved inline viewport, if any. pub fn leave_alt_screen(&mut self) -> Result<()> { - if !self.alt_screen_enabled { - return Ok(()); - } - // Disable alternate scroll when leaving alt-screen - let _ = execute!(self.terminal.backend_mut(), DisableAlternateScroll); - let _ = execute!(self.terminal.backend_mut(), LeaveAlternateScreen); - if let Some(saved) = self.alt_saved_viewport.take() { - self.terminal.set_viewport_area(saved); - } - self.alt_screen_active.store(false, Ordering::Relaxed); - Ok(()) + self.screen_session.leave(&mut self.terminal) } pub fn insert_history_lines(&mut self, lines: Vec>) { @@ -887,7 +854,7 @@ impl Tui { #[cfg(unix)] let mut prepared_resume = self .suspend_context - .prepare_resume_action(&mut self.alt_saved_viewport); + .prepare_resume_action(&self.screen_session); // Precompute any viewport updates that need a cursor-position query before entering // the synchronized update, to avoid racing with the event reader. @@ -898,7 +865,7 @@ impl Tui { stdout().sync_update(|_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { - prepared.apply(&mut self.terminal)?; + prepared.apply(&mut self.terminal, &self.screen_session)?; } let terminal = &mut self.terminal; @@ -936,8 +903,9 @@ impl Tui { #[cfg(unix)] { let area = terminal.viewport_area; - let inline_area_bottom = if self.alt_screen_active.load(Ordering::Relaxed) { - self.alt_saved_viewport + let inline_area_bottom = if self.screen_session.is_active() { + self.screen_session + .saved_viewport() .map(|r| r.bottom().saturating_sub(1)) .unwrap_or_else(|| area.bottom().saturating_sub(1)) } else { @@ -1023,14 +991,14 @@ impl Tui { #[cfg(unix)] let mut prepared_resume = self .suspend_context - .prepare_resume_action(&mut self.alt_saved_viewport); + .prepare_resume_action(&self.screen_session); ensure_virtual_terminal_processing()?; stdout().sync_update(|_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { - prepared.apply(&mut self.terminal)?; + prepared.apply(&mut self.terminal, &self.screen_session)?; } let terminal = &mut self.terminal; @@ -1050,8 +1018,9 @@ impl Tui { #[cfg(unix)] { let area = terminal.viewport_area; - let inline_area_bottom = if self.alt_screen_active.load(Ordering::Relaxed) { - self.alt_saved_viewport + let inline_area_bottom = if self.screen_session.is_active() { + self.screen_session + .saved_viewport() .map(|r| r.bottom().saturating_sub(1)) .unwrap_or_else(|| area.bottom().saturating_sub(1)) } else { diff --git a/codex-rs/tui/src/tui/event_stream.rs b/codex-rs/tui/src/tui/event_stream.rs index 9a5e416fd6..a396aaef1f 100644 --- a/codex-rs/tui/src/tui/event_stream.rs +++ b/codex-rs/tui/src/tui/event_stream.rs @@ -34,6 +34,8 @@ use tokio_stream::wrappers::WatchStream; use tokio_stream::wrappers::errors::BroadcastStreamRecvError; use super::TuiEvent; +#[cfg(unix)] +use super::screen_session::ScreenSession; /// Result type produced by an event source. pub type EventResult = std::io::Result; @@ -145,7 +147,7 @@ pub struct TuiEventStream, + screen_session: ScreenSession, } impl TuiEventStream { @@ -154,7 +156,7 @@ impl TuiEventStream { draw_rx: broadcast::Receiver<()>, terminal_focused: Arc, #[cfg(unix)] suspend_context: crate::tui::job_control::SuspendContext, - #[cfg(unix)] alt_screen_active: Arc, + #[cfg(unix)] screen_session: ScreenSession, ) -> Self { let resume_stream = WatchStream::from_changes(broker.resume_events_rx()); Self { @@ -166,7 +168,7 @@ impl TuiEventStream { #[cfg(unix)] suspend_context, #[cfg(unix)] - alt_screen_active, + screen_session, } } @@ -240,7 +242,7 @@ impl TuiEventStream { #[cfg(unix)] if crate::tui::job_control::SUSPEND_KEY.is_press(key_event) { self.broker.pause_events(); - let suspend_result = self.suspend_context.suspend(&self.alt_screen_active); + let suspend_result = self.suspend_context.suspend(&self.screen_session); self.broker.resume_events(); if let Err(err) = suspend_result { tracing::warn!( @@ -374,7 +376,7 @@ mod tests { #[cfg(unix)] crate::tui::job_control::SuspendContext::new(), #[cfg(unix)] - Arc::new(AtomicBool::new(false)), + ScreenSession::new(), ) } diff --git a/codex-rs/tui/src/tui/job_control.rs b/codex-rs/tui/src/tui/job_control.rs index bc46a0a895..5d8b449dd5 100644 --- a/codex-rs/tui/src/tui/job_control.rs +++ b/codex-rs/tui/src/tui/job_control.rs @@ -3,23 +3,19 @@ use std::io::stdout; use std::sync::Arc; use std::sync::Mutex; use std::sync::PoisonError; -use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU16; use std::sync::atomic::Ordering; use crossterm::cursor::MoveTo; use crossterm::cursor::Show; use crossterm::event::KeyCode; -use crossterm::terminal::EnterAlternateScreen; -use crossterm::terminal::LeaveAlternateScreen; use ratatui::crossterm::execute; use ratatui::layout::Rect; use crate::key_hint; -use super::DisableAlternateScroll; -use super::EnableAlternateScroll; use super::Terminal; +use super::screen_session::ScreenSession; pub const SUSPEND_KEY: key_hint::KeyBinding = key_hint::ctrl(KeyCode::Char('z')); @@ -60,11 +56,11 @@ impl SuspendContext { /// otherwise record `RealignInline`. /// - Update the cached inline cursor row so suspend can place the cursor meaningfully. /// - Trigger SIGTSTP so the process can be resumed and continue drawing with the saved state. - pub(crate) fn suspend(&self, alt_screen_active: &Arc) -> Result<()> { - if alt_screen_active.load(Ordering::Relaxed) { - // Leave alt-screen so the terminal returns to the normal buffer while suspended; also turn off alt-scroll. - let _ = execute!(stdout(), DisableAlternateScroll); - let _ = execute!(stdout(), LeaveAlternateScreen); + pub(crate) fn suspend(&self, screen_session: &ScreenSession) -> Result<()> { + let mut first_error = None; + if screen_session.is_active() { + let mut terminal_stdout = stdout(); + first_error = screen_session.suspend_to_writer(&mut terminal_stdout).err(); self.set_resume_action(ResumeAction::RestoreAlt); } else { self.set_resume_action(ResumeAction::RealignInline); @@ -93,8 +89,10 @@ impl SuspendContext { cursor_y = self.cursor_y(), "restored terminal state after resume" ); - - Ok(()) + match first_error { + Some(err) => Err(err), + None => Ok(()), + } } /// Consume the pending resume intent and precompute any viewport changes needed post-resume. @@ -103,11 +101,12 @@ impl SuspendContext { /// resumes; returns `None` when there was no pending suspend intent. pub(crate) fn prepare_resume_action( &self, - alt_saved_viewport: &mut Option, + screen_session: &ScreenSession, ) -> Option { - let action = self.take_resume_action()?; - match action { - ResumeAction::RealignInline => { + match self.take_resume_action() { + None if screen_session.is_suspended() => Some(PreparedResumeAction::RestoreAltScreen), + None => None, + Some(ResumeAction::RealignInline) => { let viewport = Rect::new( /*x*/ 0, self.cursor_y(), @@ -116,10 +115,8 @@ impl SuspendContext { ); Some(PreparedResumeAction::RealignViewport(viewport)) } - ResumeAction::RestoreAlt => { - if let Some(saved) = alt_saved_viewport.as_mut() { - saved.y = self.cursor_y(); - } + Some(ResumeAction::RestoreAlt) => { + screen_session.update_saved_viewport_y(self.cursor_y()); Some(PreparedResumeAction::RestoreAltScreen) } } @@ -178,19 +175,17 @@ pub(crate) enum PreparedResumeAction { } impl PreparedResumeAction { - pub(crate) fn apply(self, terminal: &mut Terminal) -> Result<()> { + pub(crate) fn apply( + self, + terminal: &mut Terminal, + screen_session: &ScreenSession, + ) -> Result<()> { match self { PreparedResumeAction::RealignViewport(area) => { terminal.set_viewport_area(area); } PreparedResumeAction::RestoreAltScreen => { - execute!(terminal.backend_mut(), EnterAlternateScreen)?; - // Enable "alternate scroll" so terminals may translate wheel to arrows - execute!(terminal.backend_mut(), EnableAlternateScroll)?; - if let Ok(size) = terminal.size() { - terminal.set_viewport_area(Rect::new(0, 0, size.width, size.height)); - terminal.clear()?; - } + screen_session.resume(terminal)?; } } Ok(()) diff --git a/codex-rs/tui/src/tui/screen_session.rs b/codex-rs/tui/src/tui/screen_session.rs new file mode 100644 index 0000000000..520ab80421 --- /dev/null +++ b/codex-rs/tui/src/tui/screen_session.rs @@ -0,0 +1,329 @@ +//! Alternate-screen ownership and physical terminal transitions. + +use std::io; +use std::io::Write; +use std::num::NonZeroUsize; +use std::sync::Arc; +use std::sync::Mutex; +use std::sync::PoisonError; + +use crossterm::terminal::EnterAlternateScreen; +use crossterm::terminal::LeaveAlternateScreen; +use ratatui::crossterm::execute; +use ratatui::layout::Rect; + +use super::DisableAlternateScroll; +use super::EnableAlternateScroll; +use super::Terminal; + +const INITIAL_OWNER_COUNT: usize = 1; + +#[derive(Clone)] +pub(super) struct ScreenSession { + inner: Arc>, +} + +struct ScreenSessionInner { + availability: AltScreenAvailability, + ownership: ScreenOwnership, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum AltScreenAvailability { + Enabled, + Disabled, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ScreenOwnership { + Inline, + Alternate { + owners: NonZeroUsize, + saved_viewport: Rect, + physical: PhysicalAltScreen, + }, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum PhysicalAltScreen { + Active, + Suspended, +} + +impl ScreenSession { + pub(super) fn new() -> Self { + Self { + inner: Arc::new(Mutex::new(ScreenSessionInner { + availability: AltScreenAvailability::Enabled, + ownership: ScreenOwnership::Inline, + })), + } + } + + pub(super) fn set_enabled(&self, enabled: bool) { + self.lock().availability = match enabled { + true => AltScreenAvailability::Enabled, + false => AltScreenAvailability::Disabled, + }; + } + + pub(super) fn is_active(&self) -> bool { + matches!(self.lock().ownership, ScreenOwnership::Alternate { .. }) + } + + pub(super) fn is_suspended(&self) -> bool { + matches!( + self.lock().ownership, + ScreenOwnership::Alternate { + physical: PhysicalAltScreen::Suspended, + .. + } + ) + } + + pub(super) fn saved_viewport(&self) -> Option { + match self.lock().ownership { + ScreenOwnership::Inline => None, + ScreenOwnership::Alternate { saved_viewport, .. } => Some(saved_viewport), + } + } + + pub(super) fn update_saved_viewport_y(&self, y: u16) { + if let ScreenOwnership::Alternate { saved_viewport, .. } = &mut self.lock().ownership { + saved_viewport.y = y; + } + } + + pub(super) fn enter(&self, terminal: &mut Terminal) -> io::Result<()> { + let was_active = self.is_active(); + let saved_viewport = terminal.viewport_area; + let command_result = self.acquire(terminal, saved_viewport); + let layout_result = if !was_active && self.is_active() { + expand_to_full_screen(terminal) + } else { + Ok(()) + }; + merge_results(command_result, layout_result) + } + + pub(super) fn leave(&self, terminal: &mut Terminal) -> io::Result<()> { + let saved_viewport = self.saved_viewport(); + let was_active = saved_viewport.is_some(); + let result = self.release(terminal); + if was_active + && !self.is_active() + && let Some(saved_viewport) = saved_viewport + { + terminal.set_viewport_area(saved_viewport); + } + result + } + + pub(super) fn suspend(&self, terminal: &mut Terminal) -> io::Result<()> { + let was_suspended = self.is_suspended(); + let result = self.suspend_commands(terminal); + if !was_suspended + && self.is_suspended() + && let Some(saved_viewport) = self.saved_viewport() + { + terminal.set_viewport_area(saved_viewport); + } + result + } + + pub(super) fn suspend_to_writer(&self, writer: &mut impl Write) -> io::Result<()> { + self.suspend_commands(&mut WriterCommands(writer)) + } + + pub(super) fn resume(&self, terminal: &mut Terminal) -> io::Result<()> { + let was_suspended = self.is_suspended(); + let command_result = self.resume_commands(terminal); + let layout_result = if was_suspended && !self.is_suspended() { + expand_to_full_screen(terminal) + } else { + Ok(()) + }; + merge_results(command_result, layout_result) + } + + fn acquire(&self, commands: &mut impl ScreenCommands, saved_viewport: Rect) -> io::Result<()> { + let mut inner = self.lock(); + if inner.availability == AltScreenAvailability::Disabled { + return Ok(()); + } + match &mut inner.ownership { + ScreenOwnership::Inline => { + commands.enter_alternate_screen()?; + inner.ownership = ScreenOwnership::Alternate { + owners: NonZeroUsize::MIN, + saved_viewport, + physical: PhysicalAltScreen::Active, + }; + if let Err(enable_error) = commands.enable_alternate_scroll() { + let disable_result = commands.disable_alternate_scroll(); + let leave_result = commands.leave_alternate_screen(); + if leave_result.is_ok() { + inner.ownership = ScreenOwnership::Inline; + } + return merge_results( + Err(enable_error), + merge_results(disable_result, leave_result), + ); + } + Ok(()) + } + ScreenOwnership::Alternate { owners, .. } => { + let next = owners + .get() + .checked_add(INITIAL_OWNER_COUNT) + .ok_or_else(|| io::Error::other("alternate-screen owner count overflow"))?; + *owners = NonZeroUsize::new(next) + .ok_or_else(|| io::Error::other("alternate-screen owner count became zero"))?; + Ok(()) + } + } + } + + fn release(&self, commands: &mut impl ScreenCommands) -> io::Result<()> { + let mut inner = self.lock(); + let ScreenOwnership::Alternate { + owners, physical, .. + } = &mut inner.ownership + else { + return Ok(()); + }; + if owners.get() > INITIAL_OWNER_COUNT { + *owners = NonZeroUsize::new(owners.get() - INITIAL_OWNER_COUNT).ok_or_else(|| { + io::Error::other("nested alternate-screen owner count became zero") + })?; + return Ok(()); + } + if *physical == PhysicalAltScreen::Suspended { + inner.ownership = ScreenOwnership::Inline; + return Ok(()); + } + + let disable_result = commands.disable_alternate_scroll(); + let leave_result = commands.leave_alternate_screen(); + if leave_result.is_ok() { + inner.ownership = ScreenOwnership::Inline; + } + merge_results(disable_result, leave_result) + } + + fn suspend_commands(&self, commands: &mut impl ScreenCommands) -> io::Result<()> { + let mut inner = self.lock(); + let ScreenOwnership::Alternate { physical, .. } = &mut inner.ownership else { + return Ok(()); + }; + if *physical == PhysicalAltScreen::Suspended { + return Ok(()); + } + let disable_result = commands.disable_alternate_scroll(); + let leave_result = commands.leave_alternate_screen(); + if leave_result.is_ok() { + *physical = PhysicalAltScreen::Suspended; + } + merge_results(disable_result, leave_result) + } + + fn resume_commands(&self, commands: &mut impl ScreenCommands) -> io::Result<()> { + let mut inner = self.lock(); + let ScreenOwnership::Alternate { physical, .. } = &mut inner.ownership else { + return Ok(()); + }; + if *physical == PhysicalAltScreen::Active { + return Ok(()); + } + commands.enter_alternate_screen()?; + *physical = PhysicalAltScreen::Active; + if let Err(enable_error) = commands.enable_alternate_scroll() { + let disable_result = commands.disable_alternate_scroll(); + let leave_result = commands.leave_alternate_screen(); + if leave_result.is_ok() { + *physical = PhysicalAltScreen::Suspended; + } + return merge_results( + Err(enable_error), + merge_results(disable_result, leave_result), + ); + } + Ok(()) + } + + fn lock(&self) -> std::sync::MutexGuard<'_, ScreenSessionInner> { + self.inner.lock().unwrap_or_else(PoisonError::into_inner) + } +} + +/// Command sink shared by terminal-backed transitions, job-control writes, and state tests. +trait ScreenCommands { + fn enter_alternate_screen(&mut self) -> io::Result<()>; + fn leave_alternate_screen(&mut self) -> io::Result<()>; + fn enable_alternate_scroll(&mut self) -> io::Result<()>; + fn disable_alternate_scroll(&mut self) -> io::Result<()>; +} + +impl ScreenCommands for Terminal { + fn enter_alternate_screen(&mut self) -> io::Result<()> { + execute!(self.backend_mut(), EnterAlternateScreen) + } + + fn leave_alternate_screen(&mut self) -> io::Result<()> { + execute!(self.backend_mut(), LeaveAlternateScreen) + } + + fn enable_alternate_scroll(&mut self) -> io::Result<()> { + execute!(self.backend_mut(), EnableAlternateScroll) + } + + fn disable_alternate_scroll(&mut self) -> io::Result<()> { + execute!(self.backend_mut(), DisableAlternateScroll) + } +} + +struct WriterCommands<'a, W>(&'a mut W); + +impl ScreenCommands for WriterCommands<'_, W> { + fn enter_alternate_screen(&mut self) -> io::Result<()> { + execute!(self.0, EnterAlternateScreen) + } + + fn leave_alternate_screen(&mut self) -> io::Result<()> { + execute!(self.0, LeaveAlternateScreen) + } + + fn enable_alternate_scroll(&mut self) -> io::Result<()> { + execute!(self.0, EnableAlternateScroll) + } + + fn disable_alternate_scroll(&mut self) -> io::Result<()> { + execute!(self.0, DisableAlternateScroll) + } +} + +fn expand_to_full_screen(terminal: &mut Terminal) -> io::Result<()> { + let size = terminal.size()?; + terminal.set_viewport_area(Rect::new( + /*x*/ 0, + /*y*/ 0, + size.width, + size.height, + )); + terminal.clear() +} + +fn merge_results(first: io::Result<()>, second: io::Result<()>) -> io::Result<()> { + match (first, second) { + (Ok(()), Ok(())) => Ok(()), + (Err(err), Ok(())) | (Ok(()), Err(err)) => Err(err), + (Err(first), Err(second)) => Err(io::Error::new( + first.kind(), + format!("{first}; additionally: {second}"), + )), + } +} + +#[cfg(test)] +#[path = "screen_session_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/tui/screen_session_tests.rs b/codex-rs/tui/src/tui/screen_session_tests.rs new file mode 100644 index 0000000000..19f5a5a620 --- /dev/null +++ b/codex-rs/tui/src/tui/screen_session_tests.rs @@ -0,0 +1,184 @@ +use std::io; + +use pretty_assertions::assert_eq; +use ratatui::layout::Rect; + +use super::ScreenCommands; +use super::ScreenSession; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum Effect { + Enter, + Leave, + EnableScroll, + DisableScroll, +} + +const ENTER_AND_LEAVE: &[Effect] = &[ + Effect::Enter, + Effect::EnableScroll, + Effect::DisableScroll, + Effect::Leave, +]; +const LEAVE: &[Effect] = &[Effect::DisableScroll, Effect::Leave]; + +#[derive(Default)] +struct FakeCommands { + effects: Vec, + failures: Vec, +} + +impl FakeCommands { + fn failing(failures: Vec) -> Self { + Self { + failures, + ..Self::default() + } + } + + fn record(&mut self, effect: Effect) -> io::Result<()> { + self.effects.push(effect); + if let Some(index) = self.failures.iter().position(|failure| *failure == effect) { + self.failures.remove(index); + return Err(io::Error::other(format!("failed {effect:?}"))); + } + Ok(()) + } +} + +impl ScreenCommands for FakeCommands { + fn enter_alternate_screen(&mut self) -> io::Result<()> { + self.record(Effect::Enter) + } + + fn leave_alternate_screen(&mut self) -> io::Result<()> { + self.record(Effect::Leave) + } + + fn enable_alternate_scroll(&mut self) -> io::Result<()> { + self.record(Effect::EnableScroll) + } + + fn disable_alternate_scroll(&mut self) -> io::Result<()> { + self.record(Effect::DisableScroll) + } +} + +fn acquire(session: &ScreenSession, commands: &mut FakeCommands) { + session + .acquire( + commands, + Rect { + x: 0, + y: 12, + width: 80, + height: 12, + }, + ) + .expect("acquire"); +} + +fn assert_effects(commands: &FakeCommands, expected: &[Effect]) { + assert_eq!(commands.effects, expected); +} + +#[test] +fn nested_owners_only_transition_at_outer_boundaries() { + let session = ScreenSession::new(); + let nested = session.clone(); + let mut commands = FakeCommands::default(); + acquire(&session, &mut commands); + acquire(&nested, &mut commands); + + nested.release(&mut commands).expect("nested release"); + assert!(session.is_active()); + session.release(&mut commands).expect("outer release"); + + assert!(!session.is_active()); + assert_effects(&commands, ENTER_AND_LEAVE); +} + +#[test] +fn temporary_suspend_preserves_nested_ownership_until_resume() { + let session = ScreenSession::new(); + let mut commands = FakeCommands::default(); + acquire(&session, &mut commands); + acquire(&session, &mut commands); + commands.effects.clear(); + + session.suspend_commands(&mut commands).expect("suspend"); + session + .suspend_commands(&mut commands) + .expect("repeat suspend"); + assert!(session.is_active() && session.is_suspended()); + session.resume_commands(&mut commands).expect("resume"); + session + .resume_commands(&mut commands) + .expect("repeat resume"); + session.release(&mut commands).expect("nested release"); + assert!(session.is_active()); + session.release(&mut commands).expect("outer release"); + + assert_effects( + &commands, + &[ + Effect::DisableScroll, + Effect::Leave, + Effect::Enter, + Effect::EnableScroll, + Effect::DisableScroll, + Effect::Leave, + ], + ); +} + +#[test] +fn disabled_session_ignores_ownership_requests() { + let session = ScreenSession::new(); + let mut commands = FakeCommands::default(); + session.set_enabled(/*enabled*/ false); + + acquire(&session, &mut commands); + session.release(&mut commands).expect("release"); + session.suspend_commands(&mut commands).expect("suspend"); + session.resume_commands(&mut commands).expect("resume"); + + assert!(!session.is_active()); + assert_effects(&commands, &[]); +} + +#[test] +fn partial_acquire_rolls_back_or_retains_ownership_for_retry() { + let session = ScreenSession::new(); + let mut commands = FakeCommands::failing(vec![Effect::EnableScroll]); + assert!(session.acquire(&mut commands, Rect::default()).is_err()); + assert!(!session.is_active()); + assert_effects(&commands, ENTER_AND_LEAVE); + + let session = ScreenSession::new(); + let mut commands = FakeCommands::failing(vec![Effect::EnableScroll, Effect::Leave]); + assert!(session.acquire(&mut commands, Rect::default()).is_err()); + assert!(session.is_active()); + commands.effects.clear(); + session.release(&mut commands).expect("retry cleanup"); + assert!(!session.is_active()); + assert_effects(&commands, LEAVE); +} + +#[test] +fn release_attempts_all_cleanup_and_retains_ownership_when_leave_fails() { + let session = ScreenSession::new(); + let mut commands = FakeCommands::default(); + acquire(&session, &mut commands); + commands.effects.clear(); + commands.failures = vec![Effect::DisableScroll, Effect::Leave]; + + assert!(session.release(&mut commands).is_err()); + assert!(session.is_active()); + assert_effects(&commands, LEAVE); + + commands.effects.clear(); + session.release(&mut commands).expect("retry release"); + assert!(!session.is_active()); + assert_effects(&commands, LEAVE); +}