From a4bf852207f200b7c4d14a86e66634d5ec9b4927 Mon Sep 17 00:00:00 2001 From: Knight Date: Sun, 21 Aug 2016 14:57:28 +0800 Subject: [PATCH] mv: cleanup the code --- README.md | 2 +- src/mv/Cargo.toml | 7 +- src/mv/mv.rs | 172 +++++++++++++++++++++++++++------------------- tests/test_mv.rs | 19 +++++ 4 files changed, 123 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index 5c5d42d29..009273332 100644 --- a/README.md +++ b/README.md @@ -194,7 +194,7 @@ To do * [x] mkfifo * [x] mknod * [x] mktemp -* [ ] mv (almost done, one more option) +* [x] mv * [x] nice * [x] nl * [x] nohup diff --git a/src/mv/Cargo.toml b/src/mv/Cargo.toml index 435bd5d16..de4f6eef9 100644 --- a/src/mv/Cargo.toml +++ b/src/mv/Cargo.toml @@ -9,11 +9,10 @@ path = "mv.rs" [dependencies] getopts = "*" -libc = "*" -uucore = { path="../uucore" } -[dev-dependencies] -time = "*" +[dependencies.uucore] +path = "../uucore" +default-features = false [[bin]] name = "mv" diff --git a/src/mv/mv.rs b/src/mv/mv.rs index 54460ae25..12fe9cfa3 100644 --- a/src/mv/mv.rs +++ b/src/mv/mv.rs @@ -1,22 +1,21 @@ #![crate_name = "uu_mv"] -/* - * This file is part of the uutils coreutils package. - * - * (c) Orvar Segerström - * (c) Sokovikov Evgeniy - * - * For the full copyright and license information, please view the LICENSE file - * that was distributed with this source code. - */ +// This file is part of the uutils coreutils package. +// +// (c) Orvar Segerström +// (c) Sokovikov Evgeniy +// +// For the full copyright and license information, please view the LICENSE file +// that was distributed with this source code. +// extern crate getopts; -extern crate libc; #[macro_use] extern crate uucore; use std::fs; +use std::env; use std::io::{BufRead, BufReader, Result, stdin, Write}; use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; @@ -41,7 +40,7 @@ pub enum OverwriteMode { Force, } -#[derive(Clone, Eq, PartialEq)] +#[derive(Clone, Copy, Eq, PartialEq)] pub enum BackupMode { NoBackup, SimpleBackup, @@ -52,17 +51,27 @@ pub enum BackupMode { pub fn uumain(args: Vec) -> i32 { let mut opts = getopts::Options::new(); - opts.optflagopt("", "backup", "make a backup of each existing destination file", "CONTROL"); + opts.optflagopt("", + "backup", + "make a backup of each existing destination file", + "CONTROL"); opts.optflag("b", "", "like --backup but does not accept an argument"); opts.optflag("f", "force", "do not prompt before overwriting"); opts.optflag("i", "interactive", "prompt before override"); opts.optflag("n", "no-clobber", "do not overwrite an existing file"); - opts.optflag("", "strip-trailing-slashes", "remove any trailing slashes from each SOURCE\n \ + opts.optflag("", + "strip-trailing-slashes", + "remove any trailing slashes from each SOURCE\n \ argument"); opts.optopt("S", "suffix", "override the usual backup suffix", "SUFFIX"); - opts.optopt("t", "target-directory", "move all SOURCE arguments into DIRECTORY", "DIRECTORY"); + opts.optopt("t", + "target-directory", + "move all SOURCE arguments into DIRECTORY", + "DIRECTORY"); opts.optflag("T", "no-target-directory", "treat DEST as a normal file"); - opts.optflag("u", "update", "move only when the SOURCE file is newer\n \ + opts.optflag("u", + "update", + "move only when the SOURCE file is newer\n \ than the destination file or when the\n \ destination file is missing"); opts.optflag("v", "verbose", "explain what is being done"); @@ -78,11 +87,11 @@ pub fn uumain(args: Vec) -> i32 { }; let usage = opts.usage("Move SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY."); - /* This does not exactly match the GNU implementation: - * The GNU mv defaults to Force, but if more than one of the - * overwrite options are supplied, only the last takes effect. - * To default to no-clobber in that situation seems safer: - */ + // This does not exactly match the GNU implementation: + // The GNU mv defaults to Force, but if more than one of the + // overwrite options are supplied, only the last takes effect. + // To default to no-clobber in that situation seems safer: + // let overwrite_mode = if matches.opt_present("no-clobber") { OverwriteMode::NoClobber } else if matches.opt_present("interactive") { @@ -96,15 +105,19 @@ pub fn uumain(args: Vec) -> i32 { } else if matches.opt_present("backup") { match matches.opt_str("backup") { None => BackupMode::SimpleBackup, - Some(mode) => match &mode[..] { - "simple" | "never" => BackupMode::SimpleBackup, - "numbered" | "t" => BackupMode::NumberedBackup, - "existing" | "nil" => BackupMode::ExistingBackup, - "none" | "off" => BackupMode::NoBackup, - x => { - show_error!("invalid argument ‘{}’ for ‘backup type’\n\ - Try '{} --help' for more information.", x, NAME); - return 1; + Some(mode) => { + match &mode[..] { + "simple" | "never" => BackupMode::SimpleBackup, + "numbered" | "t" => BackupMode::NumberedBackup, + "existing" | "nil" => BackupMode::ExistingBackup, + "none" | "off" => BackupMode::NoBackup, + x => { + show_error!("invalid argument ‘{}’ for ‘backup type’\n\ + Try '{} --help' for more information.", + x, + NAME); + return 1; + } } } } @@ -114,7 +127,8 @@ pub fn uumain(args: Vec) -> i32 { if overwrite_mode == OverwriteMode::NoClobber && backup_mode != BackupMode::NoBackup { show_error!("options --backup and --no-clobber are mutually exclusive\n\ - Try '{} --help' for more information.", NAME); + Try '{} --help' for more information.", + NAME); return 1; } @@ -123,12 +137,17 @@ pub fn uumain(args: Vec) -> i32 { Some(x) => x, None => { show_error!("option '--suffix' requires an argument\n\ - Try '{} --help' for more information.", NAME); + Try '{} --help' for more information.", + NAME); return 1; } } } else { - "~".to_owned() + if let (Ok(s), BackupMode::SimpleBackup) = (env::var("SIMPLE_BACKUP_SUFFIX"), backup_mode) { + s + } else { + "~".to_owned() + } }; if matches.opt_present("T") && matches.opt_present("t") { @@ -147,14 +166,11 @@ pub fn uumain(args: Vec) -> i32 { }; let paths: Vec = { - fn string_to_path<'a>(s: &'a String) -> &'a Path { - Path::new(s) - }; fn strip_slashes<'a>(p: &'a Path) -> &'a Path { p.components().as_path() } let to_owned = |p: &Path| p.to_owned(); - let arguments = matches.free.iter().map(string_to_path); + let arguments = matches.free.iter().map(Path::new); if matches.opt_present("strip-trailing-slashes") { arguments.map(strip_slashes).map(to_owned).collect() } else { @@ -177,25 +193,29 @@ fn help(usage: &str) { println!("{0} {1}\n\n\ Usage: {0} SOURCE DEST\n \ or: {0} SOURCE... DIRECTORY\n\n\ - {2}", NAME, VERSION, usage); + {2}", + NAME, + VERSION, + usage); } fn exec(files: &[PathBuf], b: Behaviour) -> i32 { - match b.target_dir { - Some(ref name) => return move_files_into_dir(files, &PathBuf::from(name), &b), - None => {} + if let Some(ref name) = b.target_dir { + return move_files_into_dir(files, &PathBuf::from(name), &b); } match files.len() { 0 | 1 => { show_error!("missing file operand\n\ - Try '{} --help' for more information.", NAME); + Try '{} --help' for more information.", + NAME); return 1; - }, + } 2 => { let source = &files[0]; let target = &files[1]; if !source.exists() { - show_error!("cannot stat ‘{}’: No such file or directory", source.display()); + show_error!("cannot stat ‘{}’: No such file or directory", + source.display()); return 1; } @@ -203,7 +223,7 @@ fn exec(files: &[PathBuf], b: Behaviour) -> i32 { if b.no_target_dir { if !source.is_dir() { show_error!("cannot overwrite directory ‘{}’ with non-directory", - target.display()); + target.display()); return 1; } @@ -211,9 +231,9 @@ fn exec(files: &[PathBuf], b: Behaviour) -> i32 { Err(e) => { show_error!("{}", e); 1 - }, - _ => 0 - } + } + _ => 0, + }; } return move_files_into_dir(&[source.clone()], target, &b); @@ -227,11 +247,13 @@ fn exec(files: &[PathBuf], b: Behaviour) -> i32 { _ => { if b.no_target_dir { show_error!("mv: extra operand ‘{}’\n\ - Try '{} --help' for more information.", files[2].display(), NAME); + Try '{} --help' for more information.", + files[2].display(), + NAME); return 1; } let target_dir = files.last().unwrap(); - move_files_into_dir(&files[0..files.len()-1], target_dir, &b); + move_files_into_dir(&files[..files.len() - 1], target_dir, &b); } } 0 @@ -258,11 +280,17 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behaviour) - if let Err(e) = rename(sourcepath, &targetpath, b) { show_error!("mv: cannot move ‘{}’ to ‘{}’: {}", - sourcepath.display(), targetpath.display(), e); + sourcepath.display(), + targetpath.display(), + e); all_successful = false; } - }; - if all_successful { 0 } else { 1 } + } + if all_successful { + 0 + } else { + 1 + } } fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> { @@ -276,7 +304,7 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> { if !read_yes() { return Ok(()); } - }, + } OverwriteMode::Force => {} }; @@ -284,7 +312,7 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> { BackupMode::NoBackup => None, BackupMode::SimpleBackup => Some(simple_backup_path(to, &b.suffix)), BackupMode::NumberedBackup => Some(numbered_backup_path(to)), - BackupMode::ExistingBackup => Some(existing_backup_path(to, &b.suffix)) + BackupMode::ExistingBackup => Some(existing_backup_path(to, &b.suffix)), }; if let Some(ref p) = backup_path { try!(fs::rename(to, p)); @@ -303,7 +331,7 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> { print!("‘{}’ -> ‘{}’", from.display(), to.display()); match backup_path { Some(path) => println!(" (backup: ‘{}’)", path.display()), - None => println!("") + None => println!(""), } } Ok(()) @@ -312,35 +340,35 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> { fn read_yes() -> bool { let mut s = String::new(); match BufReader::new(stdin()).read_line(&mut s) { - Ok(_) => match s.char_indices().nth(0) { - Some((_, x)) => x == 'y' || x == 'Y', - _ => false - }, - _ => false + Ok(_) => { + match s.chars().nth(0) { + Some(x) => x == 'y' || x == 'Y', + _ => false, + } + } + _ => false, } } fn simple_backup_path(path: &PathBuf, suffix: &str) -> PathBuf { - let mut p = path.as_os_str().to_str().unwrap().to_owned(); + let mut p = path.to_string_lossy().into_owned(); p.push_str(suffix); PathBuf::from(p) } fn numbered_backup_path(path: &PathBuf) -> PathBuf { - let mut i: u64 = 1; - loop { - let new_path = simple_backup_path(path, &format!(".~{}~", i)); - if !new_path.exists() { - return new_path; - } - i = i + 1; - } + (1_u64..) + .map(|i| path.with_extension(format!("~{}~", i))) + .skip_while(|p| p.exists()) + .next() + .expect("cannot create backup") } fn existing_backup_path(path: &PathBuf, suffix: &str) -> PathBuf { - let test_path = simple_backup_path(path, &".~1~".to_owned()); + let test_path = path.with_extension("~1~"); if test_path.exists() { - return numbered_backup_path(path); + numbered_backup_path(path) + } else { + simple_backup_path(path, suffix) } - simple_backup_path(path, suffix) } diff --git a/tests/test_mv.rs b/tests/test_mv.rs index b9b92edcb..732b66647 100644 --- a/tests/test_mv.rs +++ b/tests/test_mv.rs @@ -208,6 +208,25 @@ fn test_mv_custom_backup_suffix() { assert!(at.file_exists(&format!("{}{}", file_b, suffix))); } +#[test] +fn test_mv_custom_backup_suffix_via_env() { + let (at, mut ucmd) = at_and_ucmd(); + let file_a = "test_mv_custom_backup_suffix_file_a"; + let file_b = "test_mv_custom_backup_suffix_file_b"; + let suffix = "super-suffix-of-the-century"; + at.touch(file_a); + at.touch(file_b); + ucmd.arg("-b") + .env("SIMPLE_BACKUP_SUFFIX", suffix) + .arg(file_a) + .arg(file_b) + .succeeds().no_stderr(); + + assert!(!at.file_exists(file_a)); + assert!(at.file_exists(file_b)); + assert!(at.file_exists(&format!("{}{}", file_b, suffix))); +} + #[test] fn test_mv_backup_numbering() { let (at, mut ucmd) = at_and_ucmd();