From 77c2e4200ee3211b3e1f06edb2df53399e20578b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Sun, 8 Sep 2019 04:55:49 -0500 Subject: [PATCH 1/2] Filesystem cd refactor/cleanup. --- src/shell/filesystem_shell.rs | 54 +++++++++++------------------------ tests/command_cd_tests.rs | 27 ++++++++++++++---- 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/src/shell/filesystem_shell.rs b/src/shell/filesystem_shell.rs index 4a0d5b3f7d..221b088684 100644 --- a/src/shell/filesystem_shell.rs +++ b/src/shell/filesystem_shell.rs @@ -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()) } diff --git a/tests/command_cd_tests.rs b/tests/command_cd_tests.rs index c2d7329f37..ca066f660a 100644 --- a/tests/command_cd_tests.rs +++ b/tests/command_cd_tests.rs @@ -1,6 +1,6 @@ mod helpers; -use helpers::Playground; +use helpers::{Playground, Stub::*}; use std::path::PathBuf; #[test] @@ -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#" @@ -70,7 +87,7 @@ fn filesystem_change_current_directory_to_parent_directory() { #[test] fn file_system_change_to_home_directory() { - Playground::setup("cd_test_5", |dirs, _| { + 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!( @@ -109,4 +126,4 @@ fn filesystem_directory_not_found() { assert!(actual.contains("dir_that_does_not_exist")); assert!(actual.contains("directory not found")); -} +} \ No newline at end of file From f770409a6079dd3f616848b8486dfae4da59816a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Sun, 8 Sep 2019 05:15:55 -0500 Subject: [PATCH 2/2] cd '-' valueshell implementation and valueshell refactorings. --- src/shell/filesystem_shell.rs | 4 +- src/shell/value_shell.rs | 27 ++-- tests/command_cd_tests.rs | 243 +++++++++++++++++++++++++++++++++- 3 files changed, 258 insertions(+), 16 deletions(-) diff --git a/src/shell/filesystem_shell.rs b/src/shell/filesystem_shell.rs index 221b088684..16cbf513f8 100644 --- a/src/shell/filesystem_shell.rs +++ b/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, } @@ -211,7 +211,7 @@ impl Shell for FilesystemShell { ReturnSuccess::change_cwd( path.to_string_lossy().to_string(), )); - + Ok(stream.into()) } diff --git a/src/shell/value_shell.rs b/src/shell/value_shell.rs index e14d5603a6..010c7ae08e 100644 --- a/src/shell/value_shell.rs +++ b/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, } +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) -> ValueShell { ValueShell { path: "/".to_string(), + last_path: "/".to_string(), value, } } @@ -76,7 +84,7 @@ impl Shell for ValueShell { } fn homedir(&self) -> Option { - dirs::home_dir() + Some(PathBuf::from("/")) } fn ls(&self, args: EvaluatedWholeStreamCommandArgs) -> Result { @@ -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 { - 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(); } diff --git a/tests/command_cd_tests.rs b/tests/command_cd_tests.rs index ca066f660a..b9ad8ca12d 100644 --- a/tests/command_cd_tests.rs +++ b/tests/command_cd_tests.rs @@ -4,7 +4,7 @@ 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(), @@ -56,7 +56,7 @@ 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("-"); // + sandbox.within("odin").mkdir("-"); let actual = nu!( cwd: dirs.test(), @@ -86,7 +86,7 @@ fn filesystem_change_current_directory_to_parent_directory() { } #[test] -fn file_system_change_to_home_directory() { +fn filesystem_change_to_home_directory() { Playground::setup("cd_test_6", |dirs, _| { let actual = nu!( cwd: dirs.test(), @@ -126,4 +126,239 @@ fn filesystem_directory_not_found() { assert!(actual.contains("dir_that_does_not_exist")); assert!(actual.contains("directory not found")); -} \ No newline at end of file +} + + +#[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 ", "Jonathan Turner ", "Andrés N. Robalino "] + + [[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 ", "Jonathan Turner ", "Andrés N. Robalino "] + "# + )]); + + 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")); +}