From d0618b0b3218d18f5ae19b5700606998f5712ad4 Mon Sep 17 00:00:00 2001 From: Kevin Del Castillo Date: Mon, 6 Apr 2020 06:28:56 -0500 Subject: [PATCH] Enable the use of multiple dots in FS Shell commands (#1547) Every dot after `..` means another parent directory. --- crates/nu-cli/src/shell/filesystem_shell.rs | 105 +++++++++++--------- crates/nu-cli/tests/commands/cd.rs | 37 +++++-- crates/nu-cli/tests/commands/cp.rs | 50 ++++++++++ crates/nu-cli/tests/commands/ls.rs | 23 +++++ crates/nu-cli/tests/commands/mkdir.rs | 16 +++ crates/nu-cli/tests/commands/mv.rs | 58 ++++++++++- crates/nu-cli/tests/commands/rm.rs | 23 +++++ 7 files changed, 256 insertions(+), 56 deletions(-) diff --git a/crates/nu-cli/src/shell/filesystem_shell.rs b/crates/nu-cli/src/shell/filesystem_shell.rs index e24aebee23..8fde96668f 100644 --- a/crates/nu-cli/src/shell/filesystem_shell.rs +++ b/crates/nu-cli/src/shell/filesystem_shell.rs @@ -76,29 +76,6 @@ impl FilesystemShell { hinter: HistoryHinter {}, } } - - fn canonicalize(&self, path: impl AsRef) -> std::io::Result { - let path = if path.as_ref().is_relative() { - let components = path.as_ref().components(); - let mut result = PathBuf::from(self.path()); - for component in components { - match component { - Component::CurDir => { /* ignore current dir */ } - Component::ParentDir => { - result.pop(); - } - Component::Normal(normal) => result.push(normal), - _ => {} - } - } - - result - } else { - path.as_ref().into() - }; - - dunce::canonicalize(path) - } } impl Shell for FilesystemShell { @@ -127,7 +104,7 @@ impl Shell for FilesystemShell { let (path, p_tag) = match path { Some(p) => { let p_tag = p.tag; - let mut p = p.item; + let mut p = normalize(p.item); if p.is_dir() { if is_empty_dir(&p) { return Ok(OutputStream::empty()); @@ -210,7 +187,7 @@ impl Shell for FilesystemShell { if target == Path::new("-") { PathBuf::from(&self.last_path) } else { - let path = self.canonicalize(target).map_err(|_| { + let path = canonicalize(self.path(), target).map_err(|_| { ShellError::labeled_error( "Cannot change to directory", "directory not found", @@ -276,11 +253,9 @@ impl Shell for FilesystemShell { ) -> Result { let name_tag = name; - let mut source = PathBuf::from(path); - let mut destination = PathBuf::from(path); - - source.push(&src.item); - destination.push(&dst.item); + let path = Path::new(path); + let source = normalize(path.join(&src.item)); + let mut destination = normalize(path.join(&dst.item)); let sources: Vec<_> = match glob::glob(&source.to_string_lossy()) { Ok(files) => files.collect(), @@ -548,7 +523,7 @@ impl Shell for FilesystemShell { name: Tag, path: &str, ) -> Result { - let full_path = PathBuf::from(path); + let path = Path::new(path); if directories.is_empty() { return Err(ShellError::labeled_error( @@ -559,11 +534,7 @@ impl Shell for FilesystemShell { } for dir in directories.iter() { - let create_at = { - let mut loc = full_path.clone(); - loc.push(&dir.item); - loc - }; + let create_at = normalize(path.join(&dir.item)); let dir_res = std::fs::create_dir_all(create_at); if let Err(reason) = dir_res { @@ -586,11 +557,9 @@ impl Shell for FilesystemShell { ) -> Result { let name_tag = name; - let mut source = PathBuf::from(path); - let mut destination = PathBuf::from(path); - - source.push(&src.item); - destination.push(&dst.item); + let path = Path::new(path); + let source = normalize(path.join(&src.item)); + let mut destination = normalize(path.join(&dst.item)); let sources: Vec<_> = match glob::glob(&source.to_string_lossy()) { Ok(files) => files.collect(), @@ -605,11 +574,12 @@ impl Shell for FilesystemShell { if sources.is_empty() { return Err(ShellError::labeled_error( - "Invalid File or Pattern.", - "Invalid File or Pattern", + "Invalid file or pattern.", + "invalid file or pattern", src.tag, )); } + let destination_file_name = { match destination.file_name() { Some(name) => PathBuf::from(name), @@ -991,6 +961,7 @@ impl Shell for FilesystemShell { )); } + let path = Path::new(path); let mut all_targets: HashMap = HashMap::new(); for target in targets { let all_dots = target @@ -1006,8 +977,7 @@ impl Shell for FilesystemShell { )); } - let mut path = PathBuf::from(path); - path.push(&target.item); + let path = normalize(path.join(&target.item)); match glob::glob(&path.to_string_lossy()) { Ok(files) => { for file in files { @@ -1180,3 +1150,48 @@ fn is_hidden_dir(dir: impl AsRef) -> bool { } } } + +fn normalize(path: impl AsRef) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.as_ref().components() { + match component { + Component::Normal(normal) => { + if let Some(normal) = normal.to_str() { + if normal.chars().all(|c| c == '.') { + for _ in 0..(normal.len() - 1) { + normalized.push(".."); + } + } else { + normalized.push(normal); + } + } else { + normalized.push(normal); + } + } + c => normalized.push(c.as_os_str()), + } + } + + normalized +} + +fn canonicalize(relative_to: impl AsRef, path: impl AsRef) -> std::io::Result { + let path = if path.as_ref().is_relative() { + let mut result = relative_to.as_ref().to_path_buf(); + normalize(path.as_ref()) + .components() + .for_each(|component| match component { + Component::ParentDir => { + result.pop(); + } + Component::Normal(normal) => result.push(normal), + _ => {} + }); + + result + } else { + path.as_ref().into() + }; + + dunce::canonicalize(path) +} diff --git a/crates/nu-cli/tests/commands/cd.rs b/crates/nu-cli/tests/commands/cd.rs index 8e66103f8b..666d3d91b0 100644 --- a/crates/nu-cli/tests/commands/cd.rs +++ b/crates/nu-cli/tests/commands/cd.rs @@ -85,9 +85,26 @@ fn filesystem_change_current_directory_to_parent_directory() { }) } +#[test] +fn filesystem_change_current_directory_to_two_parents_up_using_multiple_dots() { + Playground::setup("cd_test_6", |dirs, sandbox| { + sandbox.within("foo").mkdir("bar"); + + let actual = nu!( + cwd: dirs.test().join("foo/bar"), + r#" + cd ... + pwd | echo $it + "# + ); + + assert_eq!(PathBuf::from(actual), *dirs.test()); + }) +} + #[test] fn filesystem_change_current_directory_to_parent_directory_after_delete_cwd() { - Playground::setup("cd_test_5_1", |dirs, sandbox| { + Playground::setup("cd_test_7", |dirs, sandbox| { sandbox.within("foo").mkdir("bar"); let actual = nu!( @@ -109,7 +126,7 @@ fn filesystem_change_current_directory_to_parent_directory_after_delete_cwd() { #[test] fn filesystem_change_to_home_directory() { - Playground::setup("cd_test_6", |dirs, _| { + Playground::setup("cd_test_8", |dirs, _| { let actual = nu!( cwd: dirs.test(), r#" @@ -124,7 +141,7 @@ fn filesystem_change_to_home_directory() { #[test] fn filesystem_change_to_a_directory_containing_spaces() { - Playground::setup("cd_test_7", |dirs, sandbox| { + Playground::setup("cd_test_9", |dirs, sandbox| { sandbox.mkdir("robalino turner katz"); let actual = nu!( @@ -144,7 +161,7 @@ fn filesystem_change_to_a_directory_containing_spaces() { #[test] fn filesystem_not_a_directory() { - Playground::setup("cd_test_8", |dirs, sandbox| { + Playground::setup("cd_test_10", |dirs, sandbox| { sandbox.with_files(vec![EmptyFile("ferris_did_it.txt")]); let actual = nu_error!( @@ -178,7 +195,7 @@ fn filesystem_directory_not_found() { #[test] fn valuesystem_change_from_current_path_using_relative_path() { - Playground::setup("cd_test_9", |dirs, sandbox| { + Playground::setup("cd_test_11", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", r#" @@ -209,7 +226,7 @@ fn valuesystem_change_from_current_path_using_relative_path() { #[test] fn valuesystem_change_from_current_path_using_absolute_path() { - Playground::setup("cd_test_10", |dirs, sandbox| { + Playground::setup("cd_test_12", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", r#" @@ -243,7 +260,7 @@ fn valuesystem_change_from_current_path_using_absolute_path() { #[test] fn valuesystem_switch_back_to_previous_working_path() { - Playground::setup("cd_test_11", |dirs, sandbox| { + Playground::setup("cd_test_13", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", r#" @@ -279,7 +296,7 @@ fn valuesystem_switch_back_to_previous_working_path() { #[test] fn valuesystem_change_from_current_path_using_relative_path_and_dash() { - Playground::setup("cd_test_12", |dirs, sandbox| { + Playground::setup("cd_test_14", |dirs, sandbox| { sandbox .with_files(vec![FileWithContent( "sample.toml", @@ -313,7 +330,7 @@ fn valuesystem_change_from_current_path_using_relative_path_and_dash() { #[test] fn valuesystem_change_current_path_to_parent_path() { - Playground::setup("cd_test_13", |dirs, sandbox| { + Playground::setup("cd_test_15", |dirs, sandbox| { sandbox .with_files(vec![FileWithContent( "sample.toml", @@ -340,7 +357,7 @@ fn valuesystem_change_current_path_to_parent_path() { #[test] fn valuesystem_change_to_a_path_containing_spaces() { - Playground::setup("cd_test_15", |dirs, sandbox| { + Playground::setup("cd_test_17", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", r#" diff --git a/crates/nu-cli/tests/commands/cp.rs b/crates/nu-cli/tests/commands/cp.rs index 4c9362ab53..6de98a71f1 100644 --- a/crates/nu-cli/tests/commands/cp.rs +++ b/crates/nu-cli/tests/commands/cp.rs @@ -184,3 +184,53 @@ fn copies_same_file_twice() { assert!(dirs.test().join("sample.ini").exists()); }); } + +#[test] +fn copy_files_using_glob_two_parents_up_using_multiple_dots() { + Playground::setup("cp_test_9", |dirs, sandbox| { + sandbox.within("foo").mkdir("bar").with_files(vec![ + EmptyFile("jonathan.json"), + EmptyFile("andres.xml"), + EmptyFile("yehuda.yaml"), + EmptyFile("kevin.txt"), + EmptyFile("many_more.ppl"), + ]); + + nu!( + cwd: dirs.test().join("foo/bar"), + r#" + cp * ... + "# + ); + + assert!(files_exist_at( + vec![ + "yehuda.yaml", + "jonathan.json", + "andres.xml", + "kevin.txt", + "many_more.ppl" + ], + dirs.test() + )); + }) +} + +#[test] +fn copy_file_from_two_parents_up_using_multiple_dots_to_current_dir() { + Playground::setup("cp_test_10", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("hello_there")]); + sandbox.within("foo").mkdir("bar"); + + nu!( + cwd: dirs.test().join("foo/bar"), + r#" + cp .../hello_there . + "# + ); + + let expected = dirs.test().join("foo/bar/hello_there"); + + assert!(expected.exists()); + }) +} diff --git a/crates/nu-cli/tests/commands/ls.rs b/crates/nu-cli/tests/commands/ls.rs index e5628d74cc..7b686f20ec 100644 --- a/crates/nu-cli/tests/commands/ls.rs +++ b/crates/nu-cli/tests/commands/ls.rs @@ -131,3 +131,26 @@ fn fails_when_glob_doesnt_match() { assert!(actual.contains("invalid file or pattern")); }) } + +#[test] +fn list_files_from_two_parents_up_using_multiple_dots() { + Playground::setup("ls_test_6", |dirs, sandbox| { + sandbox.with_files(vec![ + EmptyFile("yahuda.yaml"), + EmptyFile("jonathan.json"), + EmptyFile("andres.xml"), + EmptyFile("kevin.txt"), + ]); + + sandbox.within("foo").mkdir("bar"); + + let actual = nu!( + cwd: dirs.test().join("foo/bar"), + r#" + ls ... | count | echo $it + "# + ); + + assert_eq!(actual, "5"); + }) +} diff --git a/crates/nu-cli/tests/commands/mkdir.rs b/crates/nu-cli/tests/commands/mkdir.rs index 5c3173eb7e..e6d545c4a4 100644 --- a/crates/nu-cli/tests/commands/mkdir.rs +++ b/crates/nu-cli/tests/commands/mkdir.rs @@ -45,3 +45,19 @@ fn creates_intermediary_directories() { assert!(expected.exists()); }) } + +#[test] +fn create_directory_two_parents_up_using_multiple_dots() { + Playground::setup("mkdir_test_4", |dirs, sandbox| { + sandbox.within("foo").mkdir("bar"); + + nu!( + cwd: dirs.test().join("foo/bar"), + "mkdir .../boo" + ); + + let expected = dirs.test().join("boo"); + + assert!(expected.exists()); + }) +} diff --git a/crates/nu-cli/tests/commands/mv.rs b/crates/nu-cli/tests/commands/mv.rs index c49c27bec7..b1fd240936 100644 --- a/crates/nu-cli/tests/commands/mv.rs +++ b/crates/nu-cli/tests/commands/mv.rs @@ -227,7 +227,7 @@ fn errors_if_source_doesnt_exist() { cwd: dirs.root(), "mv non-existing-file test_folder/" ); - assert!(actual.contains("Invalid File or Pattern")); + assert!(actual.contains("Invalid file or pattern")); }) } @@ -250,3 +250,59 @@ fn does_not_error_on_relative_parent_path() { assert!(expected.exists()); }) } + +#[test] +#[ignore] // Temporarily failling, see https://github.com/nushell/nushell/issues/1523 +fn move_files_using_glob_two_parents_up_using_multiple_dots() { + Playground::setup("mv_test_12", |dirs, sandbox| { + sandbox.within("foo").mkdir("bar").with_files(vec![ + EmptyFile("jonathan.json"), + EmptyFile("andres.xml"), + EmptyFile("yehuda.yaml"), + EmptyFile("kevin.txt"), + EmptyFile("many_more.ppl"), + ]); + + nu!( + cwd: dirs.test().join("foo/bar"), + r#" + mv * ... + "# + ); + + let files = vec![ + "yehuda.yaml", + "jonathan.json", + "andres.xml", + "kevin.txt", + "many_more.ppl", + ]; + + let original_dir = dirs.test().join("foo/bar"); + let destination_dir = dirs.test(); + + assert!(files_exist_at(files.clone(), destination_dir)); + assert!(!files_exist_at(files, original_dir)) + }) +} + +#[test] +fn move_file_from_two_parents_up_using_multiple_dots_to_current_dir() { + Playground::setup("cp_test_10", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("hello_there")]); + sandbox.within("foo").mkdir("bar"); + + nu!( + cwd: dirs.test().join("foo/bar"), + r#" + mv .../hello_there . + "# + ); + + let expected = dirs.test().join("foo/bar/hello_there"); + let original = dirs.test().join("hello_there"); + + assert!(expected.exists()); + assert!(!original.exists()); + }) +} diff --git a/crates/nu-cli/tests/commands/rm.rs b/crates/nu-cli/tests/commands/rm.rs index 1b55a42c77..13dd5003b5 100644 --- a/crates/nu-cli/tests/commands/rm.rs +++ b/crates/nu-cli/tests/commands/rm.rs @@ -242,3 +242,26 @@ fn allows_doubly_specified_file() { assert!(!actual.contains("error")) }) } + +#[test] +fn remove_files_from_two_parents_up_using_multiple_dots_and_glob() { + Playground::setup("rm_test_13", |dirs, sandbox| { + sandbox.with_files(vec![ + EmptyFile("yehuda.txt"), + EmptyFile("jonathan.txt"), + EmptyFile("kevin.txt"), + ]); + + sandbox.within("foo").mkdir("bar"); + + nu!( + cwd: dirs.test().join("foo/bar"), + "rm .../*.txt" + ); + + assert!(!files_exist_at( + vec!["yehuda.txt", "jonathan.txt", "kevin.txt"], + dirs.test() + )); + }) +}