Enable the use of multiple dots in FS Shell commands (#1547)

Every dot after `..` means another parent directory.
2025-02-16 10:32:29 +01:00 · 2020-04-06 06:28:56 -05:00 · 2020-04-06 06:28:56 -05:00 · d0618b0b32
commit d0618b0b32
parent c4daa2e40f
7 changed files with 256 additions and 56 deletions
--- 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<Path>) -> std::io::Result<PathBuf> {
-        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<OutputStream, ShellError> {
        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<OutputStream, ShellError> {
-        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<OutputStream, ShellError> {
        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<PathBuf, Tag> = 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<Path>) -> bool {
        }
    }
 }
+
+fn normalize(path: impl AsRef<Path>) -> 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>, path: impl AsRef<Path>) -> std::io::Result<PathBuf> {
+    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)
+}
--- 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#"
--- 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());
+    })
+}
--- 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");
+    })
+}
--- 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());
+    })
+}
--- 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());
+    })
+}
--- 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()
+        ));
+    })
+}