From 1d1ec4727a270c6da32c6abcd6e4d61c80c50d88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Thu, 4 Mar 2021 09:04:56 +0200 Subject: [PATCH] Refactor arguments of path subcommands & Add path join subcommand (#3123) * Refactor path subcommand argument handling DefaultArguments are no longer passed to each subcommand. Instead, each subcommand has its own PathArguments. This means that it is no longer necessary to edit every single path subcommand source file when changing the arguments struct. * Add new path join subcommand Makes it easier to create new paths. It's just a wrapper around Rust's Path.join(). --- crates/nu-command/src/commands.rs | 2 +- .../src/commands/default_context.rs | 1 + .../nu-command/src/commands/path/basename.rs | 20 ++--- .../nu-command/src/commands/path/dirname.rs | 26 +++--- crates/nu-command/src/commands/path/exists.rs | 18 ++-- crates/nu-command/src/commands/path/expand.rs | 18 ++-- .../nu-command/src/commands/path/extension.rs | 20 ++--- .../nu-command/src/commands/path/filestem.rs | 29 ++++--- crates/nu-command/src/commands/path/join.rs | 84 +++++++++++++++++++ crates/nu-command/src/commands/path/mod.rs | 42 ++++------ crates/nu-command/src/commands/path/type.rs | 18 ++-- crates/nu-command/tests/commands/path/join.rs | 45 ++++++++++ crates/nu-command/tests/commands/path/mod.rs | 1 + 13 files changed, 228 insertions(+), 96 deletions(-) create mode 100644 crates/nu-command/src/commands/path/join.rs create mode 100644 crates/nu-command/tests/commands/path/join.rs diff --git a/crates/nu-command/src/commands.rs b/crates/nu-command/src/commands.rs index 972d5d0875..6c111b56bd 100644 --- a/crates/nu-command/src/commands.rs +++ b/crates/nu-command/src/commands.rs @@ -230,7 +230,7 @@ pub(crate) use open::Open; pub(crate) use parse::Parse; pub(crate) use path::{ PathBasename, PathCommand, PathDirname, PathExists, PathExpand, PathExtension, PathFilestem, - PathType, + PathJoin, PathType, }; pub(crate) use pivot::Pivot; pub(crate) use prepend::Prepend; diff --git a/crates/nu-command/src/commands/default_context.rs b/crates/nu-command/src/commands/default_context.rs index e85429511f..a8fdf9db36 100644 --- a/crates/nu-command/src/commands/default_context.rs +++ b/crates/nu-command/src/commands/default_context.rs @@ -235,6 +235,7 @@ pub fn create_default_context(interactive: bool) -> Result, } +impl PathSubcommandArguments for PathBasenameArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + #[async_trait] impl WholeStreamCommand for PathBasename { fn name(&self) -> &str { @@ -38,13 +44,7 @@ impl WholeStreamCommand for PathBasename { async fn run(&self, args: CommandArgs) -> Result { let tag = args.call_info.name_tag.clone(); let (PathBasenameArguments { replace, rest }, input) = args.process().await?; - let args = Arc::new(DefaultArguments { - replace: replace.map(|v| v.item), - prefix: None, - suffix: None, - num_levels: None, - paths: rest, - }); + let args = Arc::new(PathBasenameArguments { replace, rest }); operate(input, &action, tag.span, args).await } @@ -85,9 +85,9 @@ impl WholeStreamCommand for PathBasename { } } -fn action(path: &Path, args: Arc) -> UntaggedValue { +fn action(path: &Path, args: &PathBasenameArguments) -> UntaggedValue { match args.replace { - Some(ref basename) => UntaggedValue::filepath(path.with_file_name(basename)), + Some(ref basename) => UntaggedValue::filepath(path.with_file_name(&basename.item)), None => UntaggedValue::string(match path.file_name() { Some(filename) => filename.to_string_lossy(), None => "".into(), diff --git a/crates/nu-command/src/commands/path/dirname.rs b/crates/nu-command/src/commands/path/dirname.rs index a1be395388..e21886e400 100644 --- a/crates/nu-command/src/commands/path/dirname.rs +++ b/crates/nu-command/src/commands/path/dirname.rs @@ -1,4 +1,4 @@ -use super::{operate, DefaultArguments}; +use super::{operate, PathSubcommandArguments}; use crate::prelude::*; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; @@ -16,6 +16,12 @@ struct PathDirnameArguments { rest: Vec, } +impl PathSubcommandArguments for PathDirnameArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + #[async_trait] impl WholeStreamCommand for PathDirname { fn name(&self) -> &str { @@ -53,12 +59,10 @@ impl WholeStreamCommand for PathDirname { }, input, ) = args.process().await?; - let args = Arc::new(DefaultArguments { - replace: replace.map(|v| v.item), - prefix: None, - suffix: None, - num_levels: num_levels.map(|v| v.item), - paths: rest, + let args = Arc::new(PathDirnameArguments { + replace, + num_levels, + rest, }); operate(input, &action, tag.span, args).await } @@ -113,8 +117,8 @@ impl WholeStreamCommand for PathDirname { } } -fn action(path: &Path, args: Arc) -> UntaggedValue { - let num_levels = args.num_levels.unwrap_or(1); +fn action(path: &Path, args: &PathDirnameArguments) -> UntaggedValue { + let num_levels = args.num_levels.as_ref().map_or(1, |tagged| tagged.item); let mut dirname = path; let mut reached_top = false; // end early if somebody passes -n 99999999 @@ -132,9 +136,9 @@ fn action(path: &Path, args: Arc) -> UntaggedValue { Some(ref newdir) => { let remainder = path.strip_prefix(dirname).unwrap_or(dirname); if !remainder.as_os_str().is_empty() { - UntaggedValue::filepath(Path::new(newdir).join(remainder)) + UntaggedValue::filepath(Path::new(&newdir.item).join(remainder)) } else { - UntaggedValue::filepath(Path::new(newdir)) + UntaggedValue::filepath(Path::new(&newdir.item)) } } None => UntaggedValue::filepath(dirname), diff --git a/crates/nu-command/src/commands/path/exists.rs b/crates/nu-command/src/commands/path/exists.rs index 9539014a87..084e646554 100644 --- a/crates/nu-command/src/commands/path/exists.rs +++ b/crates/nu-command/src/commands/path/exists.rs @@ -1,4 +1,4 @@ -use super::{operate, DefaultArguments}; +use super::{operate, PathSubcommandArguments}; use crate::prelude::*; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; @@ -12,6 +12,12 @@ struct PathExistsArguments { rest: Vec, } +impl PathSubcommandArguments for PathExistsArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + #[async_trait] impl WholeStreamCommand for PathExists { fn name(&self) -> &str { @@ -30,13 +36,7 @@ impl WholeStreamCommand for PathExists { async fn run(&self, args: CommandArgs) -> Result { let tag = args.call_info.name_tag.clone(); let (PathExistsArguments { rest }, input) = args.process().await?; - let args = Arc::new(DefaultArguments { - replace: None, - prefix: None, - suffix: None, - num_levels: None, - paths: rest, - }); + let args = Arc::new(PathExistsArguments { rest }); operate(input, &action, tag.span, args).await } @@ -59,7 +59,7 @@ impl WholeStreamCommand for PathExists { } } -fn action(path: &Path, _args: Arc) -> UntaggedValue { +fn action(path: &Path, _args: &PathExistsArguments) -> UntaggedValue { UntaggedValue::boolean(path.exists()) } diff --git a/crates/nu-command/src/commands/path/expand.rs b/crates/nu-command/src/commands/path/expand.rs index 8ebac32707..3d74c0dd8f 100644 --- a/crates/nu-command/src/commands/path/expand.rs +++ b/crates/nu-command/src/commands/path/expand.rs @@ -1,4 +1,4 @@ -use super::{operate, DefaultArguments}; +use super::{operate, PathSubcommandArguments}; use crate::prelude::*; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; @@ -12,6 +12,12 @@ struct PathExpandArguments { rest: Vec, } +impl PathSubcommandArguments for PathExpandArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + #[async_trait] impl WholeStreamCommand for PathExpand { fn name(&self) -> &str { @@ -30,13 +36,7 @@ impl WholeStreamCommand for PathExpand { async fn run(&self, args: CommandArgs) -> Result { let tag = args.call_info.name_tag.clone(); let (PathExpandArguments { rest }, input) = args.process().await?; - let args = Arc::new(DefaultArguments { - replace: None, - prefix: None, - suffix: None, - num_levels: None, - paths: rest, - }); + let args = Arc::new(PathExpandArguments { rest }); operate(input, &action, tag.span, args).await } @@ -61,7 +61,7 @@ impl WholeStreamCommand for PathExpand { } } -fn action(path: &Path, _args: Arc) -> UntaggedValue { +fn action(path: &Path, _args: &PathExpandArguments) -> UntaggedValue { let ps = path.to_string_lossy(); let expanded = shellexpand::tilde(&ps); let path: &Path = expanded.as_ref().as_ref(); diff --git a/crates/nu-command/src/commands/path/extension.rs b/crates/nu-command/src/commands/path/extension.rs index 380f84e59d..ada2cd7689 100644 --- a/crates/nu-command/src/commands/path/extension.rs +++ b/crates/nu-command/src/commands/path/extension.rs @@ -1,4 +1,4 @@ -use super::{operate, DefaultArguments}; +use super::{operate, PathSubcommandArguments}; use crate::prelude::*; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; @@ -14,6 +14,12 @@ struct PathExtensionArguments { rest: Vec, } +impl PathSubcommandArguments for PathExtensionArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + #[async_trait] impl WholeStreamCommand for PathExtension { fn name(&self) -> &str { @@ -38,13 +44,7 @@ impl WholeStreamCommand for PathExtension { async fn run(&self, args: CommandArgs) -> Result { let tag = args.call_info.name_tag.clone(); let (PathExtensionArguments { replace, rest }, input) = args.process().await?; - let args = Arc::new(DefaultArguments { - replace: replace.map(|v| v.item), - prefix: None, - suffix: None, - num_levels: None, - paths: rest, - }); + let args = Arc::new(PathExtensionArguments { replace, rest }); operate(input, &action, tag.span, args).await } @@ -74,9 +74,9 @@ impl WholeStreamCommand for PathExtension { } } -fn action(path: &Path, args: Arc) -> UntaggedValue { +fn action(path: &Path, args: &PathExtensionArguments) -> UntaggedValue { match args.replace { - Some(ref extension) => UntaggedValue::filepath(path.with_extension(extension)), + Some(ref extension) => UntaggedValue::filepath(path.with_extension(&extension.item)), None => UntaggedValue::string(match path.extension() { Some(extension) => extension.to_string_lossy(), None => "".into(), diff --git a/crates/nu-command/src/commands/path/filestem.rs b/crates/nu-command/src/commands/path/filestem.rs index e31fa63f38..d14b5f3315 100644 --- a/crates/nu-command/src/commands/path/filestem.rs +++ b/crates/nu-command/src/commands/path/filestem.rs @@ -1,4 +1,4 @@ -use super::{operate, DefaultArguments}; +use super::{operate, PathSubcommandArguments}; use crate::prelude::*; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; @@ -16,6 +16,12 @@ struct PathFilestemArguments { rest: Vec, } +impl PathSubcommandArguments for PathFilestemArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + #[async_trait] impl WholeStreamCommand for PathFilestem { fn name(&self) -> &str { @@ -53,19 +59,18 @@ impl WholeStreamCommand for PathFilestem { let tag = args.call_info.name_tag.clone(); let ( PathFilestemArguments { - replace, prefix, suffix, + replace, rest, }, input, ) = args.process().await?; - let args = Arc::new(DefaultArguments { - replace: replace.map(|v| v.item), - prefix: prefix.map(|v| v.item), - suffix: suffix.map(|v| v.item), - num_levels: None, - paths: rest, + let args = Arc::new(PathFilestemArguments { + prefix, + suffix, + replace, + rest, }); operate(input, &action, tag.span, args).await } @@ -113,14 +118,14 @@ impl WholeStreamCommand for PathFilestem { } } -fn action(path: &Path, args: Arc) -> UntaggedValue { +fn action(path: &Path, args: &PathFilestemArguments) -> UntaggedValue { let basename = match path.file_name() { Some(name) => name.to_string_lossy().to_string(), None => "".to_string(), }; let suffix = match args.suffix { - Some(ref suf) => match basename.rmatch_indices(suf).next() { + Some(ref suf) => match basename.rmatch_indices(&suf.item).next() { Some((i, _)) => basename.split_at(i).1.to_string(), None => "".to_string(), }, @@ -132,7 +137,7 @@ fn action(path: &Path, args: Arc) -> UntaggedValue { }; let prefix = match args.prefix { - Some(ref pre) => match basename.matches(pre).next() { + Some(ref pre) => match basename.matches(&pre.item).next() { Some(m) => basename.split_at(m.len()).0.to_string(), None => "".to_string(), }, @@ -151,7 +156,7 @@ fn action(path: &Path, args: Arc) -> UntaggedValue { match args.replace { Some(ref replace) => { - let new_name = prefix + replace + &suffix; + let new_name = prefix + &replace.item + &suffix; UntaggedValue::filepath(path.with_file_name(&new_name)) } None => UntaggedValue::string(stem), diff --git a/crates/nu-command/src/commands/path/join.rs b/crates/nu-command/src/commands/path/join.rs new file mode 100644 index 0000000000..bff5a01b3f --- /dev/null +++ b/crates/nu-command/src/commands/path/join.rs @@ -0,0 +1,84 @@ +use super::{operate, PathSubcommandArguments}; +use crate::prelude::*; +use nu_engine::WholeStreamCommand; +use nu_errors::ShellError; +use nu_protocol::{ColumnPath, Signature, SyntaxShape, UntaggedValue, Value}; +use nu_source::Tagged; +use std::path::Path; + +pub struct PathJoin; + +#[derive(Deserialize)] +struct PathJoinArguments { + path: Tagged, + rest: Vec, +} + +impl PathSubcommandArguments for PathJoinArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + +#[async_trait] +impl WholeStreamCommand for PathJoin { + fn name(&self) -> &str { + "path join" + } + + fn signature(&self) -> Signature { + Signature::build("path join") + .required("path", SyntaxShape::String, "Path to join the input path") + .rest(SyntaxShape::ColumnPath, "Optionally operate by column path") + } + + fn usage(&self) -> &str { + "Joins an input path with another path" + } + + async fn run(&self, args: CommandArgs) -> Result { + let tag = args.call_info.name_tag.clone(); + let (PathJoinArguments { path, rest }, input) = args.process().await?; + let args = Arc::new(PathJoinArguments { path, rest }); + operate(input, &action, tag.span, args).await + } + + #[cfg(windows)] + fn examples(&self) -> Vec { + vec![Example { + description: "Append a filename to a path", + example: "echo 'C:\\Users\\viking' | path join spam.txt", + result: Some(vec![Value::from(UntaggedValue::filepath( + "C:\\Users\\viking\\spam.txt", + ))]), + }] + } + + #[cfg(not(windows))] + fn examples(&self) -> Vec { + vec![Example { + description: "Append a filename to a path", + example: "echo '/home/viking' | path join spam.txt", + result: Some(vec![Value::from(UntaggedValue::filepath( + "/home/viking/spam.txt", + ))]), + }] + } +} + +fn action(path: &Path, args: &PathJoinArguments) -> UntaggedValue { + UntaggedValue::filepath(path.join(&args.path.item)) +} + +#[cfg(test)] +mod tests { + use super::PathJoin; + use super::ShellError; + + #[test] + fn examples_work_as_expected() -> Result<(), ShellError> { + use crate::examples::test as test_examples; + + test_examples(PathJoin {}) + } +} diff --git a/crates/nu-command/src/commands/path/mod.rs b/crates/nu-command/src/commands/path/mod.rs index cc8d0d2f56..227f731321 100644 --- a/crates/nu-command/src/commands/path/mod.rs +++ b/crates/nu-command/src/commands/path/mod.rs @@ -5,6 +5,7 @@ mod exists; mod expand; mod extension; mod filestem; +mod join; mod r#type; use crate::prelude::*; @@ -21,34 +22,24 @@ pub use exists::PathExists; pub use expand::PathExpand; pub use extension::PathExtension; pub use filestem::PathFilestem; +pub use join::PathJoin; pub use r#type::PathType; -#[derive(Deserialize)] -struct DefaultArguments { - // used by basename, dirname, extension and filestem - replace: Option, - // used by filestem - prefix: Option, - suffix: Option, - // used by dirname - num_levels: Option, - // used by all - paths: Vec, +trait PathSubcommandArguments { + fn get_column_paths(&self) -> &Vec; } -fn handle_value( - action: &F, - v: &Value, - span: Span, - args: Arc, -) -> Result +fn handle_value(action: &F, v: &Value, span: Span, args: Arc) -> Result where - F: Fn(&Path, Arc) -> UntaggedValue + Send + 'static, + T: PathSubcommandArguments + Send + 'static, + F: Fn(&Path, &T) -> UntaggedValue + Send + 'static, { let v = match &v.value { - UntaggedValue::Primitive(Primitive::FilePath(buf)) => action(buf, args).into_value(v.tag()), + UntaggedValue::Primitive(Primitive::FilePath(buf)) => { + action(buf, &args).into_value(v.tag()) + } UntaggedValue::Primitive(Primitive::String(s)) => { - action(s.as_ref(), args).into_value(v.tag()) + action(s.as_ref(), &args).into_value(v.tag()) } other => { let got = format!("got {}", other.type_name()); @@ -64,23 +55,24 @@ where Ok(v) } -async fn operate( +async fn operate( input: crate::InputStream, action: &'static F, span: Span, - args: Arc, + args: Arc, ) -> Result where - F: Fn(&Path, Arc) -> UntaggedValue + Send + Sync + 'static, + T: PathSubcommandArguments + Send + Sync + 'static, + F: Fn(&Path, &T) -> UntaggedValue + Send + Sync + 'static, { Ok(input .map(move |v| { - if args.paths.is_empty() { + if args.get_column_paths().is_empty() { ReturnSuccess::value(handle_value(&action, &v, span, Arc::clone(&args))?) } else { let mut ret = v; - for path in &args.paths { + for path in args.get_column_paths() { let cloned_args = Arc::clone(&args); ret = ret.swap_data_by_column_path( path, diff --git a/crates/nu-command/src/commands/path/type.rs b/crates/nu-command/src/commands/path/type.rs index 65debdfc7a..11dbcaa31f 100644 --- a/crates/nu-command/src/commands/path/type.rs +++ b/crates/nu-command/src/commands/path/type.rs @@ -1,4 +1,4 @@ -use super::{operate, DefaultArguments}; +use super::{operate, PathSubcommandArguments}; use crate::prelude::*; use nu_engine::filesystem::filesystem_shell::get_file_type; use nu_engine::WholeStreamCommand; @@ -13,6 +13,12 @@ struct PathTypeArguments { rest: Vec, } +impl PathSubcommandArguments for PathTypeArguments { + fn get_column_paths(&self) -> &Vec { + &self.rest + } +} + #[async_trait] impl WholeStreamCommand for PathType { fn name(&self) -> &str { @@ -31,13 +37,7 @@ impl WholeStreamCommand for PathType { async fn run(&self, args: CommandArgs) -> Result { let tag = args.call_info.name_tag.clone(); let (PathTypeArguments { rest }, input) = args.process().await?; - let args = Arc::new(DefaultArguments { - replace: None, - prefix: None, - suffix: None, - num_levels: None, - paths: rest, - }); + let args = Arc::new(PathTypeArguments { rest }); operate(input, &action, tag.span, args).await } @@ -50,7 +50,7 @@ impl WholeStreamCommand for PathType { } } -fn action(path: &Path, _args: Arc) -> UntaggedValue { +fn action(path: &Path, _args: &PathTypeArguments) -> UntaggedValue { let meta = std::fs::symlink_metadata(path); UntaggedValue::string(match &meta { Ok(md) => get_file_type(md), diff --git a/crates/nu-command/tests/commands/path/join.rs b/crates/nu-command/tests/commands/path/join.rs new file mode 100644 index 0000000000..9a0eb9014b --- /dev/null +++ b/crates/nu-command/tests/commands/path/join.rs @@ -0,0 +1,45 @@ +use nu_test_support::{nu, pipeline}; + +use super::join_path_sep; + +#[test] +fn returns_path_joined_with_column_path() { + let actual = nu!( + cwd: "tests", pipeline( + r#" + echo [ [name]; [eggs] ] + | path join spam.txt name + | get name + "# + )); + + let expected = join_path_sep(&["eggs", "spam.txt"]); + assert_eq!(actual.out, expected); +} + +#[test] +fn appends_slash_when_joined_with_empty_path() { + let actual = nu!( + cwd: "tests", pipeline( + r#" + echo "/some/dir" + | path join '' + "# + )); + + let expected = join_path_sep(&["/some/dir", ""]); + assert_eq!(actual.out, expected); +} + +#[test] +fn returns_joined_path_when_joining_empty_path() { + let actual = nu!( + cwd: "tests", pipeline( + r#" + echo "" + | path join foo.txt + "# + )); + + assert_eq!(actual.out, "foo.txt"); +} diff --git a/crates/nu-command/tests/commands/path/mod.rs b/crates/nu-command/tests/commands/path/mod.rs index 99b6a32339..e9c3211cb0 100644 --- a/crates/nu-command/tests/commands/path/mod.rs +++ b/crates/nu-command/tests/commands/path/mod.rs @@ -4,6 +4,7 @@ mod exists; mod expand; mod extension; mod filestem; +mod join; mod type_; use std::path::MAIN_SEPARATOR;