From 7ebaa50c5f23ac6e02d28950a693cf3fc99efb15 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 12:47:27 -0300 Subject: [PATCH 1/6] refactor(render): move render service into top-level context This commit moves the synchronous shell-backed render service out of infrastructure and into a top-level Render context. The existing RenderServiceApi behavior is preserved so App still renders patchset previews through the same service boundary while the code moves closer to the upcoming Render actor. This commit is part of the architecture's refactoring phase 7. Signed-off-by: lorenzoberts --- src/app/mod.rs | 2 +- src/infrastructure/mod.rs | 1 - src/main.rs | 3 ++- src/{infrastructure => }/render/mod.rs | 0 src/{infrastructure => }/render/tests.rs | 7 +++---- src/{infrastructure => }/render/trait.rs | 0 6 files changed, 6 insertions(+), 7 deletions(-) rename src/{infrastructure => }/render/mod.rs (100%) rename src/{infrastructure => }/render/tests.rs (84%) rename src/{infrastructure => }/render/trait.rs (100%) diff --git a/src/app/mod.rs b/src/app/mod.rs index a53f662..8abc63b 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -22,7 +22,6 @@ use crate::{ env::EnvTrait, file_system::FileSystemTrait, monitoring::logging::garbage_collector::collect_garbage, - render::RenderServiceApi, shell::{ShellCommand, ShellTrait}, }, lore::{ @@ -33,6 +32,7 @@ use crate::{ }, domain::patch::{Author, Patch}, }, + render::RenderServiceApi, ui::popup::info_popup::InfoPopUp, }; use screens::{ diff --git a/src/infrastructure/mod.rs b/src/infrastructure/mod.rs index 85d0bd5..dd6b5be 100644 --- a/src/infrastructure/mod.rs +++ b/src/infrastructure/mod.rs @@ -3,6 +3,5 @@ pub mod errors; pub mod file_system; pub mod monitoring; pub mod net; -pub mod render; pub mod shell; pub mod terminal; diff --git a/src/main.rs b/src/main.rs index b52a2dc..5575d4c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,6 +5,7 @@ mod handler; mod infrastructure; mod lore; mod macros; +mod render; mod render_prefs; mod ui; @@ -19,7 +20,6 @@ use infrastructure::{ file_system::OsFileSystem, monitoring::{init_monitoring, InitMonitoringProduct}, net::UreqNetClient, - render::{RenderServiceApi, ShellRenderService}, shell::OsShell, terminal::{init, restore}, }; @@ -32,6 +32,7 @@ use lore::{ persistence::{FileLorePersistence, MailingListsCacheStore, UserLoreStateStore}, }, }; +use render::{RenderServiceApi, ShellRenderService}; use std::{ops::ControlFlow, sync::Arc}; use tracing::{event, Level}; diff --git a/src/infrastructure/render/mod.rs b/src/render/mod.rs similarity index 100% rename from src/infrastructure/render/mod.rs rename to src/render/mod.rs diff --git a/src/infrastructure/render/tests.rs b/src/render/tests.rs similarity index 84% rename from src/infrastructure/render/tests.rs rename to src/render/tests.rs index 3a19fc1..483536a 100644 --- a/src/infrastructure/render/tests.rs +++ b/src/render/tests.rs @@ -1,11 +1,10 @@ use std::sync::Arc; -use crate::infrastructure::{ - render::{RenderServiceApi, ShellRenderService}, - shell::OsShell, -}; +use crate::render::{RenderServiceApi, ShellRenderService}; use crate::{app::cover_renderer::CoverRenderer, app::patch_renderer::PatchRenderer}; +use crate::infrastructure::shell::OsShell; + #[test] fn shell_render_service_produces_one_preview_per_patch() { let svc: Arc = Arc::new(ShellRenderService::new(Arc::new(OsShell))); diff --git a/src/infrastructure/render/trait.rs b/src/render/trait.rs similarity index 100% rename from src/infrastructure/render/trait.rs rename to src/render/trait.rs From a07186b6d2eda31c572987b4b355e1ca1e8272cb Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 13:12:06 -0300 Subject: [PATCH 2/6] refactor(render): add preview request and response DTOs This commit introduces RenderPatchsetRequest and rendered preview DTOs for the Render context. The synchronous render service now accepts an explicit request payload and returns rendered preview entries instead of a raw Vec, while App preserves the existing details preview behavior by converting the rendered strings into Ratatui text at the current boundary. This commit is part of the architecture's refactoring phase 7. Signed-off-by: lorenzoberts --- src/app/mod.rs | 23 ++++++++++++++--------- src/render/dto.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/render/mod.rs | 44 ++++++++++++++++++++++---------------------- src/render/tests.rs | 9 +++++---- src/render/trait.rs | 9 +++------ 5 files changed, 88 insertions(+), 41 deletions(-) create mode 100644 src/render/dto.rs diff --git a/src/app/mod.rs b/src/app/mod.rs index 8abc63b..48d5660 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -32,7 +32,7 @@ use crate::{ }, domain::patch::{Author, Patch}, }, - render::RenderServiceApi, + render::{RenderPatchsetRequest, RenderServiceApi}, ui::popup::info_popup::InfoPopUp, }; use screens::{ @@ -219,14 +219,15 @@ impl App { Err(e) => bail!("{e:#?}"), }; - let preview_lines = self + let render_request = RenderPatchsetRequest::new( + details.raw_patches.clone(), + *self.state.config.patch_renderer(), + *self.state.config.cover_renderer(), + ); + let rendered_preview = self .services .render - .render_patchset_preview( - &details.raw_patches, - self.state.config.patch_renderer(), - self.state.config.cover_renderer(), - ) + .render_patchset_preview(render_request) .map_err(|e| eyre!("{e}"))?; let mut patches_preview: Vec = Vec::new(); @@ -234,11 +235,15 @@ impl App { let mut tested_by: Vec> = Vec::new(); let mut acked_by: Vec> = Vec::new(); - for (line, tag_summary) in preview_lines.iter().zip(details.tag_summary.iter()) { + for (entry, tag_summary) in rendered_preview + .entries + .iter() + .zip(details.tag_summary.iter()) + { reviewed_by.push(tag_summary.reviewed_by.clone()); tested_by.push(tag_summary.tested_by.clone()); acked_by.push(tag_summary.acked_by.clone()); - patches_preview.push(line.as_str().into_text()?); + patches_preview.push(entry.rendered_text.as_str().into_text()?); } let has_cover_letter = representative_patch.number_in_series() == 0; diff --git a/src/render/dto.rs b/src/render/dto.rs new file mode 100644 index 0000000..65666d3 --- /dev/null +++ b/src/render/dto.rs @@ -0,0 +1,44 @@ +use crate::render_prefs::{CoverRenderer, PatchRenderer}; + +/// Input needed to render all previews for a patchset. +pub struct RenderPatchsetRequest { + pub raw_patches: Vec, + pub patch_renderer: PatchRenderer, + pub cover_renderer: CoverRenderer, +} + +impl RenderPatchsetRequest { + pub fn new( + raw_patches: Vec, + patch_renderer: PatchRenderer, + cover_renderer: CoverRenderer, + ) -> Self { + Self { + raw_patches, + patch_renderer, + cover_renderer, + } + } +} + +/// Rendered preview payload for a full patchset. +pub struct RenderedPatchsetPreview { + pub entries: Vec, +} + +impl RenderedPatchsetPreview { + pub fn new(entries: Vec) -> Self { + Self { entries } + } +} + +/// Rendered preview payload for a single patch or cover letter entry. +pub struct RenderedPatchPreview { + pub rendered_text: String, +} + +impl RenderedPatchPreview { + pub fn new(rendered_text: String) -> Self { + Self { rendered_text } + } +} diff --git a/src/render/mod.rs b/src/render/mod.rs index b9215a5..7aacf1a 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -1,7 +1,9 @@ //! Rich patch/cover preview via external programs (`bat`, `delta`, `diff-so-fancy`). +pub mod dto; mod r#trait; +pub use dto::{RenderPatchsetRequest, RenderedPatchPreview, RenderedPatchsetPreview}; pub use r#trait::{RenderError, RenderServiceApi}; use std::sync::Arc; @@ -13,9 +15,6 @@ use crate::{ infrastructure::shell::ShellTrait, lore::infrastructure::patchset_parser::split_cover, }; -use crate::app::cover_renderer::CoverRenderer; -use crate::app::patch_renderer::PatchRenderer; - /// [`RenderServiceApi`] backed by [`ShellTrait`] and the existing `app` renderer helpers. pub struct ShellRenderService { shell: Arc, @@ -30,16 +29,14 @@ impl ShellRenderService { impl RenderServiceApi for ShellRenderService { fn render_patchset_preview( &self, - raw_patches: &[String], - patch_renderer: &PatchRenderer, - cover_renderer: &CoverRenderer, - ) -> Result, RenderError> { + request: RenderPatchsetRequest, + ) -> Result { let shell = self.shell.as_ref(); - let mut previews = Vec::with_capacity(raw_patches.len()); - for raw_patch in raw_patches { + let mut previews = Vec::with_capacity(request.raw_patches.len()); + for raw_patch in request.raw_patches { let raw_patch_expanded = raw_patch.replace('\t', " "); let (raw_cover, raw_diff) = split_cover(&raw_patch_expanded); - let rendered_cover = match render_cover(shell, raw_cover, cover_renderer) { + let rendered_cover = match render_cover(shell, raw_cover, &request.cover_renderer) { Ok(render) => render, Err(_) => { event!( @@ -49,19 +46,22 @@ impl RenderServiceApi for ShellRenderService { raw_cover.to_string() } }; - let rendered_patch = match render_patch_preview(shell, raw_diff, patch_renderer) { - Ok(render) => render, - Err(_) => { - event!( - Level::ERROR, - "Failed to render patch preview with external program", - ); - raw_diff.to_string() - } - }; - previews.push(format!("{rendered_cover}---\n{rendered_patch}")); + let rendered_patch = + match render_patch_preview(shell, raw_diff, &request.patch_renderer) { + Ok(render) => render, + Err(_) => { + event!( + Level::ERROR, + "Failed to render patch preview with external program", + ); + raw_diff.to_string() + } + }; + previews.push(RenderedPatchPreview::new(format!( + "{rendered_cover}---\n{rendered_patch}" + ))); } - Ok(previews) + Ok(RenderedPatchsetPreview::new(previews)) } } diff --git a/src/render/tests.rs b/src/render/tests.rs index 483536a..32f0b15 100644 --- a/src/render/tests.rs +++ b/src/render/tests.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use crate::render::{RenderServiceApi, ShellRenderService}; +use crate::render::{RenderPatchsetRequest, RenderServiceApi, ShellRenderService}; use crate::{app::cover_renderer::CoverRenderer, app::patch_renderer::PatchRenderer}; use crate::infrastructure::shell::OsShell; @@ -9,9 +9,10 @@ use crate::infrastructure::shell::OsShell; fn shell_render_service_produces_one_preview_per_patch() { let svc: Arc = Arc::new(ShellRenderService::new(Arc::new(OsShell))); let raw = vec!["subject\n\nbody\n---\n+line\n".to_string()]; + let request = RenderPatchsetRequest::new(raw, PatchRenderer::Default, CoverRenderer::Default); let out = svc - .render_patchset_preview(&raw, &PatchRenderer::Default, &CoverRenderer::Default) + .render_patchset_preview(request) .expect("default renderers should not spawn"); - assert_eq!(out.len(), 1); - assert!(out[0].contains("---")); + assert_eq!(out.entries.len(), 1); + assert!(out.entries[0].rendered_text.contains("---")); } diff --git a/src/render/trait.rs b/src/render/trait.rs index f42b9f4..ebec07a 100644 --- a/src/render/trait.rs +++ b/src/render/trait.rs @@ -1,8 +1,7 @@ use mockall::automock; use thiserror::Error; -use crate::app::cover_renderer::CoverRenderer; -use crate::app::patch_renderer::PatchRenderer; +use super::dto::{RenderPatchsetRequest, RenderedPatchsetPreview}; /// Failure while running an external preview renderer (bat, delta, etc.). /// @@ -25,8 +24,6 @@ pub trait RenderServiceApi: Send + Sync { /// `"{cover}---\\n{patch}"` per entry. fn render_patchset_preview( &self, - raw_patches: &[String], - patch_renderer: &PatchRenderer, - cover_renderer: &CoverRenderer, - ) -> Result, RenderError>; + request: RenderPatchsetRequest, + ) -> Result; } From 2a4835b56a12f7c949e0ac9394a01ea18f7b01e0 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 13:28:34 -0300 Subject: [PATCH 3/6] refactor(render): add Render actor and async handle This commit introduces RenderMessage, RenderHandle, and RenderActor around the existing synchronous render service. The actor owns the render core, receives requests over mpsc, replies over oneshot, and runs shell-backed rendering through spawn_blocking while preserving the current fallback behavior. It also adds actor-level tests for patchset preview rendering, single patch rendering, and sequential requests through the same Render actor. This commit is part of the architecture's refactoring phase 7. Signed-off-by: lorenzoberts --- src/render/actor.rs | 207 +++++++++++++++++++++++++++++++++++++++++ src/render/handle.rs | 59 ++++++++++++ src/render/messages.rs | 30 ++++++ src/render/mod.rs | 3 + src/render/trait.rs | 2 + 5 files changed, 301 insertions(+) create mode 100644 src/render/actor.rs create mode 100644 src/render/handle.rs create mode 100644 src/render/messages.rs diff --git a/src/render/actor.rs b/src/render/actor.rs new file mode 100644 index 0000000..3b9cb9f --- /dev/null +++ b/src/render/actor.rs @@ -0,0 +1,207 @@ +use tokio::{ + sync::{mpsc, oneshot}, + task, +}; + +use crate::render::{ + handle::RenderHandle, + messages::{RenderMessage, RenderResult}, + RenderError, RenderPatchsetRequest, RenderServiceApi, +}; + +pub const DEFAULT_RENDER_CHANNEL_SIZE: usize = 32; + +pub struct RenderActor { + core: Option>, + rx: mpsc::Receiver, +} + +impl RenderActor { + pub fn new(core: Box, rx: mpsc::Receiver) -> Self { + Self { + core: Some(core), + rx, + } + } + + pub fn spawn(core: Box) -> RenderHandle { + let (tx, rx) = mpsc::channel(DEFAULT_RENDER_CHANNEL_SIZE); + tracing::debug!( + channel_size = DEFAULT_RENDER_CHANNEL_SIZE, + "spawning render actor" + ); + tokio::spawn(Self::new(core, rx).run()); + RenderHandle::new(tx) + } + + pub async fn run(mut self) { + tracing::info!("render actor started"); + while let Some(message) = self.rx.recv().await { + self.handle_message(message).await; + } + tracing::info!("render actor stopped"); + } + + async fn handle_message(&mut self, message: RenderMessage) { + let message_name = message.name(); + tracing::debug!(message = message_name, "render request received"); + + match message { + RenderMessage::RenderPatchsetPreview { request, reply } => { + tracing::debug!( + patch_count = request.raw_patches.len(), + patch_renderer = %request.patch_renderer, + cover_renderer = %request.cover_renderer, + "rendering patchset preview" + ); + let result = self + .with_core(move |core| core.render_patchset_preview(request)) + .await + .and_then(|result| result); + send_render_reply(message_name, reply, result); + } + RenderMessage::RenderSinglePatch { + raw_patch, + patch_renderer, + cover_renderer, + reply, + } => { + tracing::debug!( + patch_renderer = %patch_renderer, + cover_renderer = %cover_renderer, + "rendering single patch preview" + ); + let result = self + .with_core(move |core| { + let request = RenderPatchsetRequest::new( + vec![raw_patch], + patch_renderer, + cover_renderer, + ); + core.render_patchset_preview(request) + }) + .await + .and_then(|result| result) + .and_then(|preview| { + preview.entries.into_iter().next().ok_or_else(|| { + RenderError::Failed("render returned no preview entries".to_string()) + }) + }); + send_render_reply(message_name, reply, result); + } + } + } + + async fn with_core(&mut self, operation: F) -> RenderResult + where + T: Send + 'static, + F: FnOnce(&dyn RenderServiceApi) -> T + Send + 'static, + { + let core = self + .core + .take() + .ok_or_else(|| RenderError::ActorUnavailable("core unavailable".to_string()))?; + let (core, result) = task::spawn_blocking(move || { + let result = operation(core.as_ref()); + (core, result) + }) + .await + .map_err(|e| RenderError::ActorUnavailable(e.to_string()))?; + self.core = Some(core); + Ok(result) + } +} + +fn send_render_reply( + message_name: &'static str, + reply: oneshot::Sender>, + result: RenderResult, +) { + if let Err(error) = &result { + tracing::warn!( + message = message_name, + error = %error, + "render request failed" + ); + } + + if reply.send(result).is_err() { + tracing::warn!( + message = message_name, + "render reply receiver dropped before response" + ); + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use crate::{ + infrastructure::shell::OsShell, + render::{RenderPatchsetRequest, ShellRenderService}, + render_prefs::{CoverRenderer, PatchRenderer}, + }; + + use super::*; + + fn spawn_test_actor() -> RenderHandle { + RenderActor::spawn(Box::new(ShellRenderService::new(Arc::new(OsShell)))) + } + + #[tokio::test] + async fn render_patchset_preview_returns_one_entry_per_patch() { + let handle = spawn_test_actor(); + let request = RenderPatchsetRequest::new( + vec![ + "subject\n\nbody\n---\n+line\n".to_string(), + "other\n\nbody\n---\n-line\n".to_string(), + ], + PatchRenderer::Default, + CoverRenderer::Default, + ); + + let rendered = handle + .render_patchset_preview(request) + .await + .expect("default renderers should not spawn"); + + assert_eq!(rendered.entries.len(), 2); + assert!(rendered.entries[0].rendered_text.contains("+line")); + assert!(rendered.entries[1].rendered_text.contains("-line")); + } + + #[tokio::test] + async fn render_single_patch_returns_first_preview_entry() { + let handle = spawn_test_actor(); + + let rendered = handle + .render_single_patch( + "subject\n\nbody\n---\n+line\n".to_string(), + PatchRenderer::Default, + CoverRenderer::Default, + ) + .await + .expect("default renderers should not spawn"); + + assert!(rendered.rendered_text.contains("+line")); + } + + #[tokio::test] + async fn render_actor_handles_sequential_requests() { + let handle = spawn_test_actor(); + + for content in ["+first", "+second"] { + let rendered = handle + .render_single_patch( + format!("subject\n\nbody\n---\n{content}\n"), + PatchRenderer::Default, + CoverRenderer::Default, + ) + .await + .expect("default renderers should not spawn"); + + assert!(rendered.rendered_text.contains(content)); + } + } +} diff --git a/src/render/handle.rs b/src/render/handle.rs new file mode 100644 index 0000000..9459425 --- /dev/null +++ b/src/render/handle.rs @@ -0,0 +1,59 @@ +use tokio::sync::{mpsc, oneshot}; + +use crate::{ + render::{ + messages::{RenderMessage, RenderResult}, + RenderError, RenderPatchsetRequest, RenderedPatchPreview, RenderedPatchsetPreview, + }, + render_prefs::{CoverRenderer, PatchRenderer}, +}; + +#[derive(Clone)] +pub struct RenderHandle { + tx: mpsc::Sender, +} + +impl RenderHandle { + pub fn new(tx: mpsc::Sender) -> Self { + Self { tx } + } + + pub async fn render_patchset_preview( + &self, + request: RenderPatchsetRequest, + ) -> RenderResult { + self.request_result(|reply| RenderMessage::RenderPatchsetPreview { request, reply }) + .await + } + + pub async fn render_single_patch( + &self, + raw_patch: String, + patch_renderer: PatchRenderer, + cover_renderer: CoverRenderer, + ) -> RenderResult { + self.request_result(|reply| RenderMessage::RenderSinglePatch { + raw_patch, + patch_renderer, + cover_renderer, + reply, + }) + .await + } + + async fn request_result( + &self, + build_message: impl FnOnce(oneshot::Sender>) -> RenderMessage, + ) -> RenderResult + where + T: Send + 'static, + { + let (reply, rx) = oneshot::channel(); + self.tx + .send(build_message(reply)) + .await + .map_err(|_| RenderError::ActorUnavailable("request channel closed".to_string()))?; + rx.await + .map_err(|_| RenderError::ActorUnavailable("reply channel closed".to_string()))? + } +} diff --git a/src/render/messages.rs b/src/render/messages.rs new file mode 100644 index 0000000..ee9954a --- /dev/null +++ b/src/render/messages.rs @@ -0,0 +1,30 @@ +use tokio::sync::oneshot; + +use crate::{ + render::{RenderError, RenderPatchsetRequest, RenderedPatchPreview, RenderedPatchsetPreview}, + render_prefs::{CoverRenderer, PatchRenderer}, +}; + +pub type RenderResult = Result; + +pub enum RenderMessage { + RenderPatchsetPreview { + request: RenderPatchsetRequest, + reply: oneshot::Sender>, + }, + RenderSinglePatch { + raw_patch: String, + patch_renderer: PatchRenderer, + cover_renderer: CoverRenderer, + reply: oneshot::Sender>, + }, +} + +impl RenderMessage { + pub fn name(&self) -> &'static str { + match self { + RenderMessage::RenderPatchsetPreview { .. } => "RenderPatchsetPreview", + RenderMessage::RenderSinglePatch { .. } => "RenderSinglePatch", + } + } +} diff --git a/src/render/mod.rs b/src/render/mod.rs index 7aacf1a..ca9d6e1 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -1,6 +1,9 @@ //! Rich patch/cover preview via external programs (`bat`, `delta`, `diff-so-fancy`). +pub mod actor; pub mod dto; +pub mod handle; +pub mod messages; mod r#trait; pub use dto::{RenderPatchsetRequest, RenderedPatchPreview, RenderedPatchsetPreview}; diff --git a/src/render/trait.rs b/src/render/trait.rs index ebec07a..7b07ac2 100644 --- a/src/render/trait.rs +++ b/src/render/trait.rs @@ -12,6 +12,8 @@ use super::dto::{RenderPatchsetRequest, RenderedPatchsetPreview}; pub enum RenderError { #[error("render failed: {0}")] Failed(String), + #[error("render actor unavailable: {0}")] + ActorUnavailable(String), } /// Abstraction for rich-text patch/cover preview (shell-backed renderers). From 1887ff57f80766e3aff56da28e3860195c452d42 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 13:44:07 -0300 Subject: [PATCH 4/6] refactor(render): route App previews through Render actor This commit wires App to RenderHandle for patchset preview rendering and starts RenderActor during application startup. Patchset details still build the same preview state, but the shell-backed rendering work now goes through the Render actor instead of a synchronous App service dependency. This commit is part of the architecture's refactoring phase 7. Signed-off-by: lorenzoberts --- src/app/mod.rs | 7 ++++--- src/main.rs | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 48d5660..030c556 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -32,7 +32,7 @@ use crate::{ }, domain::patch::{Author, Patch}, }, - render::{RenderPatchsetRequest, RenderServiceApi}, + render::{handle::RenderHandle, RenderPatchsetRequest}, ui::popup::info_popup::InfoPopUp, }; use screens::{ @@ -49,7 +49,7 @@ pub use view_model::AppViewModel; /// Injected capabilities used by `App` orchestration (not screen state). pub struct AppServices { pub lore_api: LoreApiHandle, - pub render: Box, + pub render: RenderHandle, pub shell: Box, pub fs: Box, pub env: Box, @@ -82,7 +82,7 @@ impl App { shell: Box, env: Box, lore_api: LoreApiHandle, - render: Box, + render: RenderHandle, ) -> color_eyre::Result { let config = config_service.snapshot(); @@ -228,6 +228,7 @@ impl App { .services .render .render_patchset_preview(render_request) + .await .map_err(|e| eyre!("{e}"))?; let mut patches_preview: Vec = Vec::new(); diff --git a/src/main.rs b/src/main.rs index 5575d4c..5cdba04 100644 --- a/src/main.rs +++ b/src/main.rs @@ -32,7 +32,7 @@ use lore::{ persistence::{FileLorePersistence, MailingListsCacheStore, UserLoreStateStore}, }, }; -use render::{RenderServiceApi, ShellRenderService}; +use render::{actor::RenderActor, ShellRenderService}; use std::{ops::ControlFlow, sync::Arc}; use tracing::{event, Level}; @@ -136,7 +136,7 @@ async fn main() -> color_eyre::Result<()> { )); let parser = Arc::new(MboxPatchsetParser::new(fs_arc.clone())); - let render: Box = Box::new(ShellRenderService::new(shell_arc.clone())); + let render = RenderActor::spawn(Box::new(ShellRenderService::new(shell_arc.clone()))); let lore_api = LoreApiActor::spawn(LoreService::new( gateway.clone(), From 428ef08f903a35e61ea04c0407640ea4442bdc11 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 13:45:22 -0300 Subject: [PATCH 5/6] refactor(app): centralize patchset details state construction This commit moves patchset details state assembly into PatchsetDetailsState::from_rendered_preview. App::open_patchset_details now focuses on orchestrating Lore and Render actor calls while the details screen state owns conversion from rendered preview strings into the existing Ratatui text payloads. This commit is part of the architecture's refactoring phase 7. Signed-off-by: lorenzoberts --- src/app/mod.rs | 54 +++++----------------------- src/app/screens/details_actions.rs | 56 +++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 47 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 030c556..7032b55 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -6,15 +6,10 @@ pub mod screens; pub mod state; pub mod view_model; -use ansi_to_tui::IntoText; use color_eyre::eyre::{bail, eyre}; -use ratatui::text::Text; use tracing::{event, Level}; -use std::{ - collections::{HashMap, HashSet}, - path::PathBuf, -}; +use std::path::PathBuf; use crate::{ config::ConfigServiceApi, @@ -30,7 +25,7 @@ use crate::{ errors::LoreError, handle::LoreApiHandle, }, - domain::patch::{Author, Patch}, + domain::patch::Patch, }, render::{handle::RenderHandle, RenderPatchsetRequest}, ui::popup::info_popup::InfoPopUp, @@ -231,46 +226,13 @@ impl App { .await .map_err(|e| eyre!("{e}"))?; - let mut patches_preview: Vec = Vec::new(); - let mut reviewed_by: Vec> = Vec::new(); - let mut tested_by: Vec> = Vec::new(); - let mut acked_by: Vec> = Vec::new(); - - for (entry, tag_summary) in rendered_preview - .entries - .iter() - .zip(details.tag_summary.iter()) - { - reviewed_by.push(tag_summary.reviewed_by.clone()); - tested_by.push(tag_summary.tested_by.clone()); - acked_by.push(tag_summary.acked_by.clone()); - patches_preview.push(entry.rendered_text.as_str().into_text()?); - } - - let has_cover_letter = representative_patch.number_in_series() == 0; - let patches_to_reply = vec![false; details.raw_patches.len()]; - - self.state.lore.details = Some(PatchsetDetailsState { + self.state.lore.details = Some(PatchsetDetailsState::from_rendered_preview( representative_patch, - raw_patches: details.raw_patches, - patchset_path: details.patchset_path, - patches_preview, - patches_to_reply, - has_cover_letter, - preview_index: 0, - preview_scroll_offset: 0, - preview_pan: 0, - preview_fullscreen: false, - patchset_actions: HashMap::from([ - (PatchsetAction::Bookmark, is_patchset_bookmarked), - (PatchsetAction::ReplyWithReviewedBy, false), - (PatchsetAction::Apply, false), - ]), - reviewed_by, - tested_by, - acked_by, - last_screen: self.state.navigation.current_screen.clone(), - }); + details, + rendered_preview, + is_patchset_bookmarked, + self.state.navigation.current_screen.clone(), + )?); Ok(B4Result::PatchFound) } diff --git a/src/app/screens/details_actions.rs b/src/app/screens/details_actions.rs index 75db551..c0ccc04 100644 --- a/src/app/screens/details_actions.rs +++ b/src/app/screens/details_actions.rs @@ -1,3 +1,4 @@ +use ansi_to_tui::IntoText; use ratatui::text::Text; use std::collections::{HashMap, HashSet}; @@ -9,7 +10,11 @@ use crate::{ file_system::FileSystemTrait, shell::{ShellCommand, ShellTrait}, }, - lore::domain::patch::{Author, Patch}, + lore::{ + application::dto::PatchsetDetails, + domain::patch::{Author, Patch}, + }, + render::RenderedPatchsetPreview, }; use super::CurrentScreen; @@ -53,6 +58,55 @@ pub enum PatchsetAction { } impl PatchsetDetailsState { + pub fn from_rendered_preview( + representative_patch: Patch, + details: PatchsetDetails, + rendered_preview: RenderedPatchsetPreview, + is_patchset_bookmarked: bool, + last_screen: CurrentScreen, + ) -> color_eyre::Result { + let mut patches_preview: Vec = Vec::new(); + let mut reviewed_by: Vec> = Vec::new(); + let mut tested_by: Vec> = Vec::new(); + let mut acked_by: Vec> = Vec::new(); + + for (entry, tag_summary) in rendered_preview + .entries + .iter() + .zip(details.tag_summary.iter()) + { + reviewed_by.push(tag_summary.reviewed_by.clone()); + tested_by.push(tag_summary.tested_by.clone()); + acked_by.push(tag_summary.acked_by.clone()); + patches_preview.push(entry.rendered_text.as_str().into_text()?); + } + + let has_cover_letter = representative_patch.number_in_series() == 0; + let patches_to_reply = vec![false; details.raw_patches.len()]; + + Ok(Self { + representative_patch, + raw_patches: details.raw_patches, + patchset_path: details.patchset_path, + patches_preview, + patches_to_reply, + has_cover_letter, + preview_index: 0, + preview_scroll_offset: 0, + preview_pan: 0, + preview_fullscreen: false, + patchset_actions: HashMap::from([ + (PatchsetAction::Bookmark, is_patchset_bookmarked), + (PatchsetAction::ReplyWithReviewedBy, false), + (PatchsetAction::Apply, false), + ]), + reviewed_by, + tested_by, + acked_by, + last_screen, + }) + } + pub fn preview_next_patch(&mut self) { if (self.preview_index + 1) < self.patches_preview.len() { self.preview_index += 1; From 1868807790bd698e856b7fb934a0052adb315023 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 13:49:36 -0300 Subject: [PATCH 6/6] refactor(render): move renderer helpers into Render context This commit moves the patch and cover renderer helpers out of App and into the Render context. Render no longer depends on App modules for preview rendering, and call sites now import renderer preferences from render_prefs directly. This commit completes the architecture's refactoring phase 7. Signed-off-by: lorenzoberts --- src/app/mod.rs | 2 -- src/main.rs | 3 ++- src/{app => render}/cover_renderer.rs | 6 ++++-- src/render/handle.rs | 2 ++ src/render/messages.rs | 2 ++ src/render/mod.rs | 7 +++++-- src/{app => render}/patch_renderer.rs | 6 ++++-- src/render/tests.rs | 2 +- 8 files changed, 20 insertions(+), 10 deletions(-) rename src/{app => render}/cover_renderer.rs (89%) rename src/{app => render}/patch_renderer.rs (96%) diff --git a/src/app/mod.rs b/src/app/mod.rs index 7032b55..ea4e5e8 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1,7 +1,5 @@ pub mod commands; -pub mod cover_renderer; pub mod errors; -pub mod patch_renderer; pub mod screens; pub mod state; pub mod view_model; diff --git a/src/main.rs b/src/main.rs index 5cdba04..44299bc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,7 +9,7 @@ mod render; mod render_prefs; mod ui; -use app::{patch_renderer::PatchRenderer, App}; +use app::App; use clap::Parser; use cli::Cli; use color_eyre::eyre::{bail, eyre}; @@ -33,6 +33,7 @@ use lore::{ }, }; use render::{actor::RenderActor, ShellRenderService}; +use render_prefs::PatchRenderer; use std::{ops::ControlFlow, sync::Arc}; use tracing::{event, Level}; diff --git a/src/app/cover_renderer.rs b/src/render/cover_renderer.rs similarity index 89% rename from src/app/cover_renderer.rs rename to src/render/cover_renderer.rs index 4cb8e0e..e136c70 100644 --- a/src/app/cover_renderer.rs +++ b/src/render/cover_renderer.rs @@ -2,8 +2,10 @@ use tracing::{event, Level}; use color_eyre::eyre::eyre; -use crate::infrastructure::shell::{ShellCommand, ShellTrait}; -pub use crate::render_prefs::CoverRenderer; +use crate::{ + infrastructure::shell::{ShellCommand, ShellTrait}, + render_prefs::CoverRenderer, +}; pub fn render_cover( shell: &dyn ShellTrait, diff --git a/src/render/handle.rs b/src/render/handle.rs index 9459425..46038c7 100644 --- a/src/render/handle.rs +++ b/src/render/handle.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] // Some protocol methods are reserved for future App flows. + use tokio::sync::{mpsc, oneshot}; use crate::{ diff --git a/src/render/messages.rs b/src/render/messages.rs index ee9954a..c06a0ee 100644 --- a/src/render/messages.rs +++ b/src/render/messages.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] // Follow-up phases may wire additional render message variants. + use tokio::sync::oneshot; use crate::{ diff --git a/src/render/mod.rs b/src/render/mod.rs index ca9d6e1..383ea52 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -1,9 +1,11 @@ //! Rich patch/cover preview via external programs (`bat`, `delta`, `diff-so-fancy`). pub mod actor; +mod cover_renderer; pub mod dto; pub mod handle; pub mod messages; +mod patch_renderer; mod r#trait; pub use dto::{RenderPatchsetRequest, RenderedPatchPreview, RenderedPatchsetPreview}; @@ -14,8 +16,9 @@ use std::sync::Arc; use tracing::{event, Level}; use crate::{ - app::cover_renderer::render_cover, app::patch_renderer::render_patch_preview, - infrastructure::shell::ShellTrait, lore::infrastructure::patchset_parser::split_cover, + infrastructure::shell::ShellTrait, + lore::infrastructure::patchset_parser::split_cover, + render::{cover_renderer::render_cover, patch_renderer::render_patch_preview}, }; /// [`RenderServiceApi`] backed by [`ShellTrait`] and the existing `app` renderer helpers. diff --git a/src/app/patch_renderer.rs b/src/render/patch_renderer.rs similarity index 96% rename from src/app/patch_renderer.rs rename to src/render/patch_renderer.rs index 4ee2db5..a43ccee 100644 --- a/src/app/patch_renderer.rs +++ b/src/render/patch_renderer.rs @@ -1,8 +1,10 @@ use color_eyre::eyre::eyre; use tracing::{event, Level}; -use crate::infrastructure::shell::{ShellCommand, ShellTrait}; -pub use crate::render_prefs::PatchRenderer; +use crate::{ + infrastructure::shell::{ShellCommand, ShellTrait}, + render_prefs::PatchRenderer, +}; /// Cleans patch contents before rendering for preview. Currently, it only trims /// the trailing signature delimiter (the `--` at the end of the patch) if it diff --git a/src/render/tests.rs b/src/render/tests.rs index 32f0b15..164bae4 100644 --- a/src/render/tests.rs +++ b/src/render/tests.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use crate::render::{RenderPatchsetRequest, RenderServiceApi, ShellRenderService}; -use crate::{app::cover_renderer::CoverRenderer, app::patch_renderer::PatchRenderer}; +use crate::render_prefs::{CoverRenderer, PatchRenderer}; use crate::infrastructure::shell::OsShell;