From c78f1b7c08e54eca2b14fe382824f5e282421c66 Mon Sep 17 00:00:00 2001 From: rain-bus Date: Fri, 5 Jun 2026 19:17:36 +0800 Subject: [PATCH] refactor: consolidate form/sync dispatch and trim unused helpers --- src/app/cred.rs | 6 +---- src/app/form.rs | 42 ++++--------------------------- src/app/home_ops.rs | 16 +----------- src/app/settings.rs | 34 +++++++++---------------- src/cli.rs | 12 +++------ src/config.rs | 26 +++++++++++++++++++ src/connection.rs | 5 +--- src/sync.rs | 18 ++++++++------ src/ui.rs | 51 ++++++++++++++++++++++---------------- src/ui/app.rs | 7 ++---- src/ui/view.rs | 37 +++++++++++++++++++++++---- src/ui/view/action_menu.rs | 2 +- src/ui/view/home_list.rs | 42 +++---------------------------- src/ui/view/import.rs | 28 ++------------------- 14 files changed, 131 insertions(+), 195 deletions(-) diff --git a/src/app/cred.rs b/src/app/cred.rs index 3fc532e..cc2cac5 100644 --- a/src/app/cred.rs +++ b/src/app/cred.rs @@ -91,11 +91,7 @@ impl FormNav for CredFormState { impl TextEditing for CredFormState { fn active_text(&self) -> &str { - match self.active { - CredFormField::Name => &self.name, - CredFormField::Value => &self.value, - _ => "", - } + self.field_value(self.active) } fn active_text_mut(&mut self) -> Option<&mut String> { diff --git a/src/app/form.rs b/src/app/form.rs index c538dd4..4bc5ca7 100644 --- a/src/app/form.rs +++ b/src/app/form.rs @@ -220,19 +220,7 @@ impl FormNav for FormState { impl TextEditing for FormState { fn active_text(&self) -> &str { - match self.active { - FormField::Name => &self.name, - FormField::Host => &self.host, - FormField::Port => &self.port, - FormField::User => &self.user, - FormField::CredId => &self.auth_ref, - FormField::Secret => &self.secret, - FormField::Command => &self.command, - FormField::SyncArgs => &self.sync_args, - FormField::LocalArgs => &self.local_args, - FormField::Tags => &self.tags, - _ => "", - } + self.field_value(self.active) } fn active_text_mut(&mut self) -> Option<&mut String> { @@ -296,14 +284,7 @@ impl App { self.config.deleted.insert(name.clone(), ts); if let Some(auth_ref) = profile.auth_ref() { - let still_used = self - .config - .connections - .values() - .any(|p| p.auth_ref() == Some(auth_ref)); - if !still_used { - self.config.credentials.entries.shift_remove(auth_ref); - } + self.config.prune_credential_if_unused(auth_ref, None); } self.config.save()?; self.session.home.selected = self @@ -435,7 +416,9 @@ impl App { fn save_form_credential(&mut self, name: &str, auth_ref: &str, old_auth_ref: Option) { if self.session.form.is_shell { - self.remove_unused_old_credential(name, old_auth_ref); + if let Some(old) = old_auth_ref { + self.config.prune_credential_if_unused(&old, Some(name)); + } } else if !self.session.form.secret.is_empty() { let secret = resolve_secret(&self.session.form.secret); let entry = match self.session.form.auth_kind { @@ -448,21 +431,6 @@ impl App { .insert(auth_ref.to_string(), entry); } } - - fn remove_unused_old_credential(&mut self, editing_name: &str, old_auth_ref: Option) { - let Some(old_auth_ref) = old_auth_ref else { - return; - }; - let still_used = self - .config - .connections - .iter() - .filter(|(conn_name, _)| conn_name.as_str() != editing_name) - .any(|(_, profile)| profile.auth_ref() == Some(old_auth_ref.as_str())); - if !still_used { - self.config.credentials.entries.shift_remove(&old_auth_ref); - } - } } fn parse_tags(raw: &str) -> Vec { diff --git a/src/app/home_ops.rs b/src/app/home_ops.rs index 3f73f0d..f936ce8 100644 --- a/src/app/home_ops.rs +++ b/src/app/home_ops.rs @@ -1,6 +1,5 @@ use anyhow::Result; -use crate::config::SyncBackend; use super::{App, Mode}; impl App { @@ -13,16 +12,6 @@ impl App { self.session.mode = Mode::ActionMenu; } - pub fn enter_combined_import(&mut self) -> Result<()> { - self.session.import.candidates = crate::import::load_candidates(&self.config)?; - self.session.import.selected = vec![false; self.session.import.candidates.len()]; - self.session.import.shell_candidates = self.config.local_shell_candidates(); - self.session.import.shell_selected = vec![false; self.session.import.shell_candidates.len()]; - self.session.import.cursor = 0; - self.session.mode = Mode::ImportSelector; - Ok(()) - } - pub fn enter_quick_select(&mut self) { if self.entries().is_empty() { self.toast("no connections available", false); @@ -64,10 +53,7 @@ impl App { } pub fn sync_with_toast(&mut self) { - let result = match self.config.settings.backend { - SyncBackend::Gist => crate::sync::gist::sync(&mut self.config), - SyncBackend::Webdav => crate::sync::webdav::sync(&mut self.config), - }; + let result = crate::sync::run_sync(&mut self.config); match result { Ok(report) => self.toast(report.to_string(), true), Err(err) => self.toast(err.to_string(), false), diff --git a/src/app/settings.rs b/src/app/settings.rs index 6964e94..3e6ecaf 100644 --- a/src/app/settings.rs +++ b/src/app/settings.rs @@ -1,6 +1,12 @@ use super::{FormNav, TextEditing}; use crate::config::SyncBackend; +/// Trim whitespace and return None if the result is empty. +fn trimmed_opt(s: &str) -> Option { + let s = s.trim().to_string(); + if s.is_empty() { None } else { Some(s) } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SettingsField { SyncPassword, @@ -128,14 +134,7 @@ impl Default for SettingsState { impl TextEditing for SettingsState { fn active_text(&self) -> &str { - match self.active { - SettingsField::SyncPassword => &self.password, - SettingsField::GistId => &self.gist_id, - SettingsField::WebdavUrl => &self.webdav_url, - SettingsField::WebdavUser => &self.webdav_user, - SettingsField::WebdavPassword => &self.webdav_password, - _ => "", - } + self.field_text(self.active) } fn active_text_mut(&mut self) -> Option<&mut String> { @@ -187,22 +186,13 @@ impl App { } pub fn save_settings(&mut self) -> Result<()> { - let pw = self.session.settings.password.trim().to_string(); - self.config.settings.sync_password = if pw.is_empty() { None } else { Some(pw) }; + self.config.settings.sync_password = trimmed_opt(&self.session.settings.password); self.config.settings.backend = self.session.settings.backend; self.config.settings.sync_on_start = self.session.settings.sync_on_start; - let gist = self.session.settings.gist_id.trim().to_string(); - self.config.settings.gist_id = if gist.is_empty() { None } else { Some(gist) }; - let url = self.session.settings.webdav_url.trim().to_string(); - self.config.settings.webdav_url = if url.is_empty() { None } else { Some(url) }; - let user = self.session.settings.webdav_user.trim().to_string(); - self.config.settings.webdav_user = if user.is_empty() { None } else { Some(user) }; - let wd_pw = self.session.settings.webdav_password.trim().to_string(); - self.config.settings.webdav_password = if wd_pw.is_empty() { - None - } else { - Some(wd_pw) - }; + self.config.settings.gist_id = trimmed_opt(&self.session.settings.gist_id); + self.config.settings.webdav_url = trimmed_opt(&self.session.settings.webdav_url); + self.config.settings.webdav_user = trimmed_opt(&self.session.settings.webdav_user); + self.config.settings.webdav_password = trimmed_opt(&self.session.settings.webdav_password); self.config.save()?; self.session.mode = Mode::Home; Ok(()) diff --git a/src/cli.rs b/src/cli.rs index 5273b01..3d4491b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,5 +1,5 @@ use crate::config::{ConnectionType, CredentialEntry, SshellConfig, SyncBackend, config_path, find_binary}; -use crate::sync::{gist, webdav}; +use crate::sync; use crate::{connection, import, ui}; use anyhow::{Context, Result, bail}; use clap::{Parser, Subcommand}; @@ -55,10 +55,7 @@ pub fn run() -> Result<()> { fn run_sync() -> Result<()> { let mut cfg = SshellConfig::load()?; - let report = match cfg.settings.backend { - SyncBackend::Gist => gist::sync(&mut cfg)?, - SyncBackend::Webdav => webdav::sync(&mut cfg)?, - }; + let report = sync::run_sync(&mut cfg)?; println!("{report}"); Ok(()) } @@ -143,13 +140,10 @@ fn check_connection( } ConnectionType::Shell { command, - sync_args, - local_args, .. } => { println!("type: shell"); - let mut merged_args = sync_args.clone(); - merged_args.extend(local_args.clone()); + let merged_args = profile.merged_shell_args(); println!("command: {command} {}", merged_args.join(" ")); } } diff --git a/src/config.rs b/src/config.rs index 1164cd4..445b494 100644 --- a/src/config.rs +++ b/src/config.rs @@ -199,6 +199,19 @@ impl SshellConfig { .unwrap_or(0) + 1 } + + /// Remove a credential if it is no longer referenced by any connection. + /// `exclude` is an optional connection name to skip during the check (e.g. the one being renamed). + pub fn prune_credential_if_unused(&mut self, auth_ref: &str, exclude: Option<&str>) { + let still_used = self + .connections + .iter() + .filter(|(name, _)| Some(name.as_str()) != exclude) + .any(|(_, profile)| profile.auth_ref() == Some(auth_ref)); + if !still_used { + self.credentials.entries.shift_remove(auth_ref); + } + } } impl CredentialEntry { @@ -235,6 +248,19 @@ impl ConnectionProfile { ConnectionType::Shell { sync, .. } => *sync, } } + + /// For Shell connections, returns the merged sync_args + local_args. + /// Returns an empty vec for SSH connections. + pub fn merged_shell_args(&self) -> Vec { + match &self.kind { + ConnectionType::Shell { sync_args, local_args, .. } => { + let mut out = sync_args.clone(); + out.extend(local_args.iter().cloned()); + out + } + ConnectionType::Ssh { .. } => Vec::new(), + } + } } pub fn expand_user_path(value: &str) -> PathBuf { diff --git a/src/connection.rs b/src/connection.rs index c02807f..bc51b8d 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -49,12 +49,9 @@ pub fn connect(name: &str, cfg: &SshellConfig) -> Result<()> { } => connect_ssh(cfg, host, *port, user, auth_ref), ConnectionType::Shell { command, - sync_args, - local_args, .. } => { - let mut merged_args = sync_args.clone(); - merged_args.extend(local_args.clone()); + let merged_args = profile.merged_shell_args(); exec_shell(command, &merged_args) } } diff --git a/src/sync.rs b/src/sync.rs index 72da688..010a7cb 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -2,6 +2,8 @@ mod crypto; pub mod gist; pub mod webdav; +use crate::config::SyncBackend; + use crate::config::ConnectionType; use crate::config::{ConnectionSource, CredentialStore, SshellConfig}; use anyhow::{Context, Result}; @@ -197,13 +199,7 @@ pub(crate) fn bidirectional_merge( if let Some(profile) = removed && let Some(auth_ref) = profile.auth_ref() { - let still_used = cfg - .connections - .values() - .any(|p| p.auth_ref() == Some(auth_ref)); - if !still_used { - cfg.credentials.entries.shift_remove(auth_ref); - } + cfg.prune_credential_if_unused(auth_ref, None); } report.deleted += 1; } @@ -378,3 +374,11 @@ pub(crate) fn count_synced(cfg: &SshellConfig) -> usize { pub(crate) fn to_toml_value(val: &T) -> Result { toml::Value::try_from(val).map_err(|e| anyhow::anyhow!("toml conversion failed: {e}")) } + +/// Run sync using the configured backend. +pub fn run_sync(cfg: &mut crate::config::SshellConfig) -> Result { + match cfg.settings.backend { + SyncBackend::Gist => gist::sync(cfg), + SyncBackend::Webdav => webdav::sync(cfg), + } +} diff --git a/src/ui.rs b/src/ui.rs index 6772edf..a11008e 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -75,32 +75,41 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut crate::app::App) { }); use view::View; - let (title, hints): (&str, Vec<_>) = match app.session.mode { - Mode::Home => (view::HomeListView.title(), view::HomeListView.hints()), - Mode::ActionMenu => (view::ActionMenuView.title(), view::ActionMenuView.hints()), - Mode::Search => (view::SearchView.title(), view::SearchView.hints()), - Mode::QuickSelect => (view::QuickSelectView.title(), view::QuickSelectView.hints()), - Mode::DeleteConfirm => (view::DeleteConfirmView.title(), view::DeleteConfirmView.hints()), - Mode::Form => (view::FormView.title(), view::FormView.hints()), - Mode::ImportSelector => (view::ImportView.title(), view::ImportView.hints()), - Mode::Credentials => (view::CredListView.title(), view::CredListView.hints()), - Mode::CredForm => (view::CredFormView.title(), view::CredFormView.hints()), - Mode::Settings => (view::SettingsView.title(), view::SettingsView.hints()), + let (title, hints): (&str, Vec<_>) = { + let v: &dyn View = match app.session.mode { + Mode::Home => &view::HomeListView, + Mode::ActionMenu => &view::ActionMenuView, + Mode::Search => &view::SearchView, + Mode::QuickSelect => &view::QuickSelectView, + Mode::DeleteConfirm => &view::DeleteConfirmView, + Mode::Form => &view::FormView, + Mode::ImportSelector => &view::ImportView, + Mode::Credentials => &view::CredListView, + Mode::CredForm => &view::CredFormView, + Mode::Settings => &view::SettingsView, + }; + (v.title(), v.hints()) }; draw_header(frame, app, title, shell[0]); match app.session.mode { - Mode::Home => view::HomeListView.draw(frame, app, content), - Mode::ActionMenu => view::HomeListView.draw(frame, app, content), - Mode::Search => view::SearchView.draw(frame, app, content), - Mode::QuickSelect => view::QuickSelectView.draw(frame, app, content), - Mode::DeleteConfirm => view::DeleteConfirmView.draw(frame, app, content), - Mode::Form => view::FormView.draw(frame, app, content), - Mode::ImportSelector => view::ImportView.draw(frame, app, content), - Mode::Credentials => view::CredListView.draw(frame, app, content), - Mode::CredForm => view::CredFormView.draw(frame, app, content), - Mode::Settings => view::SettingsView.draw(frame, app, content), + Mode::Home | Mode::ActionMenu | Mode::DeleteConfirm => { + view::HomeListView.draw(frame, app, content) + } + _ => { + let v: &dyn View = match app.session.mode { + Mode::Search => &view::SearchView, + Mode::QuickSelect => &view::QuickSelectView, + Mode::Form => &view::FormView, + Mode::ImportSelector => &view::ImportView, + Mode::Credentials => &view::CredListView, + Mode::CredForm => &view::CredFormView, + Mode::Settings => &view::SettingsView, + _ => unreachable!(), + }; + v.draw(frame, app, content); + } } draw_help(frame, &hints, shell[2]); diff --git a/src/ui/app.rs b/src/ui/app.rs index 97e2ee3..6b25752 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -1,5 +1,5 @@ use crate::app::{App, Mode}; -use crate::config::{ConnectionType, SyncBackend}; +use crate::config::ConnectionType; use anyhow::Result; use crossterm::{ cursor::{Hide, Show}, @@ -26,10 +26,7 @@ pub fn run() -> Result<()> { let mut app = App::load()?; if app.config.settings.sync_on_start { - let result = match app.config.settings.backend { - SyncBackend::Gist => crate::sync::gist::sync(&mut app.config), - SyncBackend::Webdav => crate::sync::webdav::sync(&mut app.config), - }; + let result = crate::sync::run_sync(&mut app.config); if let Err(err) = result { app.toast(err.to_string(), false); } diff --git a/src/ui/view.rs b/src/ui/view.rs index 37c6cc6..0da488e 100644 --- a/src/ui/view.rs +++ b/src/ui/view.rs @@ -7,12 +7,14 @@ mod import; mod settings; use crate::app::{App, FormAction, FormNav}; +use crate::ui::BLUE; use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use ratatui::{ Frame, layout::Rect, - widgets::Row, + style::{Modifier, Style}, + widgets::{Cell, Row}, }; /// A View represents a full screen that handles both rendering and key events. @@ -38,17 +40,30 @@ pub use settings::SettingsView; /// Scroll a 1:1 row list so the selected index stays visible. /// `area_height` includes the block borders and table header. pub fn scroll_rows<'a>(rows: Vec>, selected: usize, area_height: u16) -> Vec> { + scroll_indexed_rows(rows, &[selected], 0, area_height) +} + +/// Scroll a row list that mixes data rows with non-data rows (e.g. section headers). +/// `entry_row` maps each data index to its visual row index in `rows`. +/// `selected` is the currently selected data index. +pub fn scroll_indexed_rows<'a>( + rows: Vec>, + entry_row: &[usize], + selected: usize, + area_height: u16, +) -> Vec> { let visible = area_height.saturating_sub(3) as usize; // 2 borders + 1 header let total = rows.len(); - if visible == 0 || total <= visible { + if visible == 0 || total <= visible || entry_row.is_empty() { return rows; } - let scroll = if selected < visible / 2 { + let sel_row = entry_row[selected.min(entry_row.len() - 1)]; + let scroll = if sel_row < visible / 2 { 0 - } else if selected + visible / 2 >= total { + } else if sel_row + visible / 2 >= total { total.saturating_sub(visible) } else { - selected - visible / 2 + sel_row - visible / 2 }; rows.into_iter().skip(scroll).take(visible).collect() } @@ -79,3 +94,15 @@ pub fn handle_form_nav(form: &mut F, key: KeyEvent) -> Option None, } } + +/// A section header row with a label and count, used in table views. +pub fn section_row(label: &str, count: usize) -> Row<'static> { + Row::new([ + Cell::from(format!(" {label} ({count})")) + .style(Style::default().fg(BLUE).add_modifier(Modifier::BOLD)), + Cell::from(""), + Cell::from(""), + Cell::from(""), + ]) + .height(1) +} diff --git a/src/ui/view/action_menu.rs b/src/ui/view/action_menu.rs index f12986e..4454ef4 100644 --- a/src/ui/view/action_menu.rs +++ b/src/ui/view/action_menu.rs @@ -93,7 +93,7 @@ impl View for ActionMenuView { ListAction::Select => { app.session.mode = Mode::Home; match app.session.action_menu.cursor { - 0 => app.enter_combined_import()?, + 0 => app.enter_import_selector()?, 1 => app.sync_with_toast(), 2 => app.enter_credentials(), 3 => app.enter_settings(), diff --git a/src/ui/view/home_list.rs b/src/ui/view/home_list.rs index 9e8f76b..1acdf71 100644 --- a/src/ui/view/home_list.rs +++ b/src/ui/view/home_list.rs @@ -6,6 +6,7 @@ use crate::ui::component::{badge_span, draw_input, panel, tag_badge}; use crate::ui::{ACCENT, BLUE, GREEN, MUTED, PANEL_ALT, PURPLE, RED, SELECTED_BG, TEXT, YELLOW}; use super::View; +use super::{section_row, scroll_indexed_rows}; use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; @@ -162,21 +163,7 @@ pub fn draw_connection_list(frame: &mut Frame<'_>, app: &App, area: Rect) { } // Scroll to keep selected entry visible - let visible = area.height.saturating_sub(3) as usize; // 2 borders + 1 header - if visible > 0 && !entry_row.is_empty() { - let sel_row = entry_row[app.session.home.selected.min(entry_row.len() - 1)]; - let total = rows.len(); - if total > visible { - let scroll = if sel_row < visible / 2 { - 0 - } else if sel_row + visible / 2 >= total { - total.saturating_sub(visible) - } else { - sel_row - visible / 2 - }; - rows = rows.into_iter().skip(scroll).take(visible).collect(); - } - } + rows = scroll_indexed_rows(rows, &entry_row, app.session.home.selected, area.height); let title = if app.session.mode == Mode::QuickSelect { "Connections - Quick Select" @@ -203,17 +190,6 @@ pub fn draw_connection_list(frame: &mut Frame<'_>, app: &App, area: Rect) { frame.render_widget(table, area); } -fn section_row(label: &str, count: usize) -> Row<'static> { - Row::new([ - Cell::from(format!(" {label} ({count})")) - .style(Style::default().fg(BLUE).add_modifier(Modifier::BOLD)), - Cell::from(""), - Cell::from(""), - Cell::from(""), - ]) - .height(1) -} - fn connection_row( app: &App, idx: usize, @@ -252,11 +228,9 @@ fn connection_row( } ConnectionType::Shell { command, - sync_args, - local_args, .. } => { - let merged_args = shell_args(sync_args, local_args); + let merged_args = profile.merged_shell_args(); if merged_args.is_empty() { command.clone() } else { @@ -378,14 +352,12 @@ pub fn draw_detail_panel(frame: &mut Frame<'_>, app: &App, area: Rect) { shell_name, auth_ref, command, - sync_args, - local_args, sync, .. } => { lines.push(detail_text("Shell", shell_name)); lines.push(detail_text("Command", command)); - let merged_args = shell_args(sync_args, local_args); + let merged_args = profile.merged_shell_args(); if !merged_args.is_empty() { lines.push(detail_text("Args", &merged_args.join(" "))); } @@ -437,12 +409,6 @@ fn detail_text(label: &str, value: &str) -> Line<'static> { ]) } -fn shell_args(sync_args: &[String], local_args: &[String]) -> Vec { - let mut out = sync_args.to_vec(); - out.extend(local_args.iter().cloned()); - out -} - fn detail_line(label: &str, spans: Vec>) -> Line<'static> { let mut out = vec![Span::styled( format!(" {:<11}", label), diff --git a/src/ui/view/import.rs b/src/ui/view/import.rs index c6ed3bf..dcda973 100644 --- a/src/ui/view/import.rs +++ b/src/ui/view/import.rs @@ -3,6 +3,7 @@ use crate::ui::component::{ListAction, handle_list_nav, panel, panel_with_subtit use crate::ui::{BLUE, GREEN, MUTED, SELECTED_BG, TEXT}; use super::View; +use super::{section_row, scroll_indexed_rows}; use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; @@ -134,21 +135,7 @@ fn draw_import(frame: &mut Frame<'_>, app: &App, area: Rect) { } // Scroll to keep selected entry visible - let visible = area.height.saturating_sub(3) as usize; - if visible > 0 && !entry_row.is_empty() { - let sel_row = entry_row[app.session.import.cursor.min(entry_row.len() - 1)]; - let total_rows = rows.len(); - if total_rows > visible { - let scroll = if sel_row < visible / 2 { - 0 - } else if sel_row + visible / 2 >= total_rows { - total_rows.saturating_sub(visible) - } else { - sel_row - visible / 2 - }; - rows = rows.into_iter().skip(scroll).take(visible).collect(); - } - } + rows = scroll_indexed_rows(rows, &entry_row, app.session.import.cursor, area.height); let table = Table::new( rows, @@ -172,17 +159,6 @@ fn draw_import(frame: &mut Frame<'_>, app: &App, area: Rect) { frame.render_widget(table, area); } -fn section_row(label: &str, count: usize) -> Row<'static> { - Row::new([ - Cell::from(format!(" {label} ({count})")) - .style(Style::default().fg(BLUE).add_modifier(Modifier::BOLD)), - Cell::from(""), - Cell::from(""), - Cell::from(""), - ]) - .height(1) -} - // ── Key handling ─────────────────────────────────────────────── fn handle_import(app: &mut App, key: KeyEvent) -> Result<()> {