Merge pull request #620 from androbtech/valueshell-cd

Valueshell cd and FileSystemShell cd refactoring.

src/shell/filesystem_shell.rs

@@ -15,7 +15,7 @@ use std::path::{Path, PathBuf};
 
 pub struct FilesystemShell {
     pub(crate) path: String,
-    last_path: String,
+    pub(crate) last_path: String,
     completer: NuCompleter,
     hinter: HistoryHinter,
 }
@@ -185,29 +185,20 @@ impl Shell for FilesystemShell {
             },
             Some(v) => {
                 let target = v.as_path()?;
-                let path = PathBuf::from(self.path());
-                match dunce::canonicalize(path.join(&target).as_path()) {
-                    Ok(p) => p,
-                    Err(_) => {
-                        let error = Err(ShellError::labeled_error(
-                            "Can not change to directory",
-                            "directory not found",
-                            v.span().clone(),
-                        ));
 
-                        if let Some(t) = target.to_str() {
-                            if t == "-" {
-                                match dunce::canonicalize(PathBuf::from(self.last_path.clone()).as_path()) {
-                                    Ok(p) => p,
-                                    Err(_) => {
-                                        return error;
-                                    }
-                                }
-                            } else {
-                                return error;
-                            }
-                        } else {
-                            return error;
+                if PathBuf::from("-") == target {
+                    PathBuf::from(&self.last_path)
+                } else {
+                    let path = PathBuf::from(self.path());
+
+                    match dunce::canonicalize(path.join(&target)) {
+                        Ok(p) => p,
+                        Err(_) => {
+                            return Err(ShellError::labeled_error(
+                                "Can not change to directory",
+                                "directory not found",
+                                v.span().clone(),
+                            ))
                         }
                     }
                 }
@@ -215,23 +206,12 @@ impl Shell for FilesystemShell {
         };
 
         let mut stream = VecDeque::new();
-        match std::env::set_current_dir(&path) {
-            Ok(_) => {}
-            Err(_) => {
-                if let Some(directory) = args.nth(0) {
-                    return Err(ShellError::labeled_error(
-                        "Can not change to directory",
-                        "directory not found",
-                        directory.span(),
-                    ));
-                } else {
-                    return Err(ShellError::string("Can not change to directory"));
-                }
-            }
-        }
-        stream.push_back(ReturnSuccess::change_cwd(
+
+        stream.push_back(
+            ReturnSuccess::change_cwd(
             path.to_string_lossy().to_string(),
         ));
+
         Ok(stream.into())
     }
 

src/shell/value_shell.rs

@@ -10,16 +10,24 @@ use crate::utils::ValueStructure;
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct ValueShell {
     pub(crate) path: String,
+    pub(crate) last_path: String,
     pub(crate) value: Tagged<Value>,
 }
 
+impl std::fmt::Debug for ValueShell {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "ValueShell @ {}", self.path)
+    }
+}
+
 impl ValueShell {
     pub fn new(value: Tagged<Value>) -> ValueShell {
         ValueShell {
             path: "/".to_string(),
+            last_path: "/".to_string(),
             value,
         }
     }
@@ -76,7 +84,7 @@ impl Shell for ValueShell {
     }
 
     fn homedir(&self) -> Option<PathBuf> {
-        dirs::home_dir()
+        Some(PathBuf::from("/"))
     }
 
     fn ls(&self, args: EvaluatedWholeStreamCommandArgs) -> Result<OutputStream, ShellError> {
@@ -126,6 +134,8 @@ impl Shell for ValueShell {
 
                 if target == PathBuf::from("..") {
                     cwd.pop();
+                } else if target == PathBuf::from("-") {
+                    cwd = PathBuf::from(&self.last_path);
                 } else {
                     match target.to_str() {
                         Some(target) => match target.chars().nth(0) {
@@ -200,19 +210,16 @@ impl Shell for ValueShell {
     }
 
     fn pwd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result<OutputStream, ShellError> {
-        let path = PathBuf::from(&self.path());
-
         let mut stream = VecDeque::new();
-        stream.push_back(ReturnSuccess::value(
-            Value::Primitive(Primitive::String(path.to_string_lossy().to_string()))
-                .simple_spanned(args.call_info.name_span),
-        ));
-
+        stream.push_back(ReturnSuccess::value(Tagged::from_item(
+            Value::string(self.path()),
+            args.call_info.name_span,
+        )));
         Ok(stream.into())
     }
 
     fn set_path(&mut self, path: String) {
-        let _ = std::env::set_current_dir(&path);
+        self.last_path = self.path.clone();
         self.path = path.clone();
     }
 

tests/command_cd_tests.rs

@@ -1,10 +1,10 @@
 mod helpers;
 
-use helpers::Playground;
+use helpers::{Playground, Stub::*};
 use std::path::PathBuf;
 
 #[test]
-fn filesytem_change_from_current_directory_using_relative_path() {
+fn filesystem_change_from_current_directory_using_relative_path() {
     Playground::setup("cd_test_1", |dirs, _| {
         let actual = nu!(
             cwd: dirs.root(),
@@ -53,9 +53,26 @@ fn filesystem_switch_back_to_previous_working_directory() {
     })
 }
 
+#[test]
+fn filesytem_change_from_current_directory_using_relative_path_and_dash() {
+    Playground::setup("cd_test_4", |dirs, sandbox| {
+        sandbox.within("odin").mkdir("-");
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                cd odin/-
+                pwd | echo $it
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), dirs.test().join("odin").join("-"));
+    })
+}
+
 #[test]
 fn filesystem_change_current_directory_to_parent_directory() {
-    Playground::setup("cd_test_4", |dirs, _| {
+    Playground::setup("cd_test_5", |dirs, _| {
         let actual = nu!(
             cwd: dirs.test(),
             r#"
@@ -69,8 +86,8 @@ fn filesystem_change_current_directory_to_parent_directory() {
 }
 
 #[test]
-fn file_system_change_to_home_directory() {
-    Playground::setup("cd_test_5", |dirs, _| {
+fn filesystem_change_to_home_directory() {
+    Playground::setup("cd_test_6", |dirs, _| {
         let actual = nu!(
             cwd: dirs.test(),
             r#"
@@ -85,7 +102,7 @@ fn file_system_change_to_home_directory() {
 
 #[test]
 fn filesystem_change_to_a_directory_containing_spaces() {
-    Playground::setup("cd_test_6", |dirs, sandbox| {
+    Playground::setup("cd_test_7", |dirs, sandbox| {
         sandbox.mkdir("robalino turner katz");
 
         let actual = nu!(
@@ -110,3 +127,238 @@ fn filesystem_directory_not_found() {
     assert!(actual.contains("dir_that_does_not_exist"));
     assert!(actual.contains("directory not found"));
 }
+
+
+#[test]
+fn valuesystem_change_from_current_path_using_relative_path() {
+    Playground::setup("cd_test_8", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContent(
+                "sample.toml",
+                r#"
+                    [[bin]]
+                    path = "src/plugins/turner.rs"
+
+                    [[bin]]
+                    path = "src/plugins/robalino.rs"
+
+                    [[bin]]
+                    path = "src/plugins/katz.rs"
+                "#
+        )]);
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                enter sample.toml
+                cd bin
+                pwd | echo $it
+                exit
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), PathBuf::from("/bin"));
+    })
+}
+
+#[test]
+fn valuesystem_change_from_current_path_using_absolute_path() {
+    Playground::setup("cd_test_9", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContent(
+                "sample.toml",
+                r#"
+                    [dependencies]
+                    turner-ts = "0.1.1"
+                    robalino-tkd = "0.0.1"
+                    katz-ember = "0.2.3"
+
+                    [[bin]]
+                    path = "src/plugins/arepa.rs"
+
+                    [[bin]]
+                    path = "src/plugins/bbq.rs"
+                "#
+        )]);
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                enter sample.toml
+                cd bin
+                cd /dependencies
+                pwd | echo $it
+                exit
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), PathBuf::from("/dependencies"));
+    }) 
+}
+
+#[test]
+fn valuesystem_switch_back_to_previous_working_path() {
+    Playground::setup("cd_test_10", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContent(
+                "sample.toml",
+                r#"
+                    [dependencies]
+                    turner-ts = "0.1.1"
+                    robalino-tkd = "0.0.1"
+                    katz-ember = "0.2.3"
+                    odin-gf = "0.2.1"
+
+                    [[bin]]
+                    path = "src/plugins/arepa.rs"
+
+                    [[bin]]
+                    path = "src/plugins/bbq.rs"
+                "#
+        )]);
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                enter sample.toml
+                cd dependencies
+                cd /bin
+                cd -
+                pwd | echo $it
+                exit
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), PathBuf::from("/dependencies"));
+    })
+}
+
+#[test]
+fn valuesystem_change_from_current_path_using_relative_path_and_dash() {
+    Playground::setup("cd_test_11", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContent(
+                "sample.toml",
+                r#"
+                    [package]
+                    - = ["Yehuda Katz <wycats@gmail.com>", "Jonathan Turner <jonathan.d.turner@gmail.com>", "Andrés N. Robalino <andres@androbtech.com>"]
+
+                    [[bin]]
+                    path = "src/plugins/arepa.rs"
+
+                    [[bin]]
+                    path = "src/plugins/bbq.rs"
+                "#
+        )]);
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                enter sample.toml
+                cd package/-
+                cd /bin
+                cd -
+                pwd | echo $it
+                exit
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), PathBuf::from("/package/-"));
+    })
+}
+
+
+#[test]
+fn valuesystem_change_current_path_to_parent_path() {
+    Playground::setup("cd_test_12", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContent(
+                "sample.toml",
+                r#"
+                    [package]
+                    emberenios = ["Yehuda Katz <wycats@gmail.com>", "Jonathan Turner <jonathan.d.turner@gmail.com>", "Andrés N. Robalino <andres@androbtech.com>"]
+                "#
+        )]);
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                enter sample.toml
+                cd package/emberenios
+                cd ..
+                pwd | echo $it
+                exit
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), PathBuf::from("/package"));
+    })
+}
+
+#[test]
+fn valuesystem_change_to_home_directory() {
+    Playground::setup("cd_test_13", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContent(
+                "sample.toml",
+                r#"
+                    [paquete]
+                    el = "pollo loco"
+                "#
+        )]);
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                enter sample.toml
+                cd paquete
+                cd ~
+                pwd | echo $it
+                exit
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), PathBuf::from("/"));
+    })
+}
+
+#[test]
+fn valuesystem_change_to_a_path_containing_spaces() {
+    Playground::setup("cd_test_14", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContent(
+                "sample.toml",
+                r#"
+                    ["pa que te"]
+                    el = "pollo loco"
+                "#
+        )]);
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                enter sample.toml
+                cd "pa que te"
+                pwd | echo $it
+                exit
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), PathBuf::from("/").join("pa que te"));
+    })
+}
+
+#[test]
+fn valuesystem_path_not_found() {
+    let actual = nu_error!(
+        cwd: "tests/fixtures/formats",
+        r#"
+            enter cargo_sample.toml
+            cd im_a_path_that_does_not_exist
+            exit
+        "#
+    );
+
+    assert!(actual.contains("Can not change to path inside"));
+    assert!(actual.contains("No such path exists"));
+}