Improve directories exclusion computation

Author: ClementNerma

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "trasher"
-version = "4.1.0"
+version = "4.2.0"
 authors = ["Clément Nerma <clement.nerma@gmail.com>"]
 edition = "2021"
 license = "Apache-2.0"

Cargo.toml

@@ -9,12 +9,12 @@ use crate::fuzzy::FuzzyFinderItem;
 
 use super::{args::*, bail, fsutils::*, items::*};
 
-pub fn list(action: ListTrashItems, config: &Config) -> Result<()> {
+pub fn list(action: ListTrashItems, exclude_dirs: &[PathBuf]) -> Result<()> {
     let ListTrashItems { name } = action;
 
     debug!("Listing trash items...");
 
-    let mut items = list_all_trash_items(config)?;
+    let mut items = list_all_trash_items(exclude_dirs)?;
 
     if items.is_empty() {
         info!("All trashes are empty.");
@@ -36,7 +36,7 @@ pub fn list(action: ListTrashItems, config: &Config) -> Result<()> {
     Ok(())
 }
 
-pub fn remove(action: MoveToTrash, config: &Config) -> Result<()> {
+pub fn remove(action: MoveToTrash, exclude_dirs: &[PathBuf]) -> Result<()> {
     let MoveToTrash {
         paths,
         permanently,
@@ -100,7 +100,7 @@ pub fn remove(action: MoveToTrash, config: &Config) -> Result<()> {
             data.trash_filename()
         );
 
-        let trash_dir = determine_trash_dir_for(&path, config).with_context(|| {
+        let trash_dir = determine_trash_dir_for(&path, exclude_dirs).with_context(|| {
             format!(
                 "Failed to determine path to the trash directory for item: {}",
                 path.display()
@@ -157,12 +157,12 @@ pub fn remove(action: MoveToTrash, config: &Config) -> Result<()> {
     Ok(())
 }
 
-pub fn drop(action: DropItem, config: &Config) -> Result<()> {
+pub fn drop(action: DropItem, exclude_dirs: &[PathBuf]) -> Result<()> {
     let DropItem { filename, id } = action;
 
     debug!("Listing trash items...");
 
-    let item = expect_single_trash_item(&filename, id.as_deref(), config)?;
+    let item = expect_single_trash_item(&filename, id.as_deref(), exclude_dirs)?;
 
     debug!("Permanently removing item from trash...");
 
@@ -177,7 +177,7 @@ pub fn drop(action: DropItem, config: &Config) -> Result<()> {
     result.with_context(|| format!("Failed to remove item '{}' from trash", item.data.filename))
 }
 
-pub fn path_of(action: GetItemPath, config: &Config) -> Result<()> {
+pub fn path_of(action: GetItemPath, exclude_dirs: &[PathBuf]) -> Result<()> {
     let GetItemPath {
         filename,
         id,
@@ -186,7 +186,7 @@ pub fn path_of(action: GetItemPath, config: &Config) -> Result<()> {
 
     debug!("Listing trash items...");
 
-    let item = expect_single_trash_item(&filename, id.as_deref(), config)?;
+    let item = expect_single_trash_item(&filename, id.as_deref(), exclude_dirs)?;
     let item_path = item.complete_trash_item_path();
 
     match item_path.to_str() {
@@ -206,16 +206,16 @@ pub fn path_of(action: GetItemPath, config: &Config) -> Result<()> {
     Ok(())
 }
 
-pub fn restore(action: RestoreItem, config: &Config) -> Result<()> {
+pub fn restore(action: RestoreItem, exclude_dirs: &[PathBuf]) -> Result<()> {
     let RestoreItem { filename, to, id } = action;
 
     debug!("Listing trash items...");
 
     let Some(filename) = filename else {
-        return restore_with_ui(config);
+        return restore_with_ui(exclude_dirs);
     };
 
-    let item = expect_single_trash_item(&filename, id.as_deref(), config)?;
+    let item = expect_single_trash_item(&filename, id.as_deref(), exclude_dirs)?;
 
     let item_path = item.complete_trash_item_path();
 
@@ -252,8 +252,8 @@ pub fn restore(action: RestoreItem, config: &Config) -> Result<()> {
     result.with_context(|| format!("Failed to restore item '{}' from trash", item.data.filename))
 }
 
-pub fn restore_with_ui(config: &Config) -> Result<()> {
-    let items = list_all_trash_items(config)?;
+pub fn restore_with_ui(exclude_dirs: &[PathBuf]) -> Result<()> {
+    let items = list_all_trash_items(exclude_dirs)?;
 
     if items.is_empty() {
         info!("Trash is empty");
@@ -282,15 +282,15 @@ pub fn restore_with_ui(config: &Config) -> Result<()> {
             to: None,
             id: Some(to_remove.data.compute_id().to_owned()),
         },
-        config,
+        exclude_dirs,
     )?;
 
     Ok(())
 }
 
-pub fn empty(config: &Config) -> Result<()> {
-    let trash_dirs = list_trash_dirs(config)?;
-    let items = list_all_trash_items(config)?;
+pub fn empty(exclude_dirs: &[PathBuf]) -> Result<()> {
+    let trash_dirs = list_trash_dirs(exclude_dirs)?;
+    let items = list_all_trash_items(exclude_dirs)?;
 
     if items.is_empty() {
         info!("Trash is empty");
@@ -368,11 +368,11 @@ pub fn empty(config: &Config) -> Result<()> {
     Ok(())
 }
 
-pub fn trash_path(config: &Config) -> Result<()> {
+pub fn trash_path(exclude_dirs: &[PathBuf]) -> Result<()> {
     let current_dir =
         std::env::current_dir().context("Failed to determine path to the current directory")?;
 
-    let trash_dir = determine_trash_dir_for(&current_dir, config)?;
+    let trash_dir = determine_trash_dir_for(&current_dir, exclude_dirs)?;
 
     println!("{}", trash_dir.display());
 

src/actions.rs

@@ -1,11 +1,11 @@
 use std::path::PathBuf;
 
-use clap::{Args, Parser, Subcommand};
+use clap::{Parser, Subcommand};
 use log::LevelFilter;
 
 #[derive(Parser)]
 #[clap(author, version, about, long_about = None)]
-pub struct CmdArgs {
+pub struct Args {
     #[clap(
         short,
         long,
@@ -15,21 +15,16 @@ pub struct CmdArgs {
     )]
     pub verbosity: LevelFilter,
 
-    #[clap(subcommand)]
-    pub action: Action,
-
-    #[clap(flatten)]
-    pub config: Config,
-}
-
-#[derive(Args)]
-pub struct Config {
     #[clap(
         global = true,
+        short,
         long,
         help = "Disallow making a filesystem-local trash directory in some paths"
     )]
     pub exclude: Vec<PathBuf>,
+
+    #[clap(subcommand)]
+    pub action: Action,
 }
 
 #[derive(Subcommand)]

src/args.rs

@@ -3,8 +3,7 @@ use std::{
     collections::BTreeSet,
     ffi::OsStr,
     fs,
-    path::Component,
-    path::{Path, PathBuf},
+    path::{Component, Path, PathBuf},
     rc::Rc,
 };
 
@@ -17,8 +16,6 @@ use log::{debug, error, warn};
 use mountpoints::mountpaths;
 use walkdir::WalkDir;
 
-use crate::Config;
-
 use super::items::TrashItemInfos;
 
 /// Name of the trash directory
@@ -42,26 +39,20 @@ pub static ALWAYS_EXCLUDE_DIRS: &[&str] = &[
     "/var/lib/docker",
 ];
 
-/// Determine path to the trash directory for a given item and create it if required
-pub fn determine_trash_dir_for(item: &Path, config: &Config) -> Result<PathBuf> {
-    debug!("Determining trasher directory for item: {}", item.display());
-
-    let home_dir = dirs::home_dir().context("Failed to determine path to user's home directory")?;
+/// Compute the list of directories to exclude
+pub fn compute_exclusions(exclude_dirs: &[PathBuf]) -> Result<Vec<PathBuf>> {
+    debug!("Computing directories to exclude...");
 
-    let mut exclude = config
-        .exclude
+    let mut exclude = exclude_dirs
         .iter()
-        .filter_map(|dir| {
-            if !dir.is_dir() {
-                None
-            } else {
-                Some(fs::canonicalize(dir).with_context(|| {
-                    format!(
-                        "Failed to canonicalize excluded directory: {}",
-                        dir.display()
-                    )
-                }))
-            }
+        .filter(|dir| dir.is_dir())
+        .map(|dir| {
+            fs::canonicalize(dir).with_context(|| {
+                format!(
+                    "Failed to canonicalize excluded directory: {}",
+                    dir.display()
+                )
+            })
         })
         .collect::<Result<Vec<_>, _>>()?;
 
@@ -73,14 +64,23 @@ pub fn determine_trash_dir_for(item: &Path, config: &Config) -> Result<PathBuf>
             .map(Path::to_owned),
     );
 
+    Ok(exclude)
+}
+
+/// Determine path to the trash directory for a given item and create it if required
+pub fn determine_trash_dir_for(item: &Path, exclude_dirs: &[PathBuf]) -> Result<PathBuf> {
+    debug!("Determining trasher directory for item: {}", item.display());
+
+    let home_dir = dirs::home_dir().context("Failed to determine path to user's home directory")?;
+
     // Don't canonicalize excluded item paths
     // NOTE: Only works if item path is absolute
-    if exclude.iter().any(|dir| item.starts_with(dir)) {
+    if exclude_dirs.iter().any(|dir| item.starts_with(dir)) {
         return Ok(home_dir.join(TRASH_DIR_NAME));
     }
 
     let item = fs::canonicalize(item)
-        .with_context(|| format!("Failed to canonicalize item path: {}\n\nTip: you can exclude this directory using --exclude.", item.display()))?;
+        .with_context(|| format!("Failed to canonicalize item path: {}", item.display()))?;
 
     let mut mountpoints = mountpaths().context("Failed to list system mountpoints")?;
 
@@ -125,7 +125,7 @@ pub fn determine_trash_dir_for(item: &Path, config: &Config) -> Result<PathBuf>
             continue;
         }
 
-        if exclude.iter().any(|parent| item.starts_with(parent)) {
+        if exclude_dirs.iter().any(|parent| item.starts_with(parent)) {
             found = None;
             break;
         }
@@ -140,25 +140,28 @@ pub fn determine_trash_dir_for(item: &Path, config: &Config) -> Result<PathBuf>
 }
 
 /// List all trash directories
-pub fn list_trash_dirs(config: &Config) -> Result<BTreeSet<PathBuf>> {
+pub fn list_trash_dirs(exclude_dirs: &[PathBuf]) -> Result<BTreeSet<PathBuf>> {
     let canon_root = fs::canonicalize("/").context("Failed to canonicalize the root directory")?;
 
     let trash_dirs = mountpaths()
         .context("Failed to list system mountpoints")?
         .iter()
         .chain([canon_root].iter())
-        .filter(|path| match fs::metadata(path) {
-            Ok(_) => true,
-
-            Err(err) => {
-                warn!("Warning: Skipping mountpoint {}: {err}", path.display());
-                false
+        .filter(|dir| {
+            !exclude_dirs
+                .iter()
+                .any(|excluded| dir.starts_with(excluded))
+        })
+        .filter_map(|dir| match fs::metadata(dir) {
+            Ok(_) => Some(dir.join(TRASH_DIR_NAME)),
+            Err(_) => {
+                warn!("Skipping unavailable directory: {}", dir.display());
+                None
             }
         })
-        .map(|path| determine_trash_dir_for(path, config))
-        .collect::<Result<Vec<_>, _>>()?;
+        .collect();
 
-    Ok(trash_dirs.into_iter().filter(|dir| dir.is_dir()).collect())
+    Ok(trash_dirs)
 }
 
 /// List and parse all items in the trash
@@ -213,8 +216,8 @@ pub fn list_trash_items(trash_dir: &Path) -> Result<Vec<TrashedItem>> {
 }
 
 /// List all trash items
-pub fn list_all_trash_items(config: &Config) -> Result<Vec<TrashedItem>> {
-    let all_trash_items = list_trash_dirs(config)?
+pub fn list_all_trash_items(exclude_dirs: &[PathBuf]) -> Result<Vec<TrashedItem>> {
+    let all_trash_items = list_trash_dirs(exclude_dirs)?
         .into_iter()
         .map(|trash_dir| list_trash_items(&trash_dir))
         .collect::<Result<Vec<_>, _>>()?;
@@ -229,9 +232,9 @@ pub fn list_all_trash_items(config: &Config) -> Result<Vec<TrashedItem>> {
 pub fn expect_trash_item(
     filename: &str,
     id: Option<&str>,
-    config: &Config,
+    exclude_dirs: &[PathBuf],
 ) -> Result<FoundTrashItems> {
-    let mut candidates = list_all_trash_items(config)?
+    let mut candidates = list_all_trash_items(exclude_dirs)?
         .into_iter()
         .filter(|trashed| trashed.data.filename == filename)
         .collect::<Vec<_>>();
@@ -257,9 +260,9 @@ pub fn expect_trash_item(
 pub fn expect_single_trash_item(
     filename: &str,
     id: Option<&str>,
-    config: &Config,
+    exclude_dirs: &[PathBuf],
 ) -> Result<TrashedItem> {
-    match expect_trash_item(filename, id, config)? {
+    match expect_trash_item(filename, id, exclude_dirs)? {
         FoundTrashItems::Single(item) => Ok(item),
         FoundTrashItems::Multi(candidates) => bail!(
             "Multiple items with this filename were found in the trash:\n\n{}",