From 25031de6912d681378e57972de12491240e8603e Mon Sep 17 00:00:00 2001 From: ymgyt Date: Fri, 9 Aug 2024 06:17:33 +0900 Subject: [PATCH] refactor(term): abstract interactor --- crates/synd_term/src/application/builder.rs | 6 +- crates/synd_term/src/application/mod.rs | 56 +++++++++++++++---- .../src/interact/integration_interactor.rs | 34 ----------- crates/synd_term/src/interact/interactor.rs | 18 ------ crates/synd_term/src/interact/mock.rs | 38 +++++++++++++ crates/synd_term/src/interact/mod.rs | 36 +++++++++--- crates/synd_term/src/interact/process.rs | 25 +++++++++ crates/synd_term/tests/test/helper.rs | 4 +- 8 files changed, 141 insertions(+), 76 deletions(-) delete mode 100644 crates/synd_term/src/interact/integration_interactor.rs delete mode 100644 crates/synd_term/src/interact/interactor.rs create mode 100644 crates/synd_term/src/interact/mock.rs create mode 100644 crates/synd_term/src/interact/process.rs diff --git a/crates/synd_term/src/application/builder.rs b/crates/synd_term/src/application/builder.rs index 4f059dc2..ec1063c8 100644 --- a/crates/synd_term/src/application/builder.rs +++ b/crates/synd_term/src/application/builder.rs @@ -2,7 +2,7 @@ use crate::{ application::{Application, Authenticator, Cache, Clock, Config}, client::{github::GithubClient, Client}, config::Categories, - interact::Interactor, + interact::Interact, terminal::Terminal, ui::theme::Theme, }; @@ -23,7 +23,7 @@ pub struct ApplicationBuilder< pub(super) theme: Theme, pub(super) authenticator: Option, - pub(super) interactor: Option, + pub(super) interactor: Option>, pub(super) github_client: Option, pub(super) clock: Option>, pub(super) dry_run: bool, @@ -174,7 +174,7 @@ impl ApplicationBuilder { } #[must_use] - pub fn interactor(self, interactor: Interactor) -> Self { + pub fn interactor(self, interactor: Box) -> Self { Self { interactor: Some(interactor), ..self diff --git a/crates/synd_term/src/application/mod.rs b/crates/synd_term/src/application/mod.rs index 996b77e7..7396b2f3 100644 --- a/crates/synd_term/src/application/mod.rs +++ b/crates/synd_term/src/application/mod.rs @@ -18,6 +18,7 @@ use synd_auth::device_flow::DeviceAuthorizationResponse; use synd_feed::types::FeedUrl; use tokio::time::{Instant, Sleep}; use update_informer::Version; +use url::Url; use crate::{ application::event::KeyEventResult, @@ -29,7 +30,7 @@ use crate::{ }, command::{ApiResponse, Command}, config::{self, Categories}, - interact::Interactor, + interact::{Interact, ProcessInteractor}, job::Jobs, keymap::{KeymapId, Keymaps}, terminal::Terminal, @@ -87,7 +88,7 @@ pub struct Application { jobs: Jobs, background_jobs: Jobs, components: Components, - interactor: Interactor, + interactor: Box, authenticator: Authenticator, in_flight: InFlight, cache: Cache, @@ -149,7 +150,7 @@ impl Application { jobs: Jobs::new(NonZero::new(90).unwrap()), background_jobs: Jobs::new(NonZero::new(10).unwrap()), components: Components::new(&config.features), - interactor: interactor.unwrap_or_else(Interactor::new), + interactor: interactor.unwrap_or_else(|| Box::new(ProcessInteractor::new())), authenticator: authenticator.unwrap_or_else(Authenticator::new), in_flight: InFlight::new().with_throbber_timer_interval(config.throbber_timer_interval), cache, @@ -898,9 +899,17 @@ impl Application { impl Application { fn prompt_feed_subscription(&mut self) { - let input = self + let input = match self .interactor - .open_editor(InputParser::SUSBSCRIBE_FEED_PROMPT); + .open_editor(InputParser::SUSBSCRIBE_FEED_PROMPT) + { + Ok(input) => input, + Err(err) => { + tracing::warn!("{err}"); + // TODO: Handle error case + return; + } + }; tracing::debug!("Got user modified feed subscription: {input}"); // the terminal state becomes strange after editing in the editor self.terminal.force_redraw(); @@ -936,9 +945,17 @@ impl Application { return; }; - let input = self + let input = match self .interactor - .open_editor(InputParser::edit_feed_prompt(feed)); + .open_editor(InputParser::edit_feed_prompt(feed).as_str()) + { + Ok(input) => input, + Err(err) => { + // TODO: handle error case + tracing::warn!("{err}"); + return; + } + }; // the terminal state becomes strange after editing in the editor self.terminal.force_redraw(); @@ -1071,14 +1088,28 @@ impl Application { else { return; }; - self.interactor.open_browser(feed_website_url); + match Url::parse(&feed_website_url) { + Ok(url) => { + self.interactor.open_browser(url).ok(); + } + Err(err) => { + tracing::warn!("Try to open invalid feed url: {feed_website_url} {err}"); + } + }; } fn open_entry(&mut self) { let Some(entry_website_url) = self.components.entries.selected_entry_website_url() else { return; }; - self.interactor.open_browser(entry_website_url); + match Url::parse(entry_website_url) { + Ok(url) => { + self.interactor.open_browser(url).ok(); + } + Err(err) => { + tracing::warn!("Try to open invalid entry url: {entry_website_url} {err}"); + } + }; } fn open_notification(&mut self) { @@ -1090,7 +1121,7 @@ impl Application { else { return; }; - self.interactor.open_browser(notification_url.as_str()); + self.interactor.open_browser(notification_url).ok(); } } @@ -1264,8 +1295,9 @@ impl Application { .set_device_authorization_response(device_authorization.clone()); self.should_render(); // try to open input screen in the browser - self.interactor - .open_browser(device_authorization.verification_uri().to_string()); + if let Ok(url) = Url::parse(device_authorization.verification_uri().to_string().as_str()) { + self.interactor.open_browser(url).ok(); + } let authenticator = self.authenticator.clone(); let now = self.now(); diff --git a/crates/synd_term/src/interact/integration_interactor.rs b/crates/synd_term/src/interact/integration_interactor.rs deleted file mode 100644 index e62184f6..00000000 --- a/crates/synd_term/src/interact/integration_interactor.rs +++ /dev/null @@ -1,34 +0,0 @@ -use std::{cell::RefCell, ffi::OsStr}; - -pub type Interactor = TestInteractor; - -pub struct TestInteractor { - editor_buffer: RefCell>, - browser_urls: RefCell>, -} - -impl TestInteractor { - pub fn new() -> Self { - Self { - editor_buffer: RefCell::new(Vec::new()), - browser_urls: RefCell::new(Vec::new()), - } - } - - pub fn with_buffer(mut self, editor_buffer: Vec) -> Self { - self.editor_buffer = RefCell::new(editor_buffer); - self - } - - #[allow(clippy::unused_self, clippy::needless_pass_by_value)] - pub fn open_browser>(&self, url: S) { - self.browser_urls - .borrow_mut() - .push(url.as_ref().to_string_lossy().to_string()); - } - - #[allow(clippy::unused_self, clippy::needless_pass_by_value)] - pub fn open_editor>(&self, _initial_content: S) -> String { - self.editor_buffer.borrow_mut().remove(0) - } -} diff --git a/crates/synd_term/src/interact/interactor.rs b/crates/synd_term/src/interact/interactor.rs deleted file mode 100644 index e13dbfec..00000000 --- a/crates/synd_term/src/interact/interactor.rs +++ /dev/null @@ -1,18 +0,0 @@ -use std::ffi::OsStr; - -pub struct Interactor {} - -impl Interactor { - pub fn new() -> Self { - Self {} - } - - pub fn open_browser>(&self, url: S) { - // try to open input screen in the browser - open::that(url).ok(); - } - - pub fn open_editor>(&self, initial_content: S) -> String { - edit::edit(initial_content).expect("Got user modified input") - } -} diff --git a/crates/synd_term/src/interact/mock.rs b/crates/synd_term/src/interact/mock.rs new file mode 100644 index 00000000..579824a3 --- /dev/null +++ b/crates/synd_term/src/interact/mock.rs @@ -0,0 +1,38 @@ +use std::cell::RefCell; + +use crate::interact::{Interact, OpenBrowser, OpenBrowserError, OpenEditor, OpenEditorError}; + +pub struct MockInteractor { + editor_buffer: RefCell>, + browser_urls: RefCell>, +} + +impl MockInteractor { + pub fn new() -> Self { + Self { + editor_buffer: RefCell::new(Vec::new()), + browser_urls: RefCell::new(Vec::new()), + } + } + + #[must_use] + pub fn with_buffer(mut self, editor_buffer: Vec) -> Self { + self.editor_buffer = RefCell::new(editor_buffer); + self + } +} + +impl OpenBrowser for MockInteractor { + fn open_browser(&self, url: url::Url) -> Result<(), OpenBrowserError> { + self.browser_urls.borrow_mut().push(url.to_string()); + Ok(()) + } +} + +impl OpenEditor for MockInteractor { + fn open_editor(&self, _initial_content: &str) -> Result { + Ok(self.editor_buffer.borrow_mut().remove(0)) + } +} + +impl Interact for MockInteractor {} diff --git a/crates/synd_term/src/interact/mod.rs b/crates/synd_term/src/interact/mod.rs index 92c44c8a..dec5f27c 100644 --- a/crates/synd_term/src/interact/mod.rs +++ b/crates/synd_term/src/interact/mod.rs @@ -1,9 +1,31 @@ -#[cfg(not(feature = "integration"))] -mod interactor; -#[cfg(not(feature = "integration"))] -pub use interactor::Interactor; +use std::io; #[cfg(feature = "integration")] -mod integration_interactor; -#[cfg(feature = "integration")] -pub use integration_interactor::Interactor; +pub mod mock; +mod process; +pub use process::ProcessInteractor; + +use thiserror::Error; +use url::Url; + +pub trait Interact: OpenBrowser + OpenEditor {} + +#[derive(Debug, Error)] +pub enum OpenBrowserError { + #[error("failed to open browser: {0}")] + Io(#[from] io::Error), +} + +pub trait OpenBrowser { + fn open_browser(&self, url: Url) -> Result<(), OpenBrowserError>; +} + +#[derive(Debug, Error)] +#[error("failed to open editor: {message}")] +pub struct OpenEditorError { + message: String, +} + +pub trait OpenEditor { + fn open_editor(&self, initial_content: &str) -> Result; +} diff --git a/crates/synd_term/src/interact/process.rs b/crates/synd_term/src/interact/process.rs new file mode 100644 index 00000000..aa68aa56 --- /dev/null +++ b/crates/synd_term/src/interact/process.rs @@ -0,0 +1,25 @@ +use crate::interact::{Interact, OpenBrowser, OpenBrowserError, OpenEditor}; + +pub struct ProcessInteractor {} + +impl ProcessInteractor { + pub fn new() -> Self { + Self {} + } +} + +impl OpenBrowser for ProcessInteractor { + fn open_browser(&self, url: url::Url) -> Result<(), super::OpenBrowserError> { + open::that(url.as_str()).map_err(OpenBrowserError::from) + } +} + +impl OpenEditor for ProcessInteractor { + fn open_editor(&self, initial_content: &str) -> Result { + edit::edit(initial_content).map_err(|err| super::OpenEditorError { + message: err.to_string(), + }) + } +} + +impl Interact for ProcessInteractor {} diff --git a/crates/synd_term/tests/test/helper.rs b/crates/synd_term/tests/test/helper.rs index 1971a91d..faac2e93 100644 --- a/crates/synd_term/tests/test/helper.rs +++ b/crates/synd_term/tests/test/helper.rs @@ -23,7 +23,7 @@ use synd_term::{ auth::Credential, client::{github::GithubClient as TermGithubClient, Client}, config::Categories, - interact::Interactor, + interact::mock::MockInteractor, terminal::Terminal, types::Time, ui::theme::Theme, @@ -190,7 +190,7 @@ impl TestCase { } else { Vec::new() }; - Interactor::new().with_buffer(buffer) + Box::new(MockInteractor::new().with_buffer(buffer)) }; let github_client = {