From 6cbdc96f87463cb408d78cc0ada51022df0086b9 Mon Sep 17 00:00:00 2001 From: Alexander Mohr Date: Mon, 21 Apr 2025 20:07:47 +0200 Subject: [PATCH] fix clippy --- src/lib/config.rs | 15 +++--- src/lib/desktop.rs | 36 ++++++++++----- src/lib/gui.rs | 86 +++++++++++++++++----------------- src/lib/mode.rs | 112 ++++++++++++++++++++++++++------------------- src/main.rs | 2 +- 5 files changed, 138 insertions(+), 113 deletions(-) diff --git a/src/lib/config.rs b/src/lib/config.rs index 66896ec..b1ca6e2 100644 --- a/src/lib/config.rs +++ b/src/lib/config.rs @@ -17,8 +17,7 @@ pub enum ConfigurationError { impl fmt::Display for ConfigurationError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - ConfigurationError::Open(e) => write!(f, "{e}"), - ConfigurationError::Parse(e) => write!(f, "{e}"), + ConfigurationError::Open(e) | ConfigurationError::Parse(e) => write!(f, "{e}"), } } } @@ -102,7 +101,7 @@ impl FromStr for Anchor { "left" => Ok(Anchor::Left), "bottom" => Ok(Anchor::Bottom), "right" => Ok(Anchor::Right), - other => Err(format!("Invalid anchor: {}", other)), + other => Err(format!("Invalid anchor: {other}")), } } } @@ -705,7 +704,7 @@ pub fn resolve_path( /// * no config file exists /// * config file and args cannot be merged pub fn load_config(args_opt: Option) -> Result { - let config_path = conf_path(args_opt.as_ref().map(|c| c.config.clone()).flatten()); + let config_path = conf_path(args_opt.as_ref().and_then(|c| c.config.clone())); match config_path { Ok(path) => { let toml_content = @@ -740,20 +739,22 @@ pub fn load_config(args_opt: Option) -> Result Err(ConfigurationError::Open(format!("{e}"))), } } + +#[must_use] pub fn expand_path(input: &str) -> PathBuf { let mut path = input.to_string(); // Expand ~ to home directory - if path.starts_with("~") { + if path.starts_with('~') { if let Some(home_dir) = dirs::home_dir() { - path = path.replacen("~", home_dir.to_str().unwrap_or(""), 1); + path = path.replacen('~', home_dir.to_str().unwrap_or(""), 1); } } // Expand $VAR style environment variables if path.contains('$') { for (key, value) in env::vars() { - let var_pattern = format!("${}", key); + let var_pattern = format!("${key}"); if path.contains(&var_pattern) { path = path.replace(&var_pattern, &value); } diff --git a/src/lib/desktop.rs b/src/lib/desktop.rs index 72e88a3..f3239a4 100644 --- a/src/lib/desktop.rs +++ b/src/lib/desktop.rs @@ -14,6 +14,7 @@ use regex::Regex; #[derive(Debug)] pub enum DesktopError { MissingIcon, + ParsingError(String), } /// # Errors @@ -60,6 +61,10 @@ fn fetch_icon_from_theme(icon_name: &str) -> Result { } } +/// # Errors +/// +/// Will return `Err` +/// * if it was not able to find any icon pub fn fetch_icon_from_common_dirs(icon_name: &str) -> Result { let mut paths = vec![ PathBuf::from("/usr/local/share/icons"), @@ -72,16 +77,22 @@ pub fn fetch_icon_from_common_dirs(icon_name: &str) -> Result Option> { @@ -102,9 +113,10 @@ fn find_file_case_insensitive(folder: &Path, file_name: &Regex) -> Option Vec { let mut paths = vec![ PathBuf::from("/usr/share/applications"), diff --git a/src/lib/gui.rs b/src/lib/gui.rs index d0d19a6..7204a8f 100644 --- a/src/lib/gui.rs +++ b/src/lib/gui.rs @@ -155,11 +155,11 @@ fn build_ui( window.set_namespace(Some("worf")); } - config.location.as_ref().map(|location| { + if let Some(location) = config.location.as_ref() { for anchor in location { window.set_anchor(anchor.into(), true); } - }); + } let outer_box = gtk4::Box::new(config.orientation.unwrap().into(), 0); outer_box.set_widget_name("outer-box"); @@ -210,16 +210,15 @@ fn build_ui( &item_provider.lock().unwrap().get_elements(None), &list_items, &inner_box, - &config, - &sender, - &app, + config, + sender, + app, &window, ); let items_sort = ArcMenuMap::clone(&list_items); - inner_box.set_sort_func(move |child1, child2| { - sort_menu_items_by_score(child1, child2, items_sort.clone()) - }); + inner_box + .set_sort_func(move |child1, child2| sort_menu_items_by_score(child1, child2, &items_sort)); let items_focus = ArcMenuMap::clone(&list_items); inner_box.connect_map(move |fb| { @@ -235,7 +234,7 @@ fn build_ui( setup_key_event_handler( &window, - entry.clone(), + &entry, inner_box, app.clone(), sender.clone(), @@ -246,7 +245,7 @@ fn build_ui( window.set_child(Widget::NONE); window.show(); - animate_window_show(config.clone(), window.clone(), outer_box); + animate_window_show(config, window.clone(), outer_box); } fn build_ui_from_menu_items( @@ -262,30 +261,26 @@ fn build_ui_from_menu_items( let mut arc_lock = list_items.lock().unwrap(); inner_box.unset_sort_func(); - loop { - if let Some(b) = inner_box.child_at_index(0) { - inner_box.remove(&b); - } else { - break; - } + while let Some(b) = inner_box.child_at_index(0) { + inner_box.remove(&b); } for entry in items { arc_lock.insert( - add_menu_item(&inner_box, entry, config, sender, &list_items, app, window), + add_menu_item(inner_box, entry, config, sender, list_items, app, window), (*entry).clone(), ); } } - let lic = list_items.clone(); - inner_box - .set_sort_func(move |child2, child1| sort_menu_items_by_score(child1, child2, lic.clone())); + let lic = ArcMenuMap::clone(list_items); + inner_box.set_sort_func(move |child2, child1| sort_menu_items_by_score(child1, child2, &lic)); inner_box.invalidate_sort(); } +#[allow(clippy::too_many_arguments)] // todo fix this fn setup_key_event_handler( window: &ApplicationWindow, - entry: SearchEntry, + entry: &SearchEntry, inner_box: FlowBox, app: Application, sender: MenuItemSender, @@ -307,13 +302,14 @@ fn setup_key_event_handler( &config, &item_provider, &window_clone, - &key_value, + key_value, ) }); window.add_controller(key_controller); } +#[allow(clippy::too_many_arguments)] // todo refactor this? fn handle_key_press( search_entry: &SearchEntry, inner_box: &FlowBox, @@ -323,54 +319,54 @@ fn handle_key_press( config: &Config, item_provider: &ArcProvider, window_clone: &ApplicationWindow, - keyboard_key: &Key, + keyboard_key: Key, ) -> Propagation { let update_view = |query: &String, items: Vec>| { build_ui_from_menu_items( &items, - &list_items, - &inner_box, - &config, - &sender, - &app, - &window_clone, + list_items, + inner_box, + config, + sender, + app, + window_clone, ); - filter_widgets(query, list_items, &config, &inner_box); - select_first_visible_child(&list_items, &inner_box); + filter_widgets(query, list_items, config, inner_box); + select_first_visible_child(list_items, inner_box); }; let update_view_from_provider = |query: &String| { - let filtered_list = item_provider.lock().unwrap().get_elements(Some(&query)); - update_view(query, filtered_list) + let filtered_list = item_provider.lock().unwrap().get_elements(Some(query)); + update_view(query, filtered_list); }; match keyboard_key { - &Key::Escape => { + Key::Escape => { if let Err(e) = sender.send(Err(anyhow!("No item selected"))) { log::error!("failed to send message {e}"); } - close_gui(app.clone(), window_clone.clone(), &config); + close_gui(app.clone(), window_clone.clone(), config); } - &Key::Return => { + Key::Return => { if let Err(e) = handle_selected_item( - &sender, + sender, app.clone(), window_clone.clone(), - &config, - &inner_box, - &list_items, + config, + inner_box, + list_items, ) { log::error!("{e}"); } } - &Key::BackSpace => { + Key::BackSpace => { let mut query = search_entry.text().to_string(); query.pop(); search_entry.set_text(&query); update_view_from_provider(&query); } - &Key::Tab => { + Key::Tab => { if let Some(fb) = inner_box.selected_children().first() { if let Some(child) = fb.child() { let expander = child.downcast::().ok(); @@ -381,7 +377,7 @@ fn handle_key_press( let menu_item = lock.get(fb); if let Some(menu_item) = menu_item { if let Some(new_items) = - item_provider.lock().unwrap().get_sub_elements(&menu_item) + item_provider.lock().unwrap().get_sub_elements(menu_item) { let query = menu_item.label.clone(); drop(lock); @@ -411,7 +407,7 @@ fn handle_key_press( fn sort_menu_items_by_score( child1: &FlowBoxChild, child2: &FlowBoxChild, - items_lock: ArcMenuMap, + items_lock: &ArcMenuMap, ) -> Ordering { let lock = items_lock.lock().unwrap(); let m1 = lock.get(child1); @@ -444,7 +440,7 @@ fn sort_menu_items_by_score( } } -fn animate_window_show(config: Config, window: ApplicationWindow, outer_box: gtk4::Box) { +fn animate_window_show(config: &Config, window: ApplicationWindow, outer_box: gtk4::Box) { let display = window.display(); if let Some(surface) = window.surface() { // todo this does not work for multi monitor systems diff --git a/src/lib/mode.rs b/src/lib/mode.rs index 9fc85f5..38c119b 100644 --- a/src/lib/mode.rs +++ b/src/lib/mode.rs @@ -1,5 +1,5 @@ use std::os::unix::prelude::CommandExt; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::{env, fmt, fs, io}; @@ -89,7 +89,7 @@ impl DRunProvider { "Skipping desktop entry for {name:?} because action {action:?} does not exist" ); continue; - }; + } let icon = file .entry @@ -176,13 +176,13 @@ impl FileItemProvider { } } - fn resolve_icon_for_name(&self, path: PathBuf) -> String { - let result = tree_magic_mini::from_filepath(&path); + fn resolve_icon_for_name(path: &Path) -> String { + let result = tree_magic_mini::from_filepath(path); if let Some(result) = result { if result.starts_with("image") { "image-x-generic".to_owned() } else if result.starts_with("inode") { - return result.replace("/", "-"); + return result.replace('/', "-"); } else if result.starts_with("text") { if result.contains("plain") { "text-x-generic".to_owned() @@ -225,7 +225,7 @@ impl ItemProvider for FileItemProvider { }; let mut trimmed_search = search.unwrap_or(&default_path).to_owned(); - if !trimmed_search.starts_with("/") && !trimmed_search.starts_with("~") { + if !trimmed_search.starts_with('/') && !trimmed_search.starts_with('~') { trimmed_search = format!("{default_path}/{trimmed_search}"); } @@ -241,38 +241,45 @@ impl ItemProvider for FileItemProvider { } if path.is_dir() { - for entry in path.read_dir().unwrap() { - if let Ok(entry) = entry { - let mut path_str = entry.path().to_str().unwrap_or("").to_string(); - if trimmed_search.starts_with("~") { - if let Some(home_dir) = dirs::home_dir() { - path_str = path_str.replace(home_dir.to_str().unwrap_or(""), "~"); + if let Ok(entries) = path.read_dir() { + for entry in entries.flatten() { + if let Some(mut path_str) = + entry.path().to_str().map(std::string::ToString::to_string) + { + if trimmed_search.starts_with('~') { + if let Some(home_dir) = dirs::home_dir() { + if let Some(home_str) = home_dir.to_str() { + path_str = path_str.replace(home_str, "~"); + } + } } - } - if entry.path().is_dir() { - path_str += "/"; - } + if entry.path().is_dir() { + path_str.push('/'); + } - items.push({ - MenuItem { + items.push(MenuItem { label: path_str.clone(), - icon_path: Some(self.resolve_icon_for_name(entry.path())), + icon_path: Some(FileItemProvider::::resolve_icon_for_name( + &entry.path(), + )), action: Some(format!("xdg-open {path_str}")), sub_elements: vec![], working_dir: None, initial_sort_score: 0, search_sort_score: 0.0, data: Some(self.menu_item_data.clone()), - } - }); + }); + } } } } else { items.push({ MenuItem { - label: trimmed_search.to_owned(), - icon_path: Some(self.resolve_icon_for_name(PathBuf::from(&trimmed_search))), + label: trimmed_search.clone(), + icon_path: Some(FileItemProvider::::resolve_icon_for_name( + &PathBuf::from(&trimmed_search), + )), action: Some(format!("xdg-open {trimmed_search}")), sub_elements: vec![], working_dir: None, @@ -329,7 +336,7 @@ impl ItemProvider for MathProvider { let item = MenuItem { label: result, icon_path: None, - action: search.map(|s| s.to_string()), + action: search.map(String::from), sub_elements: vec![], working_dir: None, initial_sort_score: 0, @@ -361,17 +368,17 @@ enum AutoRunType { #[derive(Clone)] struct AutoItemProvider { - drun_provider: DRunProvider, - file_provider: FileItemProvider, - math_provider: MathProvider, + drun: DRunProvider, + file: FileItemProvider, + math: MathProvider, } impl AutoItemProvider { fn new() -> Self { AutoItemProvider { - drun_provider: DRunProvider::new(AutoRunType::DRun), - file_provider: FileItemProvider::new(AutoRunType::File), - math_provider: MathProvider::new(AutoRunType::Math), + drun: DRunProvider::new(AutoRunType::DRun), + file: FileItemProvider::new(AutoRunType::File), + math: MathProvider::new(AutoRunType::Math), } } } @@ -381,21 +388,21 @@ impl ItemProvider for AutoItemProvider { if let Some(search) = search_opt { let trimmed_search = search.trim(); if trimmed_search.is_empty() { - self.drun_provider.get_elements(search_opt) + self.drun.get_elements(search_opt) } else if MathProvider::::contains_math_functions_or_starts_with_number( trimmed_search, ) { - self.math_provider.get_elements(search_opt) - } else if trimmed_search.starts_with("$") - || trimmed_search.starts_with("/") - || trimmed_search.starts_with("~") + self.math.get_elements(search_opt) + } else if trimmed_search.starts_with('$') + || trimmed_search.starts_with('/') + || trimmed_search.starts_with('~') { - self.file_provider.get_elements(search_opt) + self.file.get_elements(search_opt) } else { - self.drun_provider.get_elements(search_opt) + self.drun.get_elements(search_opt) } } else { - self.drun_provider.get_elements(search_opt) + self.drun.get_elements(search_opt) } } @@ -411,7 +418,7 @@ impl ItemProvider for AutoItemProvider { /// /// Will return `Err` if it was not able to spawn the process pub fn d_run(config: &Config) -> Result<(), ModeError> { - let provider = DRunProvider::new("".to_owned()); + let provider = DRunProvider::new(String::new()); let cache_path = provider.cache_path.clone(); let mut cache = provider.cache.clone(); @@ -427,10 +434,14 @@ pub fn d_run(config: &Config) -> Result<(), ModeError> { Ok(()) } +/// # Errors +/// +/// Will return `Err` +/// * if it was not able to spawn the process pub fn auto(config: &Config) -> Result<(), ModeError> { let provider = AutoItemProvider::new(); - let cache_path = provider.drun_provider.cache_path.clone(); - let mut cache = provider.drun_provider.cache.clone(); + let cache_path = provider.drun.cache_path.clone(); + let mut cache = provider.drun.cache.clone(); // todo ues a arc instead of cloning the config let selection_result = gui::show(config.clone(), provider); @@ -459,8 +470,12 @@ pub fn auto(config: &Config) -> Result<(), ModeError> { Ok(()) } +/// # Errors +/// +/// Will return `Err` +/// * if it was not able to spawn the process pub fn file(config: &Config) -> Result<(), ModeError> { - let provider = FileItemProvider::new("".to_owned()); + let provider = FileItemProvider::new(String::new()); // todo ues a arc instead of cloning the config let selection_result = gui::show(config.clone(), provider); @@ -478,8 +493,8 @@ pub fn file(config: &Config) -> Result<(), ModeError> { Ok(()) } -pub fn math(config: &Config) -> Result<(), ModeError> { - let provider = MathProvider::new("".to_owned()); +pub fn math(config: &Config) { + let provider = MathProvider::new(String::new); // todo ues a arc instead of cloning the config let selection_result = gui::show(config.clone(), provider); @@ -489,10 +504,11 @@ pub fn math(config: &Config) -> Result<(), ModeError> { log::error!("No item selected"); } } - - Ok(()) } +/// # Errors +/// +/// todo pub fn dmenu(_: &Config) -> Result<(), ModeError> { Ok(()) } @@ -504,7 +520,7 @@ fn update_drun_cache_and_run( ) -> Result<(), ModeError> { if let Some(cache_path) = cache_path { *cache.entry(selection_result.label).or_insert(0) += 1; - if let Err(e) = save_cache_file(&cache_path, &cache) { + if let Err(e) = save_cache_file(&cache_path, cache) { log::warn!("cannot save drun cache {e:?}"); } } @@ -583,7 +599,7 @@ fn spawn_fork(cmd: &str, working_dir: Option<&String>) -> Result<(), ModeError> if let Some(dir) = working_dir { env::set_current_dir(dir) - .map_err(|e| ModeError::RunError(format!("cannot set workdir {e}")))? + .map_err(|e| ModeError::RunError(format!("cannot set workdir {e}")))?; } let exec = parts[0].replace('"', ""); diff --git a/src/main.rs b/src/main.rs index 65f3df5..cae7ce3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -30,7 +30,7 @@ fn main() -> anyhow::Result<()> { mode::file(&config).map_err(|e| anyhow!(e))?; } Mode::Math => { - mode::math(&config).map_err(|e| anyhow!(e))?; + mode::math(&config); } Mode::Auto => { mode::auto(&config).map_err(|e| anyhow!(e))?;