diff --git a/src/app/actions/apply.rs b/src/app/actions/apply.rs new file mode 100644 index 0000000..6db047b --- /dev/null +++ b/src/app/actions/apply.rs @@ -0,0 +1,468 @@ +use std::path::Path; + +use chrono::Utc; + +use crate::{ + config::{ConfigSnapshot, KernelTree}, + infrastructure::{ + file_system::FileSystemTrait, + shell::{ShellCommand, ShellTrait}, + }, +}; + +pub(crate) struct ApplyPatchsetRequest { + pub patch_title: String, + pub patchset_path: String, +} + +pub(crate) fn apply_patchset( + request: &ApplyPatchsetRequest, + fs: &dyn FileSystemTrait, + shell: &dyn ShellTrait, + config: &ConfigSnapshot, +) -> Result { + let kernel_tree = validate_kernel_tree(fs, config)?; + check_git_state(fs, shell, kernel_tree)?; + + let original_branch = get_current_branch(shell, kernel_tree)?; + let target_branch = create_target_branch(shell, kernel_tree, config)?; + + let git_am_result = run_git_am(request, shell, kernel_tree, config); + switch_to_branch(shell, kernel_tree, &original_branch)?; + + match git_am_result { + Ok(_) => Ok(format!( + " Patchset '{}' applied successfully!\n\n - Kernel Tree: '{}'\n\n - Base Branch: '{}'\n\n - Applied branch: '{}'", + request.patch_title, + kernel_tree.path(), + kernel_tree.branch(), + &target_branch + )), + Err(e) => Err(format!(" `git am` failed\n{}{}", &original_branch, e)), + } +} + +fn validate_kernel_tree<'a>( + fs: &dyn FileSystemTrait, + config: &'a ConfigSnapshot, +) -> Result<&'a KernelTree, String> { + let kernel_tree_id = if let Some(target) = config.target_kernel_tree() { + target + } else { + return Err("target kernel tree unset".to_string()); + }; + + let kernel_tree = if let Some(tree) = config.get_kernel_tree(kernel_tree_id) { + tree + } else { + return Err(format!("invalid target kernel tree '{kernel_tree_id}'")); + }; + + let kernel_tree_path = Path::new(kernel_tree.path()); + if !fs.is_dir(kernel_tree_path) { + return Err(format!("{} isn't a directory", kernel_tree.path())); + } else if !fs.is_dir(&kernel_tree_path.join(".git")) { + return Err(format!("{} isn't a git repository", kernel_tree.path())); + } + + Ok(kernel_tree) +} + +fn check_git_state( + fs: &dyn FileSystemTrait, + shell: &dyn ShellTrait, + kernel_tree: &KernelTree, +) -> Result<(), String> { + let kernel_tree_path = Path::new(kernel_tree.path()); + + if fs.is_dir(&kernel_tree_path.join(".git/rebase-merge")) { + return Err("rebase in progress. \nrun `git rebase --abort` before continuing".to_string()); + } else if fs.is_file(&kernel_tree_path.join(".git/MERGE_HEAD")) { + return Err("merge in progress. \nrun `git merge --abort` before continuing".to_string()); + } else if fs.is_file(&kernel_tree_path.join(".git/BISECT_LOG")) { + return Err("bisect in progress. \nrun `git bisect reset` before continuing".to_string()); + } else if fs.is_dir(&kernel_tree_path.join(".git/rebase-apply")) { + return Err( + "`git am` already in progress. \nrun `git am --abort` before continuing".to_string(), + ); + } + + let status_out = shell + .execute( + &ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["status", "--porcelain"]), + ) + .map_err(|e| format!("failed to check git status {e}"))?; + + let status_output = String::from_utf8_lossy(&status_out.stdout); + if !status_output.is_empty() { + return Err(format!( + "there are staged and/or unstaged changes\n{status_output}" + )); + } + + let show_ref_out = shell + .execute( + &ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["show-ref", "--verify", "--quiet"]) + .arg(format!("refs/heads/{}", kernel_tree.branch())), + ) + .map_err(|e| format!("failed to verify branch: {e}"))?; + + if !show_ref_out.success { + return Err(format!( + "invalid branch '{}' for '{}'", + kernel_tree.branch(), + kernel_tree.path() + )); + } + + Ok(()) +} + +fn get_current_branch(shell: &dyn ShellTrait, kernel_tree: &KernelTree) -> Result { + let out = shell + .execute( + &ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["rev-parse", "--abbrev-ref", "HEAD"]), + ) + .map_err(|e| format!("failed to get current branch: {e}"))?; + + let mut branch = String::from_utf8_lossy(&out.stdout).to_string(); + branch.pop(); + Ok(branch) +} + +fn switch_to_branch( + shell: &dyn ShellTrait, + kernel_tree: &KernelTree, + branch: &str, +) -> Result<(), String> { + let out = shell + .execute( + &ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["switch", branch]), + ) + .map_err(|e| format!("failed to switch branch: {e}"))?; + + if !out.success { + return Err(format!( + "failed to switch to branch '{}': {}", + branch, + String::from_utf8_lossy(&out.stderr) + )); + } + + Ok(()) +} + +fn create_target_branch( + shell: &dyn ShellTrait, + kernel_tree: &KernelTree, + config: &ConfigSnapshot, +) -> Result { + switch_to_branch(shell, kernel_tree, kernel_tree.branch())?; + + let target_branch_name = format!( + "{}{}", + config.git_am_branch_prefix(), + Utc::now().format("%Y-%m-%d-%H-%M-%S") + ); + + let out = shell + .execute( + &ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["checkout", "-b", &target_branch_name]), + ) + .map_err(|e| format!("failed to create target branch: {e}"))?; + + if !out.success { + return Err(format!( + "failed to create branch '{}': {}", + target_branch_name, + String::from_utf8_lossy(&out.stderr) + )); + } + + Ok(target_branch_name) +} + +fn run_git_am( + request: &ApplyPatchsetRequest, + shell: &dyn ShellTrait, + kernel_tree: &KernelTree, + config: &ConfigSnapshot, +) -> Result<(), String> { + let mut git_am_cmd = ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["am", &request.patchset_path]); + for opt in config.git_am_options().split_whitespace() { + git_am_cmd = git_am_cmd.arg(opt); + } + + let out = shell + .execute(&git_am_cmd) + .map_err(|e| format!("failed to execute git-am: {e}"))?; + + if !out.success { + let _ = shell.execute( + &ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["am", "--abort"]), + ); + + return Err(String::from_utf8_lossy(&out.stderr).to_string()); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::{ + collections::VecDeque, + sync::{Arc, Mutex}, + }; + + use crate::{ + config::{ConfigSnapshot, ConfigState}, + infrastructure::{ + file_system::MockFileSystemTrait, + shell::{MockShellTrait, ShellCommand, ShellOutput}, + }, + }; + + use super::*; + + const KERNEL_TREE_PATH: &str = "/kernel"; + const BASE_BRANCH: &str = "main"; + const PATCHSET_PATH: &str = "/tmp/patchset.mbx"; + + fn config() -> ConfigSnapshot { + serde_json::from_value::(serde_json::json!({ + "kernel_trees": { + "linux": { + "path": KERNEL_TREE_PATH, + "branch": BASE_BRANCH + } + }, + "target_kernel_tree": "linux", + "git_am_options": "--signoff --3way", + "git_am_branch_prefix": "patchset-" + })) + .expect("test config should deserialize") + .to_snapshot() + } + + fn config_without_target() -> ConfigSnapshot { + ConfigState::default().to_snapshot() + } + + fn request() -> ApplyPatchsetRequest { + ApplyPatchsetRequest { + patch_title: "[PATCH] test".to_string(), + patchset_path: PATCHSET_PATH.to_string(), + } + } + + fn output( + stdout: impl Into>, + stderr: impl Into>, + success: bool, + ) -> ShellOutput { + ShellOutput { + stdout: stdout.into(), + stderr: stderr.into(), + success, + } + } + + fn clean_fs() -> MockFileSystemTrait { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_dir() + .returning(|path| matches!(path.to_str(), Some("/kernel") | Some("/kernel/.git"))); + fs.expect_is_file().returning(|_| false); + fs + } + + fn shell_with_outputs( + outputs: Vec, + ) -> (MockShellTrait, Arc>>>) { + let calls = Arc::new(Mutex::new(Vec::new())); + let outputs = Arc::new(Mutex::new(VecDeque::from(outputs))); + let mut shell = MockShellTrait::new(); + let calls_for_execute = Arc::clone(&calls); + let outputs_for_execute = Arc::clone(&outputs); + shell.expect_execute().returning(move |cmd| { + calls_for_execute.lock().unwrap().push(command_parts(cmd)); + Ok(outputs_for_execute + .lock() + .unwrap() + .pop_front() + .expect("test should provide one output per shell command")) + }); + (shell, calls) + } + + fn command_parts(cmd: &ShellCommand) -> Vec { + let mut parts = vec![cmd.program.clone()]; + parts.extend(cmd.args.clone()); + parts + } + + fn command(parts: &[&str]) -> Vec { + parts.iter().map(|part| part.to_string()).collect() + } + + #[test] + fn apply_success_runs_expected_git_sequence() { + let fs = clean_fs(); + let (shell, calls) = shell_with_outputs(vec![ + output("", "", true), + output("", "", true), + output("feature\n", "", true), + output("", "", true), + output("", "", true), + output("", "", true), + output("", "", true), + ]); + + let result = apply_patchset(&request(), &fs, &shell, &config()).unwrap(); + + assert!(result.contains("Patchset '[PATCH] test' applied successfully")); + assert!(result.contains("Applied branch: 'patchset-")); + let calls = calls.lock().unwrap(); + assert_eq!( + &calls[0], + &command(&["git", "-C", KERNEL_TREE_PATH, "status", "--porcelain"]) + ); + assert_eq!( + &calls[1], + &command(&[ + "git", + "-C", + KERNEL_TREE_PATH, + "show-ref", + "--verify", + "--quiet", + "refs/heads/main" + ]) + ); + assert_eq!( + &calls[2], + &command(&[ + "git", + "-C", + KERNEL_TREE_PATH, + "rev-parse", + "--abbrev-ref", + "HEAD" + ]) + ); + assert_eq!( + &calls[3], + &command(&["git", "-C", KERNEL_TREE_PATH, "switch", BASE_BRANCH]) + ); + assert_eq!( + &calls[4][0..5], + command(&["git", "-C", KERNEL_TREE_PATH, "checkout", "-b"]).as_slice() + ); + assert!(calls[4][5].starts_with("patchset-")); + assert_eq!( + &calls[5], + &command(&[ + "git", + "-C", + KERNEL_TREE_PATH, + "am", + PATCHSET_PATH, + "--signoff", + "--3way" + ]) + ); + assert_eq!( + &calls[6], + &command(&["git", "-C", KERNEL_TREE_PATH, "switch", "feature"]) + ); + } + + #[test] + fn dirty_worktree_rejects_before_branch_creation() { + let fs = clean_fs(); + let (shell, calls) = shell_with_outputs(vec![output(" M file.rs\n", "", true)]); + + let result = apply_patchset(&request(), &fs, &shell, &config()).unwrap_err(); + + assert!(result.contains("there are staged and/or unstaged changes")); + assert_eq!(1, calls.lock().unwrap().len()); + } + + #[test] + fn missing_target_kernel_tree_rejects_before_shell_commands() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_dir().times(0); + fs.expect_is_file().times(0); + let mut shell = MockShellTrait::new(); + shell.expect_execute().times(0); + + let result = apply_patchset(&request(), &fs, &shell, &config_without_target()).unwrap_err(); + + assert_eq!("target kernel tree unset", result); + } + + #[test] + fn invalid_configured_branch_rejects_before_branch_creation() { + let fs = clean_fs(); + let (shell, calls) = shell_with_outputs(vec![ + output("", "", true), + output("", "missing branch", false), + ]); + + let result = apply_patchset(&request(), &fs, &shell, &config()).unwrap_err(); + + assert!(result.contains("invalid branch 'main'")); + assert_eq!(2, calls.lock().unwrap().len()); + } + + #[test] + fn failed_git_am_aborts_and_switches_back() { + let fs = clean_fs(); + let (shell, calls) = shell_with_outputs(vec![ + output("", "", true), + output("", "", true), + output("feature\n", "", true), + output("", "", true), + output("", "", true), + output("", "apply failed", false), + output("", "", true), + output("", "", true), + ]); + + let result = apply_patchset(&request(), &fs, &shell, &config()).unwrap_err(); + + assert!(result.contains("`git am` failed")); + assert!(result.contains("feature")); + assert!(result.contains("apply failed")); + let calls = calls.lock().unwrap(); + assert_eq!( + &calls[6], + &command(&["git", "-C", KERNEL_TREE_PATH, "am", "--abort"]) + ); + assert_eq!( + &calls[7], + &command(&["git", "-C", KERNEL_TREE_PATH, "switch", "feature"]) + ); + } +} diff --git a/src/app/actions/mod.rs b/src/app/actions/mod.rs new file mode 100644 index 0000000..82d0967 --- /dev/null +++ b/src/app/actions/mod.rs @@ -0,0 +1,48 @@ +pub(crate) mod apply; +pub(crate) mod reviewed_reply; + +use color_eyre::Result; + +use crate::{ + config::ConfigSnapshot, + infrastructure::{file_system::FileSystemTrait, shell::ShellTrait}, + lore::application::handle::LoreApiHandle, +}; + +use apply::ApplyPatchsetRequest; +use reviewed_reply::{ReviewedReplyRequest, ReviewedReplyResult}; + +pub(crate) struct PatchsetActionService<'a> { + fs: &'a dyn FileSystemTrait, + shell: &'a dyn ShellTrait, + lore_api: &'a LoreApiHandle, +} + +impl<'a> PatchsetActionService<'a> { + pub(crate) fn new( + fs: &'a dyn FileSystemTrait, + shell: &'a dyn ShellTrait, + lore_api: &'a LoreApiHandle, + ) -> Self { + Self { + fs, + shell, + lore_api, + } + } + + pub(crate) fn apply_patchset( + &self, + request: &ApplyPatchsetRequest, + config: &ConfigSnapshot, + ) -> Result { + apply::apply_patchset(request, self.fs, self.shell, config) + } + + pub(crate) async fn execute_reviewed_reply( + &self, + request: ReviewedReplyRequest, + ) -> Result { + reviewed_reply::execute_reviewed_reply(request, self.lore_api, self.shell).await + } +} diff --git a/src/app/actions/reviewed_reply.rs b/src/app/actions/reviewed_reply.rs new file mode 100644 index 0000000..25a7ba7 --- /dev/null +++ b/src/app/actions/reviewed_reply.rs @@ -0,0 +1,207 @@ +use std::{collections::HashSet, path::PathBuf, str}; + +use color_eyre::{eyre::eyre, Result}; + +use crate::{ + infrastructure::shell::{ShellCommand, ShellTrait}, + lore::application::handle::LoreApiHandle, +}; + +pub(crate) struct ReviewedReplyRequest { + pub raw_patches: Vec, + pub patches_to_reply: Vec, + pub successful_indexes: HashSet, + pub git_send_email_options: String, +} + +pub(crate) enum ReviewedReplyResult { + NoAction { successful_indexes: HashSet }, + MissingGitIdentity { successful_indexes: HashSet }, + Completed { successful_indexes: HashSet }, +} + +impl ReviewedReplyResult { + pub(crate) fn into_successful_indexes(self) -> HashSet { + match self { + ReviewedReplyResult::NoAction { successful_indexes } + | ReviewedReplyResult::MissingGitIdentity { successful_indexes } + | ReviewedReplyResult::Completed { successful_indexes } => successful_indexes, + } + } +} + +pub(crate) async fn execute_reviewed_reply( + request: ReviewedReplyRequest, + lore_api: &LoreApiHandle, + shell: &dyn ShellTrait, +) -> Result { + if !request.patches_to_reply.contains(&true) { + return Ok(ReviewedReplyResult::NoAction { + successful_indexes: request.successful_indexes, + }); + } + + let (git_user_name, git_user_email) = lore_api + .get_git_signature(String::new()) + .await + .map_err(|e| eyre!("{e:#?}"))?; + + let mut successful_indexes = request.successful_indexes; + let Some(git_signature) = git_signature(git_user_name, git_user_email) else { + println!("`git config user.name` or `git config user.email` not set\nAborting..."); + return Ok(ReviewedReplyResult::MissingGitIdentity { successful_indexes }); + }; + + let mktemp_cmd = ShellCommand::new("mktemp").arg("--directory"); + let tmp_out = shell + .execute(&mktemp_cmd) + .map_err(|e| eyre!("failed to create temp directory: {}", e))?; + let tmp_dir_str = str::from_utf8(&tmp_out.stdout) + .map_err(|e| eyre!("invalid utf-8 in temp dir path: {}", e))? + .trim() + .to_string(); + let tmp_dir = PathBuf::from(tmp_dir_str); + + let git_reply_commands = lore_api + .prepare_reply_commands( + tmp_dir, + "all".to_string(), + request.raw_patches, + request.patches_to_reply.clone(), + git_signature, + request.git_send_email_options, + ) + .await + .map_err(|e| eyre!("{e:#?}"))?; + + record_successful_reply_indexes( + shell, + &mut successful_indexes, + &request.patches_to_reply, + git_reply_commands, + ); + + Ok(ReviewedReplyResult::Completed { successful_indexes }) +} + +fn record_successful_reply_indexes( + shell: &dyn ShellTrait, + successful_indexes: &mut HashSet, + patches_to_reply: &[bool], + git_reply_commands: Vec, +) { + let reply_indexes: Vec = selected_reply_indexes(patches_to_reply); + for (i, command) in git_reply_commands.into_iter().enumerate() { + let success = shell.spawn_interactive(&command).unwrap_or(false); + if success { + successful_indexes.insert(reply_indexes[i]); + } + } +} + +fn selected_reply_indexes(patches_to_reply: &[bool]) -> Vec { + patches_to_reply + .iter() + .enumerate() + .filter_map(|(i, &val)| if val { Some(i) } else { None }) + .collect() +} + +fn git_signature(git_user_name: String, git_user_email: String) -> Option { + if git_user_name.is_empty() || git_user_email.is_empty() { + None + } else { + Some(format!("{git_user_name} <{git_user_email}>")) + } +} + +#[cfg(test)] +mod tests { + use std::{collections::HashSet, io}; + + use crate::infrastructure::shell::{MockShellTrait, ShellError}; + + use super::*; + + fn command(name: &str) -> ShellCommand { + ShellCommand::new("git").arg("send-email").arg(name) + } + + #[test] + fn selected_reply_indexes_preserves_original_patch_indexes() { + let indexes = selected_reply_indexes(&[false, true, false, true]); + + assert_eq!(vec![1, 3], indexes); + } + + #[test] + fn git_signature_requires_name_and_email() { + assert_eq!( + Some("User ".to_string()), + git_signature("User".to_string(), "user@example.com".to_string()) + ); + assert_eq!( + None, + git_signature(String::new(), "user@example.com".to_string()) + ); + assert_eq!(None, git_signature("User".to_string(), String::new())); + } + + #[test] + fn record_successful_reply_indexes_records_only_successful_commands() { + let mut shell = MockShellTrait::new(); + shell + .expect_spawn_interactive() + .times(2) + .returning(|cmd| Ok(cmd.args.last().is_some_and(|arg| arg == "first"))); + let mut successful_indexes = HashSet::from([0]); + + record_successful_reply_indexes( + &shell, + &mut successful_indexes, + &[false, true, false, true], + vec![command("first"), command("second")], + ); + + assert_eq!(HashSet::from([0, 1]), successful_indexes); + } + + #[test] + fn record_successful_reply_indexes_treats_shell_error_as_failure() { + let mut shell = MockShellTrait::new(); + shell + .expect_spawn_interactive() + .times(1) + .returning(|_| Err(ShellError::IoError(io::Error::other("failed")))); + let mut successful_indexes = HashSet::new(); + + record_successful_reply_indexes( + &shell, + &mut successful_indexes, + &[true], + vec![command("first")], + ); + + assert!(successful_indexes.is_empty()); + } + + #[test] + fn reviewed_reply_result_returns_successful_indexes_for_each_status() { + let no_action = ReviewedReplyResult::NoAction { + successful_indexes: HashSet::from([1]), + }; + let missing_identity = ReviewedReplyResult::MissingGitIdentity { + successful_indexes: HashSet::from([2]), + }; + let completed = ReviewedReplyResult::Completed { + successful_indexes: HashSet::from([3]), + }; + + assert_eq!(HashSet::from([1]), no_action.into_successful_indexes()); + assert_eq!( + HashSet::from([2]), + missing_identity.into_successful_indexes() + ); + assert_eq!(HashSet::from([3]), completed.into_successful_indexes()); + } +} diff --git a/src/app/actor.rs b/src/app/actor.rs index 435b660..3e44bb9 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -6,16 +6,16 @@ //! [`InputEvent`](crate::input::event::InputEvent) from the channel registered //! with [`InputHandle`](crate::input::handle::InputHandle). //! -//! The actor stops when the input event channel closes (user quit) or when -//! initialization or I/O returns an unrecoverable error. Startup dependency -//! checks run inside [`AppActor::run`] before the first frame. +//! The actor stops when the input event channel closes (user quit) or when I/O +//! returns an unrecoverable error. Startup dependency checks run before this +//! actor is spawned. use std::ops::ControlFlow; -use tracing::{event, Level}; +use color_eyre::{eyre::eyre, Result}; +use tokio::{spawn, sync::mpsc}; use crate::{ app::{ - errors::AppError, flows::{ bookmarked::handle_bookmarked_patchsets, details_actions::handle_patchset_details, edit_config::handle_edit_config, latest::handle_latest_patchsets, @@ -27,7 +27,6 @@ use crate::{ App, }, input::{event::InputEvent, handle::InputHandle}, - render_prefs::PatchRenderer, terminal::{handle::TerminalHandle, messages::TerminalFrame}, ui::handle::UiHandle, }; @@ -44,7 +43,7 @@ pub struct AppActor { terminal_handle: TerminalHandle, ui_handle: UiHandle, input_handle: InputHandle, - event_rx: tokio::sync::mpsc::Receiver, + event_rx: mpsc::Receiver, } impl AppActor { @@ -55,7 +54,7 @@ impl AppActor { terminal_handle: TerminalHandle, ui_handle: UiHandle, input_handle: InputHandle, - event_rx: tokio::sync::mpsc::Receiver, + event_rx: mpsc::Receiver, ) -> AppHandle { tracing::debug!("spawning app actor"); let actor = Self { @@ -65,71 +64,11 @@ impl AppActor { input_handle, event_rx, }; - AppHandle::new(tokio::spawn(actor.run())) + AppHandle::new(spawn(actor.run())) } - /// Verifies required and optional external binaries. - /// - /// A missing `b4` is a hard failure; all other missing binaries only emit - /// warnings. This replicates the former `check_external_deps` free function - /// that lived in `main.rs`. - fn initialize(&self) -> Result<(), AppError> { - let env = &*self.app.services.env; - let config = &self.app.state.config; - - if !env.which("b4") { - event!( - Level::ERROR, - "b4 is not installed, patchsets cannot be downloaded" - ); - return Err(AppError::Dependencies( - "b4 is not installed; patchsets cannot be downloaded".to_string(), - )); - } - - if !env.which("git") { - event!(Level::WARN, "git is not installed, send-email won't work"); - } - - match config.patch_renderer() { - PatchRenderer::Bat => { - if !env.which("bat") { - event!( - Level::WARN, - "bat is not installed, patch rendering will fallback to default" - ); - } - } - PatchRenderer::Delta => { - if !env.which("delta") { - event!( - Level::WARN, - "delta is not installed, patch rendering will fallback to default", - ); - } - } - PatchRenderer::DiffSoFancy => { - if !env.which("diff-so-fancy") { - event!( - Level::WARN, - "diff-so-fancy is not installed, patch rendering will fallback to default", - ); - } - } - _ => {} - } - - Ok(()) - } - - async fn run(mut self) -> color_eyre::Result<()> { + async fn run(mut self) -> Result<()> { tracing::info!("app actor started"); - - let init_result = self.initialize(); - if let Err(ref e) = init_result { - tracing::warn!(error = %e, "app actor initialization failed"); - } - init_result?; tracing::info!("app actor initialized"); let mut loading = TerminalLoadingIndicator::new(self.terminal_handle.clone()); @@ -141,7 +80,7 @@ impl AppActor { .ui_handle .build_scene(self.app.present()) .await - .map_err(|e| color_eyre::eyre::eyre!("{e}"))?; + .map_err(|e| eyre!("{e}"))?; self.terminal_handle .draw(TerminalFrame::Main(Box::new(scene))) .await @@ -178,7 +117,7 @@ async fn on_input( input: InputEvent, terminal_handle: &TerminalHandle, loading: &mut TerminalLoadingIndicator, -) -> color_eyre::Result> { +) -> Result> { if let Some(popup) = app.state.popup.as_mut() { if input == InputEvent::ClosePopup { app.state.popup = None; @@ -219,7 +158,6 @@ mod tests { use crate::{ app::{ - errors::AppError, screens::{ bookmarked::BookmarkedPatchsetsState, mail_list::MailingListSelectionState, CurrentScreen, @@ -228,9 +166,7 @@ mod tests { AppServices, }, config::{ConfigHandle, ConfigState}, - infrastructure::{ - env::MockEnvTrait, file_system::MockFileSystemTrait, shell::MockShellTrait, - }, + infrastructure::{file_system::MockFileSystemTrait, shell::MockShellTrait}, input::{event::InputEvent, handle::InputHandle, messages::InputMessage}, lore::{ application::{ @@ -258,7 +194,7 @@ mod tests { ConfigHandle::new(config_tx) } - fn minimal_app_with_env(env: MockEnvTrait) -> App { + fn minimal_app() -> App { let (lore_tx, _lore_rx) = mpsc::channel(1); let (render_tx, _render_rx) = mpsc::channel(1); @@ -295,18 +231,11 @@ mod tests { render: RenderHandle::new(render_tx), shell: Box::new(MockShellTrait::new()), fs: Box::new(MockFileSystemTrait::new()), - env: Box::new(env), config: dummy_config_handle(), }, } } - fn minimal_app() -> App { - let mut env = MockEnvTrait::new(); - env.expect_which().returning(|_| true); - minimal_app_with_env(env) - } - #[tokio::test(flavor = "multi_thread")] async fn input_channel_close_stops_actor_and_returns_ok() { let mut session = MockTerminalSessionApi::new(); @@ -387,9 +316,6 @@ mod tests { .expect("bootstrap must succeed with mock infrastructure"); assert_eq!(1, bootstrap.mailing_lists.len()); - let mut env = MockEnvTrait::new(); - env.expect_which().returning(|_| true); - let mut session = MockTerminalSessionApi::new(); session .expect_draw() @@ -405,7 +331,6 @@ mod tests { bootstrap, Box::new(MockFileSystemTrait::new()), Box::new(MockShellTrait::new()), - Box::new(env), lore_api.clone(), render.clone(), ) @@ -425,36 +350,4 @@ mod tests { lore_api.shutdown().await; render.shutdown().await; } - - #[tokio::test(flavor = "multi_thread")] - async fn missing_b4_causes_actor_to_return_dependencies_error() { - let mut env = MockEnvTrait::new(); - env.expect_which() - .withf(|name| name == "b4") - .returning(|_| false); - - let mut session = MockTerminalSessionApi::new(); - session.expect_draw().returning(|_| Ok(())); - - let terminal_handle = TerminalActor::spawn(Box::new(session)); - let ui_handle = UiActor::spawn(); - - let (_event_tx, event_rx) = mpsc::channel::(1); - let (input_tx, _input_rx) = mpsc::channel::(1); - let input_handle = InputHandle::new(input_tx); - - let handle = AppActor::spawn( - minimal_app_with_env(env), - terminal_handle, - ui_handle, - input_handle, - event_rx, - ); - - let result = handle.run_until_done().await; - let err = result.unwrap_err(); - assert!(err - .downcast_ref::() - .is_some_and(|e| matches!(e, AppError::Dependencies(_)))); - } } diff --git a/src/app/dependencies.rs b/src/app/dependencies.rs new file mode 100644 index 0000000..3a074fc --- /dev/null +++ b/src/app/dependencies.rs @@ -0,0 +1,121 @@ +use tracing::{event, Level}; + +use crate::{ + app::errors::AppError, config::ConfigSnapshot, infrastructure::env::EnvTrait, + render_prefs::PatchRenderer, +}; + +/// Verifies required and optional external binaries before the terminal starts. +/// +/// A missing `b4` is a hard failure; all other missing binaries only emit +/// warnings. This keeps fatal startup failures out of terminal raw mode. +pub(crate) fn check_external_deps( + env: &dyn EnvTrait, + config: &ConfigSnapshot, +) -> Result<(), AppError> { + if !env.which("b4") { + event!( + Level::ERROR, + "b4 is not installed, patchsets cannot be downloaded" + ); + return Err(AppError::Dependencies( + "b4 is not installed; patchsets cannot be downloaded".to_string(), + )); + } + + if !env.which("git") { + event!(Level::WARN, "git is not installed, send-email won't work"); + } + + match config.patch_renderer() { + PatchRenderer::Bat => { + if !env.which("bat") { + event!( + Level::WARN, + "bat is not installed, patch rendering will fallback to default" + ); + } + } + PatchRenderer::Delta => { + if !env.which("delta") { + event!( + Level::WARN, + "delta is not installed, patch rendering will fallback to default", + ); + } + } + PatchRenderer::DiffSoFancy => { + if !env.which("diff-so-fancy") { + event!( + Level::WARN, + "diff-so-fancy is not installed, patch rendering will fallback to default", + ); + } + } + _ => {} + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use crate::{ + config::{ConfigState, ValidatedConfigUpdate}, + infrastructure::env::MockEnvTrait, + render_prefs::PatchRenderer, + }; + + use super::*; + + #[test] + fn missing_b4_returns_dependencies_error() { + let mut env = MockEnvTrait::new(); + env.expect_which() + .withf(|name| name == "b4") + .returning(|_| false); + + let err = check_external_deps(&env, &ConfigState::default().to_snapshot()).unwrap_err(); + + assert!(matches!(err, AppError::Dependencies(_))); + } + + #[test] + fn missing_git_is_not_fatal() { + let mut env = MockEnvTrait::new(); + env.expect_which() + .withf(|name| name == "b4") + .returning(|_| true); + env.expect_which() + .withf(|name| name == "git") + .returning(|_| false); + + let result = check_external_deps(&env, &ConfigState::default().to_snapshot()); + + assert!(result.is_ok()); + } + + #[test] + fn missing_configured_renderer_is_not_fatal() { + let mut state = ConfigState::default(); + state.apply_update(&ValidatedConfigUpdate { + patch_renderer: Some(PatchRenderer::Bat), + ..Default::default() + }); + + let mut env = MockEnvTrait::new(); + env.expect_which() + .withf(|name| name == "b4") + .returning(|_| true); + env.expect_which() + .withf(|name| name == "git") + .returning(|_| true); + env.expect_which() + .withf(|name| name == "bat") + .returning(|_| false); + + let result = check_external_deps(&env, &state.to_snapshot()); + + assert!(result.is_ok()); + } +} diff --git a/src/app/flows/bookmarked.rs b/src/app/flows/bookmarked.rs index b91cbd2..e804ae5 100644 --- a/src/app/flows/bookmarked.rs +++ b/src/app/flows/bookmarked.rs @@ -1,15 +1,19 @@ use tracing::debug; +use color_eyre::Result; + use crate::{ - app::{loading::LoadingIndicator, popup::AppPopup, screens::CurrentScreen, App, B4Result}, + app::{loading::LoadingIndicator, popup::AppPopup, screens::CurrentScreen, App}, input::event::InputEvent, }; +use super::open_patchset::apply_open_patchset_result; + pub async fn handle_bookmarked_patchsets( app: &mut App, input: InputEvent, loading: &mut dyn LoadingIndicator, -) -> color_eyre::Result<()> { +) -> Result<()> { match input { InputEvent::OpenHelp => { let popup = generate_help_popup(); @@ -39,20 +43,7 @@ pub async fn handle_bookmarked_patchsets( // If a patchset has been bookmarked via the UI, b4 was already // successful for it, but patchsets may also arrive here from // other sources where the fetch could fail. - if let Ok(b4_result) = result { - match b4_result { - B4Result::PatchFound => { - app.set_current_screen(CurrentScreen::PatchsetDetails); - } - B4Result::PatchNotFound(err_cause) => { - app.state.popup = Some(AppPopup::info( - "Error", - format!("The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset."), - )); - app.set_current_screen(CurrentScreen::BookmarkedPatchsets); - } - } - } + apply_open_patchset_result(app, CurrentScreen::BookmarkedPatchsets, result); } _ => {} } diff --git a/src/app/flows/details_actions.rs b/src/app/flows/details_actions.rs index 28b18ae..15a101c 100644 --- a/src/app/flows/details_actions.rs +++ b/src/app/flows/details_actions.rs @@ -1,5 +1,6 @@ use std::time::Duration; +use color_eyre::Result; use ratatui::crossterm::event::KeyCode; use tracing::debug; @@ -15,7 +16,7 @@ pub async fn handle_patchset_details( app: &mut App, input: InputEvent, terminal_handle: &TerminalHandle, -) -> color_eyre::Result<()> { +) -> Result<()> { let patchset_details_and_actions = app .state .lore @@ -108,7 +109,7 @@ pub async fn handle_patchset_details( async fn preview_scroll_lines( amount: ScrollAmount, terminal_handle: &TerminalHandle, -) -> color_eyre::Result { +) -> Result { let (_, height) = terminal_handle.size().await?; Ok(match amount { ScrollAmount::Line => 1, diff --git a/src/app/flows/edit_config.rs b/src/app/flows/edit_config.rs index 44b2157..73a04b7 100644 --- a/src/app/flows/edit_config.rs +++ b/src/app/flows/edit_config.rs @@ -1,3 +1,4 @@ +use color_eyre::Result; use tracing::debug; use crate::{ @@ -5,7 +6,7 @@ use crate::{ input::event::InputEvent, }; -pub async fn handle_edit_config(app: &mut App, input: InputEvent) -> color_eyre::Result<()> { +pub async fn handle_edit_config(app: &mut App, input: InputEvent) -> Result<()> { let Some(is_editing) = app .state .config_state diff --git a/src/app/flows/latest.rs b/src/app/flows/latest.rs index 2193243..3953585 100644 --- a/src/app/flows/latest.rs +++ b/src/app/flows/latest.rs @@ -1,15 +1,19 @@ use tracing::debug; +use color_eyre::Result; + use crate::{ - app::{loading::LoadingIndicator, popup::AppPopup, screens::CurrentScreen, App, B4Result}, + app::{loading::LoadingIndicator, popup::AppPopup, screens::CurrentScreen, App}, input::event::InputEvent, }; +use super::open_patchset::apply_open_patchset_result; + pub async fn handle_latest_patchsets( app: &mut App, input: InputEvent, loading: &mut dyn LoadingIndicator, -) -> color_eyre::Result<()> { +) -> Result<()> { match input { InputEvent::OpenHelp => { let popup = generate_help_popup(); @@ -71,20 +75,7 @@ pub async fn handle_latest_patchsets( loading.start("Loading patchset".to_string()); let result = app.open_patchset_details().await; loading.stop()?; - if let Ok(b4_result) = result { - match b4_result { - B4Result::PatchFound => { - app.set_current_screen(CurrentScreen::PatchsetDetails); - } - B4Result::PatchNotFound(err_cause) => { - app.state.popup = Some(AppPopup::info( - "Error", - format!("The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset."), - )); - app.set_current_screen(CurrentScreen::LatestPatchsets); - } - } - } + apply_open_patchset_result(app, CurrentScreen::LatestPatchsets, result); } _ => {} } diff --git a/src/app/flows/mail_list.rs b/src/app/flows/mail_list.rs index bea7031..82ac011 100644 --- a/src/app/flows/mail_list.rs +++ b/src/app/flows/mail_list.rs @@ -1,17 +1,18 @@ use std::ops::ControlFlow; +use color_eyre::Result; +use tracing::debug; + use crate::{ app::{loading::LoadingIndicator, popup::AppPopup, screens::CurrentScreen, App}, input::event::InputEvent, }; -use tracing::debug; - pub async fn handle_mailing_list_selection( app: &mut App, input: InputEvent, loading: &mut dyn LoadingIndicator, -) -> color_eyre::Result> { +) -> Result> { match input { InputEvent::OpenHelp => { let popup = generate_help_popup(); diff --git a/src/app/flows/mod.rs b/src/app/flows/mod.rs index fdc72d8..ae496cc 100644 --- a/src/app/flows/mod.rs +++ b/src/app/flows/mod.rs @@ -3,3 +3,4 @@ pub(crate) mod details_actions; pub(crate) mod edit_config; pub(crate) mod latest; pub(crate) mod mail_list; +mod open_patchset; diff --git a/src/app/flows/open_patchset.rs b/src/app/flows/open_patchset.rs new file mode 100644 index 0000000..3431254 --- /dev/null +++ b/src/app/flows/open_patchset.rs @@ -0,0 +1,90 @@ +use color_eyre::Result; + +use crate::app::{popup::AppPopup, screens::CurrentScreen, App, B4Result}; + +#[derive(Debug, PartialEq)] +enum OpenPatchsetAction { + ShowDetails, + ShowError { origin: CurrentScreen, body: String }, +} + +pub(super) fn apply_open_patchset_result( + app: &mut App, + origin: CurrentScreen, + result: Result, +) { + match resolve_open_patchset_result(origin, result) { + OpenPatchsetAction::ShowDetails => { + app.set_current_screen(CurrentScreen::PatchsetDetails); + } + OpenPatchsetAction::ShowError { origin, body } => { + app.state.popup = Some(AppPopup::info("Error", body)); + app.set_current_screen(origin); + } + } +} + +fn resolve_open_patchset_result( + origin: CurrentScreen, + result: Result, +) -> OpenPatchsetAction { + match result { + Ok(B4Result::PatchFound) => OpenPatchsetAction::ShowDetails, + Ok(B4Result::PatchNotFound(err_cause)) => OpenPatchsetAction::ShowError { + origin, + body: format!( + "The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset." + ), + }, + Err(error) => OpenPatchsetAction::ShowError { + origin, + body: format!("The selected patchset couldn't be opened.\nReason: {error:#}"), + }, + } +} + +#[cfg(test)] +mod tests { + use color_eyre::eyre::eyre; + + use super::*; + + #[test] + fn patch_found_opens_details() { + let action = + resolve_open_patchset_result(CurrentScreen::LatestPatchsets, Ok(B4Result::PatchFound)); + + assert_eq!(OpenPatchsetAction::ShowDetails, action); + } + + #[test] + fn patch_not_found_shows_retrieval_error_on_origin_screen() { + let action = resolve_open_patchset_result( + CurrentScreen::BookmarkedPatchsets, + Ok(B4Result::PatchNotFound("not found by b4".to_string())), + ); + + assert_eq!( + OpenPatchsetAction::ShowError { + origin: CurrentScreen::BookmarkedPatchsets, + body: "The selected patchset couldn't be retrieved.\nReason: not found by b4\nPlease choose another patchset.".to_string(), + }, + action, + ); + } + + #[test] + fn unexpected_error_shows_open_error_on_origin_screen() { + let action = resolve_open_patchset_result( + CurrentScreen::LatestPatchsets, + Err(eyre!("render actor unavailable")), + ); + + let OpenPatchsetAction::ShowError { origin, body } = action else { + panic!("unexpected errors should show an error popup"); + }; + assert_eq!(CurrentScreen::LatestPatchsets, origin); + assert!(body.contains("The selected patchset couldn't be opened.")); + assert!(body.contains("render actor unavailable")); + } +} diff --git a/src/app/handle.rs b/src/app/handle.rs index c4e9777..3a9fd76 100644 --- a/src/app/handle.rs +++ b/src/app/handle.rs @@ -1,3 +1,4 @@ +use color_eyre::Result; use tokio::task::JoinHandle; /// Handle to the `AppActor` task. @@ -6,17 +7,17 @@ use tokio::task::JoinHandle; /// The actor runs until the input event channel closes (user exits) or an /// unrecoverable error propagates from the run loop. pub struct AppHandle { - join: JoinHandle>, + join: JoinHandle>, } impl AppHandle { - pub(crate) fn new(join: JoinHandle>) -> Self { + pub(crate) fn new(join: JoinHandle>) -> Self { Self { join } } /// Blocks the caller until the actor task completes, propagating any error /// returned by the actor's run loop. - pub async fn run_until_done(self) -> color_eyre::Result<()> { + pub async fn run_until_done(self) -> Result<()> { self.join.await? } } diff --git a/src/app/integration_tests/actor_lifecycle.rs b/src/app/integration_tests/actor_lifecycle.rs new file mode 100644 index 0000000..acbdc7d --- /dev/null +++ b/src/app/integration_tests/actor_lifecycle.rs @@ -0,0 +1,66 @@ +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, +}; + +use tokio::sync::mpsc; + +use crate::{ + app::actor::AppActor, + input::{actor::InputActor, event::InputEvent}, + terminal::{actor::TerminalActor, messages::TerminalFrame, session::MockTerminalSessionApi}, + ui::actor::UiActor, +}; + +use super::helpers::app_harness::minimal_app; + +#[tokio::test(flavor = "multi_thread")] +async fn main_like_lifecycle_shuts_input_down_before_terminal() { + let input_shutdown_complete = Arc::new(AtomicBool::new(false)); + let input_shutdown_complete_for_terminal = Arc::clone(&input_shutdown_complete); + + let mut session = MockTerminalSessionApi::new(); + session + .expect_draw() + .withf(|frame| matches!(frame, TerminalFrame::Main(_))) + .times(1..) + .returning(|_| Ok(())); + session.expect_poll_event().returning(|_| Ok(None)); + session.expect_shutdown().times(1).returning(move || { + assert!( + input_shutdown_complete_for_terminal.load(Ordering::SeqCst), + "terminal shutdown should happen after input shutdown" + ); + Ok(()) + }); + + let terminal_handle = TerminalActor::spawn(Box::new(session)); + let ui_handle = UiActor::spawn(); + let app = minimal_app(); + let initial_input_context = app.input_context(); + + let input_handle = InputActor::spawn(terminal_handle.clone(), initial_input_context); + let input_shutdown_handle = input_handle.clone(); + let (app_input_tx, app_input_rx) = mpsc::channel::(8); + input_handle + .subscribe_app(app_input_tx.clone()) + .await + .unwrap(); + + let app_handle = AppActor::spawn( + app, + terminal_handle.clone(), + ui_handle.clone(), + input_handle, + app_input_rx, + ); + + app_input_tx.send(InputEvent::Quit).await.unwrap(); + app_handle.run_until_done().await.unwrap(); + + input_shutdown_handle.shutdown().await.unwrap(); + input_shutdown_complete.store(true, Ordering::SeqCst); + + ui_handle.shutdown().await; + terminal_handle.shutdown().await.unwrap(); +} diff --git a/src/app/integration_tests/flow_errors.rs b/src/app/integration_tests/flow_errors.rs new file mode 100644 index 0000000..a41ac61 --- /dev/null +++ b/src/app/integration_tests/flow_errors.rs @@ -0,0 +1,114 @@ +use std::ops::ControlFlow; + +use crate::{ + app::{ + flows::{ + bookmarked::handle_bookmarked_patchsets, latest::handle_latest_patchsets, + mail_list::handle_mailing_list_selection, + }, + popup::AppPopup, + screens::CurrentScreen, + }, + input::event::InputEvent, +}; + +use super::helpers::{ + app_harness::AppHarness, + loading::FakeLoadingIndicator, + lore::{lore_handle_with_patch_details_failure, lore_handle_with_successful_patch_flow}, + render::{render_handle_with_preview_failure, render_handle_with_successful_preview}, +}; + +#[tokio::test] +async fn latest_render_failure_shows_popup_and_stays_on_latest() { + let mut harness = AppHarness::with_handles( + lore_handle_with_successful_patch_flow(), + render_handle_with_preview_failure(), + ); + let mut loading = FakeLoadingIndicator::default(); + + let result = handle_mailing_list_selection( + &mut harness.app, + InputEvent::OpenLatestPatchsets, + &mut loading, + ) + .await + .unwrap(); + assert_eq!(ControlFlow::Continue(()), result); + + handle_latest_patchsets( + &mut harness.app, + InputEvent::OpenPatchsetDetails, + &mut loading, + ) + .await + .unwrap(); + + assert_eq!( + CurrentScreen::LatestPatchsets, + harness.app.state.navigation.current_screen + ); + assert!(harness.app.state.lore.details.is_none()); + assert_info_popup_contains( + harness.app.state.popup.as_ref(), + "Error", + &[ + "The selected patchset couldn't be opened.", + "render actor unavailable in test", + ], + ); +} + +#[tokio::test] +async fn bookmarked_lore_failure_shows_popup_and_stays_on_bookmarks() { + let mut harness = AppHarness::with_bookmark( + lore_handle_with_patch_details_failure(), + render_handle_with_successful_preview(), + ); + let mut loading = FakeLoadingIndicator::default(); + + let result = handle_mailing_list_selection( + &mut harness.app, + InputEvent::OpenBookmarkedPatchsets, + &mut loading, + ) + .await + .unwrap(); + assert_eq!(ControlFlow::Continue(()), result); + + handle_bookmarked_patchsets( + &mut harness.app, + InputEvent::OpenPatchsetDetails, + &mut loading, + ) + .await + .unwrap(); + + assert_eq!( + CurrentScreen::BookmarkedPatchsets, + harness.app.state.navigation.current_screen + ); + assert!(harness.app.state.lore.details.is_none()); + assert_info_popup_contains( + harness.app.state.popup.as_ref(), + "Error", + &[ + "The selected patchset couldn't be opened.", + "lore actor unavailable in test", + ], + ); +} + +fn assert_info_popup_contains(popup: Option<&AppPopup>, expected_title: &str, expected: &[&str]) { + let Some(AppPopup::Info { title, body, .. }) = popup else { + panic!("expected info popup"); + }; + + assert_eq!(expected_title, title); + for fragment in expected { + assert!( + body.contains(fragment), + "expected popup body to contain {fragment:?}, got {body:?}" + ); + } +} diff --git a/src/app/integration_tests/flow_navigation.rs b/src/app/integration_tests/flow_navigation.rs new file mode 100644 index 0000000..fb09436 --- /dev/null +++ b/src/app/integration_tests/flow_navigation.rs @@ -0,0 +1,109 @@ +use std::ops::ControlFlow; + +use crate::{ + app::{ + flows::{ + bookmarked::handle_bookmarked_patchsets, details_actions::handle_patchset_details, + latest::handle_latest_patchsets, mail_list::handle_mailing_list_selection, + }, + screens::CurrentScreen, + }, + input::event::InputEvent, +}; + +use super::helpers::{ + app_harness::{dummy_terminal_handle, AppHarness}, + loading::FakeLoadingIndicator, + lore::lore_handle_with_successful_patch_flow, + render::render_handle_with_successful_preview, +}; + +#[tokio::test] +async fn latest_list_opens_details_and_back_returns_to_latest() { + let mut harness = AppHarness::with_handles( + lore_handle_with_successful_patch_flow(), + render_handle_with_successful_preview(), + ); + let mut loading = FakeLoadingIndicator::default(); + + let result = handle_mailing_list_selection( + &mut harness.app, + InputEvent::OpenLatestPatchsets, + &mut loading, + ) + .await + .unwrap(); + assert_eq!(ControlFlow::Continue(()), result); + assert_eq!( + CurrentScreen::LatestPatchsets, + harness.app.state.navigation.current_screen + ); + + handle_latest_patchsets( + &mut harness.app, + InputEvent::OpenPatchsetDetails, + &mut loading, + ) + .await + .unwrap(); + assert_eq!( + CurrentScreen::PatchsetDetails, + harness.app.state.navigation.current_screen + ); + assert!(harness.app.state.lore.details.is_some()); + + handle_patchset_details(&mut harness.app, InputEvent::Back, &dummy_terminal_handle()) + .await + .unwrap(); + + assert_eq!( + CurrentScreen::LatestPatchsets, + harness.app.state.navigation.current_screen + ); + assert!(harness.app.state.lore.details.is_none()); +} + +#[tokio::test] +async fn bookmarked_list_opens_details_and_back_returns_to_bookmarks() { + let mut harness = AppHarness::with_bookmark( + lore_handle_with_successful_patch_flow(), + render_handle_with_successful_preview(), + ); + let mut loading = FakeLoadingIndicator::default(); + + let result = handle_mailing_list_selection( + &mut harness.app, + InputEvent::OpenBookmarkedPatchsets, + &mut loading, + ) + .await + .unwrap(); + assert_eq!(ControlFlow::Continue(()), result); + assert_eq!( + CurrentScreen::BookmarkedPatchsets, + harness.app.state.navigation.current_screen + ); + + handle_bookmarked_patchsets( + &mut harness.app, + InputEvent::OpenPatchsetDetails, + &mut loading, + ) + .await + .unwrap(); + assert_eq!( + CurrentScreen::PatchsetDetails, + harness.app.state.navigation.current_screen + ); + assert!(harness.app.state.lore.details.is_some()); + + handle_patchset_details(&mut harness.app, InputEvent::Back, &dummy_terminal_handle()) + .await + .unwrap(); + + assert_eq!( + CurrentScreen::BookmarkedPatchsets, + harness.app.state.navigation.current_screen + ); + assert!(harness.app.state.lore.details.is_none()); +} diff --git a/src/app/integration_tests/helpers/app_harness.rs b/src/app/integration_tests/helpers/app_harness.rs new file mode 100644 index 0000000..24f1dff --- /dev/null +++ b/src/app/integration_tests/helpers/app_harness.rs @@ -0,0 +1,85 @@ +use tokio::sync::mpsc; + +use crate::{ + app::App, + config::{ConfigHandle, ConfigState}, + infrastructure::{file_system::MockFileSystemTrait, shell::MockShellTrait}, + lore::application::{cache::BootstrapLoreData, handle::LoreApiHandle}, + render::handle::RenderHandle, + terminal::{handle::TerminalHandle, messages::TerminalMessage}, +}; + +use super::lore::{sample_mailing_list, sample_patch}; + +pub(crate) struct AppHarness { + pub(crate) app: App, +} + +impl AppHarness { + pub(crate) fn new() -> Self { + Self { app: minimal_app() } + } + + pub(crate) fn with_handles(lore_api: LoreApiHandle, render: RenderHandle) -> Self { + Self { + app: app_with_bootstrap_and_handles(bootstrap_data(), lore_api, render), + } + } + + pub(crate) fn with_bookmark(lore_api: LoreApiHandle, render: RenderHandle) -> Self { + let mut bootstrap = bootstrap_data(); + bootstrap.bookmarks = vec![sample_patch()]; + Self { + app: app_with_bootstrap_and_handles(bootstrap, lore_api, render), + } + } +} + +pub(crate) fn minimal_app() -> App { + app_with_bootstrap_and_handles(bootstrap_data(), dummy_lore_handle(), dummy_render_handle()) +} + +pub(crate) fn app_with_bootstrap_and_handles( + bootstrap: BootstrapLoreData, + lore_api: LoreApiHandle, + render: RenderHandle, +) -> App { + App::new( + ConfigState::default().to_snapshot(), + dummy_config_handle(), + bootstrap, + Box::new(MockFileSystemTrait::new()), + Box::new(MockShellTrait::new()), + lore_api, + render, + ) + .expect("minimal app should build") +} + +pub(crate) fn dummy_config_handle() -> ConfigHandle { + let (tx, _rx) = mpsc::channel(1); + ConfigHandle::new(tx) +} + +pub(crate) fn dummy_lore_handle() -> LoreApiHandle { + let (tx, _rx) = mpsc::channel(1); + LoreApiHandle::new(tx) +} + +pub(crate) fn dummy_render_handle() -> RenderHandle { + let (tx, _rx) = mpsc::channel(1); + RenderHandle::new(tx) +} + +pub(crate) fn dummy_terminal_handle() -> TerminalHandle { + let (tx, _rx) = mpsc::channel::(1); + TerminalHandle::new(tx) +} + +fn bootstrap_data() -> BootstrapLoreData { + BootstrapLoreData { + mailing_lists: vec![sample_mailing_list()], + bookmarks: vec![], + reviewed: Default::default(), + } +} diff --git a/src/app/integration_tests/helpers/loading.rs b/src/app/integration_tests/helpers/loading.rs new file mode 100644 index 0000000..edf4607 --- /dev/null +++ b/src/app/integration_tests/helpers/loading.rs @@ -0,0 +1,20 @@ +use color_eyre::Result; + +use crate::app::loading::LoadingIndicator; + +#[derive(Default)] +pub(crate) struct FakeLoadingIndicator { + pub(crate) starts: Vec, + pub(crate) stop_count: usize, +} + +impl LoadingIndicator for FakeLoadingIndicator { + fn start(&mut self, title: String) { + self.starts.push(title); + } + + fn stop(&mut self) -> Result<()> { + self.stop_count += 1; + Ok(()) + } +} diff --git a/src/app/integration_tests/helpers/lore.rs b/src/app/integration_tests/helpers/lore.rs new file mode 100644 index 0000000..afa791e --- /dev/null +++ b/src/app/integration_tests/helpers/lore.rs @@ -0,0 +1,116 @@ +use std::collections::HashSet; + +use serde_xml_rs::from_str; +use tokio::{spawn, sync::mpsc}; + +use crate::lore::{ + application::{ + dto::{PatchTagSummary, PatchsetDetails}, + errors::LoreError, + handle::LoreApiHandle, + messages::LoreApiMessage, + }, + domain::{mailing_list::MailingList, patch::Patch}, +}; + +pub(crate) fn lore_handle_with_successful_patch_flow() -> LoreApiHandle { + let (tx, mut rx) = mpsc::channel(8); + spawn(async move { + while let Some(message) = rx.recv().await { + match message { + LoreApiMessage::FetchFeedPage { reply, .. } => { + reply.send(Ok(vec![sample_patch()])).ok(); + } + LoreApiMessage::FetchPatchsetDetails { reply, .. } => { + reply.send(Ok(sample_patchset_details())).ok(); + } + LoreApiMessage::Shutdown => break, + other => panic!("unexpected lore message: {}", other.name()), + } + } + }); + LoreApiHandle::new(tx) +} + +pub(crate) fn lore_handle_with_patch_details_failure() -> LoreApiHandle { + let (tx, mut rx) = mpsc::channel(8); + spawn(async move { + while let Some(message) = rx.recv().await { + match message { + LoreApiMessage::FetchPatchsetDetails { reply, .. } => { + reply + .send(Err(LoreError::ActorUnavailable( + "lore actor unavailable in test".to_string(), + ))) + .ok(); + } + LoreApiMessage::Shutdown => break, + other => panic!("unexpected lore message: {}", other.name()), + } + } + }); + LoreApiHandle::new(tx) +} + +pub(crate) fn lore_handle_with_persistence() -> LoreApiHandle { + let (tx, mut rx) = mpsc::channel(8); + spawn(async move { + while let Some(message) = rx.recv().await { + match message { + LoreApiMessage::SaveBookmarks { reply, .. } => { + reply.send(Ok(())).ok(); + } + LoreApiMessage::SaveReviewed { reply, .. } => { + reply.send(Ok(())).ok(); + } + LoreApiMessage::Shutdown => break, + other => panic!("unexpected lore message: {}", other.name()), + } + } + }); + LoreApiHandle::new(tx) +} + +pub(crate) fn sample_mailing_list() -> MailingList { + MailingList::new("test-list", "Test list") +} + +pub(crate) fn sample_patch() -> Patch { + from_str( + r#" + + + Foo Bar + foo@bar.example + + [PATCH 1/1] test patch + 2024-07-06T19:15:48Z + + urn:uuid:123-abcd-1f2a3b + + + "#, + ) + .expect("sample patch XML should deserialize") +} + +pub(crate) fn sample_patchset_details() -> PatchsetDetails { + PatchsetDetails { + patchset_path: "/tmp/patchset.mbx".to_string(), + raw_patches: vec![sample_raw_patch()], + tag_summary: vec![empty_tag_summary()], + } +} + +pub(crate) fn sample_raw_patch() -> String { + "Subject: [PATCH] test\n\nBody\n---\n file.txt | 1 +\n 1 file changed, 1 insertion(+)\n" + .to_string() +} + +pub(crate) fn empty_tag_summary() -> PatchTagSummary { + PatchTagSummary { + reviewed_by: HashSet::new(), + tested_by: HashSet::new(), + acked_by: HashSet::new(), + } +} diff --git a/src/app/integration_tests/helpers/mod.rs b/src/app/integration_tests/helpers/mod.rs new file mode 100644 index 0000000..68091d1 --- /dev/null +++ b/src/app/integration_tests/helpers/mod.rs @@ -0,0 +1,4 @@ +pub(crate) mod app_harness; +pub(crate) mod loading; +pub(crate) mod lore; +pub(crate) mod render; diff --git a/src/app/integration_tests/helpers/render.rs b/src/app/integration_tests/helpers/render.rs new file mode 100644 index 0000000..fa04aeb --- /dev/null +++ b/src/app/integration_tests/helpers/render.rs @@ -0,0 +1,46 @@ +use tokio::{spawn, sync::mpsc}; + +use crate::render::{ + handle::RenderHandle, messages::RenderMessage, RenderError, RenderedPatchPreview, + RenderedPatchsetPreview, +}; + +pub(crate) fn render_handle_with_successful_preview() -> RenderHandle { + let (tx, mut rx) = mpsc::channel(8); + spawn(async move { + while let Some(message) = rx.recv().await { + match message { + RenderMessage::RenderPatchsetPreview { reply, .. } => { + reply.send(Ok(sample_rendered_preview())).ok(); + } + RenderMessage::Shutdown => break, + } + } + }); + RenderHandle::new(tx) +} + +pub(crate) fn render_handle_with_preview_failure() -> RenderHandle { + let (tx, mut rx) = mpsc::channel(8); + spawn(async move { + while let Some(message) = rx.recv().await { + match message { + RenderMessage::RenderPatchsetPreview { reply, .. } => { + reply + .send(Err(RenderError::ActorUnavailable( + "render actor unavailable in test".to_string(), + ))) + .ok(); + } + RenderMessage::Shutdown => break, + } + } + }); + RenderHandle::new(tx) +} + +pub(crate) fn sample_rendered_preview() -> RenderedPatchsetPreview { + RenderedPatchsetPreview::new(vec![RenderedPatchPreview::new( + "Subject: [PATCH] test\n---\n+added line".to_string(), + )]) +} diff --git a/src/app/integration_tests/mod.rs b/src/app/integration_tests/mod.rs new file mode 100644 index 0000000..65db299 --- /dev/null +++ b/src/app/integration_tests/mod.rs @@ -0,0 +1,47 @@ +mod actor_lifecycle; +mod flow_errors; +mod flow_navigation; +mod helpers; +mod patchset_actions; + +use crate::app::{loading::LoadingIndicator, screens::CurrentScreen}; + +use helpers::{ + app_harness::AppHarness, + loading::FakeLoadingIndicator, + lore::{sample_patch, sample_patchset_details}, + render::sample_rendered_preview, +}; + +#[test] +fn helper_builds_minimal_app_on_initial_screen() { + let harness = AppHarness::new(); + + assert_eq!( + CurrentScreen::MailingListSelection, + harness.app.state.navigation.current_screen + ); +} + +#[test] +fn helpers_provide_sample_patchset_inputs() { + let patch = sample_patch(); + let details = sample_patchset_details(); + let rendered = sample_rendered_preview(); + + assert_eq!("[PATCH 1/1] test patch", patch.title()); + assert_eq!(1, details.raw_patches.len()); + assert_eq!(1, details.tag_summary.len()); + assert_eq!(1, rendered.entries.len()); +} + +#[test] +fn fake_loading_indicator_records_start_and_stop() { + let mut loading = FakeLoadingIndicator::default(); + + loading.start("Loading patchset".to_string()); + loading.stop().unwrap(); + + assert_eq!(vec!["Loading patchset"], loading.starts); + assert_eq!(1, loading.stop_count); +} diff --git a/src/app/integration_tests/patchset_actions.rs b/src/app/integration_tests/patchset_actions.rs new file mode 100644 index 0000000..31f3eae --- /dev/null +++ b/src/app/integration_tests/patchset_actions.rs @@ -0,0 +1,373 @@ +use std::{ + collections::{HashMap, HashSet, VecDeque}, + sync::{Arc, Mutex}, +}; + +use tokio::{spawn, sync::mpsc}; + +use crate::{ + app::{ + popup::AppPopup, + screens::{ + details_actions::{PatchsetAction, PatchsetDetailsState}, + CurrentScreen, + }, + App, + }, + config::{ConfigSnapshot, ConfigState}, + infrastructure::{ + file_system::MockFileSystemTrait, + shell::{MockShellTrait, ShellCommand, ShellOutput}, + }, + lore::application::{ + cache::BootstrapLoreData, handle::LoreApiHandle, messages::LoreApiMessage, + }, +}; + +use super::helpers::{ + app_harness::{dummy_config_handle, dummy_render_handle}, + lore::{ + lore_handle_with_persistence, sample_mailing_list, sample_patch, sample_patchset_details, + }, + render::sample_rendered_preview, +}; + +const KERNEL_TREE_PATH: &str = "/kernel"; +const BASE_BRANCH: &str = "main"; + +type ReviewedState = HashMap>; +type SharedReviewedState = Arc>>; + +#[tokio::test] +async fn apply_success_sets_success_popup_and_resets_apply_action() { + let (shell, _calls) = shell_with_outputs(vec![ + output("", "", true), + output("", "", true), + output("feature\n", "", true), + output("", "", true), + output("", "", true), + output("", "", true), + output("", "", true), + ]); + let mut app = app_with_apply_details(clean_fs(), shell); + + app.consolidate_patchset_actions().await.unwrap(); + + assert_apply_action(&app, false); + assert_info_popup_contains( + app.state.popup.as_ref(), + "Patchset Apply Success", + &[ + "applied successfully", + "Kernel Tree: '/kernel'", + "Applied branch: 'patchset-", + ], + ); +} + +#[tokio::test] +async fn apply_failure_sets_failure_popup_and_resets_apply_action() { + let (shell, calls) = shell_with_outputs(vec![ + output("", "", true), + output("", "", true), + output("feature\n", "", true), + output("", "", true), + output("", "", true), + output("", "apply failed", false), + output("", "", true), + output("", "", true), + ]); + let mut app = app_with_apply_details(clean_fs(), shell); + + app.consolidate_patchset_actions().await.unwrap(); + + assert_apply_action(&app, false); + assert_info_popup_contains( + app.state.popup.as_ref(), + "Patchset Apply Fail", + &["`git am` failed", "feature", "apply failed"], + ); + let calls = calls.lock().unwrap(); + assert_eq!( + command(&["git", "-C", KERNEL_TREE_PATH, "am", "--abort"]), + calls[6] + ); +} + +#[tokio::test] +async fn reviewed_reply_success_records_persists_and_resets_reply_action() { + let saved_reviewed = Arc::new(Mutex::new(None)); + let lore_api = reviewed_reply_lore_handle(Arc::clone(&saved_reviewed)); + let mut shell = MockShellTrait::new(); + shell + .expect_execute() + .withf(|cmd| cmd.program == "mktemp" && cmd.args == ["--directory"]) + .times(1) + .returning(|_| Ok(output("/tmp/reviewed-reply\n", "", true))); + shell + .expect_spawn_interactive() + .times(1) + .returning(|_| Ok(true)); + let mut app = app_with_reviewed_reply_details(shell, lore_api); + let message_id = selected_message_id(&app); + + app.consolidate_patchset_actions().await.unwrap(); + + assert_reply_action_reset(&app); + assert_eq!( + HashSet::from([0]), + app.state.user_state.reviewed_patchsets[&message_id] + ); + let saved = saved_reviewed + .lock() + .unwrap() + .clone() + .expect("reviewed state should be persisted"); + assert_eq!(HashSet::from([0]), saved[&message_id]); +} + +#[tokio::test] +async fn reviewed_reply_failure_does_not_record_failed_index() { + let saved_reviewed = Arc::new(Mutex::new(None)); + let lore_api = reviewed_reply_lore_handle(Arc::clone(&saved_reviewed)); + let mut shell = MockShellTrait::new(); + shell + .expect_execute() + .withf(|cmd| cmd.program == "mktemp" && cmd.args == ["--directory"]) + .times(1) + .returning(|_| Ok(output("/tmp/reviewed-reply\n", "", true))); + shell + .expect_spawn_interactive() + .times(1) + .returning(|_| Ok(false)); + let mut app = app_with_reviewed_reply_details(shell, lore_api); + let message_id = selected_message_id(&app); + + app.consolidate_patchset_actions().await.unwrap(); + + assert_reply_action_reset(&app); + assert!(app.state.user_state.reviewed_patchsets[&message_id].is_empty()); + let saved = saved_reviewed + .lock() + .unwrap() + .clone() + .expect("reviewed state should be persisted"); + assert!(saved[&message_id].is_empty()); +} + +fn app_with_apply_details(fs: MockFileSystemTrait, shell: MockShellTrait) -> App { + app_with_details( + fs, + shell, + lore_handle_with_persistence(), + apply_details_state(), + ) +} + +fn app_with_reviewed_reply_details(shell: MockShellTrait, lore_api: LoreApiHandle) -> App { + app_with_details( + MockFileSystemTrait::new(), + shell, + lore_api, + reviewed_reply_details_state(), + ) +} + +fn app_with_details( + fs: MockFileSystemTrait, + shell: MockShellTrait, + lore_api: LoreApiHandle, + details: PatchsetDetailsState, +) -> App { + let mut app = App::new( + apply_config(), + dummy_config_handle(), + BootstrapLoreData { + mailing_lists: vec![sample_mailing_list()], + bookmarks: vec![], + reviewed: Default::default(), + }, + Box::new(fs), + Box::new(shell), + lore_api, + dummy_render_handle(), + ) + .expect("app should build"); + + app.state.navigation.current_screen = CurrentScreen::PatchsetDetails; + app.state.lore.details = Some(details); + app +} + +fn apply_details_state() -> PatchsetDetailsState { + let mut details = PatchsetDetailsState::from_rendered_preview( + sample_patch(), + sample_patchset_details(), + sample_rendered_preview(), + false, + CurrentScreen::LatestPatchsets, + ); + details.toggle_apply_action(); + details +} + +fn reviewed_reply_details_state() -> PatchsetDetailsState { + let mut details = PatchsetDetailsState::from_rendered_preview( + sample_patch(), + sample_patchset_details(), + sample_rendered_preview(), + false, + CurrentScreen::LatestPatchsets, + ); + details.toggle_reply_with_reviewed_by_action(false); + details +} + +fn reviewed_reply_lore_handle(saved_reviewed: SharedReviewedState) -> LoreApiHandle { + let (tx, mut rx) = mpsc::channel(8); + spawn(async move { + while let Some(message) = rx.recv().await { + match message { + LoreApiMessage::SaveBookmarks { reply, .. } => { + reply.send(Ok(())).ok(); + } + LoreApiMessage::GetGitSignature { reply, .. } => { + reply + .send(Ok(( + "Reviewer".to_string(), + "reviewer@example.com".to_string(), + ))) + .ok(); + } + LoreApiMessage::PrepareReplyCommands { reply, .. } => { + reply + .send(Ok(vec![ + ShellCommand::new("git").args(["send-email", "--annotate"]) + ])) + .ok(); + } + LoreApiMessage::SaveReviewed { reviewed, reply } => { + *saved_reviewed.lock().unwrap() = Some(reviewed); + reply.send(Ok(())).ok(); + } + LoreApiMessage::Shutdown => break, + other => panic!("unexpected lore message: {}", other.name()), + } + } + }); + LoreApiHandle::new(tx) +} + +fn apply_config() -> ConfigSnapshot { + serde_json::from_value::(serde_json::json!({ + "kernel_trees": { + "linux": { + "path": KERNEL_TREE_PATH, + "branch": BASE_BRANCH + } + }, + "target_kernel_tree": "linux", + "git_am_options": "--signoff --3way", + "git_am_branch_prefix": "patchset-" + })) + .expect("test config should deserialize") + .to_snapshot() +} + +fn clean_fs() -> MockFileSystemTrait { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_dir() + .returning(|path| matches!(path.to_str(), Some("/kernel") | Some("/kernel/.git"))); + fs.expect_is_file().returning(|_| false); + fs +} + +fn shell_with_outputs(outputs: Vec) -> (MockShellTrait, Arc>>>) { + let calls = Arc::new(Mutex::new(Vec::new())); + let outputs = Arc::new(Mutex::new(VecDeque::from(outputs))); + let mut shell = MockShellTrait::new(); + let calls_for_execute = Arc::clone(&calls); + let outputs_for_execute = Arc::clone(&outputs); + shell.expect_execute().returning(move |cmd| { + calls_for_execute.lock().unwrap().push(command_parts(cmd)); + Ok(outputs_for_execute + .lock() + .unwrap() + .pop_front() + .expect("test should provide one output per shell command")) + }); + (shell, calls) +} + +fn output(stdout: impl Into>, stderr: impl Into>, success: bool) -> ShellOutput { + ShellOutput { + stdout: stdout.into(), + stderr: stderr.into(), + success, + } +} + +fn command_parts(cmd: &ShellCommand) -> Vec { + let mut parts = vec![cmd.program.clone()]; + parts.extend(cmd.args.clone()); + parts +} + +fn command(parts: &[&str]) -> Vec { + parts.iter().map(|part| part.to_string()).collect() +} + +fn assert_apply_action(app: &App, expected: bool) { + let details = app + .state + .lore + .details + .as_ref() + .expect("details should remain loaded"); + assert_eq!( + Some(&expected), + details.patchset_actions.get(&PatchsetAction::Apply) + ); +} + +fn selected_message_id(app: &App) -> String { + app.state + .lore + .details + .as_ref() + .expect("details should remain loaded") + .representative_patch + .message_id() + .href + .clone() +} + +fn assert_reply_action_reset(app: &App) { + let details = app + .state + .lore + .details + .as_ref() + .expect("details should remain loaded"); + assert_eq!( + Some(&false), + details + .patchset_actions + .get(&PatchsetAction::ReplyWithReviewedBy) + ); + assert_eq!(vec![false], details.patches_to_reply); +} + +fn assert_info_popup_contains(popup: Option<&AppPopup>, expected_title: &str, expected: &[&str]) { + let Some(AppPopup::Info { title, body, .. }) = popup else { + panic!("expected info popup"); + }; + + assert_eq!(expected_title, title); + for fragment in expected { + assert!( + body.contains(fragment), + "expected popup body to contain {fragment:?}, got {body:?}" + ); + } +} diff --git a/src/app/loading.rs b/src/app/loading.rs index 1f60a70..d1fdc32 100644 --- a/src/app/loading.rs +++ b/src/app/loading.rs @@ -3,10 +3,16 @@ use std::{ atomic::{AtomicBool, Ordering}, Arc, }, + thread, time::Duration, }; -use tokio::task::JoinHandle; +use color_eyre::{eyre::eyre, Report, Result}; +use tokio::{ + runtime::Handle, + spawn, + task::{block_in_place, JoinHandle}, +}; use crate::terminal::{handle::TerminalHandle, messages::TerminalFrame, TerminalError}; @@ -14,7 +20,7 @@ pub(crate) const LOADING_FRAME_INTERVAL: Duration = Duration::from_millis(200); pub(crate) trait LoadingIndicator: Send { fn start(&mut self, title: String); - fn stop(&mut self) -> color_eyre::Result<()>; + fn stop(&mut self) -> Result<()>; } pub(crate) struct TerminalLoadingIndicator { @@ -44,7 +50,7 @@ impl LoadingIndicator for TerminalLoadingIndicator { let terminal_handle = self.terminal_handle.clone(); self.running = Some(running); - self.spinner_task = Some(tokio::spawn(async move { + self.spinner_task = Some(spawn(async move { while running_clone.load(Ordering::Relaxed) { if terminal_handle .draw(TerminalFrame::Loading(title.clone())) @@ -54,14 +60,14 @@ impl LoadingIndicator for TerminalLoadingIndicator { break; } - std::thread::sleep(LOADING_FRAME_INTERVAL); + thread::sleep(LOADING_FRAME_INTERVAL); } })); - std::thread::sleep(LOADING_FRAME_INTERVAL); + thread::sleep(LOADING_FRAME_INTERVAL); } - fn stop(&mut self) -> color_eyre::Result<()> { + fn stop(&mut self) -> Result<()> { let Some(spinner_task) = self.spinner_task.take() else { return Ok(()); }; @@ -70,16 +76,16 @@ impl LoadingIndicator for TerminalLoadingIndicator { running.store(false, Ordering::Relaxed); } - tokio::task::block_in_place(|| { - tokio::runtime::Handle::current().block_on(async { spinner_task.await.ok() }); + block_in_place(|| { + Handle::current().block_on(async { spinner_task.await.ok() }); }); Ok(()) } } -pub(crate) fn terminal_error(error: TerminalError) -> color_eyre::Report { - color_eyre::eyre::eyre!("{error}") +pub(crate) fn terminal_error(error: TerminalError) -> Report { + eyre!("{error}") } #[cfg(test)] @@ -102,7 +108,7 @@ mod tests { let mut loading = TerminalLoadingIndicator::new(handle); loading.start("Fetching mailing lists".to_string()); - std::thread::sleep(LOADING_FRAME_INTERVAL); + thread::sleep(LOADING_FRAME_INTERVAL); loading.stop().unwrap(); } } diff --git a/src/app/mod.rs b/src/app/mod.rs index 0e87528..39edf71 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -9,7 +9,9 @@ //! [`AppActor`](crate::app::actor::AppActor) into [`crate::app::flows`]; //! presentation data crosses the UI boundary only through [`AppViewModel`] via //! [`App::present`]. +pub(crate) mod actions; pub mod actor; +pub(crate) mod dependencies; pub mod errors; pub(crate) mod flows; pub mod handle; @@ -21,18 +23,23 @@ pub mod state; pub mod updates; pub mod view_model; -use color_eyre::eyre::{bail, eyre}; -use tracing::{debug, event, info, warn, Level}; +#[cfg(test)] +mod integration_tests; -use std::path::PathBuf; +use color_eyre::{ + eyre::{bail, eyre}, + Result, +}; +use tracing::{debug, event, info, warn, Level}; use crate::{ + app::actions::{ + apply::ApplyPatchsetRequest, reviewed_reply::ReviewedReplyRequest, PatchsetActionService, + }, config::{ConfigHandle, ConfigSnapshot}, infrastructure::{ - env::EnvTrait, - file_system::FileSystemTrait, - monitoring::logging::garbage_collector::collect_garbage, - shell::{ShellCommand, ShellTrait}, + file_system::FileSystemTrait, monitoring::logging::garbage_collector::collect_garbage, + shell::ShellTrait, }, lore::{ application::{ @@ -61,7 +68,6 @@ pub struct AppServices { pub render: RenderHandle, pub shell: Box, pub fs: Box, - pub env: Box, pub config: ConfigHandle, } @@ -92,10 +98,9 @@ impl App { bootstrap: BootstrapLoreData, fs: Box, shell: Box, - env: Box, lore_api: LoreApiHandle, render: RenderHandle, - ) -> color_eyre::Result { + ) -> Result { event!(Level::INFO, "patch-hub started"); collect_garbage(&config); @@ -130,7 +135,6 @@ impl App { render, shell, fs, - env, config: config_handle, }, }) @@ -163,7 +167,7 @@ impl App { } /// Fetches (or re-fetches) the current page of latest patchsets from Lore. - pub async fn fetch_latest_current_page(&mut self) -> color_eyre::Result<()> { + pub async fn fetch_latest_current_page(&mut self) -> Result<()> { let lore_api = &self.services.lore_api; let latest_patchsets = &mut self.state.lore.latest_patchsets; if let Some(patchsets) = latest_patchsets.as_mut() { @@ -184,7 +188,7 @@ impl App { } /// Refreshes available mailing lists and updates [`LoreUiState::mailing_list_selection`]. - pub async fn refresh_mailing_lists(&mut self) -> color_eyre::Result<()> { + pub async fn refresh_mailing_lists(&mut self) -> Result<()> { debug!("refreshing mailing lists"); let result = self .state @@ -200,7 +204,7 @@ impl App { } /// Loads patchset details into [`LoreUiState::details`]. - pub async fn open_patchset_details(&mut self) -> color_eyre::Result { + pub async fn open_patchset_details(&mut self) -> Result { let representative_patch: Patch; let mut is_patchset_bookmarked = true; @@ -291,7 +295,7 @@ impl App { /// # Panics /// /// Panics if [`LoreUiState::details`] is `None`. - pub async fn consolidate_patchset_actions(&mut self) -> color_eyre::Result<()> { + pub async fn consolidate_patchset_actions(&mut self) -> Result<()> { debug!("consolidating patchset actions"); self.sync_patchset_bookmark().await?; self.execute_reviewed_reply().await?; @@ -300,7 +304,7 @@ impl App { Ok(()) } - async fn sync_patchset_bookmark(&mut self) -> color_eyre::Result<()> { + async fn sync_patchset_bookmark(&mut self) -> Result<()> { let details = self .state .lore @@ -340,88 +344,38 @@ impl App { Ok(()) } - async fn execute_reviewed_reply(&mut self) -> color_eyre::Result<()> { + async fn execute_reviewed_reply(&mut self) -> Result<()> { let details = self .state .lore .details .as_ref() .expect("invariant: details must be loaded before executing reviewed reply"); - let representative_patch = details.representative_patch.clone(); - let patchset_actions = &details.patchset_actions; - let raw_patches = details.raw_patches.clone(); - let patches_to_reply = details.patches_to_reply.clone(); - - if let Some(true) = patchset_actions.get(&PatchsetAction::ReplyWithReviewedBy) { - debug!( - msg_id = representative_patch.message_id().href, - "executing reviewed-by reply" - ); - let mut successful_indexes = self + if patchset_action_selected(details, &PatchsetAction::ReplyWithReviewedBy) { + let message_id = details.representative_patch.message_id().href.clone(); + debug!(msg_id = message_id, "executing reviewed-by reply"); + let successful_indexes = self .state .user_state .reviewed_patchsets - .remove(&representative_patch.message_id().href) + .remove(&message_id) .unwrap_or_default(); - - let (git_user_name, git_user_email) = self - .services - .lore_api - .get_git_signature(String::new()) - .await - .map_err(|e| eyre!("{e:#?}"))?; - - if git_user_name.is_empty() || git_user_email.is_empty() { - println!("`git config user.name` or `git config user.email` not set\nAborting..."); - } else { - let mktemp_cmd = ShellCommand::new("mktemp").arg("--directory"); - let tmp_out = self - .services - .shell - .execute(&mktemp_cmd) - .map_err(|e| eyre!("failed to create temp directory: {}", e))?; - let tmp_dir_str = std::str::from_utf8(&tmp_out.stdout) - .map_err(|e| eyre!("invalid utf-8 in temp dir path: {}", e))? - .trim() - .to_string(); - let tmp_dir = PathBuf::from(tmp_dir_str); - - let git_signature = format!("{git_user_name} <{git_user_email}>"); - let git_reply_commands = self - .services - .lore_api - .prepare_reply_commands( - tmp_dir, - "all".to_string(), - raw_patches, - patches_to_reply.clone(), - git_signature, - self.state.config.git_send_email_options().to_string(), - ) - .await - .map_err(|e| eyre!("{e:#?}"))?; - - let reply_indexes: Vec = patches_to_reply - .iter() - .enumerate() - .filter_map(|(i, &val)| if val { Some(i) } else { None }) - .collect(); - for (i, command) in git_reply_commands.into_iter().enumerate() { - let success = self - .services - .shell - .spawn_interactive(&command) - .unwrap_or(false); - if success { - successful_indexes.insert(reply_indexes[i]); - } - } - } - - self.state.user_state.reviewed_patchsets.insert( - representative_patch.message_id().href.clone(), + let request = reviewed_reply_request( + details, successful_indexes, + self.state.config.git_send_email_options().to_string(), ); + let action_service = PatchsetActionService::new( + &*self.services.fs, + &*self.services.shell, + &self.services.lore_api, + ); + let result = action_service.execute_reviewed_reply(request).await?; + + self.state + .user_state + .reviewed_patchsets + .insert(message_id.clone(), result.into_successful_indexes()); self.services .lore_api @@ -430,7 +384,7 @@ impl App { .map_err(|e| eyre!("{e:#?}"))?; info!( - msg_id = representative_patch.message_id().href, + msg_id = message_id, "reviewed-by reply sent and state persisted" ); self.state @@ -444,27 +398,22 @@ impl App { } fn execute_apply_patchset(&mut self) { - if let Some(true) = self + let details = self .state .lore .details .as_ref() - .expect("invariant: details must be loaded before executing apply patchset") - .patchset_actions - .get(&PatchsetAction::Apply) - { + .expect("invariant: details must be loaded before executing apply patchset"); + + if patchset_action_selected(details, &PatchsetAction::Apply) { debug!("applying patchset via git-am"); - let popup = match self - .state - .lore - .details - .as_ref() - .expect("invariant: details must be loaded before applying patchset") - .apply_patchset( - &*self.services.fs, - &*self.services.shell, - &self.state.config, - ) { + let request = apply_patchset_request(details); + let action_service = PatchsetActionService::new( + &*self.services.fs, + &*self.services.shell, + &self.services.lore_api, + ); + let popup = match action_service.apply_patchset(&request, &self.state.config) { Ok(msg) => popup::AppPopup::info("Patchset Apply Success", msg), Err(msg) => popup::AppPopup::info("Patchset Apply Fail", msg), }; @@ -490,7 +439,7 @@ impl App { } /// Applies edited values from [`ConfigUiState::edit_config`] into [`AppState::config`]. - pub async fn consolidate_edit_config(&mut self) -> color_eyre::Result<()> { + pub async fn consolidate_edit_config(&mut self) -> Result<()> { if let Some(edit_config) = &self.state.config_state.edit_config { debug!("validating and applying config update"); let draft = edit_config.to_update_draft(); @@ -518,3 +467,123 @@ impl App { view_model::project_state(&self.state) } } + +fn patchset_action_selected(details: &PatchsetDetailsState, action: &PatchsetAction) -> bool { + matches!(details.patchset_actions.get(action), Some(true)) +} + +fn reviewed_reply_request( + details: &PatchsetDetailsState, + successful_indexes: std::collections::HashSet, + git_send_email_options: String, +) -> ReviewedReplyRequest { + ReviewedReplyRequest { + raw_patches: details.raw_patches.clone(), + patches_to_reply: details.patches_to_reply.clone(), + successful_indexes, + git_send_email_options, + } +} + +fn apply_patchset_request(details: &PatchsetDetailsState) -> ApplyPatchsetRequest { + ApplyPatchsetRequest { + patch_title: details.representative_patch.title().clone(), + patchset_path: details.patchset_path.clone(), + } +} + +#[cfg(test)] +mod tests { + use std::collections::{HashMap, HashSet}; + + use serde_xml_rs::from_str; + + use super::*; + + fn test_patch() -> Patch { + from_str( + r#" + + + Foo Bar + foo@bar.foo.bar + + [PATCH 1/1] test patch + 2024-07-06T19:15:48Z + + urn:uuid:123-abcd-1f2a3b + + + "#, + ) + .expect("test patch XML should deserialize") + } + + fn details_state() -> PatchsetDetailsState { + PatchsetDetailsState { + representative_patch: test_patch(), + raw_patches: vec!["raw patch 0".to_string(), "raw patch 1".to_string()], + patches_preview: vec!["preview 0".to_string(), "preview 1".to_string()], + has_cover_letter: false, + patches_to_reply: vec![false, true], + patchset_path: "/tmp/patchset.mbx".to_string(), + preview_index: 0, + preview_scroll_offset: 0, + preview_pan: 0, + preview_fullscreen: false, + patchset_actions: HashMap::from([ + (PatchsetAction::Bookmark, false), + (PatchsetAction::ReplyWithReviewedBy, true), + (PatchsetAction::Apply, true), + ]), + reviewed_by: vec![HashSet::new(), HashSet::new()], + tested_by: vec![HashSet::new(), HashSet::new()], + acked_by: vec![HashSet::new(), HashSet::new()], + last_screen: CurrentScreen::LatestPatchsets, + } + } + + #[test] + fn patchset_action_selected_reads_action_map() { + let mut details = details_state(); + + assert!(patchset_action_selected(&details, &PatchsetAction::Apply)); + assert!(patchset_action_selected( + &details, + &PatchsetAction::ReplyWithReviewedBy + )); + + details + .patchset_actions + .insert(PatchsetAction::Apply, false); + + assert!(!patchset_action_selected(&details, &PatchsetAction::Apply)); + } + + #[test] + fn reviewed_reply_request_copies_reply_inputs() { + let details = details_state(); + let request = reviewed_reply_request( + &details, + HashSet::from([4usize]), + "--dry-run --suppress-cc=all".to_string(), + ); + + assert_eq!(details.raw_patches, request.raw_patches); + assert_eq!(details.patches_to_reply, request.patches_to_reply); + assert_eq!(HashSet::from([4]), request.successful_indexes); + assert_eq!( + "--dry-run --suppress-cc=all", + request.git_send_email_options + ); + } + + #[test] + fn apply_patchset_request_copies_apply_inputs() { + let details = details_state(); + let request = apply_patchset_request(&details); + + assert_eq!("[PATCH 1/1] test patch", request.patch_title); + assert_eq!("/tmp/patchset.mbx", request.patchset_path); + } +} diff --git a/src/app/popup.rs b/src/app/popup.rs index 139a393..75157e4 100644 --- a/src/app/popup.rs +++ b/src/app/popup.rs @@ -4,11 +4,12 @@ //! Each variant owns all data required to present the popup and tracks scroll //! position so the presentation layer receives a read-only snapshot. -use crate::{app::screens::details_actions::PatchsetDetailsState, input::event::InputEvent}; +use std::collections::HashSet; -// --------------------------------------------------------------------------- -// Main enum -// --------------------------------------------------------------------------- +use crate::{ + app::screens::details_actions::PatchsetDetailsState, input::event::InputEvent, + lore::domain::patch::Author, +}; /// Concrete, cloneable popup state stored in `AppState`. #[derive(Clone, Debug)] @@ -39,10 +40,6 @@ pub enum AppPopup { } impl AppPopup { - // ----------------------------------------------------------------------- - // Factories - // ----------------------------------------------------------------------- - /// Create an informational text popup. pub fn info(title: impl Into, body: impl Into) -> Self { let title = title.into(); @@ -76,18 +73,17 @@ impl AppPopup { let i = details.preview_index; let mut columns: usize = 0; - let mut format_section = - |authors: &std::collections::HashSet| -> String { - let mut text = String::new(); - for author in authors { - let line = format!(" - {author}\n"); - if line.len() > columns { - columns = line.len(); - } - text.push_str(&line); + let mut format_section = |authors: &HashSet| -> String { + let mut text = String::new(); + for author in authors { + let line = format!(" - {author}\n"); + if line.len() > columns { + columns = line.len(); } - text - }; + text.push_str(&line); + } + text + }; let reviewed_by = format_section(&details.reviewed_by[i]); let tested_by = format_section(&details.tested_by[i]); @@ -107,11 +103,6 @@ impl AppPopup { dimensions: (50, 40), } } - - // ----------------------------------------------------------------------- - // Input handling - // ----------------------------------------------------------------------- - /// Advance scroll position in response to a navigation input. /// /// All popup variants share identical two-axis scroll semantics. @@ -151,11 +142,6 @@ impl AppPopup { } } } - -// --------------------------------------------------------------------------- -// Help builder -// --------------------------------------------------------------------------- - /// Fluent builder for `AppPopup::Help`. /// /// Mirrors the API of the old `HelpPopUpBuilder` so handler call sites change diff --git a/src/app/screens/details_actions.rs b/src/app/screens/details_actions.rs index 7218225..d07f1d2 100644 --- a/src/app/screens/details_actions.rs +++ b/src/app/screens/details_actions.rs @@ -1,12 +1,8 @@ use std::collections::{HashMap, HashSet}; -use std::path::Path; + +use ansi_to_tui::IntoText; use crate::{ - config::{ConfigSnapshot, KernelTree}, - infrastructure::{ - file_system::FileSystemTrait, - shell::{ShellCommand, ShellTrait}, - }, lore::{ application::dto::PatchsetDetails, domain::patch::{Author, Patch}, @@ -48,6 +44,10 @@ pub struct PatchsetDetailsState { const LAST_LINE_PADDING: usize = 10; +fn rendered_preview_height(preview: &str) -> usize { + preview.into_text().unwrap_or_default().height() +} + #[derive(Clone, Hash, Eq, PartialEq)] pub enum PatchsetAction { Bookmark, @@ -123,8 +123,7 @@ impl PatchsetDetailsState { /// Scroll `n` lines down pub fn preview_scroll_down(&mut self, n: usize) { - // TODO: Support for renderers (only considers base preview string) - let number_of_lines = self.patches_preview[self.preview_index].lines().count(); + let number_of_lines = rendered_preview_height(&self.patches_preview[self.preview_index]); if (self.preview_scroll_offset + n) <= number_of_lines { self.preview_scroll_offset += n; } @@ -137,8 +136,7 @@ impl PatchsetDetailsState { /// Scroll to the last line pub fn go_to_last_line(&mut self) { - // TODO: Support for renderers (only considers base preview string) - let number_of_lines = self.patches_preview[self.preview_index].lines().count(); + let number_of_lines = rendered_preview_height(&self.patches_preview[self.preview_index]); self.preview_scroll_offset = number_of_lines.saturating_sub(LAST_LINE_PADDING); } @@ -219,257 +217,81 @@ impl PatchsetDetailsState { pub fn actions_require_user_io(&self) -> bool { self.patches_to_reply.contains(&true) } +} - /// Checks if there is a `target_kernel_tree` and if it is in `Config::kernel_trees` and if - /// that kernel tree is a valid git directory. - /// - /// Returns the a valid `KernelTree` or a `String` with the error message on failure. - fn validate_kernel_tree<'a>( - &self, - fs: &dyn FileSystemTrait, - config: &'a ConfigSnapshot, - ) -> Result<&'a KernelTree, String> { - let kernel_tree_id = if let Some(target) = config.target_kernel_tree() { - target - } else { - return Err("target kernel tree unset".to_string()); - }; - - let kernel_tree = if let Some(tree) = config.get_kernel_tree(kernel_tree_id) { - tree - } else { - return Err(format!("invalid target kernel tree '{kernel_tree_id}'")); - }; - - let kernel_tree_path = Path::new(kernel_tree.path()); - if !fs.is_dir(kernel_tree_path) { - return Err(format!("{} isn't a directory", kernel_tree.path())); - } else if !fs.is_dir(&kernel_tree_path.join(".git")) { - return Err(format!("{} isn't a git repository", kernel_tree.path())); - } - - Ok(kernel_tree) - } - - // Ensures the kernel directory is not currently in another git operation, - // that it does not have unstaged or uncommited changes, and that the base branch - // is valid. - // - // Returns `()` on success and a `String` with an error message on failure. - fn check_git_state( - &self, - fs: &dyn FileSystemTrait, - shell: &dyn ShellTrait, - kernel_tree: &KernelTree, - ) -> Result<(), String> { - let kernel_tree_path = Path::new(kernel_tree.path()); - - if fs.is_dir(&kernel_tree_path.join(".git/rebase-merge")) { - return Err( - "rebase in progress. \nrun `git rebase --abort` before continuing".to_string(), - ); - } else if fs.is_file(&kernel_tree_path.join(".git/MERGE_HEAD")) { - return Err( - "merge in progress. \nrun `git merge --abort` before continuing".to_string(), - ); - } else if fs.is_file(&kernel_tree_path.join(".git/BISECT_LOG")) { - return Err( - "bisect in progress. \nrun `git bisect reset` before continuing".to_string(), - ); - } else if fs.is_dir(&kernel_tree_path.join(".git/rebase-apply")) { - return Err( - "`git am` already in progress. \nrun `git am --abort` before continuing" - .to_string(), - ); - } - - let status_out = shell - .execute( - &ShellCommand::new("git") - .arg("-C") - .arg(kernel_tree.path()) - .args(["status", "--porcelain"]), - ) - .map_err(|e| format!("failed to check git status {e}"))?; - - let status_output = String::from_utf8_lossy(&status_out.stdout); - if !status_output.is_empty() { - return Err(format!( - "there are staged and/or unstaged changes\n{status_output}" - )); - } - - let show_ref_out = shell - .execute( - &ShellCommand::new("git") - .arg("-C") - .arg(kernel_tree.path()) - .args(["show-ref", "--verify", "--quiet"]) - .arg(format!("refs/heads/{}", kernel_tree.branch())), - ) - .map_err(|e| format!("failed to verify branch: {e}"))?; - - if !show_ref_out.success { - return Err(format!( - "invalid branch '{}' for '{}'", - kernel_tree.branch(), - kernel_tree.path() - )); - } - - Ok(()) - } - - /// Get the current branch of the supplied kernel tree - /// - /// Returns the branch name as a `String` or a `String` with the error message on failure - fn get_current_branch( - &self, - shell: &dyn ShellTrait, - kernel_tree: &KernelTree, - ) -> Result { - let out = shell - .execute( - &ShellCommand::new("git") - .arg("-C") - .arg(kernel_tree.path()) - .args(["rev-parse", "--abbrev-ref", "HEAD"]), - ) - .map_err(|e| format!("failed to get current branch: {e}"))?; - - let mut branch = String::from_utf8_lossy(&out.stdout).to_string(); - branch.pop(); - Ok(branch) +#[cfg(test)] +mod tests { + use std::collections::{HashMap, HashSet}; + + use serde_xml_rs::from_str; + + use super::*; + + fn test_patch() -> Patch { + from_str( + r#" + + + Foo Bar + foo@bar.foo.bar + + [PATCH 1/1] test patch + 2024-07-06T19:15:48Z + + urn:uuid:123-abcd-1f2a3b + + + "#, + ) + .expect("test patch XML should deserialize") } - /// Switch the supplied kernel tree to the supplied branch, if it exists. - /// - /// Returns `()` on sucess and a `String` with the error message on failure. - fn switch_to_branch( - &self, - shell: &dyn ShellTrait, - kernel_tree: &KernelTree, - branch: &str, - ) -> Result<(), String> { - let out = shell - .execute( - &ShellCommand::new("git") - .arg("-C") - .arg(kernel_tree.path()) - .args(["switch", branch]), - ) - .map_err(|e| format!("failed to switch branch: {e}"))?; - - if !out.success { - return Err(format!( - "failed to switch to branch '{}': {}", - branch, - String::from_utf8_lossy(&out.stderr) - )); + fn details_state_with_preview(preview: &str) -> PatchsetDetailsState { + PatchsetDetailsState { + representative_patch: test_patch(), + raw_patches: vec!["raw patch".to_string()], + patches_preview: vec![preview.to_string()], + has_cover_letter: false, + patches_to_reply: vec![false], + patchset_path: "/tmp/patchset.mbx".to_string(), + preview_index: 0, + preview_scroll_offset: 0, + preview_pan: 0, + preview_fullscreen: false, + patchset_actions: HashMap::from([ + (PatchsetAction::Bookmark, false), + (PatchsetAction::ReplyWithReviewedBy, false), + (PatchsetAction::Apply, false), + ]), + reviewed_by: vec![HashSet::new()], + tested_by: vec![HashSet::new()], + acked_by: vec![HashSet::new()], + last_screen: CurrentScreen::LatestPatchsets, } - - Ok(()) } - /// Create a new branch suffixed with the current timestamp. - /// - /// Returns a `String` with the branch name on success or the - /// error message on failure. - fn create_target_branch( - &self, - shell: &dyn ShellTrait, - kernel_tree: &KernelTree, - config: &ConfigSnapshot, - ) -> Result { - self.switch_to_branch(shell, kernel_tree, kernel_tree.branch())?; - - let target_branch_name = format!( - "{}{}", - config.git_am_branch_prefix(), - chrono::Utc::now().format("%Y-%m-%d-%H-%M-%S") - ); - - let out = shell - .execute( - &ShellCommand::new("git") - .arg("-C") - .arg(kernel_tree.path()) - .args(["checkout", "-b", &target_branch_name]), - ) - .map_err(|e| format!("failed to create target branch: {e}"))?; - - if !out.success { - return Err(format!( - "failed to create branch '{}': {}", - target_branch_name, - String::from_utf8_lossy(&out.stderr) - )); - } + #[test] + fn rendered_height_accounts_for_rendered_text_projection() { + let preview = "\u{1b}[32mrendered line\u{1b}[0m\nsecond line"; - Ok(target_branch_name) + assert_eq!(2, rendered_preview_height(preview)); } - /// Apply the selected patchset on the given `kernel_tree` with arguments from `Config` - /// - /// Returns `()` on sucess and a `String` containing the error message on failure. - fn run_git_am( - &self, - shell: &dyn ShellTrait, - kernel_tree: &KernelTree, - config: &ConfigSnapshot, - ) -> Result<(), String> { - let mut git_am_cmd = ShellCommand::new("git") - .arg("-C") - .arg(kernel_tree.path()) - .args(["am", &self.patchset_path]); - for opt in config.git_am_options().split_whitespace() { - git_am_cmd = git_am_cmd.arg(opt); - } + #[test] + fn preview_scroll_down_uses_rendered_height() { + let mut state = details_state_with_preview("\u{1b}[32mrendered line\u{1b}[0m\nsecond line"); - let out = shell - .execute(&git_am_cmd) - .map_err(|e| format!("failed to execute git-am: {e}"))?; + state.preview_scroll_down(2); - if !out.success { - let _ = shell.execute( - &ShellCommand::new("git") - .arg("-C") - .arg(kernel_tree.path()) - .args(["am", "--abort"]), - ); + assert_eq!(2, state.preview_scroll_offset); + } - return Err(String::from_utf8_lossy(&out.stderr).to_string()); - } + #[test] + fn go_to_last_line_saturates_for_short_rendered_preview() { + let mut state = details_state_with_preview("short preview"); - Ok(()) - } + state.go_to_last_line(); - /// Try to apply the patchset to a target kernel tree and returns a `String` - /// informing if the apply succeeded or failed and why. - /// - /// Returns a `Result` containing either the success or the error message. - /// # TODO: - /// - Add unit tests - pub fn apply_patchset( - &self, - fs: &dyn FileSystemTrait, - shell: &dyn ShellTrait, - config: &ConfigSnapshot, - ) -> Result { - let kernel_tree = self.validate_kernel_tree(fs, config)?; - self.check_git_state(fs, shell, kernel_tree)?; - - let original_branch = self.get_current_branch(shell, kernel_tree)?; - let target_branch = self.create_target_branch(shell, kernel_tree, config)?; - - let git_am_result = self.run_git_am(shell, kernel_tree, config); - self.switch_to_branch(shell, kernel_tree, &original_branch)?; - - match git_am_result { - Ok(_) => { - Ok(format!(" Patchset '{}' applied successfully!\n\n - Kernel Tree: '{}'\n\n - Base Branch: '{}'\n\n - Applied branch: '{}'", self.representative_patch.title(), kernel_tree.path(), kernel_tree.branch(), &target_branch)) - }, - Err(e) => Err(format!( "`git am` failed\n{}{}", &original_branch, e)) - } + assert_eq!(0, state.preview_scroll_offset); } } diff --git a/src/app/screens/edit_config.rs b/src/app/screens/edit_config.rs index a629cb7..ef1e825 100644 --- a/src/app/screens/edit_config.rs +++ b/src/app/screens/edit_config.rs @@ -1,7 +1,11 @@ -use color_eyre::eyre::bail; +use color_eyre::{eyre::bail, Report}; use derive_getters::Getters; -use std::{collections::HashMap, fmt::Display}; +use std::{ + collections::HashMap, + fmt::{self, Display, Formatter}, + mem, +}; use crate::config::{ConfigSnapshot, ConfigUpdateDraft}; @@ -109,7 +113,7 @@ impl EditConfigState { pub fn stage_edit(&mut self) { if let Ok(editable_config) = EditableConfig::try_from(self.highlighted) { self.config_buffer - .insert(editable_config, std::mem::take(&mut self.curr_edit)); + .insert(editable_config, mem::take(&mut self.curr_edit)); } } @@ -150,7 +154,7 @@ enum EditableConfig { } impl TryFrom for EditableConfig { - type Error = color_eyre::Report; + type Error = Report; fn try_from(value: usize) -> Result { match value { @@ -168,7 +172,7 @@ impl TryFrom for EditableConfig { } impl Display for EditableConfig { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { EditableConfig::PageSize => write!(f, "Page Size"), EditableConfig::CacheDir => write!(f, "Cache Directory"), diff --git a/src/app/screens/latest.rs b/src/app/screens/latest.rs index 18e5dcb..0d59b24 100644 --- a/src/app/screens/latest.rs +++ b/src/app/screens/latest.rs @@ -1,4 +1,4 @@ -use color_eyre::eyre::bail; +use color_eyre::{eyre::bail, Result}; use crate::lore::{ application::{cache::CacheMode, errors::LoreError, handle::LoreApiHandle}, @@ -43,7 +43,7 @@ impl LatestPatchsetsState { &mut self, lore_api: &LoreApiHandle, mode: CacheMode, - ) -> color_eyre::Result<()> { + ) -> Result<()> { match lore_api .fetch_feed_page( self.target_list.clone(), diff --git a/src/app/screens/mail_list.rs b/src/app/screens/mail_list.rs index e6dff7d..8911930 100644 --- a/src/app/screens/mail_list.rs +++ b/src/app/screens/mail_list.rs @@ -1,4 +1,4 @@ -use color_eyre::eyre::bail; +use color_eyre::{eyre::bail, Result}; use crate::lore::{ application::{cache::CacheMode, handle::LoreApiHandle}, @@ -18,7 +18,7 @@ impl MailingListSelectionState { &mut self, lore_api: &LoreApiHandle, mode: CacheMode, - ) -> color_eyre::Result<()> { + ) -> Result<()> { match lore_api.fetch_available_lists(mode).await { Ok(available_mailing_lists) => { self.mailing_lists = available_mailing_lists; diff --git a/src/app/updates.rs b/src/app/updates.rs index f58c722..826ee22 100644 --- a/src/app/updates.rs +++ b/src/app/updates.rs @@ -1,3 +1,5 @@ +use color_eyre::Result; + use crate::app::{loading::LoadingIndicator, screens::CurrentScreen, App}; impl App { @@ -5,7 +7,7 @@ impl App { pub async fn process_system_updates( &mut self, loading: &mut (dyn LoadingIndicator + Send), - ) -> color_eyre::Result<()> { + ) -> Result<()> { match self.state.navigation.current_screen { CurrentScreen::MailingListSelection => { if self diff --git a/src/app/view_model.rs b/src/app/view_model.rs index c130f0a..1a5ecfc 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -16,10 +16,6 @@ use super::{ state::AppState, }; -// --------------------------------------------------------------------------- -// Shared row / helper types -// --------------------------------------------------------------------------- - /// One mailing list entry shown in the selection list. #[derive(Clone, Debug)] pub struct MailingListEntry { @@ -71,10 +67,6 @@ pub struct ConfigEntryRow { pub edit_cursor_value: String, } -// --------------------------------------------------------------------------- -// Per-screen view models -// --------------------------------------------------------------------------- - #[derive(Clone, Debug)] pub struct MailingListSelectionViewModel { pub entries: Vec, @@ -129,10 +121,6 @@ pub struct EditConfigViewModel { pub is_editing_mode: bool, } -// --------------------------------------------------------------------------- -// Popup view model -// --------------------------------------------------------------------------- - #[derive(Clone, Debug)] pub enum PopupViewBody { Text(String), @@ -156,10 +144,6 @@ pub struct PopupViewModel { pub dimensions: (u16, u16), } -// --------------------------------------------------------------------------- -// Discriminated screen enum -// --------------------------------------------------------------------------- - #[derive(Clone, Debug)] pub enum ScreenViewModel { MailingListSelection(MailingListSelectionViewModel), @@ -169,10 +153,6 @@ pub enum ScreenViewModel { EditConfig(EditConfigViewModel), } -// --------------------------------------------------------------------------- -// Top-level view model -// --------------------------------------------------------------------------- - /// Owned, typed projection of [`AppState`] for one TUI frame. /// /// Built by [`project_state`]; consumed by `UiCore::build_scene`. @@ -182,10 +162,6 @@ pub struct AppViewModel { pub popup: Option, } -// --------------------------------------------------------------------------- -// Projection – public entry point -// --------------------------------------------------------------------------- - /// Projects `state` into an owned [`AppViewModel`]. /// /// Called via [`super::App::present`]. @@ -194,11 +170,6 @@ pub fn project_state(state: &AppState) -> AppViewModel { let popup = state.popup.as_ref().map(project_popup); AppViewModel { screen, popup } } - -// --------------------------------------------------------------------------- -// Private per-screen projectors -// --------------------------------------------------------------------------- - fn project_screen(state: &AppState) -> ScreenViewModel { match state.navigation.current_screen { CurrentScreen::MailingListSelection => { diff --git a/src/cli.rs b/src/cli.rs index 59515f7..7ff96b2 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,5 +1,6 @@ use clap::Parser; -use color_eyre::eyre::eyre; +use color_eyre::{eyre::eyre, Result}; +use serde_json::to_string_pretty; use std::ops::ControlFlow; @@ -17,9 +18,9 @@ impl Cli { /// 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, config: &ConfigSnapshot) -> ControlFlow, ()> { + pub fn resolve(&self, config: &ConfigSnapshot) -> ControlFlow, ()> { if self.show_configs { - match serde_json::to_string_pretty(&config) { + match to_string_pretty(&config) { Err(err) => return ControlFlow::Break(Err(eyre!(err))), Ok(config) => println!("patch-hub configurations:\n{config}"), } diff --git a/src/config/actor.rs b/src/config/actor.rs index a7f56b7..ccb2b50 100644 --- a/src/config/actor.rs +++ b/src/config/actor.rs @@ -5,7 +5,10 @@ //! and persists successful updates through [`JsonConfigRepository`](crate::config::JsonConfigRepository). use std::ops::ControlFlow; -use tokio::sync::{mpsc, oneshot}; +use tokio::{ + spawn, + sync::{mpsc, oneshot}, +}; use crate::{ config::{ @@ -45,7 +48,7 @@ where channel_size = DEFAULT_CONFIG_CHANNEL_SIZE, "spawning config actor" ); - tokio::spawn(Self::new(state, repo, rx).run()); + spawn(Self::new(state, repo, rx).run()); ConfigHandle::new(tx) } diff --git a/src/config/repository.rs b/src/config/repository.rs index 5be91d5..67c9a37 100644 --- a/src/config/repository.rs +++ b/src/config/repository.rs @@ -1,5 +1,7 @@ use std::path::Path; +use serde_json::to_writer_pretty; + use crate::config::errors::ConfigError; use crate::config::state::ConfigState; use crate::config::DEFAULT_CONFIG_PATH_SUFFIX; @@ -57,8 +59,7 @@ impl ConfigRepository for JsonConfigRepository { .fs .create_writer(Path::new(&tmp_filename)) .map_err(ConfigError::from)?; - serde_json::to_writer_pretty(tmp_file, state) - .map_err(|e| ConfigError::Save(e.to_string()))?; + to_writer_pretty(tmp_file, state).map_err(|e| ConfigError::Save(e.to_string()))?; } self.fs .rename(Path::new(&tmp_filename), config_path) diff --git a/src/config/service.rs b/src/config/service.rs index 5a77420..d1e54ee 100644 --- a/src/config/service.rs +++ b/src/config/service.rs @@ -1,5 +1,7 @@ use std::path::Path; +use serde_json::from_str; + use crate::config::env_overrides; use crate::config::errors::ConfigError; use crate::config::parsing::{parse_cover_renderer, parse_patch_renderer}; @@ -32,7 +34,7 @@ fn load_initial_state( let fs = repo.fs(); if fs.is_file(Path::new(path)) { match fs.read_to_string(Path::new(path)) { - Ok(file_contents) => match serde_json::from_str(&file_contents) { + Ok(file_contents) => match from_str(&file_contents) { Ok(config) => return config, Err(e) => eprintln!("Failed to parse config file {path}: {e}"), }, diff --git a/src/config/state.rs b/src/config/state.rs index a71bbb2..963d965 100644 --- a/src/config/state.rs +++ b/src/config/state.rs @@ -1,7 +1,10 @@ use derive_getters::Getters; use patch_hub_proc_macros::serde_individual_default; use serde::Serialize; -use std::collections::{HashMap, HashSet}; +use std::{ + collections::{HashMap, HashSet}, + env, +}; use crate::config::update::ValidatedConfigUpdate; use crate::infrastructure::env::EnvTrait; @@ -39,7 +42,7 @@ pub struct ConfigState { impl Default for ConfigState { fn default() -> Self { - let home = std::env::var("HOME").unwrap_or_else(|_| { + let home = env::var("HOME").unwrap_or_else(|_| { eprintln!("$HOME environment variable not set, using current directory"); ".".to_string() }); diff --git a/src/infrastructure/errors.rs b/src/infrastructure/errors.rs index d07356d..452e926 100644 --- a/src/infrastructure/errors.rs +++ b/src/infrastructure/errors.rs @@ -1,4 +1,9 @@ -use std::panic; +use std::{error::Error, panic}; + +use color_eyre::{ + config::HookBuilder, + eyre::{set_hook, Result}, +}; use super::terminal::restore; @@ -8,8 +13,8 @@ use super::terminal::restore; /// 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(); +pub fn install_hooks() -> Result<()> { + let (panic_hook, eyre_hook) = HookBuilder::default().into_hooks(); // convert from a color_eyre PanicHook to a standard panic hook let panic_hook = panic_hook.into_panic_hook(); @@ -20,12 +25,10 @@ pub fn install_hooks() -> color_eyre::Result<()> { // convert from a color_eyre EyreHook to a eyre ErrorHook let eyre_hook = eyre_hook.into_eyre_hook(); - color_eyre::eyre::set_hook(Box::new( - move |error: &(dyn std::error::Error + 'static)| { - restore().unwrap(); - eyre_hook(error) - }, - ))?; + set_hook(Box::new(move |error: &(dyn Error + 'static)| { + restore().unwrap(); + eyre_hook(error) + }))?; Ok(()) } diff --git a/src/infrastructure/file_system/trait.rs b/src/infrastructure/file_system/trait.rs index 3782b0e..c1a1fb5 100644 --- a/src/infrastructure/file_system/trait.rs +++ b/src/infrastructure/file_system/trait.rs @@ -1,7 +1,7 @@ use mockall::automock; use thiserror::Error; -use std::{io, path::Path}; +use std::{fs::Metadata, io, path::Path}; #[derive(Debug, Error)] pub enum FileSystemError { @@ -20,5 +20,5 @@ pub trait FileSystemTrait: Send + Sync { fn rename(&self, from: &Path, to: &Path) -> Result<(), FileSystemError>; fn create_writer(&self, path: &Path) -> Result, FileSystemError>; fn open_bufreader(&self, path: &Path) -> Result, FileSystemError>; - fn metadata(&self, path: &Path) -> Result; + fn metadata(&self, path: &Path) -> Result; } diff --git a/src/infrastructure/monitoring/logging/garbage_collector.rs b/src/infrastructure/monitoring/logging/garbage_collector.rs index 41ab09c..48f3845 100644 --- a/src/infrastructure/monitoring/logging/garbage_collector.rs +++ b/src/infrastructure/monitoring/logging/garbage_collector.rs @@ -2,6 +2,8 @@ //! //! This module is responsible for cleaning up the log files. +use std::{fs, time::SystemTime}; + use tracing::{event, Level}; use crate::config::ConfigSnapshot; @@ -13,9 +15,9 @@ pub fn collect_garbage(config: &ConfigSnapshot) { return; } - let now = std::time::SystemTime::now(); + let now = SystemTime::now(); let logs_path = config.logs_path(); - let Ok(logs) = std::fs::read_dir(logs_path) else { + let Ok(logs) = fs::read_dir(logs_path) else { event!( Level::ERROR, "Failed to read the logs directory during garbage collection" @@ -43,7 +45,7 @@ pub fn collect_garbage(config: &ConfigSnapshot) { }; let age = age.as_secs() / 60 / 60 / 24; - if age as usize > config.max_log_age() && std::fs::remove_file(log.path()).is_err() { + if age as usize > config.max_log_age() && fs::remove_file(log.path()).is_err() { event!( Level::WARN, "Failed to remove the log file: {}", diff --git a/src/infrastructure/monitoring/logging/mod.rs b/src/infrastructure/monitoring/logging/mod.rs index bd5fb04..312c593 100644 --- a/src/infrastructure/monitoring/logging/mod.rs +++ b/src/infrastructure/monitoring/logging/mod.rs @@ -3,6 +3,7 @@ use std::{ sync::{Arc, Mutex}, }; +use chrono::Local; use multi_log_file_writer::{create_non_blocking_writer, get_fmt_layer, MultiLogFileWriter}; use tracing_appender::non_blocking::WorkerGuard; use tracing_subscriber::{reload::Handle, Layer, Registry}; @@ -43,10 +44,7 @@ pub fn init_logging_layer() -> InitLoggingLayerProduct { } fn init_logging_file_writers() -> InitLoggingFileWritersProduct { - let timestamp_log_file_name = format!( - "patch-hub_{}.log", - chrono::Local::now().format("%Y%m%d-%H%M%S") - ); + let timestamp_log_file_name = format!("patch-hub_{}.log", Local::now().format("%Y%m%d-%H%M%S")); // logging thread should be non-blocking so it does not interfere with the rest of the application let (timestamp_log_writer, timestamp_log_writer_guard) = diff --git a/src/infrastructure/monitoring/logging/multi_log_file_writer.rs b/src/infrastructure/monitoring/logging/multi_log_file_writer.rs index 87159d5..44caba6 100644 --- a/src/infrastructure/monitoring/logging/multi_log_file_writer.rs +++ b/src/infrastructure/monitoring/logging/multi_log_file_writer.rs @@ -1,9 +1,11 @@ use std::{ collections::HashMap, - fs::{File, OpenOptions}, - io::Write, + fs::{self, File, OpenOptions}, + io::{self, Write}, path::Path, sync::{Arc, Mutex}, + thread, + time::Duration, }; use tracing::{event, Level}; @@ -68,7 +70,7 @@ impl MultiLogFileWriter { if let Some(current_guard) = current_guards_by_file_name.remove(file_name) { drop(current_guard); // making sure we'll flush everything by the time we copy old file contents - std::thread::sleep(std::time::Duration::from_millis(50)); + thread::sleep(Duration::from_millis(50)); } let (file_writer, file_writer_guard) = @@ -104,7 +106,7 @@ impl MultiLogFileWriter { }; if let Some(parent_dir) = Path::new(&new_log_file_path).parent() { // Create new log dir if it doesn't exist - std::fs::create_dir_all(parent_dir).expect("to create dir"); + fs::create_dir_all(parent_dir).expect("to create dir"); } let Ok(mut new_log_file_content) = OpenOptions::new() .create(true) @@ -119,7 +121,7 @@ impl MultiLogFileWriter { return; }; - let copy_result = std::io::copy(&mut old_log_file_content, &mut new_log_file_content); + let copy_result = io::copy(&mut old_log_file_content, &mut new_log_file_content); if let Err(err) = copy_result { event!(Level::ERROR, "Could not copy old file logs: {}", err); }; @@ -127,7 +129,7 @@ impl MultiLogFileWriter { } impl Write for MultiLogFileWriter { - fn write(&mut self, buf: &[u8]) -> std::io::Result { + fn write(&mut self, buf: &[u8]) -> io::Result { for writer in self.writer_by_file_name.values() { writer .lock() @@ -138,7 +140,7 @@ impl Write for MultiLogFileWriter { Ok(buf.len()) } - fn flush(&mut self) -> std::io::Result<()> { + fn flush(&mut self) -> io::Result<()> { for writer in self.writer_by_file_name.values() { writer .lock() diff --git a/src/infrastructure/terminal.rs b/src/infrastructure/terminal.rs index ec42680..7c78a0b 100644 --- a/src/infrastructure/terminal.rs +++ b/src/infrastructure/terminal.rs @@ -15,6 +15,7 @@ use ratatui::{ Terminal, }; +use color_eyre::Result; use std::io::{self, stdout, Stdout}; /// A type alias for the terminal type used in this application @@ -34,7 +35,7 @@ pub fn restore() -> io::Result<()> { Ok(()) } -pub(crate) fn setup_user_io(terminal: &mut Terminal) -> color_eyre::Result<()> { +pub(crate) fn setup_user_io(terminal: &mut Terminal) -> Result<()> { terminal.clear()?; terminal.set_cursor_position(Position::new(0, 0))?; terminal.show_cursor()?; @@ -42,7 +43,7 @@ pub(crate) fn setup_user_io(terminal: &mut Terminal) -> color_eyr Ok(()) } -pub(crate) fn teardown_user_io(terminal: &mut Terminal) -> color_eyre::Result<()> { +pub(crate) fn teardown_user_io(terminal: &mut Terminal) -> Result<()> { enable_raw_mode()?; terminal.clear()?; Ok(()) diff --git a/src/input/actor.rs b/src/input/actor.rs index 3710720..71d6182 100644 --- a/src/input/actor.rs +++ b/src/input/actor.rs @@ -12,7 +12,7 @@ //! change key bindings without restarting the pump. use std::time::Duration; -use tokio::sync::mpsc; +use tokio::{select, spawn, sync::mpsc}; use crate::{ input::{ @@ -43,7 +43,7 @@ impl InputActor { channel_size = DEFAULT_INPUT_CHANNEL_SIZE, "spawning input actor" ); - tokio::spawn( + spawn( Self { rx, terminal_handle, @@ -66,7 +66,7 @@ impl InputActor { // the TerminalActor already consumed it from the OS queue. let (event_tx, mut event_rx) = mpsc::channel::(8); let terminal_handle = self.terminal_handle.clone(); - tokio::spawn(async move { + spawn(async move { loop { match terminal_handle.poll_event(EVENT_POLL_TIMEOUT).await { Ok(Some(event)) => { @@ -86,7 +86,7 @@ impl InputActor { }); loop { - tokio::select! { + select! { msg = self.rx.recv() => match msg { Some(InputMessage::SubscribeApp { tx }) => { tracing::debug!("app subscribed to input events"); @@ -96,7 +96,6 @@ impl InputActor { tracing::debug!(?context, "input context updated"); self.context = context; } - #[cfg(test)] Some(InputMessage::Shutdown) => { tracing::info!("input actor stopping"); break; diff --git a/src/input/handle.rs b/src/input/handle.rs index 2fc76c7..fef224d 100644 --- a/src/input/handle.rs +++ b/src/input/handle.rs @@ -38,7 +38,6 @@ impl InputHandle { /// /// Dropping all clones of the handle achieves the same effect because the /// actor's receive channel closes when its last sender is gone. - #[cfg(test)] pub async fn shutdown(&self) -> Result<(), InputError> { self.send(InputMessage::Shutdown).await } diff --git a/src/input/messages.rs b/src/input/messages.rs index cb0bca9..b496e28 100644 --- a/src/input/messages.rs +++ b/src/input/messages.rs @@ -10,6 +10,5 @@ pub enum InputMessage { /// terminal event with current application state. UpdateContext { context: InputContext }, /// Requests a clean shutdown of the actor's event loop. - #[cfg(test)] Shutdown, } diff --git a/src/lore/application/actor.rs b/src/lore/application/actor.rs index 3f41f5c..c110171 100644 --- a/src/lore/application/actor.rs +++ b/src/lore/application/actor.rs @@ -8,6 +8,7 @@ use std::ops::ControlFlow; use tokio::{ + spawn, sync::{mpsc, oneshot}, task, }; @@ -37,7 +38,7 @@ impl LoreApiActor { channel_size = DEFAULT_LORE_API_CHANNEL_SIZE, "spawning lore api actor" ); - tokio::spawn(Self::new(core, rx).run()); + spawn(Self::new(core, rx).run()); LoreApiHandle::new(tx) } diff --git a/src/lore/domain/mailing_list.rs b/src/lore/domain/mailing_list.rs index 40aead8..571b9bc 100644 --- a/src/lore/domain/mailing_list.rs +++ b/src/lore/domain/mailing_list.rs @@ -1,3 +1,5 @@ +use std::cmp::Ordering; + use derive_getters::Getters; use serde::{Deserialize, Serialize}; @@ -17,13 +19,13 @@ impl MailingList { } impl Ord for MailingList { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { + fn cmp(&self, other: &Self) -> Ordering { self.name.cmp(&other.name) } } impl PartialOrd for MailingList { - fn partial_cmp(&self, other: &Self) -> Option { + fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } diff --git a/src/lore/domain/patch.rs b/src/lore/domain/patch.rs index add0579..996d749 100644 --- a/src/lore/domain/patch.rs +++ b/src/lore/domain/patch.rs @@ -4,7 +4,7 @@ use derive_getters::Getters; use regex::Regex; use serde::{Deserialize, Serialize}; -use std::fmt::Display; +use std::fmt::{self, Display, Formatter}; #[derive(Getters, Serialize, Deserialize, Debug, Clone)] pub struct PatchFeed { @@ -39,7 +39,7 @@ pub struct Author { } impl Display for Author { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "{} <{}>", self.name, self.email)?; Ok(()) } diff --git a/src/lore/infrastructure/persistence.rs b/src/lore/infrastructure/persistence.rs index 67a94a7..b59f2c0 100644 --- a/src/lore/infrastructure/persistence.rs +++ b/src/lore/infrastructure/persistence.rs @@ -1,4 +1,6 @@ use mockall::automock; +use serde::{de::DeserializeOwned, Serialize}; +use serde_json::{from_reader, to_writer}; use std::{ collections::{HashMap, HashSet}, @@ -58,7 +60,7 @@ impl FileLorePersistence { } } - fn atomic_write_json( + fn atomic_write_json( &self, value: &T, path: &str, @@ -70,15 +72,15 @@ impl FileLorePersistence { let tmp_path = format!("{path}.tmp"); { let writer = self.fs.create_writer(Path::new(&tmp_path))?; - serde_json::to_writer(writer, value).map_err(io::Error::from)?; + to_writer(writer, value).map_err(io::Error::from)?; } self.fs.rename(Path::new(&tmp_path), Path::new(path))?; Ok(()) } - fn read_json(&self, path: &str) -> Result { + fn read_json(&self, path: &str) -> Result { let reader = self.fs.open_bufreader(Path::new(path))?; - serde_json::from_reader(reader) + from_reader(reader) .map_err(io::Error::from) .map_err(FileSystemError::from) } diff --git a/src/main.rs b/src/main.rs index cb8de29..211efb6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,17 +10,17 @@ mod render_prefs; mod terminal; mod ui; -use app::{actor::AppActor, App}; +use app::{actor::AppActor, dependencies::check_external_deps, App}; use clap::Parser; use cli::Cli; -use color_eyre::eyre::eyre; +use color_eyre::{eyre::eyre, Result}; use config::{bootstrap_parts, ConfigActor}; use infrastructure::{ env::OsEnv, - file_system::OsFileSystem, + file_system::{FileSystemTrait, OsFileSystem}, monitoring::{init_monitoring, InitMonitoringProduct}, net::UreqNetClient, - shell::OsShell, + shell::{OsShell, ShellTrait}, terminal::init, }; use input::{actor::InputActor, event::InputEvent}; @@ -41,7 +41,7 @@ use tracing::{event, Level}; use ui::actor::UiActor; #[tokio::main] -async fn main() -> color_eyre::Result<()> { +async fn main() -> Result<()> { // file writer guards should be propagated to main() so the logging thread lives enough let InitMonitoringProduct { logging_guards_by_file_name, @@ -70,14 +70,16 @@ async fn main() -> color_eyre::Result<()> { ControlFlow::Continue(()) => {} } + check_external_deps(&env, &config)?; + let config_handle = ConfigActor::spawn(config_state, config_repo); let terminal_handle = TerminalActor::spawn(Box::new(CrosstermTerminalSession::new(init()?))); let ui_handle = UiActor::spawn(); // Build shared infrastructure dependencies for LoreService let net = Arc::new(UreqNetClient::new()); - let fs_arc: Arc = Arc::new(OsFileSystem); - let shell_arc: Arc = Arc::new(OsShell); + let fs_arc: Arc = Arc::new(OsFileSystem); + let shell_arc: Arc = Arc::new(OsShell); let gateway = Arc::new(HttpLoreGateway::new(net)); let persistence = Arc::new(FileLorePersistence::new( @@ -107,7 +109,10 @@ async fn main() -> color_eyre::Result<()> { shell_arc.clone(), CacheTtl::default(), )); - let bootstrap = lore_api.get_bootstrap_data().await.unwrap_or_default(); + let bootstrap = lore_api + .get_bootstrap_data() + .await + .map_err(|error| eyre!("failed to bootstrap Lore data: {error}"))?; let app = App::new( config_handle @@ -118,7 +123,6 @@ async fn main() -> color_eyre::Result<()> { bootstrap, Box::new(OsFileSystem), Box::new(OsShell), - Box::new(env), lore_api.clone(), render.clone(), )?; @@ -128,14 +132,16 @@ async fn main() -> color_eyre::Result<()> { .subscribe_app(app_input_tx) .await .map_err(|e| eyre!("{e}"))?; + let input_shutdown_handle = input_handle.clone(); // Shutdown ordering: // 1. AppActor — exits when the user quits (input channel closes) - // 2. ConfigActor — no further configuration requests once App is gone - // 3. LoreApiActor — no further requests once App is gone - // 4. RenderActor — no further requests once App is gone - // 5. UiActor — no further scene builds once App is gone - // 6. TerminalActor — restores the terminal last so the screen stays usable + // 2. InputActor — no further terminal input is needed once App is gone + // 3. ConfigActor — no further configuration requests once App is gone + // 4. LoreApiActor — no further requests once App is gone + // 5. RenderActor — no further requests once App is gone + // 6. UiActor — no further scene builds once App is gone + // 7. TerminalActor — restores the terminal last so the screen stays usable // during the steps above AppActor::spawn( app, @@ -146,6 +152,10 @@ async fn main() -> color_eyre::Result<()> { ) .run_until_done() .await?; + input_shutdown_handle + .shutdown() + .await + .map_err(|e| eyre!("{e}"))?; config_handle.shutdown().await; lore_api.shutdown().await; render.shutdown().await; diff --git a/src/render/actor.rs b/src/render/actor.rs index 65dc87e..fa1b13b 100644 --- a/src/render/actor.rs +++ b/src/render/actor.rs @@ -9,6 +9,7 @@ use std::ops::ControlFlow; use tokio::{ + spawn, sync::{mpsc, oneshot}, task, }; @@ -40,7 +41,7 @@ impl RenderActor { channel_size = DEFAULT_RENDER_CHANNEL_SIZE, "spawning render actor" ); - tokio::spawn(Self::new(core, rx).run()); + spawn(Self::new(core, rx).run()); RenderHandle::new(tx) } diff --git a/src/render/cover_renderer.rs b/src/render/cover_renderer.rs index e136c70..fd36de3 100644 --- a/src/render/cover_renderer.rs +++ b/src/render/cover_renderer.rs @@ -1,17 +1,13 @@ use tracing::{event, Level}; -use color_eyre::eyre::eyre; +use color_eyre::{eyre::eyre, Result}; use crate::{ infrastructure::shell::{ShellCommand, ShellTrait}, render_prefs::CoverRenderer, }; -pub fn render_cover( - shell: &dyn ShellTrait, - raw: &str, - renderer: &CoverRenderer, -) -> color_eyre::Result { +pub fn render_cover(shell: &dyn ShellTrait, raw: &str, renderer: &CoverRenderer) -> Result { let text = match renderer { CoverRenderer::Default => Ok(raw.to_string()), CoverRenderer::Bat => bat_cover_renderer(shell, raw), @@ -25,7 +21,7 @@ pub fn render_cover( /// # Errors /// /// If bat isn't installed or if the command fails, an error will be returned. -fn bat_cover_renderer(shell: &dyn ShellTrait, patch: &str) -> color_eyre::Result { +fn bat_cover_renderer(shell: &dyn ShellTrait, patch: &str) -> Result { let cmd = ShellCommand::new("bat").args(["-pp", "-f", "-l", "mbx"]); let out = shell diff --git a/src/render/patch_renderer.rs b/src/render/patch_renderer.rs index a43ccee..62e50d1 100644 --- a/src/render/patch_renderer.rs +++ b/src/render/patch_renderer.rs @@ -1,4 +1,4 @@ -use color_eyre::eyre::eyre; +use color_eyre::{eyre::eyre, Result}; use tracing::{event, Level}; use crate::{ @@ -23,7 +23,7 @@ pub fn render_patch_preview( shell: &dyn ShellTrait, raw: &str, renderer: &PatchRenderer, -) -> color_eyre::Result { +) -> Result { let text = match renderer { PatchRenderer::Default => Ok(raw.to_string()), PatchRenderer::Bat => bat_patch_renderer(shell, raw), @@ -43,7 +43,7 @@ pub fn render_patch_preview( /// # Tests /// /// [tests::test_bat_patch_renderer] -fn bat_patch_renderer(shell: &dyn ShellTrait, patch: &str) -> color_eyre::Result { +fn bat_patch_renderer(shell: &dyn ShellTrait, patch: &str) -> Result { let cleaned_patch = clean_patch_for_preview(patch); let cmd = ShellCommand::new("bat").args(["-pp", "-f", "-l", "patch"]); @@ -67,7 +67,7 @@ fn bat_patch_renderer(shell: &dyn ShellTrait, patch: &str) -> color_eyre::Result /// # Tests /// /// [tests::test_delta_patch_renderer] -fn delta_patch_renderer(shell: &dyn ShellTrait, patch: &str) -> color_eyre::Result { +fn delta_patch_renderer(shell: &dyn ShellTrait, patch: &str) -> Result { let cleaned_patch = clean_patch_for_preview(patch); let cmd = ShellCommand::new("delta").args([ @@ -103,7 +103,7 @@ fn delta_patch_renderer(shell: &dyn ShellTrait, patch: &str) -> color_eyre::Resu /// # Tests /// /// [tests::test_diff_so_fancy_renderer] -fn diff_so_fancy_renderer(shell: &dyn ShellTrait, patch: &str) -> color_eyre::Result { +fn diff_so_fancy_renderer(shell: &dyn ShellTrait, patch: &str) -> Result { let cleaned_patch = clean_patch_for_preview(patch); let cmd = ShellCommand::new("diff-so-fancy"); diff --git a/src/render_prefs.rs b/src/render_prefs.rs index 1e77e4d..0216b2e 100644 --- a/src/render_prefs.rs +++ b/src/render_prefs.rs @@ -1,7 +1,8 @@ //! Patch and cover renderer preferences (serde + display), shared by config and rendering. +use std::fmt::{self, Display, Formatter}; + use serde::{Deserialize, Serialize}; -use std::fmt::Display; #[derive(Debug, Serialize, Deserialize, Clone, Copy, Default)] pub enum PatchRenderer { @@ -39,7 +40,7 @@ impl From<&str> for PatchRenderer { } impl Display for PatchRenderer { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { PatchRenderer::Default => write!(f, "default"), PatchRenderer::Bat => write!(f, "bat"), @@ -77,7 +78,7 @@ impl From<&str> for CoverRenderer { } impl Display for CoverRenderer { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { CoverRenderer::Default => write!(f, "default"), CoverRenderer::Bat => write!(f, "bat"), diff --git a/src/terminal/actor.rs b/src/terminal/actor.rs index c0746af..510b6bd 100644 --- a/src/terminal/actor.rs +++ b/src/terminal/actor.rs @@ -6,6 +6,7 @@ //! e.g. crossterm) is moved into the actor at spawn time so no other component //! holds the terminal directly. use tokio::{ + spawn, sync::{mpsc, oneshot}, task, }; @@ -38,7 +39,7 @@ impl TerminalActor { channel_size = DEFAULT_TERMINAL_CHANNEL_SIZE, "spawning terminal actor" ); - tokio::spawn(Self::new(session, rx).run()); + spawn(Self::new(session, rx).run()); TerminalHandle::new(tx) } diff --git a/src/terminal/errors.rs b/src/terminal/errors.rs index 673256f..3c845a2 100644 --- a/src/terminal/errors.rs +++ b/src/terminal/errors.rs @@ -1,10 +1,12 @@ +use std::io; + use thiserror::Error; /// Failures at the terminal actor/session boundary. #[derive(Debug, Error)] pub enum TerminalError { #[error("terminal I/O error: {0}")] - Io(#[from] std::io::Error), + Io(#[from] io::Error), #[error("terminal session error: {0}")] Session(String), #[error("terminal actor unavailable: {0}")] diff --git a/src/terminal/session.rs b/src/terminal/session.rs index 72c9895..db4b4d9 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -1,4 +1,4 @@ -use std::time::Duration; +use std::time::{Duration, Instant}; use mockall::automock; use ratatui::crossterm::event::{self, Event, KeyCode, KeyEventKind}; @@ -116,7 +116,7 @@ fn wait_for_key_press_from_session( key: KeyCode, timeout: Duration, ) -> TerminalResult { - let started_at = std::time::Instant::now(); + let started_at = Instant::now(); while started_at.elapsed() < timeout { let elapsed = started_at.elapsed(); diff --git a/src/ui/actor.rs b/src/ui/actor.rs index 032c426..0a17014 100644 --- a/src/ui/actor.rs +++ b/src/ui/actor.rs @@ -8,9 +8,12 @@ //! out of [`AppState`](crate::app::state::AppState): the app layer projects domain //! state into [`AppViewModel`](crate::app::view_model::AppViewModel) before //! crossing this boundary. -use std::ops::ControlFlow; +use std::{mem, ops::ControlFlow}; -use tokio::sync::{mpsc, oneshot}; +use tokio::{ + spawn, + sync::{mpsc, oneshot}, +}; use crate::ui::{ core::UiCore, @@ -36,7 +39,7 @@ impl UiActor { pub fn spawn() -> UiHandle { let (tx, rx) = mpsc::channel(DEFAULT_UI_CHANNEL_SIZE); tracing::debug!(channel_size = DEFAULT_UI_CHANNEL_SIZE, "spawning ui actor"); - tokio::spawn(Self::new(rx).run()); + spawn(Self::new(rx).run()); UiHandle::new(tx) } @@ -57,7 +60,7 @@ impl UiActor { match message { UiMessage::BuildScene { app_view, reply_to } => { tracing::debug!( - screen = ?std::mem::discriminant(&app_view.screen), + screen = ?mem::discriminant(&app_view.screen), has_popup = app_view.popup.is_some(), "building ui scene" ); diff --git a/src/ui/scene.rs b/src/ui/scene.rs index 65483a5..b6c6998 100644 --- a/src/ui/scene.rs +++ b/src/ui/scene.rs @@ -13,10 +13,6 @@ pub use crate::app::view_model::{ ConfigEntryRow, MailingListEntry, PatchSummaryRow, TagTrailerCounts, }; -// --------------------------------------------------------------------------- -// Mailing-list selection -// --------------------------------------------------------------------------- - /// Scene for the mailing-list selection screen. #[derive(Clone, Debug)] pub struct MailingListScene { @@ -24,10 +20,6 @@ pub struct MailingListScene { pub highlighted_index: usize, } -// --------------------------------------------------------------------------- -// Bookmarked patchsets -// --------------------------------------------------------------------------- - /// Scene for the bookmarked-patchsets screen. #[derive(Clone, Debug)] pub struct BookmarkedScene { @@ -35,10 +27,6 @@ pub struct BookmarkedScene { pub selected_index: usize, } -// --------------------------------------------------------------------------- -// Latest patchsets -// --------------------------------------------------------------------------- - /// Scene for the latest-patchsets screen. #[derive(Clone, Debug)] pub struct LatestScene { @@ -46,10 +34,6 @@ pub struct LatestScene { pub selected_index: usize, } -// --------------------------------------------------------------------------- -// Patchset details -// --------------------------------------------------------------------------- - /// Scene for the patchset-details-and-actions screen. #[derive(Clone, Debug)] pub struct PatchsetDetailsScene { @@ -78,20 +62,12 @@ pub struct PatchsetDetailsScene { pub is_current_patch_reply_staged: bool, } -// --------------------------------------------------------------------------- -// Edit config -// --------------------------------------------------------------------------- - /// Scene for the edit-configuration screen. #[derive(Clone, Debug)] pub struct EditConfigScene { pub entries: Vec, } -// --------------------------------------------------------------------------- -// Discriminated body -// --------------------------------------------------------------------------- - /// Which screen's scene the body carries. #[derive(Clone, Debug)] pub enum UiBody { @@ -102,10 +78,6 @@ pub enum UiBody { EditConfig(EditConfigScene), } -// --------------------------------------------------------------------------- -// Navigation bar -// --------------------------------------------------------------------------- - /// Pre-computed navigation-bar content. /// /// `mode_spans` is a list of styled text spans that together form the left @@ -117,10 +89,6 @@ pub struct NavigationBarScene { pub keys_hint: Span<'static>, } -// --------------------------------------------------------------------------- -// Popup -// --------------------------------------------------------------------------- - /// Structured body for each popup variant. #[derive(Clone, Debug)] pub enum PopupBody { @@ -149,10 +117,6 @@ pub struct PopupScene { pub dimensions: (u16, u16), } -// --------------------------------------------------------------------------- -// Top-level scene -// --------------------------------------------------------------------------- - /// The complete, paint-ready visual snapshot for one TUI frame. #[derive(Clone, Debug)] pub struct UiScene { diff --git a/src/ui/screens/bookmarked.rs b/src/ui/screens/bookmarked.rs index 0113d58..467002c 100644 --- a/src/ui/screens/bookmarked.rs +++ b/src/ui/screens/bookmarked.rs @@ -2,16 +2,12 @@ use ratatui::{ layout::Rect, style::{Color, Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, HighlightSpacing, List, ListItem, ListState}, + widgets::{Block, BorderType, Borders, HighlightSpacing, List, ListItem, ListState}, Frame, }; use crate::{app::view_model::BookmarkedViewModel, ui::scene::BookmarkedScene}; -// --------------------------------------------------------------------------- -// Builder -// --------------------------------------------------------------------------- - pub fn build_scene(vm: &BookmarkedViewModel) -> BookmarkedScene { BookmarkedScene { rows: vm.rows.clone(), @@ -19,10 +15,6 @@ pub fn build_scene(vm: &BookmarkedViewModel) -> BookmarkedScene { } } -// --------------------------------------------------------------------------- -// Painter -// --------------------------------------------------------------------------- - pub fn paint(f: &mut Frame, scene: &BookmarkedScene, chunk: Rect) { let mut list_items = Vec::::new(); @@ -45,7 +37,7 @@ pub fn paint(f: &mut Frame, scene: &BookmarkedScene, chunk: Rect) { let list_block = Block::default() .borders(Borders::ALL) - .border_type(ratatui::widgets::BorderType::Double) + .border_type(BorderType::Double) .style(Style::default()); let list = List::new(list_items) @@ -65,10 +57,6 @@ pub fn paint(f: &mut Frame, scene: &BookmarkedScene, chunk: Rect) { f.render_stateful_widget(list, chunk, &mut list_state); } -// --------------------------------------------------------------------------- -// Navigation-bar helpers -// --------------------------------------------------------------------------- - pub fn mode_spans() -> Vec> { vec![Span::styled( "Bookmarked Patchsets", diff --git a/src/ui/screens/details.rs b/src/ui/screens/details.rs index 47bb90c..cf1dc23 100644 --- a/src/ui/screens/details.rs +++ b/src/ui/screens/details.rs @@ -2,7 +2,7 @@ use ratatui::{ layout::{Constraint, Direction, Layout, Rect}, style::{Color, Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, Padding, Paragraph, Wrap}, + widgets::{Block, BorderType, Borders, Padding, Paragraph, Wrap}, Frame, }; @@ -10,11 +10,6 @@ use crate::{ app::view_model::PatchsetDetailsViewModel, ui::scene::{PatchsetDetailsScene, TagTrailerCounts}, }; - -// --------------------------------------------------------------------------- -// Builder -// --------------------------------------------------------------------------- - pub fn build_scene(vm: &PatchsetDetailsViewModel) -> PatchsetDetailsScene { PatchsetDetailsScene { patch_title: vm.patch_title.clone(), @@ -36,10 +31,6 @@ pub fn build_scene(vm: &PatchsetDetailsViewModel) -> PatchsetDetailsScene { } } -// --------------------------------------------------------------------------- -// Painter -// --------------------------------------------------------------------------- - pub fn paint(f: &mut Frame, scene: &PatchsetDetailsScene, chunk: Rect) { if scene.preview_fullscreen { paint_preview(f, scene, chunk); @@ -139,7 +130,7 @@ fn paint_details_and_actions( .block( Block::default() .borders(Borders::ALL) - .border_type(ratatui::widgets::BorderType::Double) + .border_type(BorderType::Double) .title(Line::styled(" Details ", Style::default().fg(Color::Green)).left_aligned()) .padding(Padding::vertical(1)), ) @@ -200,7 +191,7 @@ fn paint_details_and_actions( .block( Block::default() .borders(Borders::ALL) - .border_type(ratatui::widgets::BorderType::Double) + .border_type(BorderType::Double) .title(Line::styled(" Actions ", Style::default().fg(Color::Green)).left_aligned()) .padding(Padding::vertical(1)), ) @@ -216,7 +207,7 @@ fn paint_preview(f: &mut Frame, scene: &PatchsetDetailsScene, chunk: Rect) { .block( Block::default() .borders(Borders::ALL) - .border_type(ratatui::widgets::BorderType::Double) + .border_type(BorderType::Double) .title( Line::styled( scene.preview_title.clone(), @@ -232,10 +223,6 @@ fn paint_preview(f: &mut Frame, scene: &PatchsetDetailsScene, chunk: Rect) { f.render_widget(patch_preview, chunk); } -// --------------------------------------------------------------------------- -// Navigation-bar helpers -// --------------------------------------------------------------------------- - pub fn mode_spans() -> Vec> { vec![Span::styled( "Patchset Details and Actions", diff --git a/src/ui/screens/edit_config.rs b/src/ui/screens/edit_config.rs index 4d0c4d5..fff8895 100644 --- a/src/ui/screens/edit_config.rs +++ b/src/ui/screens/edit_config.rs @@ -8,20 +8,12 @@ use ratatui::{ use crate::{app::view_model::EditConfigViewModel, ui::scene::EditConfigScene}; -// --------------------------------------------------------------------------- -// Builder -// --------------------------------------------------------------------------- - pub fn build_scene(vm: &EditConfigViewModel) -> EditConfigScene { EditConfigScene { entries: vm.entries.clone(), } } -// --------------------------------------------------------------------------- -// Painter -// --------------------------------------------------------------------------- - pub fn paint(f: &mut Frame, scene: &EditConfigScene, chunk: Rect) { let mut constraints = Vec::new(); @@ -71,10 +63,6 @@ pub fn paint(f: &mut Frame, scene: &EditConfigScene, chunk: Rect) { } } -// --------------------------------------------------------------------------- -// Navigation-bar helpers -// --------------------------------------------------------------------------- - pub fn mode_spans(vm: &EditConfigViewModel) -> Vec> { vec![if vm.is_editing_mode { Span::styled("Editing...", Style::default().fg(Color::LightYellow)) diff --git a/src/ui/screens/latest.rs b/src/ui/screens/latest.rs index ddbb0fa..0b1dc46 100644 --- a/src/ui/screens/latest.rs +++ b/src/ui/screens/latest.rs @@ -2,16 +2,12 @@ use ratatui::{ layout::Rect, style::{Color, Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, HighlightSpacing, List, ListItem, ListState}, + widgets::{Block, BorderType, Borders, HighlightSpacing, List, ListItem, ListState}, Frame, }; use crate::{app::view_model::LatestPatchsetsViewModel, ui::scene::LatestScene}; -// --------------------------------------------------------------------------- -// Builder -// --------------------------------------------------------------------------- - pub fn build_scene(vm: &LatestPatchsetsViewModel) -> LatestScene { LatestScene { rows: vm.rows.clone(), @@ -19,10 +15,6 @@ pub fn build_scene(vm: &LatestPatchsetsViewModel) -> LatestScene { } } -// --------------------------------------------------------------------------- -// Painter -// --------------------------------------------------------------------------- - pub fn paint(f: &mut Frame, scene: &LatestScene, chunk: Rect) { let mut list_items = Vec::::new(); @@ -45,7 +37,7 @@ pub fn paint(f: &mut Frame, scene: &LatestScene, chunk: Rect) { let list_block = Block::default() .borders(Borders::ALL) - .border_type(ratatui::widgets::BorderType::Double) + .border_type(BorderType::Double) .style(Style::default()); let list = List::new(list_items) @@ -65,10 +57,6 @@ pub fn paint(f: &mut Frame, scene: &LatestScene, chunk: Rect) { f.render_stateful_widget(list, chunk, &mut list_state); } -// --------------------------------------------------------------------------- -// Navigation-bar helpers -// --------------------------------------------------------------------------- - pub fn mode_spans(vm: &LatestPatchsetsViewModel) -> Vec> { vec![Span::styled( format!( diff --git a/src/ui/screens/mailing_list.rs b/src/ui/screens/mailing_list.rs index c18c685..868cf44 100644 --- a/src/ui/screens/mailing_list.rs +++ b/src/ui/screens/mailing_list.rs @@ -2,7 +2,7 @@ use ratatui::{ layout::Rect, style::{Color, Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, HighlightSpacing, List, ListItem, ListState}, + widgets::{Block, BorderType, Borders, HighlightSpacing, List, ListItem, ListState}, Frame, }; @@ -10,11 +10,6 @@ use crate::{ app::view_model::{MailingListSelectionViewModel, TargetListStatus}, ui::scene::MailingListScene, }; - -// --------------------------------------------------------------------------- -// Builder -// --------------------------------------------------------------------------- - pub fn build_scene(vm: &MailingListSelectionViewModel) -> MailingListScene { MailingListScene { entries: vm.entries.clone(), @@ -22,10 +17,6 @@ pub fn build_scene(vm: &MailingListSelectionViewModel) -> MailingListScene { } } -// --------------------------------------------------------------------------- -// Painter -// --------------------------------------------------------------------------- - pub fn paint(f: &mut Frame, scene: &MailingListScene, chunk: Rect) { let mut list_items = Vec::::new(); @@ -44,7 +35,7 @@ pub fn paint(f: &mut Frame, scene: &MailingListScene, chunk: Rect) { let list_block = Block::default() .borders(Borders::ALL) - .border_type(ratatui::widgets::BorderType::Double) + .border_type(BorderType::Double) .style(Style::default()); let list = List::new(list_items) @@ -64,10 +55,6 @@ pub fn paint(f: &mut Frame, scene: &MailingListScene, chunk: Rect) { f.render_stateful_widget(list, chunk, &mut list_state); } -// --------------------------------------------------------------------------- -// Navigation-bar helpers -// --------------------------------------------------------------------------- - pub fn mode_spans(vm: &MailingListSelectionViewModel) -> Vec> { let text_area = match vm.target_list_status { TargetListStatus::Empty => { diff --git a/src/ui/screens/popup.rs b/src/ui/screens/popup.rs index 23e7be8..41a0e01 100644 --- a/src/ui/screens/popup.rs +++ b/src/ui/screens/popup.rs @@ -1,5 +1,5 @@ use ratatui::{ - layout::Alignment, + layout::{Alignment, Rect}, style::{Color, Modifier, Style, Stylize}, text::Line, widgets::{Block, BorderType, Borders, Clear, Paragraph, Wrap}, @@ -10,11 +10,6 @@ use crate::{ app::view_model::{PopupViewBody, PopupViewModel}, ui::scene::{PopupBody, PopupScene}, }; - -// --------------------------------------------------------------------------- -// Builder -// --------------------------------------------------------------------------- - pub fn build_scene(vm: &PopupViewModel) -> PopupScene { let body = match &vm.body { PopupViewBody::Text(text) => PopupBody::Text(text.clone()), @@ -44,11 +39,7 @@ pub fn build_scene(vm: &PopupViewModel) -> PopupScene { } } -// --------------------------------------------------------------------------- -// Painter -// --------------------------------------------------------------------------- - -pub fn paint(f: &mut Frame, scene: &PopupScene, chunk: ratatui::layout::Rect) { +pub fn paint(f: &mut Frame, scene: &PopupScene, chunk: Rect) { match &scene.body { PopupBody::Text(body) => paint_info(f, &scene.title, body, scene.scroll_offset, chunk), PopupBody::Keybinds { @@ -77,13 +68,7 @@ pub fn paint(f: &mut Frame, scene: &PopupScene, chunk: ratatui::layout::Rect) { } } -fn paint_info( - f: &mut Frame, - title: &str, - body: &str, - scroll: (u16, u16), - chunk: ratatui::layout::Rect, -) { +fn paint_info(f: &mut Frame, title: &str, body: &str, scroll: (u16, u16), chunk: Rect) { let bold_blue = Style::default() .add_modifier(Modifier::BOLD) .fg(Color::Blue); @@ -112,7 +97,7 @@ fn paint_help( description: Option<&str>, formatted_keybinds: &str, scroll: (u16, u16), - chunk: ratatui::layout::Rect, + chunk: Rect, ) { let block = Block::default() .title(title.to_string()) @@ -148,7 +133,7 @@ fn paint_review_trailers( tested_by: &str, acked_by: &str, scroll: (u16, u16), - chunk: ratatui::layout::Rect, + chunk: Rect, ) { let header_style = Style::default() .fg(Color::Cyan)