From 57101d50226dd1e13709834582f4c69041083256 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Tue, 18 Aug 2020 07:36:09 +0200 Subject: [PATCH] Run exitscripts in original dir (#2352) * Modify testcase * Run exitscript in the folder it was specified * Update documentation * Add comment * Borrow instead of clone * Does this just... work on windows? * fmt * as_str * Collapse if by order of clippy * Support windows * fmt * refactor tests * fmt * This time it will work on windows FOR SURE * Remove debug prints * Comment * Refactor tests * fmt * fix spelling * update comment --- crates/nu-cli/src/commands/autoenv.rs | 2 +- .../src/env/directory_specific_environment.rs | 22 ++++- tests/shell/pipeline/commands/internal.rs | 83 +++++++++---------- 3 files changed, 57 insertions(+), 50 deletions(-) diff --git a/crates/nu-cli/src/commands/autoenv.rs b/crates/nu-cli/src/commands/autoenv.rs index a3775772d..086790b7e 100644 --- a/crates/nu-cli/src/commands/autoenv.rs +++ b/crates/nu-cli/src/commands/autoenv.rs @@ -55,7 +55,7 @@ impl WholeStreamCommand for Autoenv { The file can contain several optional sections: env: environment variables to set when visiting the directory. The variables are unset after leaving the directory and any overwritten values are restored. scriptvars: environment variables that should be set to the return value of a script. After they have been set, they behave in the same way as variables set in the env section. - scripts: scripts to run when entering the directory or leaving it. Note that exitscripts are not run in the directory they are declared in."# + scripts: scripts to run when entering the directory or leaving it."# } fn signature(&self) -> Signature { diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 6f6b05e24..592c60a46 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -112,7 +112,7 @@ impl DirectorySpecificEnvironment { if let Some(es) = nu_env_doc.entryscripts { for s in es { - run(s.as_str())?; + run(s.as_str(), None)?; } } @@ -147,7 +147,7 @@ impl DirectorySpecificEnvironment { new_exitscripts.insert(dir, scripts); } else { for s in scripts { - run(s.as_str())?; + run(s.as_str(), Some(&dir))?; } } } @@ -215,9 +215,23 @@ impl DirectorySpecificEnvironment { } } -fn run(cmd: &str) -> Result<(), ShellError> { +fn run(cmd: &str, dir: Option<&PathBuf>) -> Result<(), ShellError> { if cfg!(target_os = "windows") { - Command::new("cmd").args(&["/C", cmd]).output()? + if let Some(dir) = dir { + let command = format!("cd {} & {}", dir.to_string_lossy(), cmd); + Command::new("cmd") + .args(&["/C", command.as_str()]) + .output()? + } else { + Command::new("cmd").args(&["/C", cmd]).output()? + } + } else if let Some(dir) = dir { + // FIXME: When nu scripting is added, cding like might not be a good idea. If nu decides to execute entryscripts when entering the dir this way, it will cause troubles. + // For now only standard shell scripts are used, so this is an issue for the future. + Command::new("sh") + .arg("-c") + .arg(format!("cd {:?}; {}", dir, cmd)) + .output()? } else { Command::new("sh").arg("-c").arg(&cmd).output()? }; diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs index 4233d4fd1..ce1c0c017 100644 --- a/tests/shell/pipeline/commands/internal.rs +++ b/tests/shell/pipeline/commands/internal.rs @@ -48,36 +48,31 @@ fn autoenv() { sandbox.mkdir("bizz/buzz"); sandbox.mkdir("foob"); - let scriptfile = if cfg!(target_os = "windows") { - FileWithContent( - ".nu-env", - r#"[env] - testkey = "testvalue" + // Windows uses a different command to create an empty file so we need to have different content on windows. + let full_nu_env = if cfg!(target_os = "windows") { + r#"[env] + testkey = "testvalue" - [scriptvars] - myscript = "echo myval" + [scriptvars] + myscript = "echo myval" - [scripts] - entryscripts = ["echo nul > hello.txt"] - exitscripts = ["echo nul > bye.txt"]"#, - ) + [scripts] + entryscripts = ["echo nul > hello.txt"] + exitscripts = ["echo nul > bye.txt"]"# } else { - FileWithContent( - ".nu-env", - r#"[env] - testkey = "testvalue" + r#"[env] + testkey = "testvalue" - [scriptvars] - myscript = "echo myval" + [scriptvars] + myscript = "echo myval" - [scripts] - entryscripts = ["touch hello.txt"] - exitscripts = ["touch bye.txt"]"#, - ) + [scripts] + entryscripts = ["touch hello.txt"] + exitscripts = ["touch bye.txt"]"# }; sandbox.with_files(vec![ - scriptfile, + FileWithContent(".nu-env", full_nu_env), FileWithContent( "foo/.nu-env", r#"[env] @@ -89,14 +84,28 @@ fn autoenv() { r#"[env] overwrite_me = "set_in_bar""#, ), - FileWithContent( - "bizz/.nu-env", - r#"[scripts] - entryscripts = ["touch hello.txt"] - exitscripts = ["touch bye.txt"]"#, - ), + FileWithContent("bizz/.nu-env", full_nu_env), ]); + //Make sure basic keys are set + let actual = nu!( + cwd: dirs.test(), + r#"autoenv trust + echo $nu.env.testkey"# + ); + assert!(actual.out.ends_with("testvalue")); + + // Make sure exitscripts are run in the directory they were specified. + let actual = nu!( + cwd: dirs.test(), + r#"autoenv trust + cd .. + cd autoenv_test + ls + ls | where name == "bye.txt" | get name"# + ); + assert!(actual.out.contains("bye.txt")); + // Make sure entry scripts are run let actual = nu!( cwd: dirs.test(), @@ -116,7 +125,7 @@ fn autoenv() { ); assert!(!actual.out.contains("bye.txt")); - // Make sure entry scripts are run when re-visiting a directory + // Make sure entryscripts are run when re-visiting a directory let actual = nu!( cwd: dirs.test(), r#"autoenv trust bizz @@ -138,14 +147,6 @@ fn autoenv() { ); assert!(!actual.out.ends_with("hello.txt")); - //Make sure basic keys are set - let actual = nu!( - cwd: dirs.test(), - r#"autoenv trust - echo $nu.env.testkey"# - ); - assert!(actual.out.ends_with("testvalue")); - //Backing out of the directory should unset the keys let actual = nu!( cwd: dirs.test(), @@ -189,14 +190,6 @@ fn autoenv() { ); assert!(actual.out.contains("hello.txt")); - // Make sure exit scripts are run - let actual = nu!( - cwd: dirs.test(), - r#"cd .. - ls | where name == "bye.txt" | get name"# - ); - assert!(actual.out.contains("bye.txt")); - //Variables set in parent directories should be set even if you directly cd to a subdir let actual = nu!( cwd: dirs.test(),