From 43850bf20e50b8db8afff3aab4e91d6e198dddb6 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Wed, 9 Feb 2022 09:56:27 -0500 Subject: [PATCH] Re-port filesystem commands (#4387) * Re-port the filesystem commands * Remove commented out section --- Cargo.lock | 7 + crates/nu-command/Cargo.toml | 1 + crates/nu-command/src/filesystem/cp.rs | 370 +++++++++++++++-------- crates/nu-command/src/filesystem/mv.rs | 187 ++++++------ crates/nu-command/src/filesystem/rm.rs | 298 +++++++++--------- crates/nu-command/src/filesystem/util.rs | 1 + crates/nu-command/tests/commands/cp.rs | 4 - crates/nu-command/tests/commands/rm.rs | 8 - crates/nu-protocol/src/config.rs | 9 + 9 files changed, 494 insertions(+), 391 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 46de16677..8885eaad7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1062,6 +1062,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs_extra" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" + [[package]] name = "futf" version = "0.1.5" @@ -2180,6 +2186,7 @@ dependencies = [ "eml-parser", "encoding_rs", "filesize", + "fs_extra", "glob", "hamcrest2", "htmlescape", diff --git a/crates/nu-command/Cargo.toml b/crates/nu-command/Cargo.toml index 5c6cdbfb0..e67d24901 100644 --- a/crates/nu-command/Cargo.toml +++ b/crates/nu-command/Cargo.toml @@ -38,6 +38,7 @@ dtparse = "1.2.0" eml-parser = "0.1.0" encoding_rs = "0.8.30" filesize = "0.2.0" +fs_extra = "1.2.0" glob = "0.3.0" htmlescape = "0.3.1" ical = "0.7.0" diff --git a/crates/nu-command/src/filesystem/cp.rs b/crates/nu-command/src/filesystem/cp.rs index 060246b33..fa05bb572 100644 --- a/crates/nu-command/src/filesystem/cp.rs +++ b/crates/nu-command/src/filesystem/cp.rs @@ -1,15 +1,20 @@ use std::path::PathBuf; -use super::util::get_interactive_confirmation; use nu_engine::env::current_dir; use nu_engine::CallExt; use nu_path::canonicalize_with; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; -use nu_protocol::{Category, PipelineData, ShellError, Signature, SyntaxShape}; +use nu_protocol::{Category, PipelineData, ShellError, Signature, Spanned, SyntaxShape}; use crate::filesystem::util::FileStructure; +const GLOB_PARAMS: glob::MatchOptions = glob::MatchOptions { + case_sensitive: true, + require_literal_separator: false, + require_literal_leading_dot: false, +}; + #[derive(Clone)] pub struct Cp; @@ -32,8 +37,9 @@ impl Command for Cp { "copy recursively through subdirectories", Some('r'), ) - .switch("force", "suppress error when no file", Some('f')) - .switch("interactive", "ask user to confirm action", Some('i')) + // TODO: add back in additional features + // .switch("force", "suppress error when no file", Some('f')) + // .switch("interactive", "ask user to confirm action", Some('i')) .category(Category::FileSystem) } @@ -44,93 +50,49 @@ impl Command for Cp { call: &Call, _input: PipelineData, ) -> Result { - let source: String = call.req(engine_state, stack, 0)?; - let destination: String = call.req(engine_state, stack, 1)?; - let interactive = call.has_flag("interactive"); - let force = call.has_flag("force"); + let src: Spanned = call.req(engine_state, stack, 0)?; + let dst: Spanned = call.req(engine_state, stack, 1)?; + let recursive = call.has_flag("recursive"); let path = current_dir(engine_state, stack)?; - let source = path.join(source.as_str()); - let destination = path.join(destination.as_str()); + let source = path.join(src.item.as_str()); + let destination = path.join(dst.item.as_str()); + + let sources: Vec<_> = match glob::glob_with(&source.to_string_lossy(), GLOB_PARAMS) { + Ok(files) => files.collect(), + Err(e) => { + return Err(ShellError::SpannedLabeledError( + e.to_string(), + "invalid pattern".to_string(), + src.span, + )) + } + }; - let mut sources = - glob::glob(&source.to_string_lossy()).map_or_else(|_| Vec::new(), Iterator::collect); if sources.is_empty() { - return Err(ShellError::FileNotFound(call.positional[0].span)); - } - - if sources.len() > 1 && !destination.is_dir() { - return Err(ShellError::MoveNotPossible { - source_message: "Can't move many files".to_string(), - source_span: call.positional[0].span, - destination_message: "into single file".to_string(), - destination_span: call.positional[1].span, - }); - } - - let any_source_is_dir = sources.iter().any(|f| matches!(f, Ok(f) if f.is_dir())); - let recursive: bool = call.has_flag("recursive"); - if any_source_is_dir && !recursive { - return Err(ShellError::MoveNotPossibleSingle( - "Directories must be copied using \"--recursive\"".to_string(), - call.positional[0].span, + return Err(ShellError::SpannedLabeledError( + "No matches found".into(), + "no matches found".into(), + src.span, )); } - if interactive && !force { - let mut remove: Vec = vec![]; - for (index, file) in sources.iter().enumerate() { - let prompt = format!( - "Are you shure that you want to copy {} to {}?", - file.as_ref() - .map_err(|err| ShellError::SpannedLabeledError( - "Reference error".into(), - err.to_string(), - call.head - ))? - .file_name() - .ok_or_else(|| ShellError::SpannedLabeledError( - "File name error".into(), - "Unable to get file name".into(), - call.head - ))? - .to_str() - .ok_or_else(|| ShellError::SpannedLabeledError( - "Unable to get str error".into(), - "Unable to convert to str file name".into(), - call.head - ))?, - destination - .file_name() - .ok_or_else(|| ShellError::SpannedLabeledError( - "File name error".into(), - "Unable to get file name".into(), - call.head - ))? - .to_str() - .ok_or_else(|| ShellError::SpannedLabeledError( - "Unable to get str error".into(), - "Unable to convert to str file name".into(), - call.head - ))?, - ); + if sources.len() > 1 && !destination.is_dir() { + return Err(ShellError::SpannedLabeledError( + "Destination must be a directory when copying multiple files".into(), + "is not a directory".into(), + dst.span, + )); + } - let input = get_interactive_confirmation(prompt)?; + let any_source_is_dir = sources.iter().any(|f| matches!(f, Ok(f) if f.is_dir())); - if !input { - remove.push(index); - } - } - - remove.reverse(); - - for index in remove { - sources.remove(index); - } - - if sources.is_empty() { - return Err(ShellError::NoFileToBeCopied()); - } + if any_source_is_dir && !recursive { + return Err(ShellError::SpannedLabeledError( + "Directories must be copied using \"--recursive\"".into(), + "resolves to a directory (not copied)".into(), + src.span, + )); } for entry in sources.into_iter().flatten() { @@ -140,7 +102,7 @@ impl Command for Cp { if entry.is_file() { let sources = sources.paths_applying_with(|(source_file, _depth_level)| { if destination.is_dir() { - let mut dest = canonicalize_with(&destination, &path)?; + let mut dest = canonicalize_with(&dst.item, &path)?; if let Some(name) = entry.file_name() { dest.push(name); } @@ -152,15 +114,8 @@ impl Command for Cp { for (src, dst) in sources { if src.is_file() { - std::fs::copy(&src, dst).map_err(|e| { - ShellError::MoveNotPossibleSingle( - format!( - "failed to move containing file \"{}\": {}", - src.to_string_lossy(), - e - ), - call.positional[0].span, - ) + std::fs::copy(src, dst).map_err(|e| { + ShellError::SpannedLabeledError(e.to_string(), e.to_string(), call.head) })?; } } @@ -171,58 +126,48 @@ impl Command for Cp { match entry.file_name() { Some(name) => destination.join(name), None => { - return Err(ShellError::FileNotFoundCustom( - format!("containing \"{:?}\" is not a valid path", entry), - call.positional[0].span, + return Err(ShellError::SpannedLabeledError( + "Copy aborted. Not a valid path".into(), + "not a valid path".into(), + dst.span, )) } } }; std::fs::create_dir_all(&destination).map_err(|e| { - ShellError::MoveNotPossibleSingle( - format!("failed to recursively fill destination: {}", e), - call.positional[1].span, - ) + ShellError::SpannedLabeledError(e.to_string(), e.to_string(), dst.span) })?; let sources = sources.paths_applying_with(|(source_file, depth_level)| { let mut dest = destination.clone(); let path = canonicalize_with(&source_file, &path)?; - let components = path + + #[allow(clippy::needless_collect)] + let comps: Vec<_> = path .components() .map(|fragment| fragment.as_os_str()) .rev() - .take(1 + depth_level); + .take(1 + depth_level) + .collect(); + + for fragment in comps.into_iter().rev() { + dest.push(fragment); + } - components.for_each(|fragment| dest.push(fragment)); Ok((PathBuf::from(&source_file), dest)) })?; - for (src, dst) in sources { - if src.is_dir() && !dst.exists() { - std::fs::create_dir_all(&dst).map_err(|e| { - ShellError::MoveNotPossibleSingle( - format!( - "failed to create containing directory \"{}\": {}", - dst.to_string_lossy(), - e - ), - call.positional[1].span, - ) + for (s, d) in sources { + if s.is_dir() && !d.exists() { + std::fs::create_dir_all(&d).map_err(|e| { + ShellError::SpannedLabeledError(e.to_string(), e.to_string(), dst.span) })?; } - if src.is_file() { - std::fs::copy(&src, &dst).map_err(|e| { - ShellError::MoveNotPossibleSingle( - format!( - "failed to move containing file \"{}\": {}", - src.to_string_lossy(), - e - ), - call.positional[0].span, - ) + if s.is_file() { + std::fs::copy(&s, &d).map_err(|e| { + ShellError::SpannedLabeledError(e.to_string(), e.to_string(), call.head) })?; } } @@ -231,4 +176,183 @@ impl Command for Cp { Ok(PipelineData::new(call.head)) } + + // let mut sources = + // glob::glob(&source.to_string_lossy()).map_or_else(|_| Vec::new(), Iterator::collect); + // if sources.is_empty() { + // return Err(ShellError::FileNotFound(call.positional[0].span)); + // } + + // if sources.len() > 1 && !destination.is_dir() { + // return Err(ShellError::MoveNotPossible { + // source_message: "Can't move many files".to_string(), + // source_span: call.positional[0].span, + // destination_message: "into single file".to_string(), + // destination_span: call.positional[1].span, + // }); + // } + + // let any_source_is_dir = sources.iter().any(|f| matches!(f, Ok(f) if f.is_dir())); + // let recursive: bool = call.has_flag("recursive"); + // if any_source_is_dir && !recursive { + // return Err(ShellError::MoveNotPossibleSingle( + // "Directories must be copied using \"--recursive\"".to_string(), + // call.positional[0].span, + // )); + // } + + // if interactive && !force { + // let mut remove: Vec = vec![]; + // for (index, file) in sources.iter().enumerate() { + // let prompt = format!( + // "Are you shure that you want to copy {} to {}?", + // file.as_ref() + // .map_err(|err| ShellError::SpannedLabeledError( + // "Reference error".into(), + // err.to_string(), + // call.head + // ))? + // .file_name() + // .ok_or_else(|| ShellError::SpannedLabeledError( + // "File name error".into(), + // "Unable to get file name".into(), + // call.head + // ))? + // .to_str() + // .ok_or_else(|| ShellError::SpannedLabeledError( + // "Unable to get str error".into(), + // "Unable to convert to str file name".into(), + // call.head + // ))?, + // destination + // .file_name() + // .ok_or_else(|| ShellError::SpannedLabeledError( + // "File name error".into(), + // "Unable to get file name".into(), + // call.head + // ))? + // .to_str() + // .ok_or_else(|| ShellError::SpannedLabeledError( + // "Unable to get str error".into(), + // "Unable to convert to str file name".into(), + // call.head + // ))?, + // ); + + // let input = get_interactive_confirmation(prompt)?; + + // if !input { + // remove.push(index); + // } + // } + + // remove.reverse(); + + // for index in remove { + // sources.remove(index); + // } + + // if sources.is_empty() { + // return Err(ShellError::NoFileToBeCopied()); + // } + // } + + // for entry in sources.into_iter().flatten() { + // let mut sources = FileStructure::new(); + // sources.walk_decorate(&entry, engine_state, stack)?; + + // if entry.is_file() { + // let sources = sources.paths_applying_with(|(source_file, _depth_level)| { + // if destination.is_dir() { + // let mut dest = canonicalize_with(&destination, &path)?; + // if let Some(name) = entry.file_name() { + // dest.push(name); + // } + // Ok((source_file, dest)) + // } else { + // Ok((source_file, destination.clone())) + // } + // })?; + + // for (src, dst) in sources { + // if src.is_file() { + // std::fs::copy(&src, dst).map_err(|e| { + // ShellError::MoveNotPossibleSingle( + // format!( + // "failed to move containing file \"{}\": {}", + // src.to_string_lossy(), + // e + // ), + // call.positional[0].span, + // ) + // })?; + // } + // } + // } else if entry.is_dir() { + // let destination = if !destination.exists() { + // destination.clone() + // } else { + // match entry.file_name() { + // Some(name) => destination.join(name), + // None => { + // return Err(ShellError::FileNotFoundCustom( + // format!("containing \"{:?}\" is not a valid path", entry), + // call.positional[0].span, + // )) + // } + // } + // }; + + // std::fs::create_dir_all(&destination).map_err(|e| { + // ShellError::MoveNotPossibleSingle( + // format!("failed to recursively fill destination: {}", e), + // call.positional[1].span, + // ) + // })?; + + // let sources = sources.paths_applying_with(|(source_file, depth_level)| { + // let mut dest = destination.clone(); + // let path = canonicalize_with(&source_file, &path)?; + // let components = path + // .components() + // .map(|fragment| fragment.as_os_str()) + // .rev() + // .take(1 + depth_level); + + // components.for_each(|fragment| dest.push(fragment)); + // Ok((PathBuf::from(&source_file), dest)) + // })?; + + // for (src, dst) in sources { + // if src.is_dir() && !dst.exists() { + // std::fs::create_dir_all(&dst).map_err(|e| { + // ShellError::MoveNotPossibleSingle( + // format!( + // "failed to create containing directory \"{}\": {}", + // dst.to_string_lossy(), + // e + // ), + // call.positional[1].span, + // ) + // })?; + // } + + // if src.is_file() { + // std::fs::copy(&src, &dst).map_err(|e| { + // ShellError::MoveNotPossibleSingle( + // format!( + // "failed to move containing file \"{}\": {}", + // src.to_string_lossy(), + // e + // ), + // call.positional[0].span, + // ) + // })?; + // } + // } + // } + // } + + // Ok(PipelineData::new(call.head)) + // } } diff --git a/crates/nu-command/src/filesystem/mv.rs b/crates/nu-command/src/filesystem/mv.rs index 737143172..1ac13a508 100644 --- a/crates/nu-command/src/filesystem/mv.rs +++ b/crates/nu-command/src/filesystem/mv.rs @@ -1,11 +1,17 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; -use super::util::get_interactive_confirmation; +// use super::util::get_interactive_confirmation; use nu_engine::env::current_dir; use nu_engine::CallExt; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; -use nu_protocol::{Category, PipelineData, ShellError, Signature, Spanned, SyntaxShape}; +use nu_protocol::{Category, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape}; + +const GLOB_PARAMS: glob::MatchOptions = glob::MatchOptions { + case_sensitive: true, + require_literal_separator: false, + require_literal_leading_dot: false, +}; #[derive(Clone)] pub struct Mv; @@ -32,8 +38,8 @@ impl Command for Mv { SyntaxShape::Filepath, "the location to move files/directories to", ) - .switch("interactive", "ask user to confirm action", Some('i')) - .switch("force", "suppress error when no file", Some('f')) + // .switch("interactive", "ask user to confirm action", Some('i')) + // .switch("force", "suppress error when no file", Some('f')) .category(Category::FileSystem) } @@ -46,99 +52,58 @@ impl Command for Mv { ) -> Result { // TODO: handle invalid directory or insufficient permissions when moving let spanned_source: Spanned = call.req(engine_state, stack, 0)?; - let destination: String = call.req(engine_state, stack, 1)?; - let interactive = call.has_flag("interactive"); - let force = call.has_flag("force"); + let spanned_destination: Spanned = call.req(engine_state, stack, 1)?; + // let interactive = call.has_flag("interactive"); + // let force = call.has_flag("force"); let path = current_dir(engine_state, stack)?; let source = path.join(spanned_source.item.as_str()); - let destination = path.join(destination.as_str()); + let destination = path.join(spanned_destination.item.as_str()); - let mut sources = - glob::glob(&source.to_string_lossy()).map_or_else(|_| Vec::new(), Iterator::collect); + let mut sources = glob::glob_with(&source.to_string_lossy(), GLOB_PARAMS) + .map_or_else(|_| Vec::new(), Iterator::collect); if sources.is_empty() { - return Err(ShellError::FileNotFound(spanned_source.span)); + return Err(ShellError::SpannedLabeledError( + "Invalid file or pattern".into(), + "invalid file or pattern".into(), + spanned_source.span, + )); } - if interactive && !force { - let mut remove: Vec = vec![]; - for (index, file) in sources.iter().enumerate() { - let prompt = format!( - "Are you shure that you want to move {} to {}?", - file.as_ref() - .map_err(|err| ShellError::SpannedLabeledError( - "Reference error".into(), - err.to_string(), - call.head - ))? - .file_name() - .ok_or_else(|| ShellError::SpannedLabeledError( - "File name error".into(), - "Unable to get file name".into(), - call.head - ))? - .to_str() - .ok_or_else(|| ShellError::SpannedLabeledError( - "Unable to get str error".into(), - "Unable to convert to str file name".into(), - call.head - ))?, - destination - .file_name() - .ok_or_else(|| ShellError::SpannedLabeledError( - "File name error".into(), - "Unable to get file name".into(), - call.head - ))? - .to_str() - .ok_or_else(|| ShellError::SpannedLabeledError( - "Unable to get str error".into(), - "Unable to convert to str file name".into(), - call.head - ))?, - ); - - let input = get_interactive_confirmation(prompt)?; - - if !input { - remove.push(index); - } - } - - remove.reverse(); - - for index in remove { - sources.remove(index); - } - - if sources.is_empty() { - return Err(ShellError::NoFileToBeMoved()); - } - } + // We have two possibilities. + // + // First, the destination exists. + // - If a directory, move everything into that directory, otherwise + // - if only a single source, overwrite the file, otherwise + // - error. + // + // Second, the destination doesn't exist, so we can only rename a single source. Otherwise + // it's an error. if (destination.exists() && !destination.is_dir() && sources.len() > 1) || (!destination.exists() && sources.len() > 1) { - return Err(ShellError::MoveNotPossible { - source_message: "Can't move many files".to_string(), - source_span: call.positional[0].span, - destination_message: "into single file".to_string(), - destination_span: call.positional[1].span, - }); + return Err(ShellError::SpannedLabeledError( + "Can only move multiple sources if destination is a directory".into(), + "destination must be a directory when multiple sources".into(), + spanned_destination.span, + )); } let some_if_source_is_destination = sources .iter() .find(|f| matches!(f, Ok(f) if destination.starts_with(f))); if destination.exists() && destination.is_dir() && sources.len() == 1 { - if let Some(Ok(_filename)) = some_if_source_is_destination { - return Err(ShellError::MoveNotPossible { - source_message: "Can't move directory".to_string(), - source_span: call.positional[0].span, - destination_message: "into itself".to_string(), - destination_span: call.positional[1].span, - }); + if let Some(Ok(filename)) = some_if_source_is_destination { + return Err(ShellError::SpannedLabeledError( + format!( + "Not possible to move {:?} to itself", + filename.file_name().expect("Invalid file name") + ), + "cannot move to itself".into(), + spanned_destination.span, + )); } } @@ -150,20 +115,41 @@ impl Command for Mv { } for entry in sources.into_iter().flatten() { - move_file(call, &entry, &destination)? + move_file( + Spanned { + item: entry, + span: spanned_source.span, + }, + Spanned { + item: destination.clone(), + span: spanned_destination.span, + }, + )? } Ok(PipelineData::new(call.head)) } } -fn move_file(call: &Call, from: &Path, to: &Path) -> Result<(), ShellError> { +fn move_file( + spanned_from: Spanned, + spanned_to: Spanned, +) -> Result<(), ShellError> { + let Spanned { + item: from, + span: from_span, + } = spanned_from; + let Spanned { + item: to, + span: to_span, + } = spanned_to; + if to.exists() && from.is_dir() && to.is_file() { return Err(ShellError::MoveNotPossible { source_message: "Can't move a directory".to_string(), - source_span: call.positional[0].span, + source_span: spanned_from.span, destination_message: "to a file".to_string(), - destination_span: call.positional[1].span, + destination_span: spanned_to.span, }); } @@ -174,29 +160,42 @@ fn move_file(call: &Call, from: &Path, to: &Path) -> Result<(), ShellError> { }; if !destination_dir_exists { - return Err(ShellError::DirectoryNotFound(call.positional[1].span)); + return Err(ShellError::DirectoryNotFound(to_span)); } - let mut to = to.to_path_buf(); + let mut to = to; if to.is_dir() { let from_file_name = match from.file_name() { Some(name) => name, - None => return Err(ShellError::DirectoryNotFound(call.positional[1].span)), + None => return Err(ShellError::DirectoryNotFound(to_span)), }; to.push(from_file_name); } - move_item(call, from, &to) + move_item(&from, from_span, &to) } -fn move_item(call: &Call, from: &Path, to: &Path) -> Result<(), ShellError> { +fn move_item(from: &Path, from_span: Span, to: &Path) -> Result<(), ShellError> { // We first try a rename, which is a quick operation. If that doesn't work, we'll try a copy // and remove the old file/folder. This is necessary if we're moving across filesystems or devices. - std::fs::rename(&from, &to).map_err(|_| ShellError::MoveNotPossible { - source_message: "failed to move".to_string(), - source_span: call.positional[0].span, - destination_message: "into".to_string(), - destination_span: call.positional[1].span, + std::fs::rename(&from, &to).or_else(|_| { + match if from.is_file() { + let mut options = fs_extra::file::CopyOptions::new(); + options.overwrite = true; + fs_extra::file::move_file(from, to, &options) + } else { + let mut options = fs_extra::dir::CopyOptions::new(); + options.overwrite = true; + options.copy_inside = true; + fs_extra::dir::move_dir(from, to, &options) + } { + Ok(_) => Ok(()), + Err(e) => Err(ShellError::SpannedLabeledError( + format!("Could not move {:?} to {:?}. {:}", from, to, e), + "could not move".into(), + from_span, + )), + } }) } diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index b49ce0629..a898ca080 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/crates/nu-command/src/filesystem/rm.rs @@ -1,32 +1,29 @@ +use std::collections::HashMap; +use std::io::ErrorKind; #[cfg(unix)] use std::os::unix::prelude::FileTypeExt; use std::path::PathBuf; -use super::util::get_interactive_confirmation; +// use super::util::get_interactive_confirmation; use nu_engine::env::current_dir; use nu_engine::CallExt; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, SyntaxShape, - Value, + Category, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Span, Spanned, + SyntaxShape, Value, +}; + +const GLOB_PARAMS: glob::MatchOptions = glob::MatchOptions { + case_sensitive: true, + require_literal_separator: false, + require_literal_leading_dot: false, }; #[derive(Clone)] pub struct Rm; -// Where self.0 is the unexpanded target's positional index (i.e. call.positional[self.0].span) -struct Target(usize, PathBuf); - -struct RmArgs { - targets: Vec, - recursive: bool, - trash: bool, - permanent: bool, - force: bool, -} - impl Command for Rm { fn name(&self) -> &str { "rm" @@ -50,7 +47,7 @@ impl Command for Rm { ) .switch("recursive", "delete subdirectories recursively", Some('r')) .switch("force", "suppress error when no file", Some('f')) - .switch("interactive", "ask user to confirm action", Some('i')) + // .switch("interactive", "ask user to confirm action", Some('i')) .rest( "rest", SyntaxShape::GlobPattern, @@ -77,142 +74,120 @@ fn rm( ) -> Result { let trash = call.has_flag("trash"); let permanent = call.has_flag("permanent"); - let interactive = call.has_flag("interactive"); + let recursive = call.has_flag("recursive"); + let force = call.has_flag("force"); + // let interactive = call.has_flag("interactive"); - if trash && permanent { - return Err(ShellError::IncompatibleParametersSingle( - "Can't use \"--trash\" with \"--permanent\"".to_string(), + let ctrlc = engine_state.ctrlc.clone(); + + let targets: Vec> = call.rest(engine_state, stack, 0)?; + let span = call.head; + + let config = stack.get_config()?; + + let rm_always_trash = config.rm_always_trash; + + #[cfg(not(feature = "trash-support"))] + { + if rm_always_trash { + return Err(ShellError::SpannedLabeledError( + "Cannot execute `rm`; the current configuration specifies \ + `rm_always_trash = true`, but the current nu executable was not \ + built with feature `trash_support`." + .into(), + "trash required to be true but not supported".into(), + span, + )); + } else if trash { + return Err(ShellError::SpannedLabeledError( + "Cannot execute `rm` with option `--trash`; feature `trash-support` not enabled" + .into(), + "this option is only available if nu is built with the `trash-support` feature" + .into(), + span, + )); + } + } + + if targets.is_empty() { + return Err(ShellError::SpannedLabeledError( + "rm requires target paths".into(), + "needs parameter".into(), + span, + )); + } + + let path = current_dir(engine_state, stack)?; + + let mut all_targets: HashMap = HashMap::new(); + for target in targets { + if path.to_string_lossy() == target.item + || path.as_os_str().to_string_lossy().starts_with(&format!( + "{}{}", + target.item, + std::path::MAIN_SEPARATOR + )) + { + return Err(ShellError::SpannedLabeledError( + "Cannot remove any parent directory".into(), + "cannot remove any parent directory".into(), + target.span, + )); + } + + let path = path.join(&target.item); + match glob::glob_with( + &path.to_string_lossy(), + glob::MatchOptions { + require_literal_leading_dot: true, + ..GLOB_PARAMS + }, + ) { + Ok(files) => { + for file in files { + match file { + Ok(ref f) => { + // It is not appropriate to try and remove the + // current directory or its parent when using + // glob patterns. + let name = f.display().to_string(); + if name.ends_with("/.") || name.ends_with("/..") { + continue; + } + + all_targets.entry(f.clone()).or_insert_with(|| target.span); + } + Err(e) => { + return Err(ShellError::SpannedLabeledError( + format!("Could not remove {:}", path.to_string_lossy()), + e.to_string(), + target.span, + )); + } + } + } + } + Err(e) => { + return Err(ShellError::SpannedLabeledError( + e.to_string(), + e.to_string(), + call.head, + )) + } + }; + } + + if all_targets.is_empty() && !force { + return Err(ShellError::SpannedLabeledError( + "No valid paths".into(), + "no valid paths".into(), call.head, )); } - let current_path = current_dir(engine_state, stack)?; - let mut paths = call - .rest::(engine_state, stack, 0)? + Ok(all_targets .into_iter() - .map(|path| current_path.join(path)) - .peekable(); - - if paths.peek().is_none() { - return Err(ShellError::FileNotFound(call.positional[0].span)); - } - - // Expand and flatten files - let resolve_path = |i: usize, path: PathBuf| { - glob::glob(&path.to_string_lossy()).map_or_else( - |_| Vec::new(), - |path_iter| path_iter.flatten().map(|f| Target(i, f)).collect(), - ) - }; - - let mut targets: Vec = vec![]; - for (i, path) in paths.enumerate() { - let mut paths: Vec = resolve_path(i, path); - - if paths.is_empty() { - return Err(ShellError::FileNotFound(call.positional[i].span)); - } - - targets.append(paths.as_mut()); - } - - let recursive = call.has_flag("recursive"); - let force = call.has_flag("force"); - - if interactive && !force { - let mut remove: Vec = vec![]; - for (index, file) in targets.iter().enumerate() { - let prompt: String = format!( - "Are you sure that you what to delete {}?", - file.1 - .file_name() - .ok_or_else(|| ShellError::SpannedLabeledError( - "File name error".into(), - "Unable to get file name".into(), - call.head - ))? - .to_str() - .ok_or_else(|| ShellError::SpannedLabeledError( - "Unable to get str error".into(), - "Unable to convert to str file name".into(), - call.head - ))?, - ); - - let input = get_interactive_confirmation(prompt)?; - - if !input { - remove.push(index); - } - } - - remove.reverse(); - - for index in remove { - targets.remove(index); - } - - if targets.is_empty() { - return Err(ShellError::NoFileToBeRemoved()); - } - } - - let args = RmArgs { - targets, - recursive, - trash, - permanent, - force, - }; - let response = rm_helper(call, args); - - // let temp = rm_helper(call, args).flatten(); - // let temp = input.flatten(call.head, move |_| rm_helper(call, args)); - - Ok(response - .into_iter() - .into_pipeline_data(engine_state.ctrlc.clone())) - // Ok(Value::Nothing { span }) -} - -fn rm_helper(call: &Call, args: RmArgs) -> Vec { - let (targets, recursive, trash, _permanent, force) = ( - args.targets, - args.recursive, - args.trash, - args.permanent, - args.force, - ); - - #[cfg(not(feature = "trash-support"))] - { - if trash { - let error = match call.get_flag_expr("trash").ok_or_else(|| { - ShellError::SpannedLabeledError( - "Flag not found".into(), - "trash flag not found".into(), - call.head, - ) - }) { - Ok(expr) => ShellError::FeatureNotEnabled(expr.span), - Err(err) => err, - }; - - return vec![Value::Error { error }]; - } - } - - if targets.is_empty() && !force { - return vec![Value::Error { - error: ShellError::FileNotFound(call.head), - }]; - } - - targets - .into_iter() - .map(move |target| { - let (i, f) = (target.0, target.1); - + .map(move |(f, _)| { let is_empty = || match f.read_dir() { Ok(mut p) => p.next().is_none(), Err(_) => false, @@ -240,9 +215,8 @@ fn rm_helper(call: &Call, args: RmArgs) -> Vec { #[cfg(feature = "trash-support")] { use std::io::Error; - result = if trash { + result = if trash || (rm_always_trash && !permanent) { trash::delete(&f).map_err(|e: trash::Error| { - use std::io::ErrorKind; Error::new(ErrorKind::Other, format!("{:?}", e)) }) } else if metadata.is_file() { @@ -261,34 +235,34 @@ fn rm_helper(call: &Call, args: RmArgs) -> Vec { } if let Err(e) = result { + let msg = format!("Could not delete because: {:}\nTry '--trash' flag", e); Value::Error { - error: ShellError::RemoveNotPossible( - format!("Could not delete because: {:}\nTry '--trash' flag", e), - call.head, - ), + error: ShellError::SpannedLabeledError(msg, e.to_string(), span), } } else { - Value::String { - val: format!("deleted {:}", f.to_string_lossy()), - span: call.positional[i].span, - } + let val = format!("deleted {:}", f.to_string_lossy()); + Value::String { val, span } } } else { + let msg = format!("Cannot remove {:}. try --recursive", f.to_string_lossy()); Value::Error { - error: ShellError::RemoveNotPossible( - "Cannot remove. try --recursive".to_string(), - call.positional[i].span, + error: ShellError::SpannedLabeledError( + msg, + "cannot remove non-empty directory".into(), + span, ), } } } else { + let msg = format!("no such file or directory: {:}", f.to_string_lossy()); Value::Error { - error: ShellError::RemoveNotPossible( - "no such file or directory".to_string(), - call.positional[i].span, + error: ShellError::SpannedLabeledError( + msg, + "no such file or directory".into(), + span, ), } } }) - .collect() + .into_pipeline_data(ctrlc)) } diff --git a/crates/nu-command/src/filesystem/util.rs b/crates/nu-command/src/filesystem/util.rs index a15c5a461..79364808a 100644 --- a/crates/nu-command/src/filesystem/util.rs +++ b/crates/nu-command/src/filesystem/util.rs @@ -96,6 +96,7 @@ pub struct Resource { impl Resource {} +#[allow(dead_code)] pub fn get_interactive_confirmation(prompt: String) -> Result> { let input = Input::new() .with_prompt(prompt) diff --git a/crates/nu-command/tests/commands/cp.rs b/crates/nu-command/tests/commands/cp.rs index 76da70753..9fe1272b7 100644 --- a/crates/nu-command/tests/commands/cp.rs +++ b/crates/nu-command/tests/commands/cp.rs @@ -31,8 +31,6 @@ fn copies_the_file_inside_directory_if_path_to_copy_is_directory() { }) } -// FIXME: jt: needs more work -#[ignore] #[test] fn error_if_attempting_to_copy_a_directory_to_another_directory() { Playground::setup("cp_test_3", |dirs, _| { @@ -77,8 +75,6 @@ fn copies_the_directory_inside_directory_if_path_to_copy_is_directory_and_with_r }) } -// FIXME: jt: needs more work -#[ignore] #[test] fn deep_copies_with_recursive_flag() { Playground::setup("cp_test_5", |dirs, sandbox| { diff --git a/crates/nu-command/tests/commands/rm.rs b/crates/nu-command/tests/commands/rm.rs index ddbe7180c..ae7e35828 100644 --- a/crates/nu-command/tests/commands/rm.rs +++ b/crates/nu-command/tests/commands/rm.rs @@ -121,8 +121,6 @@ fn removes_directory_contents_with_recursive_flag() { }) } -// FIXME: jt: needs more work -#[ignore] #[test] fn errors_if_attempting_to_delete_a_directory_with_content_without_recursive_flag() { Playground::setup("rm_test_6", |dirs, sandbox| { @@ -137,8 +135,6 @@ fn errors_if_attempting_to_delete_a_directory_with_content_without_recursive_fla }) } -// FIXME: jt: needs more work -#[ignore] #[test] fn errors_if_attempting_to_delete_single_dot_as_argument() { Playground::setup("rm_test_7", |dirs, _| { @@ -151,8 +147,6 @@ fn errors_if_attempting_to_delete_single_dot_as_argument() { }) } -// FIXME: jt: needs more work -#[ignore] #[test] fn errors_if_attempting_to_delete_two_dot_as_argument() { Playground::setup("rm_test_8", |dirs, _| { @@ -283,8 +277,6 @@ fn no_errors_if_attempting_to_delete_non_existent_file_with_f_flag() { }) } -// FIXME: jt: needs more work -#[ignore] #[test] fn rm_wildcard_keeps_dotfiles() { Playground::setup("rm_test_15", |dirs, sandbox| { diff --git a/crates/nu-protocol/src/config.rs b/crates/nu-protocol/src/config.rs index 471ca0b77..0e64dc4af 100644 --- a/crates/nu-protocol/src/config.rs +++ b/crates/nu-protocol/src/config.rs @@ -67,6 +67,7 @@ pub struct Config { pub menu_config: HashMap, pub keybindings: Vec, pub history_config: HashMap, + pub rm_always_trash: bool, } impl Default for Config { @@ -90,6 +91,7 @@ impl Default for Config { menu_config: HashMap::new(), keybindings: Vec::new(), history_config: HashMap::new(), + rm_always_trash: false, } } } @@ -194,6 +196,13 @@ impl Value { eprintln!("$config.quick_completions is not a bool") } } + "rm_always_trash" => { + if let Ok(b) = value.as_bool() { + config.rm_always_trash = b; + } else { + eprintln!("$config.rm_always_trash is not a bool") + } + } "filesize_format" => { if let Ok(v) = value.as_string() { config.filesize_format = v.to_lowercase();