From 55f740497b4498e594013cb8da11df9539d1d076 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 23:26:08 +0100 Subject: [PATCH 01/14] Beautify get_tags() helper --- src/module/bm/commands.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index 59f507f3..ee5630cb 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -153,21 +153,25 @@ fn get_filtered_files_from_backend<'a>(module: &'a Module, } fn get_tags<'a>(rt: &Runtime, sub: &ArgMatches<'a, 'a>) -> Vec { - debug!("Fetching tags from commandline"); - sub.value_of("tags").and_then(|tags| - Some(tags.split(",") - .into_iter() - .map(|s| s.to_string()) - .filter(|e| - if e.contains(" ") { - warn!("Tag contains spaces: '{}'", e); - false - } else { - true - }).collect() - ) - ).or(Some(vec![])).unwrap() + fn reject_if_with_spaces(e: &String) -> bool { + if e.contains(" ") { + warn!("Tag contains spaces: '{}'", e); + false + } else { + true + } + } + + debug!("Fetching tags from commandline"); + sub.value_of("tags").and_then(|tags| { + Some(tags.split(",") + .into_iter() + .map(|s| s.to_string()) + .filter(|e| reject_if_with_spaces(e)) + .collect() + ) + }).or(Some(vec![])).unwrap() } fn get_matcher<'a>(rt: &Runtime, sub: &ArgMatches<'a, 'a>) -> Option { From f8e870312e2a1ee30183b8d5e736a11847ad1d32 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 23:35:48 +0100 Subject: [PATCH 02/14] Outsource: ids iterator to files vector --- src/storage/backend.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index 719932ee..de58c162 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -81,9 +81,7 @@ impl StorageBackend { .and_then(|ids| { debug!("Iterating ids and building files from them"); debug!(" number of ids = {}", ids.len()); - Ok(ids.filter_map(|id| self.get_file_by_id(m, &id, p)) - .collect::>() - .into_iter()) + Ok(self.filter_map_ids_to_files(m, p, ids).into_iter()) }) .map_err(|e| { debug!("StorageBackend::iter_ids() returned error = {:?}", e); @@ -198,9 +196,8 @@ impl StorageBackend { let globstr = self.prefix_of_files_for_module(m) + "*" + &id_str[..] + ".imag"; debug!("Globbing with globstr = '{}'", globstr); glob(&globstr[..]).map(|globlist| { - let mut vec = globlist_to_file_id_vec(globlist).into_iter() - .filter_map(|id| self.get_file_by_id(m, &id, p)) - .collect::>(); + let idvec = globlist_to_file_id_vec(globlist).into_iter(); + let mut vec = self.filter_map_ids_to_files(m, p, idvec); vec.reverse(); vec.pop() }).unwrap_or({ @@ -270,6 +267,17 @@ impl StorageBackend { self.storepath.clone() + m.name() } + fn filter_map_ids_to_files<'a, HP>(&self, + m: &'a Module, + p: &Parser, + ids: IntoIter) + -> Vec> + where HP: FileHeaderParser + { + ids.filter_map(|id| self.get_file_by_id(m, &id, p)) + .collect::>() + } + } #[derive(Debug)] @@ -335,3 +343,4 @@ fn globlist_to_file_id_vec(globlist: Paths) -> Vec { .map(|pbuf| FileID::from(&pbuf)) .collect::>() } + From 5953563671006e54d412f6321a1a8dae346560e2 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 23:39:30 +0100 Subject: [PATCH 03/14] Refactor for readability --- src/storage/backend.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index de58c162..ead88f1a 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -208,12 +208,16 @@ impl StorageBackend { // The (hash)type is already in the FileID object, so we can just // build a path from the information we already have debug!("We know FileIDType, so we build the path directly now"); - if let Ok(mut fs) = FSFile::open(self.build_filepath_with_id(m, id.clone())) { + let filepath = self.build_filepath_with_id(m, id.clone()); + if let Ok(mut fs) = FSFile::open(filepath) { let mut s = String::new(); fs.read_to_string(&mut s); + debug!("Success opening file with id '{}'", id); debug!("Parsing to internal structure now"); - p.read(s).and_then(|(h, d)| Ok(File::from_parser_result(m, id.clone(), h, d))).ok() + p.read(s).and_then(|(h, d)| { + Ok(File::from_parser_result(m, id.clone(), h, d)) + }).ok() } else { debug!("No file with id '{}'", id); None From f7a92b6e7998669b6be71e7cdcbb0c62f1847ffc Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 01:14:13 +0100 Subject: [PATCH 04/14] Resort "use" First comes std stuff Second is external crates Then we have own modules All in alphabetical order and as few lines as possible if readability is preserved. --- src/cli.rs | 6 ++---- src/configuration.rs | 11 +++-------- src/main.rs | 4 ++-- src/module/bm/commands.rs | 23 +++++++++-------------- src/module/bm/header.rs | 5 +++-- src/module/bm/mod.rs | 26 +++++++++++--------------- src/module/mod.rs | 8 +++----- src/runtime.rs | 9 +++------ src/storage/backend.rs | 20 +++++++------------- src/storage/file.rs | 8 ++++---- src/storage/file_id.rs | 5 ++--- src/storage/json/parser.rs | 13 ++++++------- src/storage/parser.rs | 3 ++- 13 files changed, 57 insertions(+), 84 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 42b8d563..3b1d8b4b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,10 +1,8 @@ +use std::fmt::{Debug, Formatter, Error}; + extern crate clap; use clap::{App, ArgMatches}; -use std::fmt::Debug; -use std::fmt::Formatter; -use std::fmt::Error; - pub struct ModuleConfig { pub load : bool, } diff --git a/src/configuration.rs b/src/configuration.rs index c8eb8e67..8a6d6e31 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -1,14 +1,9 @@ -extern crate clap; - -use cli::CliConfig; - +use std::fmt::{Debug, Formatter, Error}; use std::path::Path; + use config::reader::from_file; use config::types::Config as Cfg; - -use std::fmt::Debug; -use std::fmt::Formatter; -use std::fmt::Error; +use cli::CliConfig; pub struct Configuration { pub rtp : String, diff --git a/src/main.rs b/src/main.rs index ce8e3cdb..9e1f99cf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,6 +11,8 @@ extern crate url; extern crate config; +use std::process::exit; + use cli::CliConfig; use configuration::Configuration; use runtime::{ImagLogger, Runtime}; @@ -28,8 +30,6 @@ mod module; mod storage; mod ui; -use std::process::exit; - fn main() { let yaml = load_yaml!("../etc/cli.yml"); let app = App::from_yaml(yaml); diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index ee5630cb..3b7a1d35 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -1,22 +1,17 @@ -use runtime::Runtime; -use storage::backend::{StorageBackendError, StorageBackend}; - -use module::Module; -use module::ModuleError; -use module::CommandResult; -use module::CommandEnv; - -use module::bm::header::build_header; -use module::bm::header::get_tags_from_header; -use storage::json::parser::JsonHeaderParser; -use storage::parser::{Parser, FileHeaderParser}; -use storage::file::File; -use ui::file::{FilePrinter, TablePrinter}; use std::vec::IntoIter; use clap::ArgMatches; use regex::Regex; +use module::{CommandEnv, CommandResult, Module, ModuleError}; +use module::bm::header::{build_header, get_tags_from_header}; +use runtime::Runtime; +use storage::backend::{StorageBackendError, StorageBackend}; +use storage::file::File; +use storage::json::parser::JsonHeaderParser; +use storage::parser::{Parser, FileHeaderParser}; +use ui::file::{FilePrinter, TablePrinter}; + pub fn add_command(module: &Module, env: CommandEnv) -> CommandResult { use url::Url; diff --git a/src/module/bm/header.rs b/src/module/bm/header.rs index 10f20d80..e2479048 100644 --- a/src/module/bm/header.rs +++ b/src/module/bm/header.rs @@ -1,7 +1,8 @@ -use storage::file::FileHeaderSpec as FHS; -use storage::file::FileHeaderData as FHD; use std::ops::Deref; +use storage::file::FileHeaderData as FHD; +use storage::file::FileHeaderSpec as FHS; + pub fn get_spec() -> FHS { FHS::Map { keys: vec![ url_key(), tags_key() ] } } diff --git a/src/module/bm/mod.rs b/src/module/bm/mod.rs index e963564b..216665ca 100644 --- a/src/module/bm/mod.rs +++ b/src/module/bm/mod.rs @@ -1,25 +1,21 @@ -use runtime::Runtime; -use module::Module; -use module::CommandMap; -use module::ModuleResult; -use module::ModuleError; -use std::path::Path; -use std::result::Result; -use std::fmt::Result as FMTResult; -use std::fmt::Formatter; -use std::fmt::Debug; -use clap::ArgMatches; -use regex::Regex; - mod header; mod commands; +use std::fmt::{Debug, Formatter}; +use std::fmt::Result as FMTResult; +use std::path::Path; +use std::result::Result; + +use clap::ArgMatches; +use regex::Regex; + +use module::{CommandMap, Module, ModuleError, ModuleResult}; +use runtime::Runtime; +use self::commands::*; use self::header::build_header; use storage::json::parser::JsonHeaderParser; use storage::parser::FileHeaderParser; -use self::commands::*; - pub struct BMModule { path: Option, } diff --git a/src/module/mod.rs b/src/module/mod.rs index 7d0d0562..a0d9a467 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1,15 +1,13 @@ -use runtime::Runtime; +use std::collections::HashMap; use std::error::Error; -use std::fmt::Formatter; +use std::fmt::{Debug, Display, Formatter}; use std::fmt::Result as FMTResult; -use std::fmt::Display; -use std::fmt::Debug; use std::path::Path; use std::result::Result; -use std::collections::HashMap; use clap::{App, ArgMatches}; +use runtime::Runtime; use storage::backend::{StorageBackend, StorageBackendError}; pub mod bm; diff --git a/src/runtime.rs b/src/runtime.rs index a666c1bf..92c38f6e 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1,14 +1,11 @@ +use std::fmt::{Debug, Formatter, Error}; + extern crate log; +use log::{LogRecord, LogLevel, LogLevelFilter, LogMetadata, SetLoggerError}; pub use cli::CliConfig; pub use configuration::Configuration as Cfg; -use std::fmt::Debug; -use std::fmt::Formatter; -use std::fmt::Error; - -use log::{LogRecord, LogLevel, LogLevelFilter, LogMetadata, SetLoggerError}; - pub struct ImagLogger { lvl: LogLevel, } diff --git a/src/storage/backend.rs b/src/storage/backend.rs index ead88f1a..9e56778d 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -1,27 +1,21 @@ use std::error::Error; -use std::fmt::Display; -use std::fmt::Formatter; +use std::fmt::{Display, Formatter}; use std::fmt::Result as FMTResult; -use std::path::Path; -use std::path::PathBuf; -use std::vec::Vec; use std::fs::File as FSFile; -use std::fs::create_dir_all; -use std::fs::remove_file; -use std::io::Read; -use std::io::Write; -use std::vec::IntoIter; +use std::fs::{create_dir_all, remove_file}; +use std::io::{Read, Write}; +use std::path::{Path, PathBuf}; +use std::vec::{Vec, IntoIter}; use glob::glob; use glob::Paths; +use module::Module; +use runtime::Runtime; use storage::file::File; use storage::file_id::*; use storage::parser::{FileHeaderParser, Parser, ParserError}; -use module::Module; -use runtime::Runtime; - pub type BackendOperationResult = Result; pub struct StorageBackend { diff --git a/src/storage/file.rs b/src/storage/file.rs index da7dd66e..7a14bb5d 100644 --- a/src/storage/file.rs +++ b/src/storage/file.rs @@ -2,12 +2,12 @@ use std::error::Error; use std::fmt::{Debug, Display, Formatter}; use std::fmt; -use module::Module; -use super::parser::{FileHeaderParser, Parser, ParserError}; -use storage::file_id::*; - use regex::Regex; +use module::Module; +use storage::file_id::*; +use super::parser::{FileHeaderParser, Parser, ParserError}; + #[derive(Debug)] #[derive(Clone)] pub enum FileHeaderSpec { diff --git a/src/storage/file_id.rs b/src/storage/file_id.rs index da44de36..7a1353ce 100644 --- a/src/storage/file_id.rs +++ b/src/storage/file_id.rs @@ -1,10 +1,9 @@ +use std::convert::{From, Into}; use std::error::Error; use std::fmt::{Debug, Display, Formatter}; use std::fmt; -use std::result::Result; use std::path::{Path, PathBuf}; -use std::convert::From; -use std::convert::Into; +use std::result::Result; use regex::Regex; diff --git a/src/storage/json/parser.rs b/src/storage/json/parser.rs index 40675583..942edb1b 100644 --- a/src/storage/json/parser.rs +++ b/src/storage/json/parser.rs @@ -1,16 +1,15 @@ +use std::collections::HashMap; +use std::io::stdout; +use std::error::Error; + use serde_json::{Value, from_str}; use serde_json::error::Result as R; use serde_json::Serializer; use serde::ser::Serialize; use serde::ser::Serializer as Ser; -use std::collections::HashMap; -use std::io::stdout; -use std::error::Error; - -use super::super::parser::{FileHeaderParser, ParserError}; -use super::super::file::{FileHeaderSpec, FileHeaderData}; - +use storage::parser::{FileHeaderParser, ParserError}; +use storage::file::{FileHeaderSpec, FileHeaderData}; pub struct JsonHeaderParser { spec: Option, diff --git a/src/storage/parser.rs b/src/storage/parser.rs index 373c6f61..71617724 100644 --- a/src/storage/parser.rs +++ b/src/storage/parser.rs @@ -1,8 +1,9 @@ -use regex::Regex; use std::error::Error; use std::fmt::{Debug, Display, Formatter}; use std::fmt; +use regex::Regex; + use super::file::{FileHeaderSpec, FileHeaderData}; pub struct ParserError { From 1f8bdef1ed067f7133c36b93eb8a80b6db1d6184 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 01:18:12 +0100 Subject: [PATCH 05/14] Remove unused variables --- src/ui/file.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/file.rs b/src/ui/file.rs index fe7ee83b..c454eab6 100644 --- a/src/ui/file.rs +++ b/src/ui/file.rs @@ -29,7 +29,7 @@ struct DebugPrinter { impl FilePrinter for DebugPrinter { - fn new(verbose: bool, debug: bool) -> DebugPrinter { + fn new(_: bool, debug: bool) -> DebugPrinter { DebugPrinter { debug: debug, } From 230d00f35a83f9eab6f9a34dd8b1cb8e8494ddae Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 01:23:06 +0100 Subject: [PATCH 06/14] Remove unused imports --- src/configuration.rs | 2 -- src/module/bm/commands.rs | 4 ++-- src/module/bm/mod.rs | 10 +--------- src/module/mod.rs | 5 ++--- src/storage/backend.rs | 5 +---- src/storage/file_id.rs | 2 +- src/storage/json/parser.rs | 1 - src/storage/parser.rs | 2 +- src/ui/file.rs | 1 - 9 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 8a6d6e31..a26cccff 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -15,8 +15,6 @@ pub struct Configuration { impl Configuration { pub fn new(config: &CliConfig) -> Configuration { - use std::env::home_dir; - let rtp = rtp_path(config).or(default_path()); let mut verbose = false; diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index 3b7a1d35..b4674c92 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -6,10 +6,10 @@ use regex::Regex; use module::{CommandEnv, CommandResult, Module, ModuleError}; use module::bm::header::{build_header, get_tags_from_header}; use runtime::Runtime; -use storage::backend::{StorageBackendError, StorageBackend}; +use storage::backend::StorageBackendError; use storage::file::File; use storage::json::parser::JsonHeaderParser; -use storage::parser::{Parser, FileHeaderParser}; +use storage::parser::Parser; use ui::file::{FilePrinter, TablePrinter}; pub fn add_command(module: &Module, env: CommandEnv) -> CommandResult { diff --git a/src/module/bm/mod.rs b/src/module/bm/mod.rs index 216665ca..08502a77 100644 --- a/src/module/bm/mod.rs +++ b/src/module/bm/mod.rs @@ -3,18 +3,10 @@ mod commands; use std::fmt::{Debug, Formatter}; use std::fmt::Result as FMTResult; -use std::path::Path; -use std::result::Result; -use clap::ArgMatches; -use regex::Regex; - -use module::{CommandMap, Module, ModuleError, ModuleResult}; +use module::{CommandMap, Module, ModuleResult}; use runtime::Runtime; use self::commands::*; -use self::header::build_header; -use storage::json::parser::JsonHeaderParser; -use storage::parser::FileHeaderParser; pub struct BMModule { path: Option, diff --git a/src/module/mod.rs b/src/module/mod.rs index a0d9a467..0d8de8e0 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -2,13 +2,12 @@ use std::collections::HashMap; use std::error::Error; use std::fmt::{Debug, Display, Formatter}; use std::fmt::Result as FMTResult; -use std::path::Path; use std::result::Result; -use clap::{App, ArgMatches}; +use clap::ArgMatches; use runtime::Runtime; -use storage::backend::{StorageBackend, StorageBackendError}; +use storage::backend::StorageBackend; pub mod bm; diff --git a/src/storage/backend.rs b/src/storage/backend.rs index 9e56778d..2d28e9f1 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -4,7 +4,6 @@ use std::fmt::Result as FMTResult; use std::fs::File as FSFile; use std::fs::{create_dir_all, remove_file}; use std::io::{Read, Write}; -use std::path::{Path, PathBuf}; use std::vec::{Vec, IntoIter}; use glob::glob; @@ -14,7 +13,7 @@ use module::Module; use runtime::Runtime; use storage::file::File; use storage::file_id::*; -use storage::parser::{FileHeaderParser, Parser, ParserError}; +use storage::parser::{FileHeaderParser, Parser}; pub type BackendOperationResult = Result; @@ -178,8 +177,6 @@ impl StorageBackend { pub fn get_file_by_id<'a, HP>(&self, m: &'a Module, id: &FileID, p: &Parser) -> Option> where HP: FileHeaderParser { - use std::ops::Index; - debug!("Searching for file with id '{}'", id); if id.get_type() == FileIDType::NONE { diff --git a/src/storage/file_id.rs b/src/storage/file_id.rs index 7a1353ce..a95043d1 100644 --- a/src/storage/file_id.rs +++ b/src/storage/file_id.rs @@ -2,7 +2,7 @@ use std::convert::{From, Into}; use std::error::Error; use std::fmt::{Debug, Display, Formatter}; use std::fmt; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::result::Result; use regex::Regex; diff --git a/src/storage/json/parser.rs b/src/storage/json/parser.rs index 942edb1b..39b6ad58 100644 --- a/src/storage/json/parser.rs +++ b/src/storage/json/parser.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::io::stdout; use std::error::Error; use serde_json::{Value, from_str}; diff --git a/src/storage/parser.rs b/src/storage/parser.rs index 71617724..0f30f04b 100644 --- a/src/storage/parser.rs +++ b/src/storage/parser.rs @@ -4,7 +4,7 @@ use std::fmt; use regex::Regex; -use super::file::{FileHeaderSpec, FileHeaderData}; +use super::file::FileHeaderData; pub struct ParserError { summary: String, diff --git a/src/ui/file.rs b/src/ui/file.rs index c454eab6..625eb43e 100644 --- a/src/ui/file.rs +++ b/src/ui/file.rs @@ -1,6 +1,5 @@ use std::iter::Iterator; -use runtime::Runtime; use storage::file::File; pub trait FilePrinter { From 75cedc15800e21b090e2020c8b9ada72ce38caea Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 10:45:33 +0100 Subject: [PATCH 07/14] Move to rust stable (1.4 atm) so we can use clap --- default.nix | 2 +- src/main.rs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/default.nix b/default.nix index 6408d618..f108e799 100644 --- a/default.nix +++ b/default.nix @@ -1,7 +1,7 @@ { pkgs ? (import {}) }: let - env = with pkgs.rustUnstable; [ + env = with pkgs.rustStable; [ rustc cargo pkgs.llvmPackages.lldb diff --git a/src/main.rs b/src/main.rs index 9e1f99cf..99494a4b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,3 @@ -#![feature(box_patterns)] - #[macro_use] extern crate clap; #[macro_use] extern crate log; #[macro_use] extern crate serde; From 166a0cf5dd9403b1bd185f506be7d0a5868ef3ee Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 11:00:40 +0100 Subject: [PATCH 08/14] bm: remove_command: Refactor into sub-functions --- src/module/bm/commands.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index b4674c92..03952ce2 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -49,11 +49,7 @@ pub fn list_command(module: &Module, env: CommandEnv) -> CommandResult { } pub fn remove_command(module: &Module, env: CommandEnv) -> CommandResult { - let checked : bool = run_removal_checking(&env); - debug!("Checked mode: {}", checked); - if let Some(id) = get_id(env.rt, env.matches) { - debug!("Remove by id: {}", id); - + fn remove_by_id(module: &Module, env: CommandEnv, id: String, checked: bool) -> CommandResult { let parser = Parser::new(JsonHeaderParser::new(None)); let file = env.bk .get_file_by_id(module, &id.into(), &parser) @@ -73,9 +69,9 @@ pub fn remove_command(module: &Module, env: CommandEnv) -> CommandResult { info!("Remove worked"); Ok(()) } - } else { - debug!("Remove more than one file"); + } + fn remove_by_filtering(module: &Module, env: CommandEnv, checked: bool) -> CommandResult { get_filtered_files_from_backend(module, &env).and_then(|files| { let nfiles = files.len(); info!("Removing {} Files", nfiles); @@ -106,6 +102,17 @@ pub fn remove_command(module: &Module, env: CommandEnv) -> CommandResult { } }) } + + let checked : bool = run_removal_checking(&env); + debug!("Checked mode: {}", checked); + + if let Some(id) = get_id(env.rt, env.matches) { + debug!("Remove by id: {}", id); + remove_by_id(module, env, id, checked) + } else { + debug!("Remove more than one file"); + remove_by_filtering(module, env, checked) + } } /* From e8c7df4593b5cc05eef55572e001b48eeefd9466 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 11:03:05 +0100 Subject: [PATCH 09/14] Remove variable, use expression directly --- src/module/bm/commands.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index 03952ce2..ad8f358f 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -88,10 +88,8 @@ pub fn remove_command(module: &Module, env: CommandEnv) -> CommandResult { }) .collect::>(); - let nerrs = errs.len(); - - if nerrs != 0 { - warn!("{} Errors occured while removing {} files", nerrs, nfiles); + if errs.len() != 0 { + warn!("{} Errors occured while removing {} files", errs.len(), nfiles); let moderr = ModuleError::new("File removal failed"); // TODO : Collect StorageBackendErrors From 80563cb340e19c2f29cc638242a76b03c30423dc Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 11:05:50 +0100 Subject: [PATCH 10/14] Move tag filtering to sub-function --- src/module/bm/commands.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index ad8f358f..edc4f3d6 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -123,6 +123,17 @@ fn get_filtered_files_from_backend<'a>(module: &'a Module, env: &CommandEnv) -> Result>, ModuleError> { + fn check_tags(tags: &Vec, file: &File) -> bool { + if tags.len() != 0 { + debug!("Checking tags of: {:?}", file.id()); + get_tags_from_header(&file.header()) + .iter() + .any(|t| tags.contains(t)) + } else { + true + } + } + let parser = Parser::new(JsonHeaderParser::new(None)); let tags = get_tags(env.rt, env.matches); debug!("Tags: {:?}", tags); @@ -130,13 +141,7 @@ fn get_filtered_files_from_backend<'a>(module: &'a Module, .map(|files| { let f = files.filter(|file| { debug!("Backend returns file: {:?}", file); - if tags.len() != 0 { - debug!("Checking tags of: {:?}", file.id()); - get_tags_from_header(&file.header()).iter() - .any(|t| tags.contains(t)) - } else { - true - } + check_tags(&tags, file) }).filter(|file| { debug!("Checking matches of: {:?}", file.id()); get_matcher(env.rt, env.matches) From 93393751b298b8d01999cfc6d7fb72ed70182422 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 11:06:22 +0100 Subject: [PATCH 11/14] Remove uneccessary variable --- src/module/bm/commands.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index edc4f3d6..c72266d1 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -139,7 +139,7 @@ fn get_filtered_files_from_backend<'a>(module: &'a Module, debug!("Tags: {:?}", tags); env.bk.iter_files(module, &parser) .map(|files| { - let f = files.filter(|file| { + files.filter(|file| { debug!("Backend returns file: {:?}", file); check_tags(&tags, file) }).filter(|file| { @@ -147,8 +147,7 @@ fn get_filtered_files_from_backend<'a>(module: &'a Module, get_matcher(env.rt, env.matches) .and_then(|r| Some(file.matches_with(&r))) .unwrap_or(true) - }).collect::>(); - f.into_iter() + }).collect::>().into_iter() }).map_err(|e| { debug!("Error from Backend: {:?}", e); let mut merr = ModuleError::new("Could not filter files"); From 9f59478da06db1946c8bc2db0ae75475c213f3dc Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 11:10:54 +0100 Subject: [PATCH 12/14] Readability fixes --- src/module/bm/commands.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index c72266d1..09d52c44 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -137,7 +137,8 @@ fn get_filtered_files_from_backend<'a>(module: &'a Module, let parser = Parser::new(JsonHeaderParser::new(None)); let tags = get_tags(env.rt, env.matches); debug!("Tags: {:?}", tags); - env.bk.iter_files(module, &parser) + env.bk + .iter_files(module, &parser) .map(|files| { files.filter(|file| { debug!("Backend returns file: {:?}", file); @@ -145,9 +146,11 @@ fn get_filtered_files_from_backend<'a>(module: &'a Module, }).filter(|file| { debug!("Checking matches of: {:?}", file.id()); get_matcher(env.rt, env.matches) - .and_then(|r| Some(file.matches_with(&r))) + .map(|r| file.matches_with(&r)) .unwrap_or(true) - }).collect::>().into_iter() + }) + .collect::>() + .into_iter() }).map_err(|e| { debug!("Error from Backend: {:?}", e); let mut merr = ModuleError::new("Could not filter files"); From e6a32eafc0e3ed5d742d761f355ee4ba384649b2 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 14:20:43 +0100 Subject: [PATCH 13/14] Outsource StorageBackendError building into helper function --- src/storage/backend.rs | 96 +++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 61 deletions(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index 2d28e9f1..9c0d4513 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -35,13 +35,8 @@ impl StorageBackend { }) }).or_else(|e| { debug!("Creating failed, constructing error instance"); - let mut serr = StorageBackendError::new( - "create_dir_all()", - "Could not create store directories", - Some(storepath) - ); - serr.caused_by = Some(Box::new(e)); - Err(serr) + Err(serr_build("create_dir_all()", "Could not create store directories", + Some(storepath), Some(Box::new(e)))) }) } @@ -56,13 +51,8 @@ impl StorageBackend { }) .map_err(|e| { debug!("glob() returned error: {:?}", e); - let serr = StorageBackendError::new( - "iter_ids()", - "Cannot iter on file ids", - None); - // Why the hack is Error not implemented for glob::PatternError - // serr.caused_by = Some(Box::new(e)); - serr + serr_build("iter_ids()", "Cannot iter on file ids", + None, None) }) } @@ -78,11 +68,8 @@ impl StorageBackend { }) .map_err(|e| { debug!("StorageBackend::iter_ids() returned error = {:?}", e); - let mut serr = StorageBackendError::new("iter_files()", - "Cannot iter on files", - None); - serr.caused_by = Some(Box::new(e)); - serr + serr_build("iter_files()", "Cannot iter on files", + None, Some(Box::new(e))) }) } @@ -107,23 +94,14 @@ impl StorageBackend { file.write_all(&string.clone().into_bytes()) .map_err(|ioerr| { debug!("Could not write file"); - let mut err = StorageBackendError::new( - "File::write_all()", - "Could not write out File contents", - None - ); - err.caused_by = Some(Box::new(ioerr)); - err + serr_build("File::write_all()", + "Could not write out File contents", + None, Some(Box::new(ioerr))) }) }).map_err(|writeerr| { debug!("Could not create file at '{}'", path); - let mut err = StorageBackendError::new( - "File::create()", - "Creating file on disk failed", - None - ); - err.caused_by = Some(Box::new(writeerr)); - err + serr_build("File::create()", "Creating file on disk failed", + None, Some(Box::new(writeerr))) }).and(Ok(())) } @@ -147,23 +125,15 @@ impl StorageBackend { file.write_all(&string.clone().into_bytes()) .map_err(|ioerr| { debug!("Could not write file"); - let mut err = StorageBackendError::new( - "File::write()", - "Tried to write contents of this file, though operation did not succeed", - Some(string) - ); - err.caused_by = Some(Box::new(ioerr)); - err + serr_build("File::write()", + "Tried to write contents of this file, though operation did not succeed", + Some(string), Some(Box::new(ioerr))) }) }).map_err(|writeerr| { debug!("Could not write file at '{}'", path); - let mut err = StorageBackendError::new( - "File::open()", - "Tried to update contents of this file, though file doesn't exist", - None - ); - err.caused_by = Some(Box::new(writeerr)); - err + serr_build("File::open()", + "Tried to update contents of this file, though file doesn't exist", + None, Some(Box::new(writeerr))) }).and(Ok(())) } @@ -227,13 +197,8 @@ impl StorageBackend { let fp = self.build_filepath(&file); remove_file(fp).map_err(|e| { - let mut serr = StorageBackendError::new( - "remove_file()", - "File removal failed", - Some(format!("{}", file)) - ); - serr.caused_by = Some(Box::new(e)); - serr + serr_build("remove_file()", "File removal failed", + Some(format!("{}", file)), Some(Box::new(e))) }) } @@ -323,13 +288,9 @@ fn write_with_parser<'a, HP>(f: &File, p: &Parser) -> Result Vec { .collect::>() } +/* + * Helper to build a StorageBackendError object with cause, because one line is + * less than three lines + */ +fn serr_build(action: &'static str, desc: &'static str, + data: Option, caused_by: Option>) + -> StorageBackendError +{ + let mut err = StorageBackendError::new(action, desc, data); + err.caused_by = caused_by; + err +} + From 3a8f2b549308853f1e4450a2b535619a9971dd58 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 5 Dec 2015 15:01:46 +0100 Subject: [PATCH 14/14] Refactor: dont use helper but StorageBackendError::new() directly --- src/storage/backend.rs | 74 ++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index 9c0d4513..28a75232 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -25,6 +25,8 @@ pub struct StorageBackend { impl StorageBackend { pub fn new(rt: &Runtime) -> BackendOperationResult { + use self::StorageBackendError as SBE; + let storepath = rt.get_rtp() + "/store/"; debug!("Trying to create {}", storepath); create_dir_all(&storepath).and_then(|_| { @@ -35,13 +37,15 @@ impl StorageBackend { }) }).or_else(|e| { debug!("Creating failed, constructing error instance"); - Err(serr_build("create_dir_all()", "Could not create store directories", + Err(SBE::new("create_dir_all()", "Could not create store directories", Some(storepath), Some(Box::new(e)))) }) } pub fn iter_ids(&self, m: &Module) -> Result, StorageBackendError> { + use self::StorageBackendError as SBE; + let globstr = self.prefix_of_files_for_module(m) + "*.imag"; debug!("Globstring = {}", globstr); glob(&globstr[..]) @@ -51,8 +55,7 @@ impl StorageBackend { }) .map_err(|e| { debug!("glob() returned error: {:?}", e); - serr_build("iter_ids()", "Cannot iter on file ids", - None, None) + SBE::new("iter_ids()", "Cannot iter on file ids", None, None) }) } @@ -60,6 +63,8 @@ impl StorageBackend { -> Result>, StorageBackendError> where HP: FileHeaderParser { + use self::StorageBackendError as SBE; + self.iter_ids(m) .and_then(|ids| { debug!("Iterating ids and building files from them"); @@ -68,8 +73,8 @@ impl StorageBackend { }) .map_err(|e| { debug!("StorageBackend::iter_ids() returned error = {:?}", e); - serr_build("iter_files()", "Cannot iter on files", - None, Some(Box::new(e))) + SBE::new("iter_files()", "Cannot iter on files", None, + Some(Box::new(e))) }) } @@ -81,6 +86,8 @@ impl StorageBackend { pub fn put_file(&self, f: File, p: &Parser) -> BackendOperationResult where HP: FileHeaderParser { + use self::StorageBackendError as SBE; + let written = write_with_parser(&f, p); if written.is_err() { return Err(written.err().unwrap()); } let string = written.unwrap(); @@ -94,14 +101,14 @@ impl StorageBackend { file.write_all(&string.clone().into_bytes()) .map_err(|ioerr| { debug!("Could not write file"); - serr_build("File::write_all()", - "Could not write out File contents", - None, Some(Box::new(ioerr))) + SBE::new("File::write_all()", + "Could not write out File contents", + None, Some(Box::new(ioerr))) }) }).map_err(|writeerr| { debug!("Could not create file at '{}'", path); - serr_build("File::create()", "Creating file on disk failed", - None, Some(Box::new(writeerr))) + SBE::new("File::create()", "Creating file on disk failed", None, + Some(Box::new(writeerr))) }).and(Ok(())) } @@ -112,6 +119,8 @@ impl StorageBackend { pub fn update_file(&self, f: File, p: &Parser) -> BackendOperationResult where HP: FileHeaderParser { + use self::StorageBackendError as SBE; + let contents = write_with_parser(&f, p); if contents.is_err() { return Err(contents.err().unwrap()); } let string = contents.unwrap(); @@ -125,15 +134,15 @@ impl StorageBackend { file.write_all(&string.clone().into_bytes()) .map_err(|ioerr| { debug!("Could not write file"); - serr_build("File::write()", - "Tried to write contents of this file, though operation did not succeed", - Some(string), Some(Box::new(ioerr))) + SBE::new("File::write()", + "Tried to write contents of this file, though operation did not succeed", + Some(string), Some(Box::new(ioerr))) }) }).map_err(|writeerr| { debug!("Could not write file at '{}'", path); - serr_build("File::open()", - "Tried to update contents of this file, though file doesn't exist", - None, Some(Box::new(writeerr))) + SBE::new("File::open()", + "Tried to update contents of this file, though file doesn't exist", + None, Some(Box::new(writeerr))) }).and(Ok(())) } @@ -187,6 +196,8 @@ impl StorageBackend { } pub fn remove_file(&self, m: &Module, file: File, checked: bool) -> BackendOperationResult { + use self::StorageBackendError as SBE; + if checked { error!("Checked remove not implemented yet. I will crash now"); unimplemented!() @@ -197,8 +208,8 @@ impl StorageBackend { let fp = self.build_filepath(&file); remove_file(fp).map_err(|e| { - serr_build("remove_file()", "File removal failed", - Some(format!("{}", file)), Some(Box::new(e))) + SBE::new("remove_file()", "File removal failed", + Some(format!("{}", file)), Some(Box::new(e))) }) } @@ -250,7 +261,11 @@ pub struct StorageBackendError { impl StorageBackendError { - fn new(action: S, desc: S, data: Option) -> StorageBackendError + fn new(action: S, + desc: S, + data: Option, + cause: Option>) + -> StorageBackendError where S: Into { StorageBackendError { @@ -286,11 +301,13 @@ impl<'a> Display for StorageBackendError { fn write_with_parser<'a, HP>(f: &File, p: &Parser) -> Result where HP: FileHeaderParser { + use self::StorageBackendError as SBE; + p.write(f.contents()) .or_else(|err| { - Err(serr_build("Parser::write()", - "Cannot translate internal representation of file contents into on-disk representation", - None, Some(Box::new(err)))) + Err(SBE::new("Parser::write()", + "Cannot translate internal representation of file contents into on-disk representation", + None, Some(Box::new(err)))) }) } @@ -300,16 +317,3 @@ fn globlist_to_file_id_vec(globlist: Paths) -> Vec { .collect::>() } -/* - * Helper to build a StorageBackendError object with cause, because one line is - * less than three lines - */ -fn serr_build(action: &'static str, desc: &'static str, - data: Option, caused_by: Option>) - -> StorageBackendError -{ - let mut err = StorageBackendError::new(action, desc, data); - err.caused_by = caused_by; - err -} -