From 0eb7a4f847c3ca92a7acc47b39811e54e5c673e8 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:21:28 -0300 Subject: [PATCH 01/13] refactor(terminal): add terminal actor protocol This commit introduces the Terminal actor boundary with a typed message protocol, cloneable handle, terminal session trait, actor error type, and focused actor tests. The new module follows the existing actor request/reply pattern while keeping runtime wiring unchanged. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/main.rs | 1 + src/terminal/actor.rs | 250 +++++++++++++++++++++++++++++++++++++++ src/terminal/errors.rs | 13 ++ src/terminal/handle.rs | 89 ++++++++++++++ src/terminal/messages.rs | 64 ++++++++++ src/terminal/mod.rs | 13 ++ src/terminal/session.rs | 22 ++++ 7 files changed, 452 insertions(+) create mode 100644 src/terminal/actor.rs create mode 100644 src/terminal/errors.rs create mode 100644 src/terminal/handle.rs create mode 100644 src/terminal/messages.rs create mode 100644 src/terminal/mod.rs create mode 100644 src/terminal/session.rs diff --git a/src/main.rs b/src/main.rs index beb615b..3a2fcf5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,7 @@ mod lore; mod macros; mod render; mod render_prefs; +mod terminal; mod ui; use app::App; diff --git a/src/terminal/actor.rs b/src/terminal/actor.rs new file mode 100644 index 0000000..2e635fc --- /dev/null +++ b/src/terminal/actor.rs @@ -0,0 +1,250 @@ +use tokio::{ + sync::{mpsc, oneshot}, + task, +}; + +use crate::terminal::{ + handle::TerminalHandle, + messages::{TerminalFrame, TerminalMessage, TerminalResult}, + session::TerminalSessionApi, + TerminalError, +}; + +pub const DEFAULT_TERMINAL_CHANNEL_SIZE: usize = 32; + +pub struct TerminalActor { + session: Option>, + rx: mpsc::Receiver, +} + +impl TerminalActor { + pub fn new(session: Box, rx: mpsc::Receiver) -> Self { + Self { + session: Some(session), + rx, + } + } + + pub fn spawn(session: Box) -> TerminalHandle { + let (tx, rx) = mpsc::channel(DEFAULT_TERMINAL_CHANNEL_SIZE); + tracing::debug!( + channel_size = DEFAULT_TERMINAL_CHANNEL_SIZE, + "spawning terminal actor" + ); + tokio::spawn(Self::new(session, rx).run()); + TerminalHandle::new(tx) + } + + pub async fn run(mut self) { + tracing::info!("terminal actor started"); + while let Some(message) = self.rx.recv().await { + self.handle_message(message).await; + } + tracing::info!("terminal actor stopped"); + } + + async fn handle_message(&mut self, message: TerminalMessage) { + let message_name = message.name(); + tracing::debug!(message = message_name, "terminal request received"); + + match message { + TerminalMessage::Draw { frame, reply } => { + let result = self + .with_session(move |session| session.draw(frame)) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + TerminalMessage::ReadEvent { reply } => { + let result = self + .with_session(|session| session.read_event()) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + TerminalMessage::PollEvent { timeout, reply } => { + let result = self + .with_session(move |session| session.poll_event(timeout)) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + TerminalMessage::SetupUserIo { reply } => { + let result = self + .with_session(|session| session.setup_user_io()) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + TerminalMessage::TeardownUserIo { reply } => { + let result = self + .with_session(|session| session.teardown_user_io()) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + TerminalMessage::WaitForKeyPress { + key, + timeout, + reply, + } => { + let result = self + .with_session(move |session| session.wait_for_key_press(key, timeout)) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + TerminalMessage::GetSize { reply } => { + let result = self + .with_session(|session| session.size()) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + TerminalMessage::Shutdown { reply } => { + let result = self + .with_session(|session| session.shutdown()) + .await + .and_then(|result| result); + send_terminal_reply(message_name, reply, result); + } + } + } + + async fn with_session(&mut self, operation: F) -> TerminalResult + where + T: Send + 'static, + F: FnOnce(&mut dyn TerminalSessionApi) -> T + Send + 'static, + { + let mut session = self + .session + .take() + .ok_or_else(|| TerminalError::ActorUnavailable("session unavailable".to_string()))?; + let (session, result) = task::spawn_blocking(move || { + let result = operation(session.as_mut()); + (session, result) + }) + .await + .map_err(|e| TerminalError::ActorUnavailable(e.to_string()))?; + self.session = Some(session); + Ok(result) + } +} + +fn send_terminal_reply( + message_name: &'static str, + reply: oneshot::Sender>, + result: TerminalResult, +) { + if let Err(error) = &result { + tracing::warn!( + message = message_name, + error = %error, + "terminal request failed" + ); + } + + if reply.send(result).is_err() { + tracing::warn!( + message = message_name, + "terminal reply receiver dropped before response" + ); + } +} + +#[cfg(test)] +mod tests { + use std::time::Duration; + + use ratatui::crossterm::event::KeyCode; + + use crate::{ + input::event::{KeyInput, TerminalEvent}, + terminal::session::MockTerminalSessionApi, + }; + + use super::*; + + fn spawn_test_actor(session: MockTerminalSessionApi) -> TerminalHandle { + TerminalActor::spawn(Box::new(session)) + } + + #[tokio::test] + async fn draw_returns_ok_from_actor() { + let mut session = MockTerminalSessionApi::new(); + session + .expect_draw() + .withf(|frame| frame == &TerminalFrame) + .times(1) + .returning(|_| Ok(())); + let handle = spawn_test_actor(session); + + let result = handle.draw(TerminalFrame).await; + + assert!(result.is_ok()); + } + + #[tokio::test] + async fn read_event_returns_terminal_event_from_actor() { + let expected = TerminalEvent::Key(KeyInput::press(KeyCode::Char('j'))); + let mut session = MockTerminalSessionApi::new(); + session + .expect_read_event() + .times(1) + .returning(move || Ok(Some(expected.clone()))); + let handle = spawn_test_actor(session); + + let result = handle.read_event().await.unwrap(); + + assert_eq!( + result, + Some(TerminalEvent::Key(KeyInput::press(KeyCode::Char('j')))) + ); + } + + #[tokio::test] + async fn sequential_requests_preserve_session() { + let mut session = MockTerminalSessionApi::new(); + session + .expect_draw() + .withf(|frame| frame == &TerminalFrame) + .times(1) + .returning(|_| Ok(())); + session.expect_size().times(1).returning(|| Ok((120, 40))); + let handle = spawn_test_actor(session); + + handle.draw(TerminalFrame).await.unwrap(); + let size = handle.size().await.unwrap(); + + assert_eq!(size, (120, 40)); + } + + #[tokio::test] + async fn wait_for_key_press_uses_requested_key_and_timeout() { + let mut session = MockTerminalSessionApi::new(); + session + .expect_wait_for_key_press() + .withf(|key, timeout| *key == KeyCode::Enter && *timeout == Duration::from_millis(50)) + .times(1) + .returning(|_, _| Ok(true)); + let handle = spawn_test_actor(session); + + let pressed = handle + .wait_for_key_press(KeyCode::Enter, Duration::from_millis(50)) + .await + .unwrap(); + + assert!(pressed); + } + + #[tokio::test] + async fn closed_channel_returns_actor_unavailable() { + let (tx, rx) = mpsc::channel(DEFAULT_TERMINAL_CHANNEL_SIZE); + let handle = TerminalHandle::new(tx); + drop(rx); + + let result = handle.draw(TerminalFrame).await; + + assert!(matches!(result, Err(TerminalError::ActorUnavailable(_)))); + } +} diff --git a/src/terminal/errors.rs b/src/terminal/errors.rs new file mode 100644 index 0000000..30b4770 --- /dev/null +++ b/src/terminal/errors.rs @@ -0,0 +1,13 @@ +use thiserror::Error; + +/// Failures at the terminal actor/session boundary. +#[allow(dead_code)] +#[derive(Debug, Error)] +pub enum TerminalError { + #[error("terminal I/O error: {0}")] + Io(#[from] std::io::Error), + #[error("terminal session error: {0}")] + Session(String), + #[error("terminal actor unavailable: {0}")] + ActorUnavailable(String), +} diff --git a/src/terminal/handle.rs b/src/terminal/handle.rs new file mode 100644 index 0000000..7d2498d --- /dev/null +++ b/src/terminal/handle.rs @@ -0,0 +1,89 @@ +#![allow(dead_code)] // Follow-up Phase 9 commits wire terminal handle methods. + +use std::time::Duration; + +use ratatui::crossterm::event::KeyCode; +use tokio::sync::{mpsc, oneshot}; + +use crate::{ + input::event::TerminalEvent, + terminal::{ + messages::{TerminalFrame, TerminalMessage, TerminalResult}, + TerminalError, + }, +}; + +#[derive(Clone)] +pub struct TerminalHandle { + tx: mpsc::Sender, +} + +impl TerminalHandle { + pub fn new(tx: mpsc::Sender) -> Self { + Self { tx } + } + + pub async fn draw(&self, frame: TerminalFrame) -> TerminalResult<()> { + self.request_result(|reply| TerminalMessage::Draw { frame, reply }) + .await + } + + pub async fn read_event(&self) -> TerminalResult> { + self.request_result(|reply| TerminalMessage::ReadEvent { reply }) + .await + } + + pub async fn poll_event(&self, timeout: Duration) -> TerminalResult> { + self.request_result(|reply| TerminalMessage::PollEvent { timeout, reply }) + .await + } + + pub async fn setup_user_io(&self) -> TerminalResult<()> { + self.request_result(|reply| TerminalMessage::SetupUserIo { reply }) + .await + } + + pub async fn teardown_user_io(&self) -> TerminalResult<()> { + self.request_result(|reply| TerminalMessage::TeardownUserIo { reply }) + .await + } + + pub async fn wait_for_key_press( + &self, + key: KeyCode, + timeout: Duration, + ) -> TerminalResult { + self.request_result(|reply| TerminalMessage::WaitForKeyPress { + key, + timeout, + reply, + }) + .await + } + + pub async fn size(&self) -> TerminalResult<(u16, u16)> { + self.request_result(|reply| TerminalMessage::GetSize { reply }) + .await + } + + pub async fn shutdown(&self) -> TerminalResult<()> { + self.request_result(|reply| TerminalMessage::Shutdown { reply }) + .await + } + + async fn request_result( + &self, + build_message: impl FnOnce(oneshot::Sender>) -> TerminalMessage, + ) -> TerminalResult + where + T: Send + 'static, + { + let (reply, rx) = oneshot::channel(); + self.tx + .send(build_message(reply)) + .await + .map_err(|_| TerminalError::ActorUnavailable("request channel closed".to_string()))?; + rx.await + .map_err(|_| TerminalError::ActorUnavailable("reply channel closed".to_string()))? + } +} diff --git a/src/terminal/messages.rs b/src/terminal/messages.rs new file mode 100644 index 0000000..22f17ed --- /dev/null +++ b/src/terminal/messages.rs @@ -0,0 +1,64 @@ +#![allow(dead_code)] // Follow-up Phase 9 commits wire the full terminal protocol. + +use std::time::Duration; + +use ratatui::crossterm::event::KeyCode; +use tokio::sync::oneshot; + +use crate::{input::event::TerminalEvent, terminal::TerminalError}; + +pub type TerminalResult = Result; + +/// Owned payload for a terminal draw request. +/// +/// The first protocol commit keeps this intentionally small. Runtime wiring can +/// evolve it into the final owned render payload without changing the actor +/// request/reply shape. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct TerminalFrame; + +pub enum TerminalMessage { + Draw { + frame: TerminalFrame, + reply: oneshot::Sender>, + }, + ReadEvent { + reply: oneshot::Sender>>, + }, + PollEvent { + timeout: Duration, + reply: oneshot::Sender>>, + }, + SetupUserIo { + reply: oneshot::Sender>, + }, + TeardownUserIo { + reply: oneshot::Sender>, + }, + WaitForKeyPress { + key: KeyCode, + timeout: Duration, + reply: oneshot::Sender>, + }, + GetSize { + reply: oneshot::Sender>, + }, + Shutdown { + reply: oneshot::Sender>, + }, +} + +impl TerminalMessage { + pub fn name(&self) -> &'static str { + match self { + TerminalMessage::Draw { .. } => "Draw", + TerminalMessage::ReadEvent { .. } => "ReadEvent", + TerminalMessage::PollEvent { .. } => "PollEvent", + TerminalMessage::SetupUserIo { .. } => "SetupUserIo", + TerminalMessage::TeardownUserIo { .. } => "TeardownUserIo", + TerminalMessage::WaitForKeyPress { .. } => "WaitForKeyPress", + TerminalMessage::GetSize { .. } => "GetSize", + TerminalMessage::Shutdown { .. } => "Shutdown", + } + } +} diff --git a/src/terminal/mod.rs b/src/terminal/mod.rs new file mode 100644 index 0000000..9add3dc --- /dev/null +++ b/src/terminal/mod.rs @@ -0,0 +1,13 @@ +//! Actor boundary for the Ratatui/Crossterm terminal session. +//! +//! Phase 9 introduces this module as the only owner of terminal session +//! operations. Later commits wire the existing app loop through +//! [`handle::TerminalHandle`]. + +pub mod actor; +pub mod errors; +pub mod handle; +pub mod messages; +pub mod session; + +pub use errors::TerminalError; diff --git a/src/terminal/session.rs b/src/terminal/session.rs new file mode 100644 index 0000000..103f9ea --- /dev/null +++ b/src/terminal/session.rs @@ -0,0 +1,22 @@ +use std::time::Duration; + +use mockall::automock; +use ratatui::crossterm::event::KeyCode; + +use crate::{ + input::event::TerminalEvent, + terminal::messages::{TerminalFrame, TerminalResult}, +}; + +/// Stateful terminal session owned by the terminal actor. +#[automock] +pub trait TerminalSessionApi: Send { + fn draw(&mut self, frame: TerminalFrame) -> TerminalResult<()>; + fn read_event(&mut self) -> TerminalResult>; + fn poll_event(&mut self, timeout: Duration) -> TerminalResult>; + fn setup_user_io(&mut self) -> TerminalResult<()>; + fn teardown_user_io(&mut self) -> TerminalResult<()>; + fn wait_for_key_press(&mut self, key: KeyCode, timeout: Duration) -> TerminalResult; + fn size(&self) -> TerminalResult<(u16, u16)>; + fn shutdown(&mut self) -> TerminalResult<()>; +} From 7d09870bf873bbcdf77713fb255ca308e9bcc0d5 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:23:42 -0300 Subject: [PATCH 02/13] refactor(terminal): implement crossterm session This commit adds the Ratatui/Crossterm-backed terminal session behind the TerminalSessionApi boundary. Raw event reading, polling, resize/key conversion, user-IO mode transitions, size lookup, and shutdown restoration are now modeled as terminal session operations. It also makes TerminalFrame an explicit owned draw payload so later Phase 9 commits can wire drawing through the terminal actor protocol. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/terminal/actor.rs | 10 +-- src/terminal/messages.rs | 10 +-- src/terminal/session.rs | 161 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 168 insertions(+), 13 deletions(-) diff --git a/src/terminal/actor.rs b/src/terminal/actor.rs index 2e635fc..77ee001 100644 --- a/src/terminal/actor.rs +++ b/src/terminal/actor.rs @@ -174,12 +174,12 @@ mod tests { let mut session = MockTerminalSessionApi::new(); session .expect_draw() - .withf(|frame| frame == &TerminalFrame) + .withf(|frame| frame == &TerminalFrame::Empty) .times(1) .returning(|_| Ok(())); let handle = spawn_test_actor(session); - let result = handle.draw(TerminalFrame).await; + let result = handle.draw(TerminalFrame::Empty).await; assert!(result.is_ok()); } @@ -207,13 +207,13 @@ mod tests { let mut session = MockTerminalSessionApi::new(); session .expect_draw() - .withf(|frame| frame == &TerminalFrame) + .withf(|frame| frame == &TerminalFrame::Empty) .times(1) .returning(|_| Ok(())); session.expect_size().times(1).returning(|| Ok((120, 40))); let handle = spawn_test_actor(session); - handle.draw(TerminalFrame).await.unwrap(); + handle.draw(TerminalFrame::Empty).await.unwrap(); let size = handle.size().await.unwrap(); assert_eq!(size, (120, 40)); @@ -243,7 +243,7 @@ mod tests { let handle = TerminalHandle::new(tx); drop(rx); - let result = handle.draw(TerminalFrame).await; + let result = handle.draw(TerminalFrame::Empty).await; assert!(matches!(result, Err(TerminalError::ActorUnavailable(_)))); } diff --git a/src/terminal/messages.rs b/src/terminal/messages.rs index 22f17ed..21d21b2 100644 --- a/src/terminal/messages.rs +++ b/src/terminal/messages.rs @@ -10,12 +10,12 @@ use crate::{input::event::TerminalEvent, terminal::TerminalError}; pub type TerminalResult = Result; /// Owned payload for a terminal draw request. -/// -/// The first protocol commit keeps this intentionally small. Runtime wiring can -/// evolve it into the final owned render payload without changing the actor -/// request/reply shape. #[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct TerminalFrame; +pub enum TerminalFrame { + /// Placeholder frame used while the terminal actor is wired into the app. + #[default] + Empty, +} pub enum TerminalMessage { Draw { diff --git a/src/terminal/session.rs b/src/terminal/session.rs index 103f9ea..5366779 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -1,11 +1,18 @@ use std::time::Duration; use mockall::automock; -use ratatui::crossterm::event::KeyCode; +use ratatui::crossterm::event::{self, Event, KeyCode, KeyEventKind}; use crate::{ - input::event::TerminalEvent, - terminal::messages::{TerminalFrame, TerminalResult}, + infrastructure::terminal::{ + restore, setup_user_io as setup_terminal_user_io, + teardown_user_io as teardown_terminal_user_io, Tui, + }, + input::event::{KeyInput, TerminalEvent}, + terminal::{ + messages::{TerminalFrame, TerminalResult}, + TerminalError, + }, }; /// Stateful terminal session owned by the terminal actor. @@ -20,3 +27,151 @@ pub trait TerminalSessionApi: Send { fn size(&self) -> TerminalResult<(u16, u16)>; fn shutdown(&mut self) -> TerminalResult<()>; } + +/// Ratatui/Crossterm-backed terminal session. +#[allow(dead_code)] // Wired into main in the next Phase 9 commit. +pub struct CrosstermTerminalSession { + terminal: Tui, + shutdown: bool, +} + +impl CrosstermTerminalSession { + #[allow(dead_code)] // Wired into main in the next Phase 9 commit. + pub fn new(terminal: Tui) -> Self { + Self { + terminal, + shutdown: false, + } + } + + pub fn terminal_event_from_crossterm_event(event: Event) -> Option { + match event { + Event::Key(key) if key.kind == KeyEventKind::Release => None, + Event::Key(key) => Some(TerminalEvent::Key(KeyInput::from(key))), + Event::Resize(width, height) => Some(TerminalEvent::Resize { width, height }), + _ => None, + } + } +} + +impl TerminalSessionApi for CrosstermTerminalSession { + fn draw(&mut self, frame: TerminalFrame) -> TerminalResult<()> { + match frame { + TerminalFrame::Empty => { + self.terminal.draw(|_| {})?; + } + } + Ok(()) + } + + fn read_event(&mut self) -> TerminalResult> { + Ok(Self::terminal_event_from_crossterm_event(event::read()?)) + } + + fn poll_event(&mut self, timeout: Duration) -> TerminalResult> { + if !event::poll(timeout)? { + return Ok(None); + } + + self.read_event() + } + + fn setup_user_io(&mut self) -> TerminalResult<()> { + setup_terminal_user_io(&mut self.terminal) + .map_err(|err| TerminalError::Session(err.to_string())) + } + + fn teardown_user_io(&mut self) -> TerminalResult<()> { + teardown_terminal_user_io(&mut self.terminal) + .map_err(|err| TerminalError::Session(err.to_string())) + } + + fn wait_for_key_press(&mut self, key: KeyCode, timeout: Duration) -> TerminalResult { + let started_at = std::time::Instant::now(); + + while started_at.elapsed() < timeout { + let elapsed = started_at.elapsed(); + let remaining = timeout.saturating_sub(elapsed); + let poll_timeout = remaining.min(Duration::from_millis(16)); + + if let Some(TerminalEvent::Key(input)) = self.poll_event(poll_timeout)? { + if input.code == key { + return Ok(true); + } + } + } + + Ok(false) + } + + fn size(&self) -> TerminalResult<(u16, u16)> { + let size = self.terminal.size()?; + Ok((size.width, size.height)) + } + + fn shutdown(&mut self) -> TerminalResult<()> { + if self.shutdown { + return Ok(()); + } + + restore()?; + self.shutdown = true; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use ratatui::crossterm::event::{ + Event, KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers, + }; + + use crate::input::event::{KeyInput, TerminalEvent}; + + use super::CrosstermTerminalSession; + + #[test] + fn converts_key_press_to_terminal_key_event() { + let key = KeyEvent::new(KeyCode::Char('j'), KeyModifiers::NONE); + + let event = CrosstermTerminalSession::terminal_event_from_crossterm_event(Event::Key(key)); + + assert_eq!(event, Some(TerminalEvent::Key(KeyInput::from(key)))); + } + + #[test] + fn ignores_key_release_events() { + let key = KeyEvent { + code: KeyCode::Char('j'), + modifiers: KeyModifiers::NONE, + kind: KeyEventKind::Release, + state: KeyEventState::NONE, + }; + + let event = CrosstermTerminalSession::terminal_event_from_crossterm_event(Event::Key(key)); + + assert_eq!(event, None); + } + + #[test] + fn converts_resize_events() { + let event = + CrosstermTerminalSession::terminal_event_from_crossterm_event(Event::Resize(120, 40)); + + assert_eq!( + event, + Some(TerminalEvent::Resize { + width: 120, + height: 40 + }) + ); + } + + #[test] + fn ignores_non_key_non_resize_events() { + let event = + CrosstermTerminalSession::terminal_event_from_crossterm_event(Event::FocusGained); + + assert_eq!(event, None); + } +} From d4cd499209e52f84a2648f1537a8bad6873734d5 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:25:19 -0300 Subject: [PATCH 03/13] refactor(terminal): resolve cli before terminal startup This commit moves early CLI handling ahead of terminal initialization so --show-configs can print configuration without entering the alternate screen or requiring an explicit terminal restore. Normal TUI startup still initializes the terminal only after configuration and CLI resolution have completed. It also adds focused coverage for the pre-terminal CLI resolution behavior. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/cli.rs | 45 ++++++++++++++++++++++++++++++++------------- src/main.rs | 7 ++++--- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index dd2d078..59515f7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,10 +1,9 @@ use clap::Parser; use color_eyre::eyre::eyre; -use ratatui::{prelude::Backend, Terminal}; use std::ops::ControlFlow; -use crate::{config::ConfigSnapshot, infrastructure::terminal::restore}; +use crate::config::ConfigSnapshot; #[derive(Debug, Parser)] #[command(version, about)] @@ -15,19 +14,11 @@ pub struct Cli { } impl Cli { - /// Resolves the command line arguments and applies the necessary changes to the terminal and app + /// Resolves command line arguments that may finish before the TUI starts. /// /// Some arguments may finish the program early (returning `ControlFlow::Break`) - pub fn resolve( - &self, - terminal: Terminal, - config: &ConfigSnapshot, - ) -> ControlFlow, Terminal> { + pub fn resolve(&self, config: &ConfigSnapshot) -> ControlFlow, ()> { if self.show_configs { - drop(terminal); - if let Err(err) = restore() { - return ControlFlow::Break(Err(eyre!(err))); - } match serde_json::to_string_pretty(&config) { Err(err) => return ControlFlow::Break(Err(eyre!(err))), Ok(config) => println!("patch-hub configurations:\n{config}"), @@ -36,6 +27,34 @@ impl Cli { return ControlFlow::Break(Ok(())); } - ControlFlow::Continue(terminal) + ControlFlow::Continue(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::ConfigState; + + #[test] + fn resolve_continues_when_no_early_cli_action_is_requested() { + let cli = Cli { + show_configs: false, + }; + let config = ConfigState::default().to_snapshot(); + + let result = cli.resolve(&config); + + assert!(matches!(result, ControlFlow::Continue(()))); + } + + #[test] + fn resolve_finishes_after_printing_configs() { + let cli = Cli { show_configs: true }; + let config = ConfigState::default().to_snapshot(); + + let result = cli.resolve(&config); + + assert!(matches!(result, ControlFlow::Break(Ok(())))); } } diff --git a/src/main.rs b/src/main.rs index 3a2fcf5..ac0d408 100644 --- a/src/main.rs +++ b/src/main.rs @@ -101,7 +101,6 @@ async fn main() -> color_eyre::Result<()> { let args = Cli::parse(); infrastructure::errors::install_hooks()?; - let mut terminal = init()?; let env = OsEnv; let config_service: Box = @@ -115,11 +114,13 @@ async fn main() -> color_eyre::Result<()> { logging_reload_handle, ); - match args.resolve(terminal, &config) { + match args.resolve(&config) { ControlFlow::Break(b) => return b, - ControlFlow::Continue(t) => terminal = t, + ControlFlow::Continue(()) => {} } + let terminal = init()?; + // Build shared infrastructure dependencies for LoreService let net = Arc::new(UreqNetClient::new()); let fs_arc: Arc = Arc::new(OsFileSystem); From a7696e67fea1c2c863b15c13d4c50635a5800a89 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:27:10 -0300 Subject: [PATCH 04/13] refactor(terminal): route event reads through handle This commit updates the main app loop to receive raw TerminalEvent values through TerminalHandle instead of constructing the Crossterm event source directly. The runtime now spawns a terminal actor for event reads while leaving the existing concrete terminal draw path in place for the follow-up handler and loading-screen migration. It also adds a transitional Crossterm event session so this step can move event capture behind the actor protocol without changing application input semantics. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/handler/mod.rs | 16 ++++---- src/main.rs | 4 +- src/terminal/session.rs | 88 ++++++++++++++++++++++++++++++++++------- 3 files changed, 84 insertions(+), 24 deletions(-) diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 816ccdf..c455119 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -10,11 +10,8 @@ use std::ops::ControlFlow; use crate::{ app::{screens::CurrentScreen, App}, - input::{ - event::InputEvent, - mapper::InputMapper, - terminal_source::{CrosstermEventSource, TerminalEventSource}, - }, + input::{event::InputEvent, mapper::InputMapper}, + terminal::handle::TerminalHandle, ui::draw_ui, }; @@ -60,11 +57,14 @@ where Ok(ControlFlow::Continue(terminal)) } -pub async fn run_app(mut terminal: Terminal, mut app: App) -> color_eyre::Result<()> +pub async fn run_app( + mut terminal: Terminal, + mut app: App, + terminal_handle: TerminalHandle, +) -> color_eyre::Result<()> where B: Backend + Send + 'static, { - let mut event_source = CrosstermEventSource; let mut input_mapper = InputMapper::default(); loop { @@ -77,7 +77,7 @@ where // need to refresh the UI independently of any event as doing so gravely // hinders the performance to below acceptable. // if event::poll(Duration::from_millis(16))? { - if let Some(terminal_event) = event_source.read_event()? { + if let Some(terminal_event) = terminal_handle.read_event().await? { let input = input_mapper.map_terminal_event(terminal_event, &app.input_context()); if let Some(input) = input { match input_handling(terminal, &mut app, input).await? { diff --git a/src/main.rs b/src/main.rs index ac0d408..d4a523a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,6 +37,7 @@ use lore::{ use render::{actor::RenderActor, ShellRenderService}; use render_prefs::PatchRenderer; use std::{ops::ControlFlow, sync::Arc}; +use terminal::{actor::TerminalActor, session::CrosstermEventSession}; use tracing::{event, Level}; /// Verifies required and optional external binaries before the TUI runs. @@ -120,6 +121,7 @@ async fn main() -> color_eyre::Result<()> { } let terminal = init()?; + let terminal_events = TerminalActor::spawn(Box::new(CrosstermEventSession)); // Build shared infrastructure dependencies for LoreService let net = Arc::new(UreqNetClient::new()); @@ -173,7 +175,7 @@ async fn main() -> color_eyre::Result<()> { bail!("patch-hub cannot be executed because some dependencies are missing, check logs for more information"); } - run_app(terminal, app).await?; + run_app(terminal, app, terminal_events).await?; restore()?; event!(Level::INFO, "patch-hub finished"); diff --git a/src/terminal/session.rs b/src/terminal/session.rs index 5366779..6e205b8 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -87,21 +87,7 @@ impl TerminalSessionApi for CrosstermTerminalSession { } fn wait_for_key_press(&mut self, key: KeyCode, timeout: Duration) -> TerminalResult { - let started_at = std::time::Instant::now(); - - while started_at.elapsed() < timeout { - let elapsed = started_at.elapsed(); - let remaining = timeout.saturating_sub(elapsed); - let poll_timeout = remaining.min(Duration::from_millis(16)); - - if let Some(TerminalEvent::Key(input)) = self.poll_event(poll_timeout)? { - if input.code == key { - return Ok(true); - } - } - } - - Ok(false) + wait_for_key_press_from_session(self, key, timeout) } fn size(&self) -> TerminalResult<(u16, u16)> { @@ -120,6 +106,78 @@ impl TerminalSessionApi for CrosstermTerminalSession { } } +/// Transitional Crossterm event source used while drawing still owns the +/// concrete Ratatui terminal outside the actor. +pub struct CrosstermEventSession; + +impl TerminalSessionApi for CrosstermEventSession { + fn draw(&mut self, _frame: TerminalFrame) -> TerminalResult<()> { + Err(TerminalError::Session( + "event-only terminal session cannot draw".to_string(), + )) + } + + fn read_event(&mut self) -> TerminalResult> { + Ok(CrosstermTerminalSession::terminal_event_from_crossterm_event(event::read()?)) + } + + fn poll_event(&mut self, timeout: Duration) -> TerminalResult> { + if !event::poll(timeout)? { + return Ok(None); + } + + self.read_event() + } + + fn setup_user_io(&mut self) -> TerminalResult<()> { + Err(TerminalError::Session( + "event-only terminal session cannot setup user I/O".to_string(), + )) + } + + fn teardown_user_io(&mut self) -> TerminalResult<()> { + Err(TerminalError::Session( + "event-only terminal session cannot teardown user I/O".to_string(), + )) + } + + fn wait_for_key_press(&mut self, key: KeyCode, timeout: Duration) -> TerminalResult { + wait_for_key_press_from_session(self, key, timeout) + } + + fn size(&self) -> TerminalResult<(u16, u16)> { + Err(TerminalError::Session( + "event-only terminal session has no terminal size".to_string(), + )) + } + + fn shutdown(&mut self) -> TerminalResult<()> { + Ok(()) + } +} + +fn wait_for_key_press_from_session( + session: &mut dyn TerminalSessionApi, + key: KeyCode, + timeout: Duration, +) -> TerminalResult { + let started_at = std::time::Instant::now(); + + while started_at.elapsed() < timeout { + let elapsed = started_at.elapsed(); + let remaining = timeout.saturating_sub(elapsed); + let poll_timeout = remaining.min(Duration::from_millis(16)); + + if let Some(TerminalEvent::Key(input)) = session.poll_event(poll_timeout)? { + if input.code == key { + return Ok(true); + } + } + } + + Ok(false) +} + #[cfg(test)] mod tests { use ratatui::crossterm::event::{ From 87075c51743202cd167bb7d2afd24b3cc697527b Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:29:56 -0300 Subject: [PATCH 05/13] refactor(terminal): decouple handlers from terminal loading This commit removes concrete Terminal ownership from the mailing list, latest patchsets, bookmarked patchsets, and system-update handlers. Loading screen rendering now flows through a LoadingIndicator boundary owned by the main handler loop, preserving spinner behavior without passing Terminal through normal screen handlers. The remaining direct terminal access is isolated to the app loop boundary and the patchset details interactive user-IO path, which is handled separately. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/app/updates.rs | 34 +++++------- src/handler/bookmarked.rs | 57 ++++++++----------- src/handler/latest.rs | 71 ++++++++++-------------- src/handler/mail_list.rs | 44 ++++++--------- src/handler/mod.rs | 113 +++++++++++++++++++++++++++++++++++--- 5 files changed, 187 insertions(+), 132 deletions(-) diff --git a/src/app/updates.rs b/src/app/updates.rs index 018eab0..c3876b5 100644 --- a/src/app/updates.rs +++ b/src/app/updates.rs @@ -1,19 +1,14 @@ -use ratatui::{prelude::Backend, Terminal}; - use crate::{ app::{screens::CurrentScreen, App}, - loading_screen, + handler::LoadingIndicator, }; impl App { /// Processes app-driven updates that are not direct user input. - pub async fn process_system_updates( + pub async fn process_system_updates( &mut self, - mut terminal: Terminal, - ) -> color_eyre::Result> - where - B: Backend + Send + 'static, - { + loading: &mut dyn LoadingIndicator, + ) -> color_eyre::Result<()> { match self.state.navigation.current_screen { CurrentScreen::MailingListSelection => { if self @@ -23,11 +18,10 @@ impl App { .mailing_lists .is_empty() { - terminal = loading_screen! { - terminal, "Fetching mailing lists" => { - self.refresh_mailing_lists().await - } - }; + loading.start("Fetching mailing lists".to_string()); + let result = self.refresh_mailing_lists().await; + loading.stop()?; + result?; } } CurrentScreen::LatestPatchsets => { @@ -35,12 +29,10 @@ impl App { if patchsets_state.processed_patchsets_count() == 0 { let target_list = patchsets_state.target_list().to_string(); - terminal = loading_screen! { - terminal, - format!("Fetching patchsets from {}", target_list) => { - self.fetch_latest_current_page().await - } - }; + loading.start(format!("Fetching patchsets from {}", target_list)); + let result = self.fetch_latest_current_page().await; + loading.stop()?; + result?; self.state.lore.mailing_list_selection.clear_target_list(); } @@ -59,6 +51,6 @@ impl App { _ => {} } - Ok(terminal) + Ok(()) } } diff --git a/src/handler/bookmarked.rs b/src/handler/bookmarked.rs index 6bc87e6..b2d725c 100644 --- a/src/handler/bookmarked.rs +++ b/src/handler/bookmarked.rs @@ -1,22 +1,15 @@ -use ratatui::{prelude::Backend, Terminal}; - -use std::ops::ControlFlow; - use crate::{ app::{screens::CurrentScreen, App, B4Result}, + handler::LoadingIndicator, input::event::InputEvent, - loading_screen, ui::popup::{help::HelpPopUpBuilder, info_popup::InfoPopUp, PopUp}, }; -pub async fn handle_bookmarked_patchsets( +pub async fn handle_bookmarked_patchsets( app: &mut App, input: InputEvent, - mut terminal: Terminal, -) -> color_eyre::Result>> -where - B: Backend + Send + 'static, -{ + loading: &mut dyn LoadingIndicator, +) -> color_eyre::Result<()> { match input { InputEvent::OpenHelp => { let popup = generate_help_popup(); @@ -39,34 +32,30 @@ where .select_above_patchset(); } InputEvent::OpenPatchsetDetails => { - terminal = loading_screen! { - terminal, - "Loading patchset" => { - let result = app.open_patchset_details().await; - if result.is_ok() { - // If a patchset has been bookmarked UI, this means that - // b4 was successful in fetching it, so it shouldn't be - // necessary to handle this, but we can't assume that a - // patchset in this list was bookmarked through the UI - match result.unwrap() { - B4Result::PatchFound => { - app.set_current_screen(CurrentScreen::PatchsetDetails); - } - B4Result::PatchNotFound(err_cause) => { - app.state.popup = Some(InfoPopUp::generate_info_popup( - "Error",&format!("The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset.") - )); - app.set_current_screen(CurrentScreen::BookmarkedPatchsets); - } - } + loading.start("Loading patchset".to_string()); + let result = app.open_patchset_details().await; + loading.stop()?; + if result.is_ok() { + // If a patchset has been bookmarked UI, this means that + // b4 was successful in fetching it, so it shouldn't be + // necessary to handle this, but we can't assume that a + // patchset in this list was bookmarked through the UI + match result.unwrap() { + B4Result::PatchFound => { + app.set_current_screen(CurrentScreen::PatchsetDetails); + } + B4Result::PatchNotFound(err_cause) => { + app.state.popup = Some(InfoPopUp::generate_info_popup( + "Error",&format!("The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset.") + )); + app.set_current_screen(CurrentScreen::BookmarkedPatchsets); } - color_eyre::eyre::Ok(()) } - }; + } } _ => {} } - Ok(ControlFlow::Continue(terminal)) + Ok(()) } pub fn generate_help_popup() -> Box { diff --git a/src/handler/latest.rs b/src/handler/latest.rs index 2934781..60e829a 100644 --- a/src/handler/latest.rs +++ b/src/handler/latest.rs @@ -1,22 +1,15 @@ -use ratatui::{prelude::Backend, Terminal}; - -use std::ops::ControlFlow; - use crate::{ app::{screens::CurrentScreen, App, B4Result}, + handler::LoadingIndicator, input::event::InputEvent, - loading_screen, ui::popup::{help::HelpPopUpBuilder, info_popup::InfoPopUp, PopUp}, }; -pub async fn handle_latest_patchsets( +pub async fn handle_latest_patchsets( app: &mut App, input: InputEvent, - mut terminal: Terminal, -) -> color_eyre::Result>> -where - B: Backend + Send + 'static, -{ + loading: &mut dyn LoadingIndicator, +) -> color_eyre::Result<()> { match input { InputEvent::OpenHelp => { let popup = generate_help_popup(); @@ -51,18 +44,16 @@ where .unwrap() .target_list() .to_string(); - terminal = loading_screen! { - terminal, - format!("Fetching patchsets from {}", list_name) => { - app.state - .lore - .latest_patchsets - .as_mut() - .unwrap() - .increment_page(); - app.fetch_latest_current_page().await - } - }; + loading.start(format!("Fetching patchsets from {}", list_name)); + app.state + .lore + .latest_patchsets + .as_mut() + .unwrap() + .increment_page(); + let result = app.fetch_latest_current_page().await; + loading.stop()?; + result?; } InputEvent::PreviousPage => { app.state @@ -75,30 +66,26 @@ where app.fetch_latest_current_page().await?; } InputEvent::OpenPatchsetDetails => { - terminal = loading_screen! { - terminal, - "Loading patchset" => { - let result = app.open_patchset_details().await; - if result.is_ok() { - match result.unwrap() { - B4Result::PatchFound => { - app.set_current_screen(CurrentScreen::PatchsetDetails); - } - B4Result::PatchNotFound(err_cause) => { - app.state.popup = Some(InfoPopUp::generate_info_popup( - "Error",&format!("The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset.") - )); - app.set_current_screen(CurrentScreen::LatestPatchsets); - } - } + loading.start("Loading patchset".to_string()); + let result = app.open_patchset_details().await; + loading.stop()?; + if result.is_ok() { + match result.unwrap() { + B4Result::PatchFound => { + app.set_current_screen(CurrentScreen::PatchsetDetails); + } + B4Result::PatchNotFound(err_cause) => { + app.state.popup = Some(InfoPopUp::generate_info_popup( + "Error",&format!("The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset.") + )); + app.set_current_screen(CurrentScreen::LatestPatchsets); } - color_eyre::eyre::Ok(()) } - }; + } } _ => {} } - Ok(ControlFlow::Continue(terminal)) + Ok(()) } pub fn generate_help_popup() -> Box { diff --git a/src/handler/mail_list.rs b/src/handler/mail_list.rs index 1dc3174..454b888 100644 --- a/src/handler/mail_list.rs +++ b/src/handler/mail_list.rs @@ -1,22 +1,17 @@ -use ratatui::{prelude::Backend, Terminal}; - use std::ops::ControlFlow; use crate::{ app::{screens::CurrentScreen, App}, + handler::LoadingIndicator, input::event::InputEvent, - loading_screen, ui::popup::{help::HelpPopUpBuilder, PopUp}, }; -pub async fn handle_mailing_list_selection( +pub async fn handle_mailing_list_selection( app: &mut App, input: InputEvent, - mut terminal: Terminal, -) -> color_eyre::Result>> -where - B: Backend + Send + 'static, -{ + loading: &mut dyn LoadingIndicator, +) -> color_eyre::Result> { match input { InputEvent::OpenHelp => { let popup = generate_help_popup(); @@ -39,26 +34,21 @@ where .target_list() .to_string(); - terminal = loading_screen! { - terminal, - format!("Fetching patchsets from {}", list_name) => { - let result = app.fetch_latest_current_page().await; - if result.is_ok() { - app.state.lore.mailing_list_selection.clear_target_list(); - app.set_current_screen(CurrentScreen::LatestPatchsets); - } - result - } - }; + loading.start(format!("Fetching patchsets from {}", list_name)); + let result = app.fetch_latest_current_page().await; + loading.stop()?; + if result.is_ok() { + app.state.lore.mailing_list_selection.clear_target_list(); + app.set_current_screen(CurrentScreen::LatestPatchsets); + } + result?; } } InputEvent::RefreshMailingLists => { - terminal = loading_screen! { - terminal, - "Refreshing lists" => { - app.refresh_mailing_lists().await - } - }; + loading.start("Refreshing lists".to_string()); + let result = app.refresh_mailing_lists().await; + loading.stop()?; + result?; } InputEvent::OpenEditConfig => { app.init_edit_config(); @@ -99,7 +89,7 @@ where } _ => {} } - Ok(ControlFlow::Continue(terminal)) + Ok(ControlFlow::Continue(())) } // TODO: Move this to a more appropriate place diff --git a/src/handler/mod.rs b/src/handler/mod.rs index c455119..187cc7b 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -6,7 +6,15 @@ mod mail_list; use ratatui::{prelude::Backend, Terminal}; -use std::ops::ControlFlow; +use std::{ + ops::ControlFlow, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, + thread::JoinHandle, + time::Duration, +}; use crate::{ app::{screens::CurrentScreen, App}, @@ -21,14 +29,98 @@ use edit_config::handle_edit_config; use latest::handle_latest_patchsets; use mail_list::handle_mailing_list_selection; +pub(crate) trait LoadingIndicator { + fn start(&mut self, title: String); + fn stop(&mut self) -> color_eyre::Result<()>; +} + +struct TerminalLoadingIndicator { + terminal: Option>, + running: Option>, + handle: Option>>, +} + +impl TerminalLoadingIndicator +where + B: Backend + Send + 'static, +{ + fn new(terminal: Terminal) -> Self { + Self { + terminal: Some(terminal), + running: None, + handle: None, + } + } + + fn terminal_mut(&mut self) -> color_eyre::Result<&mut Terminal> { + self.terminal + .as_mut() + .ok_or_else(|| color_eyre::eyre::eyre!("terminal unavailable while loading")) + } + + fn into_terminal(mut self) -> color_eyre::Result> { + self.stop()?; + self.terminal + .take() + .ok_or_else(|| color_eyre::eyre::eyre!("terminal unavailable after loading")) + } +} + +impl LoadingIndicator for TerminalLoadingIndicator +where + B: Backend + Send + 'static, +{ + fn start(&mut self, title: String) { + if self.handle.is_some() { + return; + } + + let Some(mut terminal) = self.terminal.take() else { + return; + }; + let loading = Arc::new(AtomicBool::new(true)); + let loading_clone = Arc::clone(&loading); + + self.running = Some(loading); + self.handle = Some(std::thread::spawn(move || { + while loading_clone.load(Ordering::Relaxed) { + terminal = crate::ui::loading_screen::render(terminal, &title); + std::thread::sleep(Duration::from_millis(200)); + } + + terminal + })); + + std::thread::sleep(Duration::from_millis(200)); + } + + fn stop(&mut self) -> color_eyre::Result<()> { + let Some(handle) = self.handle.take() else { + return Ok(()); + }; + + if let Some(running) = self.running.take() { + running.store(false, Ordering::Relaxed); + } + + self.terminal = Some( + handle + .join() + .map_err(|_| color_eyre::eyre::eyre!("loading screen thread panicked"))?, + ); + Ok(()) + } +} + async fn input_handling( - mut terminal: Terminal, + terminal: Terminal, app: &mut App, input: InputEvent, ) -> color_eyre::Result>> where B: Backend + Send + 'static, { + let mut loading = TerminalLoadingIndicator::new(terminal); if let Some(popup) = app.state.popup.as_mut() { if input == InputEvent::ClosePopup { app.state.popup = None; @@ -38,23 +130,26 @@ where } else { match app.state.navigation.current_screen { CurrentScreen::MailingListSelection => { - return handle_mailing_list_selection(app, input, terminal).await; + match handle_mailing_list_selection(app, input, &mut loading).await? { + ControlFlow::Continue(()) => {} + ControlFlow::Break(()) => return Ok(ControlFlow::Break(())), + } } CurrentScreen::BookmarkedPatchsets => { - return handle_bookmarked_patchsets(app, input, terminal).await; + handle_bookmarked_patchsets(app, input, &mut loading).await?; } CurrentScreen::PatchsetDetails => { - handle_patchset_details(app, input, &mut terminal).await?; + handle_patchset_details(app, input, loading.terminal_mut()?).await?; } CurrentScreen::EditConfig => { handle_edit_config(app, input)?; } CurrentScreen::LatestPatchsets => { - return handle_latest_patchsets(app, input, terminal).await; + handle_latest_patchsets(app, input, &mut loading).await?; } } } - Ok(ControlFlow::Continue(terminal)) + Ok(ControlFlow::Continue(loading.into_terminal()?)) } pub async fn run_app( @@ -68,7 +163,9 @@ where let mut input_mapper = InputMapper::default(); loop { - terminal = app.process_system_updates(terminal).await?; + let mut loading = TerminalLoadingIndicator::new(terminal); + app.process_system_updates(&mut loading).await?; + terminal = loading.into_terminal()?; terminal.draw(|f| draw_ui(f, &app.to_view_model()))?; From 58c3354b84f19e03aa011eca207bdbb745656f74 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:31:39 -0300 Subject: [PATCH 06/13] refactor(terminal): hide details terminal control This commit removes direct terminal access from the patchset details handler. Preview sizing and user-IO mode transitions now go through a terminal control boundary owned by the main handler loop, while the post-interactive Enter wait uses TerminalHandle instead of constructing a crossterm event source directly. This keeps the details input flow on the Phase 9 terminal protocol path without changing the existing patchset action behavior. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/handler/details_actions.rs | 47 ++++++++++++++++++++-------------- src/handler/mod.rs | 34 ++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/src/handler/details_actions.rs b/src/handler/details_actions.rs index 9c20447..cc754a1 100644 --- a/src/handler/details_actions.rs +++ b/src/handler/details_actions.rs @@ -1,19 +1,22 @@ -use ratatui::{backend::Backend, Terminal}; +use std::time::Duration; + +use ratatui::crossterm::event::KeyCode; use crate::{ app::{screens::CurrentScreen, App}, - infrastructure::terminal::{setup_user_io, teardown_user_io}, - input::{ - event::{InputEvent, ScrollAmount}, - terminal_source::{wait_for_enter_press, CrosstermEventSource}, - }, + handler::TerminalController, + input::event::{InputEvent, ScrollAmount}, + terminal::handle::TerminalHandle, ui::popup::{help::HelpPopUpBuilder, review_trailers::ReviewTrailersPopUp, PopUp}, }; -pub async fn handle_patchset_details( +const USER_IO_ENTER_POLL_TIMEOUT: Duration = Duration::from_millis(200); + +pub async fn handle_patchset_details( app: &mut App, input: InputEvent, - terminal: &mut Terminal, + terminal: &mut dyn TerminalController, + terminal_handle: &TerminalHandle, ) -> color_eyre::Result<()> { let patchset_details_and_actions = app.state.lore.details.as_mut().unwrap(); @@ -31,11 +34,11 @@ pub async fn handle_patchset_details( patchset_details_and_actions.toggle_apply_action(); } InputEvent::PreviewScrollDown(amount) => { - let lines = preview_scroll_lines(amount, terminal); + let lines = preview_scroll_lines(amount, terminal)?; patchset_details_and_actions.preview_scroll_down(lines); } InputEvent::PreviewScrollUp(amount) => { - let lines = preview_scroll_lines(amount, terminal); + let lines = preview_scroll_lines(amount, terminal)?; patchset_details_and_actions.preview_scroll_up(lines); } InputEvent::PreviewPanLeft => { @@ -77,12 +80,14 @@ pub async fn handle_patchset_details( } InputEvent::ConsolidatePatchsetActions => { if patchset_details_and_actions.actions_require_user_io() { - setup_user_io(terminal)?; + terminal.setup_user_io()?; app.consolidate_patchset_actions().await?; println!("\nPress ENTER continue..."); - let mut event_source = CrosstermEventSource; - wait_for_enter_press(&mut event_source)?; - teardown_user_io(terminal)?; + while !terminal_handle + .wait_for_key_press(KeyCode::Enter, USER_IO_ENTER_POLL_TIMEOUT) + .await? + {} + terminal.teardown_user_io()?; } else { app.consolidate_patchset_actions().await?; } @@ -93,12 +98,16 @@ pub async fn handle_patchset_details( Ok(()) } -fn preview_scroll_lines(amount: ScrollAmount, terminal: &Terminal) -> usize { - match amount { +fn preview_scroll_lines( + amount: ScrollAmount, + terminal: &dyn TerminalController, +) -> color_eyre::Result { + let (_, height) = terminal.size()?; + Ok(match amount { ScrollAmount::Line => 1, - ScrollAmount::HalfPage => terminal.size().unwrap().height as usize / 2, - ScrollAmount::Page => terminal.size().unwrap().height as usize, - } + ScrollAmount::HalfPage => height as usize / 2, + ScrollAmount::Page => height as usize, + }) } pub fn generate_help_popup() -> Box { diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 187cc7b..e031a1e 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -18,6 +18,7 @@ use std::{ use crate::{ app::{screens::CurrentScreen, App}, + infrastructure::terminal::{setup_user_io, teardown_user_io}, input::{event::InputEvent, mapper::InputMapper}, terminal::handle::TerminalHandle, ui::draw_ui, @@ -34,6 +35,12 @@ pub(crate) trait LoadingIndicator { fn stop(&mut self) -> color_eyre::Result<()>; } +pub(crate) trait TerminalController { + fn setup_user_io(&mut self) -> color_eyre::Result<()>; + fn teardown_user_io(&mut self) -> color_eyre::Result<()>; + fn size(&self) -> color_eyre::Result<(u16, u16)>; +} + struct TerminalLoadingIndicator { terminal: Option>, running: Option>, @@ -112,10 +119,33 @@ where } } +impl TerminalController for TerminalLoadingIndicator +where + B: Backend + Send + 'static, +{ + fn setup_user_io(&mut self) -> color_eyre::Result<()> { + setup_user_io(self.terminal_mut()?) + } + + fn teardown_user_io(&mut self) -> color_eyre::Result<()> { + teardown_user_io(self.terminal_mut()?) + } + + fn size(&self) -> color_eyre::Result<(u16, u16)> { + let size = self + .terminal + .as_ref() + .ok_or_else(|| color_eyre::eyre::eyre!("terminal unavailable while loading"))? + .size()?; + Ok((size.width, size.height)) + } +} + async fn input_handling( terminal: Terminal, app: &mut App, input: InputEvent, + terminal_handle: &TerminalHandle, ) -> color_eyre::Result>> where B: Backend + Send + 'static, @@ -139,7 +169,7 @@ where handle_bookmarked_patchsets(app, input, &mut loading).await?; } CurrentScreen::PatchsetDetails => { - handle_patchset_details(app, input, loading.terminal_mut()?).await?; + handle_patchset_details(app, input, &mut loading, terminal_handle).await?; } CurrentScreen::EditConfig => { handle_edit_config(app, input)?; @@ -177,7 +207,7 @@ where if let Some(terminal_event) = terminal_handle.read_event().await? { let input = input_mapper.map_terminal_event(terminal_event, &app.input_context()); if let Some(input) = input { - match input_handling(terminal, &mut app, input).await? { + match input_handling(terminal, &mut app, input, &terminal_handle).await? { ControlFlow::Continue(t) => terminal = t, ControlFlow::Break(_) => return Ok(()), } From 2fb6814697ea3c3d82811fd659967a537da7f461 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:34:22 -0300 Subject: [PATCH 07/13] refactor(terminal): remove legacy terminal source This commit removes the old input terminal source now that raw terminal event conversion and reads live behind the terminal actor session boundary. The input module documentation now describes TerminalEvent production as coming from the terminal actor, leaving InputMapper as the semantic translation layer for the current app loop. It also applies the remaining cleanup needed for the Phase 9 changes to pass the repository formatting, test, and clippy checks. This commit completes the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/app/updates.rs | 2 +- src/handler/latest.rs | 2 +- src/handler/mail_list.rs | 2 +- src/handler/mod.rs | 6 -- src/input/mod.rs | 9 ++- src/input/terminal_source.rs | 124 ----------------------------------- src/terminal/actor.rs | 3 +- 7 files changed, 9 insertions(+), 139 deletions(-) delete mode 100644 src/input/terminal_source.rs diff --git a/src/app/updates.rs b/src/app/updates.rs index c3876b5..a382b68 100644 --- a/src/app/updates.rs +++ b/src/app/updates.rs @@ -29,7 +29,7 @@ impl App { if patchsets_state.processed_patchsets_count() == 0 { let target_list = patchsets_state.target_list().to_string(); - loading.start(format!("Fetching patchsets from {}", target_list)); + loading.start(format!("Fetching patchsets from {target_list}")); let result = self.fetch_latest_current_page().await; loading.stop()?; result?; diff --git a/src/handler/latest.rs b/src/handler/latest.rs index 60e829a..ee226d2 100644 --- a/src/handler/latest.rs +++ b/src/handler/latest.rs @@ -44,7 +44,7 @@ pub async fn handle_latest_patchsets( .unwrap() .target_list() .to_string(); - loading.start(format!("Fetching patchsets from {}", list_name)); + loading.start(format!("Fetching patchsets from {list_name}")); app.state .lore .latest_patchsets diff --git a/src/handler/mail_list.rs b/src/handler/mail_list.rs index 454b888..3dd749a 100644 --- a/src/handler/mail_list.rs +++ b/src/handler/mail_list.rs @@ -34,7 +34,7 @@ pub async fn handle_mailing_list_selection( .target_list() .to_string(); - loading.start(format!("Fetching patchsets from {}", list_name)); + loading.start(format!("Fetching patchsets from {list_name}")); let result = app.fetch_latest_current_page().await; loading.stop()?; if result.is_ok() { diff --git a/src/handler/mod.rs b/src/handler/mod.rs index e031a1e..c5bab03 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -199,11 +199,6 @@ where terminal.draw(|f| draw_ui(f, &app.to_view_model()))?; - // *IMPORTANT*: Uncommenting the if below makes `patch-hub` not block - // until an event is captured. We should only do it when (if ever) we - // need to refresh the UI independently of any event as doing so gravely - // hinders the performance to below acceptable. - // if event::poll(Duration::from_millis(16))? { if let Some(terminal_event) = terminal_handle.read_event().await? { let input = input_mapper.map_terminal_event(terminal_event, &app.input_context()); if let Some(input) = input { @@ -213,6 +208,5 @@ where } } } - // } } } diff --git a/src/input/mod.rs b/src/input/mod.rs index 1aca996..840126c 100644 --- a/src/input/mod.rs +++ b/src/input/mod.rs @@ -1,13 +1,12 @@ //! Protocol boundary between terminal input and application intent. //! -//! Phase 8 keeps this module actor-free: terminal backends produce -//! [`event::TerminalEvent`] values, [`mapper::InputMapper`] translates them -//! using [`context::InputContext`], and handlers consume semantic +//! Terminal backends produce [`event::TerminalEvent`] values through the +//! terminal actor, [`mapper::InputMapper`] translates them using +//! [`context::InputContext`], and handlers consume semantic //! [`event::InputEvent`] values. The Input actor and broadcast handle are -//! introduced later once Terminal is also actorized. +//! introduced in the next phase. pub mod bindings; pub mod context; pub mod event; pub mod mapper; -pub mod terminal_source; diff --git a/src/input/terminal_source.rs b/src/input/terminal_source.rs deleted file mode 100644 index acc59ed..0000000 --- a/src/input/terminal_source.rs +++ /dev/null @@ -1,124 +0,0 @@ -use ratatui::crossterm::event::{self, Event, KeyCode, KeyEventKind}; - -use crate::input::event::{KeyInput, TerminalEvent}; - -pub trait TerminalEventSource { - fn read_event(&mut self) -> color_eyre::Result>; -} - -pub struct CrosstermEventSource; - -impl TerminalEventSource for CrosstermEventSource { - fn read_event(&mut self) -> color_eyre::Result> { - Ok(terminal_event_from_crossterm_event(event::read()?)) - } -} - -pub fn terminal_event_from_crossterm_event(event: Event) -> Option { - match event { - Event::Key(key) if key.kind == KeyEventKind::Release => None, - Event::Key(key) => Some(TerminalEvent::Key(KeyInput::from(key))), - Event::Resize(width, height) => Some(TerminalEvent::Resize { width, height }), - _ => None, - } -} - -pub fn wait_for_enter_press(event_source: &mut dyn TerminalEventSource) -> color_eyre::Result<()> { - loop { - if let Some(TerminalEvent::Key(key)) = event_source.read_event()? { - if key.code == KeyCode::Enter { - return Ok(()); - } - } - } -} - -#[cfg(test)] -mod tests { - use ratatui::crossterm::event::{ - Event, KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers, - }; - - use crate::input::{ - event::{KeyInput, TerminalEvent}, - terminal_source::{ - terminal_event_from_crossterm_event, wait_for_enter_press, TerminalEventSource, - }, - }; - - struct FakeTerminalEventSource { - events: Vec>, - } - - impl FakeTerminalEventSource { - fn new(events: Vec>) -> Self { - Self { events } - } - } - - impl TerminalEventSource for FakeTerminalEventSource { - fn read_event(&mut self) -> color_eyre::Result> { - Ok(self.events.remove(0)) - } - } - - #[test] - fn converts_key_press_to_terminal_key_event() { - let key = KeyEvent::new(KeyCode::Char('j'), KeyModifiers::NONE); - - let event = terminal_event_from_crossterm_event(Event::Key(key)); - - assert_eq!(event, Some(TerminalEvent::Key(KeyInput::from(key)))); - } - - #[test] - fn ignores_key_release_events() { - let key = KeyEvent { - code: KeyCode::Char('j'), - modifiers: KeyModifiers::NONE, - kind: KeyEventKind::Release, - state: KeyEventState::NONE, - }; - - let event = terminal_event_from_crossterm_event(Event::Key(key)); - - assert_eq!(event, None); - } - - #[test] - fn converts_resize_events() { - let event = terminal_event_from_crossterm_event(Event::Resize(120, 40)); - - assert_eq!( - event, - Some(TerminalEvent::Resize { - width: 120, - height: 40 - }) - ); - } - - #[test] - fn ignores_non_key_non_resize_events() { - let event = terminal_event_from_crossterm_event(Event::FocusGained); - - assert_eq!(event, None); - } - - #[test] - fn wait_for_enter_press_ignores_non_enter_events() { - let mut event_source = FakeTerminalEventSource::new(vec![ - None, - Some(TerminalEvent::Resize { - width: 120, - height: 40, - }), - Some(TerminalEvent::Key(KeyInput::press(KeyCode::Char('x')))), - Some(TerminalEvent::Key(KeyInput::press(KeyCode::Enter))), - ]); - - let result = wait_for_enter_press(&mut event_source); - - assert!(result.is_ok()); - } -} diff --git a/src/terminal/actor.rs b/src/terminal/actor.rs index 77ee001..2dc214d 100644 --- a/src/terminal/actor.rs +++ b/src/terminal/actor.rs @@ -5,7 +5,7 @@ use tokio::{ use crate::terminal::{ handle::TerminalHandle, - messages::{TerminalFrame, TerminalMessage, TerminalResult}, + messages::{TerminalMessage, TerminalResult}, session::TerminalSessionApi, TerminalError, }; @@ -160,6 +160,7 @@ mod tests { use crate::{ input::event::{KeyInput, TerminalEvent}, + terminal::messages::TerminalFrame, terminal::session::MockTerminalSessionApi, }; From c2c79cc2a188a19b778e4c1e355406c1da4a86f4 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:42:59 -0300 Subject: [PATCH 08/13] refactor(terminal): add owned draw payload This commit introduces an owned AppRenderSnapshot so terminal draw requests can cross the actor channel without borrowing AppState. TerminalFrame now models main UI and loading-frame draws, and CrosstermTerminalSession can render both through the existing UI drawing helpers. It also makes render state and popup values cloneable so the current UI layer can keep using AppViewModel while the terminal actor receives owned frame payloads. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/app/mod.rs | 1 + src/app/render_snapshot.rs | 25 +++++++++++++++++++++++++ src/app/screens/bookmarked.rs | 1 + src/app/screens/details_actions.rs | 1 + src/app/screens/edit_config.rs | 4 ++-- src/app/screens/latest.rs | 1 + src/app/screens/mail_list.rs | 1 + src/app/state.rs | 5 +++++ src/terminal/actor.rs | 4 ++-- src/terminal/messages.rs | 8 ++++++-- src/terminal/session.rs | 8 ++++++++ src/ui/loading_screen.rs | 2 +- src/ui/popup/help.rs | 2 +- src/ui/popup/info_popup.rs | 2 +- src/ui/popup/mod.rs | 21 ++++++++++++++++++++- src/ui/popup/review_trailers.rs | 2 +- 16 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 src/app/render_snapshot.rs diff --git a/src/app/mod.rs b/src/app/mod.rs index 1e30fc7..8c6eb07 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1,6 +1,7 @@ pub mod commands; pub mod errors; pub mod input; +pub mod render_snapshot; pub mod screens; pub mod state; pub mod updates; diff --git a/src/app/render_snapshot.rs b/src/app/render_snapshot.rs new file mode 100644 index 0000000..d0cdfaf --- /dev/null +++ b/src/app/render_snapshot.rs @@ -0,0 +1,25 @@ +use super::{App, AppState, AppViewModel}; + +/// Owned application state snapshot for terminal actor drawing. +#[derive(Clone)] +pub struct AppRenderSnapshot { + state: AppState, +} + +impl AppRenderSnapshot { + #[allow(dead_code)] // Wired into the runtime draw path in the next commit. + pub fn new(state: AppState) -> Self { + Self { state } + } + + pub fn to_view_model(&self) -> AppViewModel<'_> { + AppViewModel { state: &self.state } + } +} + +impl App { + #[allow(dead_code)] // Wired into the runtime draw path in the next commit. + pub fn render_snapshot(&self) -> AppRenderSnapshot { + AppRenderSnapshot::new(self.state.clone()) + } +} diff --git a/src/app/screens/bookmarked.rs b/src/app/screens/bookmarked.rs index 5f07539..88a2531 100644 --- a/src/app/screens/bookmarked.rs +++ b/src/app/screens/bookmarked.rs @@ -1,5 +1,6 @@ use crate::lore::domain::patch::Patch; +#[derive(Clone)] pub struct BookmarkedPatchsetsState { pub bookmarked_patchsets: Vec, pub patchset_index: usize, diff --git a/src/app/screens/details_actions.rs b/src/app/screens/details_actions.rs index c0ccc04..e9be519 100644 --- a/src/app/screens/details_actions.rs +++ b/src/app/screens/details_actions.rs @@ -19,6 +19,7 @@ use crate::{ use super::CurrentScreen; +#[derive(Clone)] pub struct PatchsetDetailsState { pub representative_patch: Patch, /// Raw patches as plain text files diff --git a/src/app/screens/edit_config.rs b/src/app/screens/edit_config.rs index 742a34b..c56d5cd 100644 --- a/src/app/screens/edit_config.rs +++ b/src/app/screens/edit_config.rs @@ -5,7 +5,7 @@ use std::{collections::HashMap, fmt::Display}; use crate::config::{ConfigSnapshot, ConfigUpdateDraft}; -#[derive(Debug, Getters)] +#[derive(Clone, Debug, Getters)] pub struct EditConfigState { #[getter(skip)] config_buffer: HashMap, @@ -137,7 +137,7 @@ impl EditConfigState { } } -#[derive(Debug, Hash, Eq, PartialEq)] +#[derive(Clone, Debug, Hash, Eq, PartialEq)] enum EditableConfig { PageSize, CacheDir, diff --git a/src/app/screens/latest.rs b/src/app/screens/latest.rs index 4a5140d..53c003d 100644 --- a/src/app/screens/latest.rs +++ b/src/app/screens/latest.rs @@ -5,6 +5,7 @@ use crate::lore::{ domain::patch::Patch, }; +#[derive(Clone)] pub struct LatestPatchsetsState { target_list: String, page_number: usize, diff --git a/src/app/screens/mail_list.rs b/src/app/screens/mail_list.rs index 0074d71..e6dff7d 100644 --- a/src/app/screens/mail_list.rs +++ b/src/app/screens/mail_list.rs @@ -5,6 +5,7 @@ use crate::lore::{ domain::mailing_list::MailingList, }; +#[derive(Clone)] pub struct MailingListSelectionState { pub mailing_lists: Vec, pub target_list: String, diff --git a/src/app/state.rs b/src/app/state.rs index da76a53..061e6a4 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -11,11 +11,13 @@ use crate::{ }; /// Navigation-only state: which screen is active. +#[derive(Clone)] pub struct NavigationState { pub current_screen: CurrentScreen, } /// Lore-related UI: mailing list picker, feed, patchset details. +#[derive(Clone)] pub struct LoreUiState { pub mailing_list_selection: MailingListSelectionState, pub latest_patchsets: Option, @@ -23,17 +25,20 @@ pub struct LoreUiState { } /// User-owned Lore data (bookmarks and review markers). +#[derive(Clone)] pub struct UserLoreState { pub bookmarked_patchsets: BookmarkedPatchsetsState, pub reviewed_patchsets: HashMap>, } /// Edit-config screen state (transient form). +#[derive(Clone)] pub struct ConfigUiState { pub edit_config: Option, } /// All application state grouped for the future App actor. +#[derive(Clone)] pub struct AppState { pub navigation: NavigationState, pub lore: LoreUiState, diff --git a/src/terminal/actor.rs b/src/terminal/actor.rs index 2dc214d..7c16833 100644 --- a/src/terminal/actor.rs +++ b/src/terminal/actor.rs @@ -175,7 +175,7 @@ mod tests { let mut session = MockTerminalSessionApi::new(); session .expect_draw() - .withf(|frame| frame == &TerminalFrame::Empty) + .withf(|frame| matches!(frame, TerminalFrame::Empty)) .times(1) .returning(|_| Ok(())); let handle = spawn_test_actor(session); @@ -208,7 +208,7 @@ mod tests { let mut session = MockTerminalSessionApi::new(); session .expect_draw() - .withf(|frame| frame == &TerminalFrame::Empty) + .withf(|frame| matches!(frame, TerminalFrame::Empty)) .times(1) .returning(|_| Ok(())); session.expect_size().times(1).returning(|| Ok((120, 40))); diff --git a/src/terminal/messages.rs b/src/terminal/messages.rs index 21d21b2..2be3252 100644 --- a/src/terminal/messages.rs +++ b/src/terminal/messages.rs @@ -5,13 +5,17 @@ use std::time::Duration; use ratatui::crossterm::event::KeyCode; use tokio::sync::oneshot; -use crate::{input::event::TerminalEvent, terminal::TerminalError}; +use crate::{ + app::render_snapshot::AppRenderSnapshot, input::event::TerminalEvent, terminal::TerminalError, +}; pub type TerminalResult = Result; /// Owned payload for a terminal draw request. -#[derive(Debug, Clone, Default, PartialEq, Eq)] +#[derive(Clone, Default)] pub enum TerminalFrame { + Main(Box), + Loading(String), /// Placeholder frame used while the terminal actor is wired into the app. #[default] Empty, diff --git a/src/terminal/session.rs b/src/terminal/session.rs index 6e205b8..02c12dd 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -13,6 +13,7 @@ use crate::{ messages::{TerminalFrame, TerminalResult}, TerminalError, }, + ui::{draw_ui, loading_screen::draw_loading_screen}, }; /// Stateful terminal session owned by the terminal actor. @@ -57,6 +58,13 @@ impl CrosstermTerminalSession { impl TerminalSessionApi for CrosstermTerminalSession { fn draw(&mut self, frame: TerminalFrame) -> TerminalResult<()> { match frame { + TerminalFrame::Main(snapshot) => { + self.terminal + .draw(|f| draw_ui(f, &snapshot.to_view_model()))?; + } + TerminalFrame::Loading(title) => { + self.terminal.draw(|f| draw_loading_screen(f, &title))?; + } TerminalFrame::Empty => { self.terminal.draw(|_| {})?; } diff --git a/src/ui/loading_screen.rs b/src/ui/loading_screen.rs index 99343b4..39e0d53 100644 --- a/src/ui/loading_screen.rs +++ b/src/ui/loading_screen.rs @@ -43,7 +43,7 @@ fn spinner() -> char { /// The actual implementation of the loading screen rendering. Currently the /// loading notification is static. -fn draw_loading_screen(f: &mut Frame, title: impl Display) { +pub(crate) fn draw_loading_screen(f: &mut Frame, title: impl Display) { let frame_area = f.area(); let loading_text = format!("{} {}", title, spinner()); diff --git a/src/ui/popup/help.rs b/src/ui/popup/help.rs index b3ea975..9b9efe7 100644 --- a/src/ui/popup/help.rs +++ b/src/ui/popup/help.rs @@ -21,7 +21,7 @@ use super::PopUp; /// The description is displayed below the title and is optional /// The keybinds (also optional) are displayed in a table format with the key on the left and the help message on the right #[allow(dead_code)] -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct HelpPopUp { title: Option, description: Option, diff --git a/src/ui/popup/info_popup.rs b/src/ui/popup/info_popup.rs index 17fc393..2691619 100644 --- a/src/ui/popup/info_popup.rs +++ b/src/ui/popup/info_popup.rs @@ -8,7 +8,7 @@ use ratatui::{ use super::PopUp; use crate::input::event::InputEvent; -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct InfoPopUp { title: String, info: String, diff --git a/src/ui/popup/mod.rs b/src/ui/popup/mod.rs index 8901514..d08fbf5 100644 --- a/src/ui/popup/mod.rs +++ b/src/ui/popup/mod.rs @@ -8,8 +8,27 @@ use std::fmt::Debug; use crate::input::event::InputEvent; +pub trait PopUpClone { + fn clone_box(&self) -> Box; +} + +impl PopUpClone for T +where + T: 'static + PopUp + Clone, +{ + fn clone_box(&self) -> Box { + Box::new(self.clone()) + } +} + +impl Clone for Box { + fn clone(&self) -> Self { + self.clone_box() + } +} + /// A trait that represents a popup that can be rendered on top of a screen -pub trait PopUp: Debug { +pub trait PopUp: Debug + Send + PopUpClone { /// Returns the dimensions of the popup in percentage of the screen /// (width, height) /// diff --git a/src/ui/popup/review_trailers.rs b/src/ui/popup/review_trailers.rs index 88466a3..06e4d92 100644 --- a/src/ui/popup/review_trailers.rs +++ b/src/ui/popup/review_trailers.rs @@ -14,7 +14,7 @@ use crate::{ use super::PopUp; -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct ReviewTrailersPopUp { reviewed_by_text: String, tested_by_text: String, From 66246eeae7dda57edbad9a6ffae8273478d094b3 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:48:23 -0300 Subject: [PATCH 09/13] refactor(terminal): wire runtime to crossterm session This commit makes TerminalActor own the concrete Tui at startup and routes the main app loop draw and read path through TerminalHandle instead of the split init plus CrosstermEventSession model. run_app now takes only App and TerminalHandle, and frame rendering uses TerminalFrame::Main with App::render_snapshot. It also rewrites the handler-side loading indicator to hold a TerminalHandle instead of Terminal, bridging preview sizing and user-IO transitions through the terminal actor while leaving handle-backed loading frames for the follow-up commit. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/app/mod.rs | 1 + src/app/render_snapshot.rs | 2 - src/handler/mod.rs | 166 +++++++++++-------------------------- src/main.rs | 7 +- src/terminal/session.rs | 2 - src/ui/loading_screen.rs | 1 + 6 files changed, 52 insertions(+), 127 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 8c6eb07..946a9b4 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -432,6 +432,7 @@ impl App { /// Borrows state for one UI frame without passing [`App`] into `ui/`. #[must_use] + #[allow(dead_code)] // Runtime draws via [`App::render_snapshot`] until the UI actor owns rendering. pub fn to_view_model(&self) -> AppViewModel<'_> { AppViewModel { state: &self.state } } diff --git a/src/app/render_snapshot.rs b/src/app/render_snapshot.rs index d0cdfaf..0ab0f32 100644 --- a/src/app/render_snapshot.rs +++ b/src/app/render_snapshot.rs @@ -7,7 +7,6 @@ pub struct AppRenderSnapshot { } impl AppRenderSnapshot { - #[allow(dead_code)] // Wired into the runtime draw path in the next commit. pub fn new(state: AppState) -> Self { Self { state } } @@ -18,7 +17,6 @@ impl AppRenderSnapshot { } impl App { - #[allow(dead_code)] // Wired into the runtime draw path in the next commit. pub fn render_snapshot(&self) -> AppRenderSnapshot { AppRenderSnapshot::new(self.state.clone()) } diff --git a/src/handler/mod.rs b/src/handler/mod.rs index c5bab03..9687185 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -4,24 +4,16 @@ mod edit_config; mod latest; mod mail_list; -use ratatui::{prelude::Backend, Terminal}; - -use std::{ - ops::ControlFlow, - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, - thread::JoinHandle, - time::Duration, -}; +use std::{future::Future, ops::ControlFlow}; use crate::{ app::{screens::CurrentScreen, App}, - infrastructure::terminal::{setup_user_io, teardown_user_io}, input::{event::InputEvent, mapper::InputMapper}, - terminal::handle::TerminalHandle, - ui::draw_ui, + terminal::{ + handle::TerminalHandle, + messages::{TerminalFrame, TerminalResult}, + TerminalError, + }, }; use bookmarked::handle_bookmarked_patchsets; @@ -41,116 +33,57 @@ pub(crate) trait TerminalController { fn size(&self) -> color_eyre::Result<(u16, u16)>; } -struct TerminalLoadingIndicator { - terminal: Option>, - running: Option>, - handle: Option>>, +struct TerminalLoadingIndicator { + terminal_handle: TerminalHandle, } -impl TerminalLoadingIndicator -where - B: Backend + Send + 'static, -{ - fn new(terminal: Terminal) -> Self { - Self { - terminal: Some(terminal), - running: None, - handle: None, - } - } - - fn terminal_mut(&mut self) -> color_eyre::Result<&mut Terminal> { - self.terminal - .as_mut() - .ok_or_else(|| color_eyre::eyre::eyre!("terminal unavailable while loading")) - } - - fn into_terminal(mut self) -> color_eyre::Result> { - self.stop()?; - self.terminal - .take() - .ok_or_else(|| color_eyre::eyre::eyre!("terminal unavailable after loading")) +impl TerminalLoadingIndicator { + fn new(terminal_handle: TerminalHandle) -> Self { + Self { terminal_handle } } } -impl LoadingIndicator for TerminalLoadingIndicator -where - B: Backend + Send + 'static, -{ - fn start(&mut self, title: String) { - if self.handle.is_some() { - return; - } - - let Some(mut terminal) = self.terminal.take() else { - return; - }; - let loading = Arc::new(AtomicBool::new(true)); - let loading_clone = Arc::clone(&loading); - - self.running = Some(loading); - self.handle = Some(std::thread::spawn(move || { - while loading_clone.load(Ordering::Relaxed) { - terminal = crate::ui::loading_screen::render(terminal, &title); - std::thread::sleep(Duration::from_millis(200)); - } - - terminal - })); - - std::thread::sleep(Duration::from_millis(200)); +impl LoadingIndicator for TerminalLoadingIndicator { + fn start(&mut self, _title: String) { + // Handle-backed loading frames are wired in the next Phase 9 commit. } fn stop(&mut self) -> color_eyre::Result<()> { - let Some(handle) = self.handle.take() else { - return Ok(()); - }; - - if let Some(running) = self.running.take() { - running.store(false, Ordering::Relaxed); - } - - self.terminal = Some( - handle - .join() - .map_err(|_| color_eyre::eyre::eyre!("loading screen thread panicked"))?, - ); Ok(()) } } -impl TerminalController for TerminalLoadingIndicator -where - B: Backend + Send + 'static, -{ +impl TerminalController for TerminalLoadingIndicator { fn setup_user_io(&mut self) -> color_eyre::Result<()> { - setup_user_io(self.terminal_mut()?) + terminal_handle_call(self.terminal_handle.setup_user_io()) } fn teardown_user_io(&mut self) -> color_eyre::Result<()> { - teardown_user_io(self.terminal_mut()?) + terminal_handle_call(self.terminal_handle.teardown_user_io()) } fn size(&self) -> color_eyre::Result<(u16, u16)> { - let size = self - .terminal - .as_ref() - .ok_or_else(|| color_eyre::eyre::eyre!("terminal unavailable while loading"))? - .size()?; - Ok((size.width, size.height)) + terminal_handle_call(self.terminal_handle.size()) } } -async fn input_handling( - terminal: Terminal, +fn terminal_handle_call( + future: impl Future>, +) -> color_eyre::Result { + tokio::task::block_in_place(|| tokio::runtime::Handle::current().block_on(future)) + .map_err(|error| color_eyre::eyre::eyre!("{error}")) +} + +fn terminal_error(error: TerminalError) -> color_eyre::Report { + color_eyre::eyre::eyre!("{error}") +} + +async fn input_handling( app: &mut App, input: InputEvent, terminal_handle: &TerminalHandle, -) -> color_eyre::Result>> -where - B: Backend + Send + 'static, -{ - let mut loading = TerminalLoadingIndicator::new(terminal); + loading: &mut TerminalLoadingIndicator, +) -> color_eyre::Result> { if let Some(popup) = app.state.popup.as_mut() { if input == InputEvent::ClosePopup { app.state.popup = None; @@ -160,51 +93,46 @@ where } else { match app.state.navigation.current_screen { CurrentScreen::MailingListSelection => { - match handle_mailing_list_selection(app, input, &mut loading).await? { + match handle_mailing_list_selection(app, input, loading).await? { ControlFlow::Continue(()) => {} ControlFlow::Break(()) => return Ok(ControlFlow::Break(())), } } CurrentScreen::BookmarkedPatchsets => { - handle_bookmarked_patchsets(app, input, &mut loading).await?; + handle_bookmarked_patchsets(app, input, loading).await?; } CurrentScreen::PatchsetDetails => { - handle_patchset_details(app, input, &mut loading, terminal_handle).await?; + handle_patchset_details(app, input, loading, terminal_handle).await?; } CurrentScreen::EditConfig => { handle_edit_config(app, input)?; } CurrentScreen::LatestPatchsets => { - handle_latest_patchsets(app, input, &mut loading).await?; + handle_latest_patchsets(app, input, loading).await?; } } } - Ok(ControlFlow::Continue(loading.into_terminal()?)) + Ok(ControlFlow::Continue(())) } -pub async fn run_app( - mut terminal: Terminal, - mut app: App, - terminal_handle: TerminalHandle, -) -> color_eyre::Result<()> -where - B: Backend + Send + 'static, -{ +pub async fn run_app(mut app: App, terminal_handle: TerminalHandle) -> color_eyre::Result<()> { let mut input_mapper = InputMapper::default(); + let mut loading = TerminalLoadingIndicator::new(terminal_handle.clone()); loop { - let mut loading = TerminalLoadingIndicator::new(terminal); app.process_system_updates(&mut loading).await?; - terminal = loading.into_terminal()?; - terminal.draw(|f| draw_ui(f, &app.to_view_model()))?; + terminal_handle + .draw(TerminalFrame::Main(Box::new(app.render_snapshot()))) + .await + .map_err(terminal_error)?; - if let Some(terminal_event) = terminal_handle.read_event().await? { + if let Some(terminal_event) = terminal_handle.read_event().await.map_err(terminal_error)? { let input = input_mapper.map_terminal_event(terminal_event, &app.input_context()); if let Some(input) = input { - match input_handling(terminal, &mut app, input, &terminal_handle).await? { - ControlFlow::Continue(t) => terminal = t, - ControlFlow::Break(_) => return Ok(()), + match input_handling(&mut app, input, &terminal_handle, &mut loading).await? { + ControlFlow::Continue(()) => {} + ControlFlow::Break(()) => return Ok(()), } } } diff --git a/src/main.rs b/src/main.rs index d4a523a..c7796ae 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,7 +37,7 @@ use lore::{ use render::{actor::RenderActor, ShellRenderService}; use render_prefs::PatchRenderer; use std::{ops::ControlFlow, sync::Arc}; -use terminal::{actor::TerminalActor, session::CrosstermEventSession}; +use terminal::{actor::TerminalActor, session::CrosstermTerminalSession}; use tracing::{event, Level}; /// Verifies required and optional external binaries before the TUI runs. @@ -120,8 +120,7 @@ async fn main() -> color_eyre::Result<()> { ControlFlow::Continue(()) => {} } - let terminal = init()?; - let terminal_events = TerminalActor::spawn(Box::new(CrosstermEventSession)); + let terminal_handle = TerminalActor::spawn(Box::new(CrosstermTerminalSession::new(init()?))); // Build shared infrastructure dependencies for LoreService let net = Arc::new(UreqNetClient::new()); @@ -175,7 +174,7 @@ async fn main() -> color_eyre::Result<()> { bail!("patch-hub cannot be executed because some dependencies are missing, check logs for more information"); } - run_app(terminal, app, terminal_events).await?; + run_app(app, terminal_handle).await?; restore()?; event!(Level::INFO, "patch-hub finished"); diff --git a/src/terminal/session.rs b/src/terminal/session.rs index 02c12dd..cbeaa4c 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -30,14 +30,12 @@ pub trait TerminalSessionApi: Send { } /// Ratatui/Crossterm-backed terminal session. -#[allow(dead_code)] // Wired into main in the next Phase 9 commit. pub struct CrosstermTerminalSession { terminal: Tui, shutdown: bool, } impl CrosstermTerminalSession { - #[allow(dead_code)] // Wired into main in the next Phase 9 commit. pub fn new(terminal: Tui) -> Self { Self { terminal, diff --git a/src/ui/loading_screen.rs b/src/ui/loading_screen.rs index 39e0d53..45767b9 100644 --- a/src/ui/loading_screen.rs +++ b/src/ui/loading_screen.rs @@ -27,6 +27,7 @@ const LOADING_AREA_EXTRA_LINES: u16 = 2; /// This function renders a loading screen taking a `terminal` instance and a /// `title`. +#[allow(dead_code)] // Runtime loading moves to [`TerminalHandle`] in the next Phase 9 commit. pub fn render(mut terminal: Terminal, title: impl Display) -> Terminal { let _ = terminal.draw(|f| draw_loading_screen(f, title)); terminal From 4e769e7134632327794e4581c54ee963bfd62f57 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:51:24 -0300 Subject: [PATCH 10/13] refactor(terminal): route loading frames through handle This commit moves loading screen rendering behind TerminalHandle instead of passing a concrete Terminal through handler code. TerminalLoadingIndicator now spawns a background task that periodically sends TerminalFrame::Loading draws while long-running work is in progress, and stop cleanly shuts the spinner task down before normal UI frames resume. It also removes the legacy loading_screen render helper and loading_screen! macro now that loading layout is drawn through CrosstermTerminalSession and the terminal actor protocol. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/handler/mod.rs | 84 ++++++++++++++++++++++++++++++++++++++-- src/macros.rs | 50 ------------------------ src/ui/loading_screen.rs | 11 +----- 3 files changed, 81 insertions(+), 64 deletions(-) diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 9687185..9c0ea69 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -4,7 +4,17 @@ mod edit_config; mod latest; mod mail_list; -use std::{future::Future, ops::ControlFlow}; +use std::{ + future::Future, + ops::ControlFlow, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, + time::Duration, +}; + +use tokio::task::JoinHandle; use crate::{ app::{screens::CurrentScreen, App}, @@ -22,6 +32,8 @@ use edit_config::handle_edit_config; use latest::handle_latest_patchsets; use mail_list::handle_mailing_list_selection; +const LOADING_FRAME_INTERVAL: Duration = Duration::from_millis(200); + pub(crate) trait LoadingIndicator { fn start(&mut self, title: String); fn stop(&mut self) -> color_eyre::Result<()>; @@ -35,20 +47,61 @@ pub(crate) trait TerminalController { struct TerminalLoadingIndicator { terminal_handle: TerminalHandle, + running: Option>, + spinner_task: Option>, } impl TerminalLoadingIndicator { fn new(terminal_handle: TerminalHandle) -> Self { - Self { terminal_handle } + Self { + terminal_handle, + running: None, + spinner_task: None, + } } } impl LoadingIndicator for TerminalLoadingIndicator { - fn start(&mut self, _title: String) { - // Handle-backed loading frames are wired in the next Phase 9 commit. + fn start(&mut self, title: String) { + if self.spinner_task.is_some() { + return; + } + + let running = Arc::new(AtomicBool::new(true)); + let running_clone = Arc::clone(&running); + let terminal_handle = self.terminal_handle.clone(); + + self.running = Some(running); + self.spinner_task = Some(tokio::spawn(async move { + while running_clone.load(Ordering::Relaxed) { + if terminal_handle + .draw(TerminalFrame::Loading(title.clone())) + .await + .is_err() + { + break; + } + + std::thread::sleep(LOADING_FRAME_INTERVAL); + } + })); + + std::thread::sleep(LOADING_FRAME_INTERVAL); } fn stop(&mut self) -> color_eyre::Result<()> { + let Some(spinner_task) = self.spinner_task.take() else { + return Ok(()); + }; + + if let Some(running) = self.running.take() { + running.store(false, Ordering::Relaxed); + } + + tokio::task::block_in_place(|| { + tokio::runtime::Handle::current().block_on(async { spinner_task.await.ok() }); + }); + Ok(()) } } @@ -138,3 +191,26 @@ pub async fn run_app(mut app: App, terminal_handle: TerminalHandle) -> color_eyr } } } + +#[cfg(test)] +mod tests { + use crate::terminal::{actor::TerminalActor, session::MockTerminalSessionApi}; + + use super::*; + + #[tokio::test(flavor = "multi_thread")] + async fn loading_indicator_draws_loading_frame_through_terminal_handle() { + let mut session = MockTerminalSessionApi::new(); + session + .expect_draw() + .withf(|frame| matches!(frame, TerminalFrame::Loading(_))) + .times(1..) + .returning(|_| Ok(())); + let handle = TerminalActor::spawn(Box::new(session)); + let mut loading = TerminalLoadingIndicator::new(handle); + + loading.start("Fetching mailing lists".to_string()); + std::thread::sleep(LOADING_FRAME_INTERVAL); + loading.stop().unwrap(); + } +} diff --git a/src/macros.rs b/src/macros.rs index dae1a5e..bf88b0c 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1,53 +1,3 @@ -#[macro_export] -/// Macro that encapsulates a piece of code that takes long to run and displays a loading screen while it runs. -/// -/// This macro takes two arguments: the terminal and the title of the loading screen (anything that implements `Display`). -/// After a `=>` token, you can pass the code that takes long to run. -/// -/// When the execution finishes, the macro will return the terminal. -/// -/// Important to notice that the code block will run in the same scope as the rest of the macro. -/// Be aware that in Rust, when using `?` or `return` inside a closure, they apply to the outer function, -/// not the closure itself. This can lead to unexpected behavior if you expect the closure to handle -/// errors or return values independently of the enclosing function. -/// -/// # Example -/// ```rust norun -/// terminal = loading_screen! { terminal, "Loading stuff" => { -/// // code that takes long to run -/// }}; -/// ``` -macro_rules! loading_screen { - { $terminal:expr, $title:expr => $inst:expr} => { - { - let loading = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)); - let loading_clone = std::sync::Arc::clone(&loading); - let mut terminal = $terminal; - - let handle = std::thread::spawn(move || { - while loading_clone.load(std::sync::atomic::Ordering::Relaxed) { - terminal = $crate::ui::loading_screen::render(terminal, $title); - std::thread::sleep(std::time::Duration::from_millis(200)); - } - - terminal - }); - - // we have to sleep so the loading thread completes at least one render - std::thread::sleep(std::time::Duration::from_millis(200)); - let inst_result = $inst; - - loading.store(false, std::sync::atomic::Ordering::Relaxed); - - let terminal = handle.join().unwrap(); - - inst_result?; - - terminal - } - }; -} - #[macro_export] macro_rules! log_on_error { ($result:expr) => { diff --git a/src/ui/loading_screen.rs b/src/ui/loading_screen.rs index 45767b9..79d3613 100644 --- a/src/ui/loading_screen.rs +++ b/src/ui/loading_screen.rs @@ -1,9 +1,8 @@ use ratatui::{ - prelude::Backend, style::{Color, Style}, text::{Line, Span}, widgets::{Block, Borders, Paragraph, Wrap}, - Frame, Terminal, + Frame, }; use std::fmt::Display; @@ -25,14 +24,6 @@ static mut SPINNER_TICK: usize = 1; const LOADING_AREA_EXTRA_FACTOR_WIDTH: f32 = 1.3; const LOADING_AREA_EXTRA_LINES: u16 = 2; -/// This function renders a loading screen taking a `terminal` instance and a -/// `title`. -#[allow(dead_code)] // Runtime loading moves to [`TerminalHandle`] in the next Phase 9 commit. -pub fn render(mut terminal: Terminal, title: impl Display) -> Terminal { - let _ = terminal.draw(|f| draw_loading_screen(f, title)); - terminal -} - /// Gets the current spinner state and updates the tick. fn spinner() -> char { let char_to_ret = SPINNER[unsafe { SPINNER_TICK }]; From 7f552231ddd3633ed8a596c4efeb5b2af193e33a Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:52:17 -0300 Subject: [PATCH 11/13] refactor(terminal): move details user-io to handle This commit removes the handler-side TerminalController boundary and routes patchset details preview sizing and interactive user-IO transitions through TerminalHandle instead. Preview scroll amounts now query terminal size asynchronously, and consolidate actions use setup_user_io, teardown_user_io, and the existing wait_for_key_press path on the terminal actor protocol. It also adds actor tests covering setup_user_io and teardown_user_io delegation through the mock terminal session. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/handler/details_actions.rs | 16 +++++++-------- src/handler/mod.rs | 36 ++-------------------------------- src/terminal/actor.rs | 21 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 43 deletions(-) diff --git a/src/handler/details_actions.rs b/src/handler/details_actions.rs index cc754a1..3d48160 100644 --- a/src/handler/details_actions.rs +++ b/src/handler/details_actions.rs @@ -4,7 +4,6 @@ use ratatui::crossterm::event::KeyCode; use crate::{ app::{screens::CurrentScreen, App}, - handler::TerminalController, input::event::{InputEvent, ScrollAmount}, terminal::handle::TerminalHandle, ui::popup::{help::HelpPopUpBuilder, review_trailers::ReviewTrailersPopUp, PopUp}, @@ -15,7 +14,6 @@ const USER_IO_ENTER_POLL_TIMEOUT: Duration = Duration::from_millis(200); pub async fn handle_patchset_details( app: &mut App, input: InputEvent, - terminal: &mut dyn TerminalController, terminal_handle: &TerminalHandle, ) -> color_eyre::Result<()> { let patchset_details_and_actions = app.state.lore.details.as_mut().unwrap(); @@ -34,11 +32,11 @@ pub async fn handle_patchset_details( patchset_details_and_actions.toggle_apply_action(); } InputEvent::PreviewScrollDown(amount) => { - let lines = preview_scroll_lines(amount, terminal)?; + let lines = preview_scroll_lines(amount, terminal_handle).await?; patchset_details_and_actions.preview_scroll_down(lines); } InputEvent::PreviewScrollUp(amount) => { - let lines = preview_scroll_lines(amount, terminal)?; + let lines = preview_scroll_lines(amount, terminal_handle).await?; patchset_details_and_actions.preview_scroll_up(lines); } InputEvent::PreviewPanLeft => { @@ -80,14 +78,14 @@ pub async fn handle_patchset_details( } InputEvent::ConsolidatePatchsetActions => { if patchset_details_and_actions.actions_require_user_io() { - terminal.setup_user_io()?; + terminal_handle.setup_user_io().await?; app.consolidate_patchset_actions().await?; println!("\nPress ENTER continue..."); while !terminal_handle .wait_for_key_press(KeyCode::Enter, USER_IO_ENTER_POLL_TIMEOUT) .await? {} - terminal.teardown_user_io()?; + terminal_handle.teardown_user_io().await?; } else { app.consolidate_patchset_actions().await?; } @@ -98,11 +96,11 @@ pub async fn handle_patchset_details( Ok(()) } -fn preview_scroll_lines( +async fn preview_scroll_lines( amount: ScrollAmount, - terminal: &dyn TerminalController, + terminal_handle: &TerminalHandle, ) -> color_eyre::Result { - let (_, height) = terminal.size()?; + let (_, height) = terminal_handle.size().await?; Ok(match amount { ScrollAmount::Line => 1, ScrollAmount::HalfPage => height as usize / 2, diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 9c0ea69..2521c57 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -5,7 +5,6 @@ mod latest; mod mail_list; use std::{ - future::Future, ops::ControlFlow, sync::{ atomic::{AtomicBool, Ordering}, @@ -19,11 +18,7 @@ use tokio::task::JoinHandle; use crate::{ app::{screens::CurrentScreen, App}, input::{event::InputEvent, mapper::InputMapper}, - terminal::{ - handle::TerminalHandle, - messages::{TerminalFrame, TerminalResult}, - TerminalError, - }, + terminal::{handle::TerminalHandle, messages::TerminalFrame, TerminalError}, }; use bookmarked::handle_bookmarked_patchsets; @@ -39,12 +34,6 @@ pub(crate) trait LoadingIndicator { fn stop(&mut self) -> color_eyre::Result<()>; } -pub(crate) trait TerminalController { - fn setup_user_io(&mut self) -> color_eyre::Result<()>; - fn teardown_user_io(&mut self) -> color_eyre::Result<()>; - fn size(&self) -> color_eyre::Result<(u16, u16)>; -} - struct TerminalLoadingIndicator { terminal_handle: TerminalHandle, running: Option>, @@ -106,27 +95,6 @@ impl LoadingIndicator for TerminalLoadingIndicator { } } -impl TerminalController for TerminalLoadingIndicator { - fn setup_user_io(&mut self) -> color_eyre::Result<()> { - terminal_handle_call(self.terminal_handle.setup_user_io()) - } - - fn teardown_user_io(&mut self) -> color_eyre::Result<()> { - terminal_handle_call(self.terminal_handle.teardown_user_io()) - } - - fn size(&self) -> color_eyre::Result<(u16, u16)> { - terminal_handle_call(self.terminal_handle.size()) - } -} - -fn terminal_handle_call( - future: impl Future>, -) -> color_eyre::Result { - tokio::task::block_in_place(|| tokio::runtime::Handle::current().block_on(future)) - .map_err(|error| color_eyre::eyre::eyre!("{error}")) -} - fn terminal_error(error: TerminalError) -> color_eyre::Report { color_eyre::eyre::eyre!("{error}") } @@ -155,7 +123,7 @@ async fn input_handling( handle_bookmarked_patchsets(app, input, loading).await?; } CurrentScreen::PatchsetDetails => { - handle_patchset_details(app, input, loading, terminal_handle).await?; + handle_patchset_details(app, input, terminal_handle).await?; } CurrentScreen::EditConfig => { handle_edit_config(app, input)?; diff --git a/src/terminal/actor.rs b/src/terminal/actor.rs index 7c16833..edde73e 100644 --- a/src/terminal/actor.rs +++ b/src/terminal/actor.rs @@ -238,6 +238,27 @@ mod tests { assert!(pressed); } + #[tokio::test] + async fn setup_user_io_delegates_to_session() { + let mut session = MockTerminalSessionApi::new(); + session.expect_setup_user_io().times(1).returning(|| Ok(())); + let handle = spawn_test_actor(session); + + handle.setup_user_io().await.unwrap(); + } + + #[tokio::test] + async fn teardown_user_io_delegates_to_session() { + let mut session = MockTerminalSessionApi::new(); + session + .expect_teardown_user_io() + .times(1) + .returning(|| Ok(())); + let handle = spawn_test_actor(session); + + handle.teardown_user_io().await.unwrap(); + } + #[tokio::test] async fn closed_channel_returns_actor_unavailable() { let (tx, rx) = mpsc::channel(DEFAULT_TERMINAL_CHANNEL_SIZE); From 89b123e735ef5f7dfdcf39de0fd01350eaf4ca58 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:53:07 -0300 Subject: [PATCH 12/13] refactor(terminal): route normal shutdown through handle This commit restores the terminal through TerminalHandle::shutdown after run_app returns instead of calling infrastructure restore directly from main. The terminal actor session remains the single owner of normal terminal lifecycle teardown in the runtime path. It also documents that panic and error hooks keep the direct restore fallback for fatal failures, and adds an actor test covering repeated shutdown requests through the mock terminal session. This commit is part of the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/infrastructure/errors.rs | 4 ++++ src/main.rs | 9 ++++++--- src/terminal/actor.rs | 10 ++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/infrastructure/errors.rs b/src/infrastructure/errors.rs index 12d2138..d07356d 100644 --- a/src/infrastructure/errors.rs +++ b/src/infrastructure/errors.rs @@ -4,6 +4,10 @@ use super::terminal::restore; /// This replaces the standard color_eyre panic and error hooks with hooks that /// restore the terminal before printing the panic or error. +/// +/// Normal application shutdown restores the terminal through +/// [`crate::terminal::handle::TerminalHandle::shutdown`]. These hooks keep a +/// direct [`super::terminal::restore`] fallback for panics and fatal errors. pub fn install_hooks() -> color_eyre::Result<()> { let (panic_hook, eyre_hook) = color_eyre::config::HookBuilder::default().into_hooks(); diff --git a/src/main.rs b/src/main.rs index c7796ae..a46ca8e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,7 +23,7 @@ use infrastructure::{ monitoring::{init_monitoring, InitMonitoringProduct}, net::UreqNetClient, shell::OsShell, - terminal::{init, restore}, + terminal::init, }; use lore::{ application::{actor::LoreApiActor, cache::CacheTtl, service::LoreService}, @@ -174,8 +174,11 @@ async fn main() -> color_eyre::Result<()> { bail!("patch-hub cannot be executed because some dependencies are missing, check logs for more information"); } - run_app(app, terminal_handle).await?; - restore()?; + run_app(app, terminal_handle.clone()).await?; + terminal_handle + .shutdown() + .await + .map_err(|error| eyre!("{error}"))?; event!(Level::INFO, "patch-hub finished"); diff --git a/src/terminal/actor.rs b/src/terminal/actor.rs index edde73e..919fb02 100644 --- a/src/terminal/actor.rs +++ b/src/terminal/actor.rs @@ -259,6 +259,16 @@ mod tests { handle.teardown_user_io().await.unwrap(); } + #[tokio::test] + async fn shutdown_delegates_to_session_and_is_safe_to_repeat() { + let mut session = MockTerminalSessionApi::new(); + session.expect_shutdown().times(2).returning(|| Ok(())); + let handle = spawn_test_actor(session); + + handle.shutdown().await.unwrap(); + handle.shutdown().await.unwrap(); + } + #[tokio::test] async fn closed_channel_returns_actor_unavailable() { let (tx, rx) = mpsc::channel(DEFAULT_TERMINAL_CHANNEL_SIZE); From 91e71ea63d3f3838d0bd3649de30791cb4f5fa08 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 14:54:17 -0300 Subject: [PATCH 13/13] refactor(terminal): complete phase 9 ownership cleanup This commit removes the transitional CrosstermEventSession and finishes Phase 9 terminal ownership cleanup. Raw Crossterm event reads now live only in CrosstermTerminalSession, infrastructure terminal helpers are narrowed to session and emergency restore use, and the terminal and input module docs describe the current pull loop through TerminalHandle. It also removes stale Phase 9 dead-code allowances and the unused App::to_view_model helper now that runtime drawing uses AppRenderSnapshot. This commit completes the architecture's refactoring phase 9. Signed-off-by: lorenzoberts --- src/app/mod.rs | 7 ----- src/app/view_model.rs | 3 +- src/infrastructure/terminal.rs | 11 ++++++-- src/input/mod.rs | 11 ++++---- src/terminal/errors.rs | 1 - src/terminal/handle.rs | 3 +- src/terminal/messages.rs | 4 +-- src/terminal/mod.rs | 9 ++++-- src/terminal/session.rs | 50 ---------------------------------- 9 files changed, 25 insertions(+), 74 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 946a9b4..5ba58de 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -429,11 +429,4 @@ impl App { pub fn set_current_screen(&mut self, new_current_screen: CurrentScreen) { self.state.navigation.current_screen = new_current_screen; } - - /// Borrows state for one UI frame without passing [`App`] into `ui/`. - #[must_use] - #[allow(dead_code)] // Runtime draws via [`App::render_snapshot`] until the UI actor owns rendering. - pub fn to_view_model(&self) -> AppViewModel<'_> { - AppViewModel { state: &self.state } - } } diff --git a/src/app/view_model.rs b/src/app/view_model.rs index e36b1da..56187a0 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -2,7 +2,8 @@ use super::state::AppState; -/// References into [`AppState`] for one Ratatui frame. Built via [`super::App::to_view_model`]. +/// References into [`AppState`] for one Ratatui frame. Built via +/// [`super::render_snapshot::AppRenderSnapshot::to_view_model`]. #[derive(Clone, Copy)] pub struct AppViewModel<'a> { pub state: &'a AppState, diff --git a/src/infrastructure/terminal.rs b/src/infrastructure/terminal.rs index c8bf6db..ec42680 100644 --- a/src/infrastructure/terminal.rs +++ b/src/infrastructure/terminal.rs @@ -1,3 +1,10 @@ +//! Low-level Crossterm/Ratatui helpers for the terminal actor session and +//! emergency restore hooks. +//! +//! Normal runtime startup uses [`init`] from `main`, session operations go +//! through [`crate::terminal::session::CrosstermTerminalSession`], and fatal +//! error hooks call [`restore`] directly. + use ratatui::{ crossterm::{ execute, @@ -27,7 +34,7 @@ pub fn restore() -> io::Result<()> { Ok(()) } -pub fn setup_user_io(terminal: &mut Terminal) -> color_eyre::Result<()> { +pub(crate) fn setup_user_io(terminal: &mut Terminal) -> color_eyre::Result<()> { terminal.clear()?; terminal.set_cursor_position(Position::new(0, 0))?; terminal.show_cursor()?; @@ -35,7 +42,7 @@ pub fn setup_user_io(terminal: &mut Terminal) -> color_eyre::Resu Ok(()) } -pub fn teardown_user_io(terminal: &mut Terminal) -> color_eyre::Result<()> { +pub(crate) fn teardown_user_io(terminal: &mut Terminal) -> color_eyre::Result<()> { enable_raw_mode()?; terminal.clear()?; Ok(()) diff --git a/src/input/mod.rs b/src/input/mod.rs index 840126c..7d7d3a5 100644 --- a/src/input/mod.rs +++ b/src/input/mod.rs @@ -1,10 +1,11 @@ //! Protocol boundary between terminal input and application intent. //! -//! Terminal backends produce [`event::TerminalEvent`] values through the -//! terminal actor, [`mapper::InputMapper`] translates them using -//! [`context::InputContext`], and handlers consume semantic -//! [`event::InputEvent`] values. The Input actor and broadcast handle are -//! introduced in the next phase. +//! Phase 9 uses a pull loop: `handler::run_app` reads raw +//! [`event::TerminalEvent`] values through the terminal actor, then +//! [`mapper::InputMapper`] translates them using [`context::InputContext`] +//! before handlers consume semantic [`event::InputEvent`] values. Phase 10 +//! will introduce an `InputActor` that broadcasts terminal events instead of +//! this direct pull loop. pub mod bindings; pub mod context; diff --git a/src/terminal/errors.rs b/src/terminal/errors.rs index 30b4770..673256f 100644 --- a/src/terminal/errors.rs +++ b/src/terminal/errors.rs @@ -1,7 +1,6 @@ use thiserror::Error; /// Failures at the terminal actor/session boundary. -#[allow(dead_code)] #[derive(Debug, Error)] pub enum TerminalError { #[error("terminal I/O error: {0}")] diff --git a/src/terminal/handle.rs b/src/terminal/handle.rs index 7d2498d..83973bb 100644 --- a/src/terminal/handle.rs +++ b/src/terminal/handle.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] // Follow-up Phase 9 commits wire terminal handle methods. - use std::time::Duration; use ratatui::crossterm::event::KeyCode; @@ -33,6 +31,7 @@ impl TerminalHandle { .await } + #[allow(dead_code)] // Reserved for Phase 10 non-blocking input polling. pub async fn poll_event(&self, timeout: Duration) -> TerminalResult> { self.request_result(|reply| TerminalMessage::PollEvent { timeout, reply }) .await diff --git a/src/terminal/messages.rs b/src/terminal/messages.rs index 2be3252..879f4c7 100644 --- a/src/terminal/messages.rs +++ b/src/terminal/messages.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] // Follow-up Phase 9 commits wire the full terminal protocol. - use std::time::Duration; use ratatui::crossterm::event::KeyCode; @@ -16,7 +14,7 @@ pub type TerminalResult = Result; pub enum TerminalFrame { Main(Box), Loading(String), - /// Placeholder frame used while the terminal actor is wired into the app. + /// Placeholder frame used in terminal actor tests. #[default] Empty, } diff --git a/src/terminal/mod.rs b/src/terminal/mod.rs index 9add3dc..3d14168 100644 --- a/src/terminal/mod.rs +++ b/src/terminal/mod.rs @@ -1,8 +1,11 @@ //! Actor boundary for the Ratatui/Crossterm terminal session. //! -//! Phase 9 introduces this module as the only owner of terminal session -//! operations. Later commits wire the existing app loop through -//! [`handle::TerminalHandle`]. +//! Phase 9 makes this module the single owner of terminal session operations. +//! The runtime pull loop in `handler::run_app` draws through +//! [`handle::TerminalHandle::draw`] and reads input through +//! [`handle::TerminalHandle::read_event`]. Phase 10 will introduce an +//! `InputActor` that broadcasts terminal events instead of the current +//! direct pull loop. pub mod actor; pub mod errors; diff --git a/src/terminal/session.rs b/src/terminal/session.rs index cbeaa4c..4ca1d20 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -112,56 +112,6 @@ impl TerminalSessionApi for CrosstermTerminalSession { } } -/// Transitional Crossterm event source used while drawing still owns the -/// concrete Ratatui terminal outside the actor. -pub struct CrosstermEventSession; - -impl TerminalSessionApi for CrosstermEventSession { - fn draw(&mut self, _frame: TerminalFrame) -> TerminalResult<()> { - Err(TerminalError::Session( - "event-only terminal session cannot draw".to_string(), - )) - } - - fn read_event(&mut self) -> TerminalResult> { - Ok(CrosstermTerminalSession::terminal_event_from_crossterm_event(event::read()?)) - } - - fn poll_event(&mut self, timeout: Duration) -> TerminalResult> { - if !event::poll(timeout)? { - return Ok(None); - } - - self.read_event() - } - - fn setup_user_io(&mut self) -> TerminalResult<()> { - Err(TerminalError::Session( - "event-only terminal session cannot setup user I/O".to_string(), - )) - } - - fn teardown_user_io(&mut self) -> TerminalResult<()> { - Err(TerminalError::Session( - "event-only terminal session cannot teardown user I/O".to_string(), - )) - } - - fn wait_for_key_press(&mut self, key: KeyCode, timeout: Duration) -> TerminalResult { - wait_for_key_press_from_session(self, key, timeout) - } - - fn size(&self) -> TerminalResult<(u16, u16)> { - Err(TerminalError::Session( - "event-only terminal session has no terminal size".to_string(), - )) - } - - fn shutdown(&mut self) -> TerminalResult<()> { - Ok(()) - } -} - fn wait_for_key_press_from_session( session: &mut dyn TerminalSessionApi, key: KeyCode,