From f415e6fd344e303ef15ff18a4544fc5c33992f3b Mon Sep 17 00:00:00 2001 From: Denis Isidoro Date: Tue, 6 Apr 2021 09:45:46 -0300 Subject: [PATCH] Use Pathbufs appropriately (#481) --- src/cmds/info.rs | 2 +- src/cmds/repo.rs | 71 ++++++++++++++++++++++++++++----------- src/filesystem.rs | 84 ++++++++++++++++++++--------------------------- src/fs.rs | 32 ++++++++++-------- 4 files changed, 107 insertions(+), 82 deletions(-) diff --git a/src/cmds/info.rs b/src/cmds/info.rs index c7f7942..8bf1223 100644 --- a/src/cmds/info.rs +++ b/src/cmds/info.rs @@ -9,7 +9,7 @@ pub enum Info { pub fn main(info: &Info) -> Result<(), Error> { match info { - Info::CheatsPath => println!("{}", pathbuf_to_string(default_cheat_pathbuf()?)?), + Info::CheatsPath => println!("{}", pathbuf_to_string(&default_cheat_pathbuf()?)?), } Ok(()) } diff --git a/src/cmds/repo.rs b/src/cmds/repo.rs index cc79047..8d8dbd3 100644 --- a/src/cmds/repo.rs +++ b/src/cmds/repo.rs @@ -1,6 +1,7 @@ use crate::filesystem; use crate::finder::structures::{Opts as FinderOpts, SuggestionType}; use crate::finder::{Finder, FinderChoice}; +use crate::fs::pathbuf_to_string; use crate::git; use anyhow::Context; use anyhow::Error; @@ -8,17 +9,28 @@ use std::fs; use std::io::Write; pub fn browse(finder: &FinderChoice) -> Result<(), Error> { - let repo_path_str = format!("{}/featured", filesystem::tmp_path_str()?); + let repo_pathbuf = { + let mut p = filesystem::tmp_pathbuf()?; + p.push("featured"); + p + }; - let _ = filesystem::remove_dir(&repo_path_str); - filesystem::create_dir(&repo_path_str)?; + let repo_path_str = pathbuf_to_string(&repo_pathbuf)?; + + let _ = filesystem::remove_dir(&repo_pathbuf); + filesystem::create_dir(&repo_pathbuf)?; let (repo_url, _, _) = git::meta("denisidoro/cheats"); git::shallow_clone(repo_url.as_str(), &repo_path_str) .with_context(|| format!("Failed to clone `{}`", repo_url))?; - let repos = fs::read_to_string(format!("{}/featured_repos.txt", &repo_path_str)) - .context("Unable to fetch featured repositories")?; + let feature_repos_file = { + let mut p = repo_pathbuf.clone(); + p.push("featured_repos.txt"); + p + }; + + let repos = fs::read_to_string(&feature_repos_file).context("Unable to fetch featured repositories")?; let opts = FinderOpts { column: Some(1), @@ -35,7 +47,7 @@ pub fn browse(finder: &FinderChoice) -> Result<(), Error> { }) .context("Failed to get repo URL from finder")?; - filesystem::remove_dir(&repo_path_str)?; + filesystem::remove_dir(&repo_pathbuf)?; add(repo, finder) } @@ -67,18 +79,19 @@ pub fn add(uri: String, finder: &FinderChoice) -> Result<(), Error> { let should_import_all = ask_if_should_import_all(finder).unwrap_or(false); let (actual_uri, user, repo) = git::meta(uri.as_str()); - let cheat_path_str = filesystem::pathbuf_to_string(filesystem::default_cheat_pathbuf()?)?; - let tmp_path_str = filesystem::tmp_path_str()?; + let cheat_pathbuf = filesystem::default_cheat_pathbuf()?; + let tmp_pathbuf = filesystem::tmp_pathbuf()?; + let tmp_path_str = pathbuf_to_string(&tmp_pathbuf)?; - let _ = filesystem::remove_dir(&tmp_path_str); - filesystem::create_dir(&tmp_path_str)?; + let _ = filesystem::remove_dir(&tmp_pathbuf); + filesystem::create_dir(&tmp_pathbuf)?; eprintln!("Cloning {} into {}...\n", &actual_uri, &tmp_path_str); git::shallow_clone(actual_uri.as_str(), &tmp_path_str) .with_context(|| format!("Failed to clone `{}`", actual_uri))?; - let all_files = filesystem::all_cheat_files(&tmp_path_str).join("\n"); + let all_files = filesystem::all_cheat_files(&tmp_pathbuf).join("\n"); let opts = FinderOpts { suggestion_type: SuggestionType::MultipleSelections, @@ -102,21 +115,41 @@ pub fn add(uri: String, finder: &FinderChoice) -> Result<(), Error> { files }; - let to_folder = format!("{}/{}__{}", cheat_path_str, user, repo).replace("./", ""); + let to_folder = { + let mut p = cheat_pathbuf.clone(); + p.push(format!("{}__{}", user, repo)); + p + }; - for f in files.split('\n') { - let from = format!("{}/{}", tmp_path_str, f).replace("./", ""); - let filename = f.replace("./", "").replace("/", "__"); - let to = format!("{}/{}", &to_folder, filename); + for file in files.split('\n') { + let from = { + let mut p = tmp_pathbuf.clone(); + p.push(file); + p + }; + let filename = file.replace("/", "__"); + let to = { + let mut p = to_folder.clone(); + p.push(filename); + p + }; fs::create_dir_all(&to_folder).unwrap_or(()); - fs::copy(&from, &to).with_context(|| format!("Failed to copy `{}` to `{}`", from, to))?; + fs::copy(&from, &to).with_context(|| { + format!( + "Failed to copy `{}` to `{}`", + pathbuf_to_string(&from).expect("unable to parse {from}"), + pathbuf_to_string(&to).expect("unable to parse {to}") + ) + })?; } - filesystem::remove_dir(&tmp_path_str)?; + filesystem::remove_dir(&tmp_pathbuf)?; eprintln!( "The following .cheat files were imported successfully:\n{}\n\nThey are now located at {}/{}", - files, cheat_path_str, to_folder + files, + pathbuf_to_string(&cheat_pathbuf)?, + pathbuf_to_string(&to_folder)? ); Ok(()) diff --git a/src/filesystem.rs b/src/filesystem.rs index f224bc1..89d042f 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -8,23 +8,16 @@ use crate::writer::Writer; use anyhow::Error; use directories_next::BaseDirs; use std::collections::HashSet; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use walkdir::WalkDir; -pub fn all_cheat_files(path_str: &str) -> Vec { - let path_str_with_trailing_slash = if path_str.ends_with('/') { - path_str.to_string() - } else { - format!("{}/", &path_str) - }; - - WalkDir::new(&path_str) +pub fn all_cheat_files(path: &Path) -> Vec { + WalkDir::new(&path) .follow_links(true) .into_iter() .filter_map(|e| e.ok()) .map(|e| e.path().to_str().unwrap_or("").to_string()) .filter(|e| e.ends_with(".cheat")) - .map(|e| e.replace(&path_str_with_trailing_slash, "")) .collect::>() } @@ -58,7 +51,7 @@ pub fn cheat_paths(path: Option) -> Result { if let Some(p) = path { Ok(p) } else { - pathbuf_to_string(default_cheat_pathbuf()?) + pathbuf_to_string(&default_cheat_pathbuf()?) } } @@ -81,19 +74,11 @@ pub fn read_all( let folders = paths_from_path_param(&paths); for folder in folders { - for file in all_cheat_files(folder) { - let full_filename = format!("{}/{}", &folder, &file); - files.push(full_filename.clone()); + let folder_pathbuf = PathBuf::from(folder); + for file in all_cheat_files(&folder_pathbuf) { + files.push(file.clone()); let index = files.len() - 1; - if read_file( - &full_filename, - index, - &mut variables, - &mut visited_lines, - writer, - stdin, - ) - .is_ok() + if read_file(&file, index, &mut variables, &mut visited_lines, writer, stdin).is_ok() && !found_something { found_something = true @@ -108,6 +93,33 @@ pub fn read_all( Ok(Some(variables)) } +pub fn tmp_pathbuf() -> Result { + let mut root = default_cheat_pathbuf()?; + root.push("tmp"); + Ok(root) +} + +pub struct Fetcher { + path: Option, +} + +impl Fetcher { + pub fn new(path: Option) -> Self { + Self { path } + } +} + +impl fetcher::Fetcher for Fetcher { + fn fetch( + &self, + stdin: &mut std::process::ChildStdin, + writer: &mut dyn Writer, + files: &mut Vec, + ) -> Result, Error> { + read_all(self.path.clone(), files, stdin, writer) + } +} + #[cfg(test)] mod tests { use super::*; @@ -165,29 +177,3 @@ mod tests { } } } - -pub fn tmp_path_str() -> Result { - let cheat_path_str = pathbuf_to_string(default_cheat_pathbuf()?)?; - Ok(format!("{}/tmp", cheat_path_str)) -} - -pub struct Fetcher { - path: Option, -} - -impl Fetcher { - pub fn new(path: Option) -> Self { - Self { path } - } -} - -impl fetcher::Fetcher for Fetcher { - fn fetch( - &self, - stdin: &mut std::process::ChildStdin, - writer: &mut dyn Writer, - files: &mut Vec, - ) -> Result, Error> { - read_all(self.path.clone(), files, stdin, writer) - } -} diff --git a/src/fs.rs b/src/fs.rs index 429a61f..8a16da2 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -28,7 +28,7 @@ where .map(|line| line.map_err(Error::from))) } -pub fn pathbuf_to_string(pathbuf: PathBuf) -> Result { +pub fn pathbuf_to_string(pathbuf: &Path) -> Result { Ok(pathbuf .as_os_str() .to_str() @@ -44,15 +44,11 @@ fn follow_symlink(pathbuf: PathBuf) -> Result { .to_str() .ok_or_else(|| InvalidPath(o.to_path_buf()))?; if o_str.starts_with('.') { - let parent = pathbuf + let p = pathbuf .parent() .ok_or_else(|| anyhow!("`{}` has no parent", pathbuf.display()))?; - let parent_str = parent - .as_os_str() - .to_str() - .ok_or_else(|| InvalidPath(parent.to_path_buf()))?; - let path_str = format!("{}/{}", parent_str, o_str); - let p = PathBuf::from(path_str); + let mut p = PathBuf::from(p); + p.push(o_str); follow_symlink(p) } else { follow_symlink(o) @@ -67,13 +63,23 @@ fn exe_pathbuf() -> Result { } pub fn exe_string() -> Result { - pathbuf_to_string(exe_pathbuf()?) + pathbuf_to_string(&exe_pathbuf()?) } -pub fn create_dir(path: &str) -> Result<(), Error> { - create_dir_all(path).with_context(|| format!("Failed to create directory `{}`", path)) +pub fn create_dir(path: &Path) -> Result<(), Error> { + create_dir_all(path).with_context(|| { + format!( + "Failed to create directory `{}`", + pathbuf_to_string(path).expect("Unable to parse {path}") + ) + }) } -pub fn remove_dir(path: &str) -> Result<(), Error> { - remove_dir_all(path).with_context(|| format!("Failed to remove directory `{}`", path)) +pub fn remove_dir(path: &Path) -> Result<(), Error> { + remove_dir_all(path).with_context(|| { + format!( + "Failed to remove directory `{}`", + pathbuf_to_string(path).expect("Unable to parse {path}") + ) + }) }