Use ShellExpandTrait::expand in more user-provided paths

ShellExpandTrait::expand was not used consistently, leading to only some
functionalities supporting things like tilde expansion.

Fixes #387

Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>

meli/src/conf.rs

@@ -34,7 +34,7 @@ use std::{
 use melib::{
     backends::{MailboxHash, TagHash},
     search::Query,
-    SortField, SortOrder, StderrLogger,
+    ShellExpandTrait, SortField, SortOrder, StderrLogger,
 };
 
 use crate::{
@@ -311,7 +311,7 @@ pub fn get_config_file() -> Result<PathBuf> {
         .set_source(Some(std::sync::Arc::new(Box::new(err))))
     })?;
     match env::var("MELI_CONFIG") {
-        Ok(path) => Ok(PathBuf::from(path)),
+        Ok(path) => Ok(PathBuf::from(path).expand()),
         Err(_) => Ok(xdg_dirs
             .place_config_file("config.toml")
             .chain_err_summary(|| {

meli/src/mail/listing.rs

@@ -31,7 +31,8 @@ use std::{
 
 use futures::future::try_join_all;
 use melib::{
-    backends::EnvelopeHashBatch, mbox::MboxMetadata, utils::datetime, Flag, FlagOp, UnixTimestamp,
+    backends::EnvelopeHashBatch, mbox::MboxMetadata, utils::datetime, Flag, FlagOp,
+    ShellExpandTrait, UnixTimestamp,
 };
 use smallvec::SmallVec;
 
@@ -777,6 +778,7 @@ pub trait MailListingTrait: ListingTrait {
                     if path.is_relative() {
                         path = context.current_dir().join(&path);
                     }
+                    path = path.expand();
                     let account = &mut context.accounts[&account_hash];
                     let format = (*format).unwrap_or_default();
                     let collection = account.collection.clone();
@@ -1742,12 +1744,13 @@ impl Component for Listing {
                             return true;
                         }
                         Action::Listing(ListingAction::Import(file_path, mailbox_path)) => {
+                            let file_path = file_path.expand();
                             let account = &mut context.accounts[self.cursor_pos.account];
                             if let Err(err) = account
                                 .mailbox_by_path(mailbox_path)
                                 .and_then(|mailbox_hash| {
                                     Ok((
-                                        std::fs::read(file_path).chain_err_summary(|| {
+                                        std::fs::read(&file_path).chain_err_summary(|| {
                                             format!("Could not read {}", file_path.display())
                                         })?,
                                         mailbox_hash,

meli/src/mail/view/utils.rs

@@ -21,10 +21,10 @@
 
 use std::{fs::File, io::Write, os::unix::fs::PermissionsExt, path::Path};
 
-use melib::Result;
+use melib::{Result, ShellExpandTrait};
 
 pub fn save_attachment(path: &Path, bytes: &[u8]) -> Result<()> {
-    let mut f = File::create(path)?;
+    let mut f = File::create(path.expand())?;
     let mut permissions = f.metadata()?.permissions();
     permissions.set_mode(0o600); // Read/write for owner only.
     f.set_permissions(permissions)?;

meli/src/manpages.rs

@@ -28,7 +28,7 @@ use std::{
 };
 
 use flate2::bufread::GzDecoder;
-use melib::log;
+use melib::{log, ShellExpandTrait};
 
 use crate::{Error, Result};
 
@@ -87,6 +87,7 @@ impl ManPages {
     pub fn install(destination: Option<PathBuf>) -> Result<PathBuf> {
         fn path_valid(p: &Path, tries: &mut Vec<PathBuf>) -> bool {
             tries.push(p.into());
+            let p = p.expand();
             p.exists()
                 && p.is_dir()
                 && fs::metadata(p)
@@ -116,6 +117,7 @@ impl ManPages {
         else {
             return Err(format!("Could not write to any of these paths: {:?}", tries).into());
         };
+        path = path.expand();
 
         for (p, dir) in [
             (Self::Main, "man1"),

meli/src/subcommands.rs

@@ -27,13 +27,13 @@ use std::{
 };
 
 use crossbeam::channel::{Receiver, Sender};
-use melib::Result;
+use melib::{Result, ShellExpandTrait};
 
 use crate::*;
 
 pub fn create_config(path: Option<PathBuf>) -> Result<()> {
     let config_path = if let Some(path) = path {
-        path
+        path.expand()
     } else {
         conf::get_config_file()?
     };
@@ -131,7 +131,7 @@ pub fn compiled_with() -> Result<()> {
 
 pub fn test_config(path: Option<PathBuf>) -> Result<()> {
     let config_path = if let Some(path) = path {
-        path
+        path.expand()
     } else {
         crate::conf::get_config_file()?
     };
@@ -144,6 +144,7 @@ pub fn view(
     sender: Sender<ThreadEvent>,
     receiver: Receiver<ThreadEvent>,
 ) -> Result<State> {
+    let path = path.expand();
     if !path.exists() {
         return Err(Error::new(format!(
             "`{}` is not a valid path",

meli/src/types/helpers.rs

@@ -30,7 +30,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use melib::{error::*, uuid::Uuid};
+use melib::{error::*, uuid::Uuid, ShellExpandTrait};
 
 /// Temporary file that can optionally cleaned up when it is dropped.
 #[derive(Debug)]
@@ -105,7 +105,8 @@ impl File {
             path.set_extension(ext);
         }
         fn inner(path: &Path, bytes: &[u8], delete_on_drop: bool) -> Result<File> {
-            let mut f = std::fs::File::create(path)?;
+            let path = path.expand();
+            let mut f = std::fs::File::create(&path)?;
             let metadata = f.metadata()?;
             let mut permissions = metadata.permissions();
 
@@ -115,7 +116,7 @@ impl File {
             f.write_all(bytes)?;
             f.flush()?;
             Ok(File {
-                path: path.to_path_buf(),
+                path,
                 delete_on_drop,
             })
         }

melib/src/addressbook.rs

@@ -27,6 +27,7 @@ use std::{
     collections::HashMap,
     hash::{Hash, Hasher},
     ops::Deref,
+    path::Path,
 };
 
 use indexmap::IndexMap;
@@ -35,6 +36,7 @@ use uuid::Uuid;
 use crate::utils::{
     datetime::{now, timestamp_to_string, UnixTimestamp},
     parsec::Parser,
+    shellexpand::ShellExpandTrait,
 };
 
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
@@ -139,8 +141,8 @@ impl AddressBook {
 
     pub fn with_account(s: &crate::conf::AccountSettings) -> Self {
         let mut ret = Self::new(s.name.clone());
-        if let Some(mutt_alias_file) = s.extra.get("mutt_alias_file").map(String::as_str) {
-            match std::fs::read_to_string(std::path::Path::new(mutt_alias_file))
+        if let Some(mutt_alias_file) = s.extra.get("mutt_alias_file") {
+            match std::fs::read_to_string(Path::new(mutt_alias_file).expand())
                 .map_err(|err| err.to_string())
                 .and_then(|contents| {
                     contents
@@ -165,7 +167,8 @@ impl AddressBook {
         }
         #[cfg(feature = "vcard")]
         if let Some(vcard_path) = s.vcard_folder() {
-            match vcard::load_cards(std::path::Path::new(vcard_path)) {
+            let expanded_path = Path::new(vcard_path).expand();
+            match vcard::load_cards(&expanded_path) {
                 Ok(cards) => {
                     for c in cards {
                         ret.add_card(c);
@@ -173,6 +176,13 @@ impl AddressBook {
                 }
                 Err(err) => {
                     log::warn!("Could not load vcards from {:?}: {}", vcard_path, err);
+                    if expanded_path.display().to_string() != vcard_path {
+                        log::warn!(
+                            "Note: vcard_folder was expanded from {} to {}",
+                            vcard_path,
+                            expanded_path.display()
+                        );
+                    }
                 }
             }
         }

melib/src/conf.rs

@@ -27,6 +27,7 @@ use crate::{
     backends::SpecialUsageMailbox,
     email::Address,
     error::{Error, ErrorKind, Result},
+    ShellExpandTrait,
 };
 pub use crate::{SortField, SortOrder};
 
@@ -107,7 +108,7 @@ impl AccountSettings {
         #[cfg(feature = "vcard")]
         {
             if let Some(folder) = self.extra.remove("vcard_folder") {
-                let path = Path::new(&folder);
+                let path = Path::new(&folder).expand();
 
                 if !matches!(path.try_exists(), Ok(true)) {
                     return Err(Error::new(format!(

melib/src/maildir/backend.rs

@@ -1078,11 +1078,11 @@ impl MaildirType {
             settings: &AccountSettings,
             p: P,
         ) -> Result<Vec<MailboxHash>> {
-            if !p.as_ref().exists() || !p.as_ref().is_dir() {
+            if !p.as_ref().try_exists().unwrap_or(false) || !p.as_ref().is_dir() {
                 return Err(Error::new(format!(
                     "Configuration error: Path \"{}\" {}",
                     p.as_ref().display(),
-                    if !p.as_ref().exists() {
+                    if !p.as_ref().try_exists().unwrap_or(false) {
                         "does not exist."
                     } else {
                         "is not a directory."
@@ -1147,7 +1147,7 @@ impl MaildirType {
             Ok(children)
         }
         let root_mailbox = PathBuf::from(&settings.root_mailbox).expand();
-        if !root_mailbox.exists() {
+        if !root_mailbox.try_exists().unwrap_or(false) {
             return Err(Error::new(format!(
                 "Configuration error ({}): root_mailbox `{}` is not a valid directory.",
                 settings.name,
@@ -1294,7 +1294,7 @@ impl MaildirType {
 
     pub fn validate_config(s: &mut AccountSettings) -> Result<()> {
         let root_mailbox = PathBuf::from(&s.root_mailbox).expand();
-        if !root_mailbox.exists() {
+        if !root_mailbox.try_exists().unwrap_or(false) {
             return Err(Error::new(format!(
                 "Configuration error ({}): root_mailbox `{}` is not a valid directory.",
                 s.name,

melib/src/maildir/mod.rs

@@ -138,7 +138,7 @@ impl MaildirMailbox {
         accept_invalid: bool,
         settings: &AccountSettings,
     ) -> Result<Self> {
-        let pathbuf = PathBuf::from(&path);
+        let pathbuf = PathBuf::from(&path).expand();
         let mut h = DefaultHasher::new();
         pathbuf.hash(&mut h);
 

melib/src/mbox/mod.rs

@@ -1298,7 +1298,7 @@ impl MboxType {
         event_consumer: BackendEventConsumer,
     ) -> Result<Box<dyn MailBackend>> {
         let path = Path::new(s.root_mailbox.as_str()).expand();
-        if !path.exists() {
+        if !path.try_exists().unwrap_or(false) {
             return Err(Error::new(format!(
                 "\"root_mailbox\" {} for account {} is not a valid path.",
                 s.root_mailbox.as_str(),
@@ -1371,8 +1371,8 @@ impl MboxType {
         for (k, f) in s.mailboxes.iter() {
             if let Some(path_str) = f.extra.get("path") {
                 let hash = MailboxHash(get_path_hash!(path_str));
-                let pathbuf: PathBuf = path_str.into();
-                if !pathbuf.exists() || pathbuf.is_dir() {
+                let pathbuf: PathBuf = Path::new(path_str).expand();
+                if !pathbuf.try_exists().unwrap_or(false) || pathbuf.is_dir() {
                     return Err(Error::new(format!(
                         "mbox mailbox configuration entry \"{}\" path value {} is not a file.",
                         k, path_str
@@ -1451,7 +1451,7 @@ impl MboxType {
             };
         }
         let path = Path::new(s.root_mailbox.as_str()).expand();
-        if !path.exists() {
+        if !path.try_exists().unwrap_or(false) {
             return Err(Error::new(format!(
                 "\"root_mailbox\" {} for account {} is not a valid path.",
                 s.root_mailbox.as_str(),

melib/src/notmuch/mod.rs

@@ -284,7 +284,15 @@ impl NotmuchDb {
         let mut dlpath = Cow::Borrowed("libnotmuch.so");
         let mut custom_dlpath = false;
         if let Some(lib_path) = s.extra.get("library_file_path") {
-            dlpath = Cow::Owned(lib_path.to_string());
+            let expanded_path = Path::new(lib_path).expand();
+            let expanded_path_string = expanded_path.display().to_string();
+            dlpath = if &expanded_path_string != lib_path
+                && expanded_path.try_exists().unwrap_or(false)
+            {
+                Cow::Owned(expanded_path_string)
+            } else {
+                Cow::Owned(lib_path.to_string())
+            };
             custom_dlpath = true;
         }
         let lib = Arc::new(NotmuchLibrary {
@@ -311,7 +319,7 @@ impl NotmuchDb {
             dlpath,
         });
         let mut path = Path::new(s.root_mailbox.as_str()).expand();
-        if !path.exists() {
+        if !path.try_exists().unwrap_or(false) {
             return Err(Error::new(format!(
                 "Notmuch `root_mailbox` {} for account {} does not exist.",
                 s.root_mailbox.as_str(),
@@ -328,7 +336,7 @@ impl NotmuchDb {
             .set_kind(ErrorKind::Configuration));
         }
         path.push(".notmuch");
-        if !path.exists() || !path.is_dir() {
+        if !path.try_exists().unwrap_or(false) || !path.is_dir() {
             return Err(Error::new(format!(
                 "Notmuch `root_mailbox` {} for account {} does not contain a `.notmuch` \
                  subdirectory.",
@@ -412,7 +420,7 @@ impl NotmuchDb {
 
     pub fn validate_config(s: &mut AccountSettings) -> Result<()> {
         let mut path = Path::new(s.root_mailbox.as_str()).expand();
-        if !path.exists() {
+        if !path.try_exists().unwrap_or(false) {
             return Err(Error::new(format!(
                 "Notmuch `root_mailbox` {} for account {} does not exist.",
                 s.root_mailbox.as_str(),
@@ -429,7 +437,7 @@ impl NotmuchDb {
             .set_kind(ErrorKind::Configuration));
         }
         path.push(".notmuch");
-        if !path.exists() || !path.is_dir() {
+        if !path.try_exists().unwrap_or(false) || !path.is_dir() {
             return Err(Error::new(format!(
                 "Notmuch `root_mailbox` {} for account {} does not contain a `.notmuch` \
                  subdirectory.",
@@ -442,7 +450,12 @@ impl NotmuchDb {
 
         let account_name = s.name.to_string();
         if let Some(lib_path) = s.extra.remove("library_file_path") {
-            if !Path::new(&lib_path).exists() || Path::new(&lib_path).is_dir() {
+            let expanded_path = Path::new(&lib_path).expand();
+            if (!Path::new(&lib_path).try_exists().unwrap_or(false)
+                || Path::new(&lib_path).is_dir())
+                && !Path::new(&expanded_path).try_exists().unwrap_or(false)
+                || Path::new(&expanded_path).is_dir()
+            {
                 return Err(Error::new(format!(
                     "Notmuch `library_file_path` setting value `{}` for account {} does not exist \
                      or is a directory.",

melib/src/utils/shellexpand.rs

@@ -29,25 +29,36 @@ use std::{
 
 use smallvec::SmallVec;
 
+// [ref:needs_dev_doc]: ShellExpandTrait
+// [ref:needs_unit_test]: ShellExpandTrait
 pub trait ShellExpandTrait {
+    // [ref:needs_dev_doc]
     fn expand(&self) -> PathBuf;
+    // [ref:needs_dev_doc]
     fn complete(&self, force: bool) -> SmallVec<[String; 128]>;
 }
 
 impl ShellExpandTrait for Path {
     fn expand(&self) -> PathBuf {
+        // [ref:TODO]: ShellExpandTrait: add support for parameters in braces ${ }
+        // https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
         let mut ret = PathBuf::new();
         for c in self.components() {
-            let c_to_str = c.as_os_str().to_str();
+            let c_to_str = c.as_os_str();
             match c_to_str {
-                Some("~") => {
+                tilde if tilde == "~" => {
                     if let Ok(home_dir) = std::env::var("HOME") {
                         ret.push(home_dir)
                     } else {
-                        return PathBuf::new();
+                        // POSIX says that if HOME is unset, the results of tilde expansion is
+                        // unspecified.
+                        // https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_01
+                        // Abort expansion.
+                        return self.to_path_buf();
                     }
                 }
-                Some(var) if var.starts_with('$') => {
+                var if var.to_string_lossy().starts_with('$') => {
+                    let var = var.to_string_lossy();
                     let env_name = var.split_at(1).1;
                     if env_name.chars().all(char::is_uppercase) {
                         ret.push(std::env::var(env_name).unwrap_or_default());
@@ -55,13 +66,9 @@ impl ShellExpandTrait for Path {
                         ret.push(c);
                     }
                 }
-                Some(_) => {
+                _ => {
                     ret.push(c);
                 }
-                None => {
-                    /* path is invalid */
-                    return PathBuf::new();
-                }
             }
         }
         ret