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 Path<xxx>Arguments. 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

@@ -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;

crates/nu-command/src/commands/default_context.rs

@@ -235,6 +235,7 @@ pub fn create_default_context(interactive: bool) -> Result<EvaluationContext, Bo
             whole_stream_command(PathExpand),
             whole_stream_command(PathExtension),
             whole_stream_command(PathFilestem),
+            whole_stream_command(PathJoin),
             whole_stream_command(PathType),
             // Url
             whole_stream_command(UrlCommand),

crates/nu-command/src/commands/path/basename.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 PathBasenameArguments {
     rest: Vec<ColumnPath>,
 }
 
+impl PathSubcommandArguments for PathBasenameArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<OutputStream, ShellError> {
         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<DefaultArguments>) -> 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(),

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<ColumnPath>,
 }
 
+impl PathSubcommandArguments for PathDirnameArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<DefaultArguments>) -> 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<DefaultArguments>) -> 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),

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<ColumnPath>,
 }
 
+impl PathSubcommandArguments for PathExistsArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<OutputStream, ShellError> {
         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<DefaultArguments>) -> UntaggedValue {
+fn action(path: &Path, _args: &PathExistsArguments) -> UntaggedValue {
     UntaggedValue::boolean(path.exists())
 }
 

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<ColumnPath>,
 }
 
+impl PathSubcommandArguments for PathExpandArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<OutputStream, ShellError> {
         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<DefaultArguments>) -> 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();

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<ColumnPath>,
 }
 
+impl PathSubcommandArguments for PathExtensionArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<OutputStream, ShellError> {
         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<DefaultArguments>) -> 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(),

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<ColumnPath>,
 }
 
+impl PathSubcommandArguments for PathFilestemArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<DefaultArguments>) -> 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<DefaultArguments>) -> 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<DefaultArguments>) -> 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),

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<String>,
+    rest: Vec<ColumnPath>,
+}
+
+impl PathSubcommandArguments for PathJoinArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<OutputStream, ShellError> {
+        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<Example> {
+        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<Example> {
+        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 {})
+    }
+}

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<String>,
-    // used by filestem
-    prefix: Option<String>,
-    suffix: Option<String>,
-    // used by dirname
-    num_levels: Option<u32>,
-    // used by all
-    paths: Vec<ColumnPath>,
+trait PathSubcommandArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath>;
 }
 
-fn handle_value<F>(
-    action: &F,
-    v: &Value,
-    span: Span,
-    args: Arc<DefaultArguments>,
-) -> Result<Value, ShellError>
+fn handle_value<F, T>(action: &F, v: &Value, span: Span, args: Arc<T>) -> Result<Value, ShellError>
 where
-    F: Fn(&Path, Arc<DefaultArguments>) -> 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<F>(
+async fn operate<F, T>(
     input: crate::InputStream,
     action: &'static F,
     span: Span,
-    args: Arc<DefaultArguments>,
+    args: Arc<T>,
 ) -> Result<OutputStream, ShellError>
 where
-    F: Fn(&Path, Arc<DefaultArguments>) -> 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,

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<ColumnPath>,
 }
 
+impl PathSubcommandArguments for PathTypeArguments {
+    fn get_column_paths(&self) -> &Vec<ColumnPath> {
+        &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<OutputStream, ShellError> {
         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<DefaultArguments>) -> 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),

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");
+}

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;