From 0ba5efd43d579a889c6bf4542f914be50b7d6f80 Mon Sep 17 00:00:00 2001 From: Zhenping Zhao Date: Thu, 21 Nov 2024 10:38:34 -0800 Subject: [PATCH] Comments, fix testcase problems found in 'toolkit check pr' --- crates/nu-cli/src/repl.rs | 1 + crates/nu-command/src/filesystem/cd.rs | 1 + crates/nu-path/src/pwd_per_drive.rs | 21 +++++++++++++++++++-- crates/nu-path/src/tilde.rs | 1 + src/main.rs | 4 +++- 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index f0b9f1eef5..eea99d6188 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -858,6 +858,7 @@ fn do_auto_cd( report_shell_error(engine_state, &err); return; }; + // Let PWD-per-drive sync with auto_cd target let _ = nu_path::set_pwd_per_drive(PathBuf::from(path.clone()).as_path()); let cwd = Value::string(cwd, span); diff --git a/crates/nu-command/src/filesystem/cd.rs b/crates/nu-command/src/filesystem/cd.rs index c4b17b688a..6cecf56638 100644 --- a/crates/nu-command/src/filesystem/cd.rs +++ b/crates/nu-command/src/filesystem/cd.rs @@ -114,6 +114,7 @@ impl Command for Cd { //FIXME: this only changes the current scope, but instead this environment variable //should probably be a block that loads the information from the state in the overlay PermissionResult::PermissionOk => { + // Let PWD-per-drive sync with cd target let _ = nu_path::set_pwd_per_drive(path.as_path()); stack.set_cwd(path)?; Ok(PipelineData::empty()) diff --git a/crates/nu-path/src/pwd_per_drive.rs b/crates/nu-path/src/pwd_per_drive.rs index 248ebb545f..1bd50f6bd9 100644 --- a/crates/nu-path/src/pwd_per_drive.rs +++ b/crates/nu-path/src/pwd_per_drive.rs @@ -159,7 +159,15 @@ mod tests { assert_eq!(map.get_pwd('C'), Some("C:\\Users\\Example".to_string())); // Get PWD for drive E (not set, should return E:\) - assert_eq!(map.get_pwd('E'), Some("E:\\".to_string())); + // 11-21-2024 happened to start nushell from drive E:, + // run toolkit test 'toolkit check pr' then this test failed + // since the singleton has its own state, so this type of test ('not set, + // should return ...') must be more careful to avoid accidentally fail. + if let Some(pwd_on_e) = get_full_path_name_w("E:") { + assert_eq!(map.get_pwd('E'), Some(pwd_on_e)); + } else { + assert_eq!(map.get_pwd('E'), Some("E:\\".to_string())); + } } } @@ -200,7 +208,16 @@ mod tests { assert_eq!(drive_map.get_pwd('D'), Some("D:\\Projects".to_string())); // Get PWD for drive E (not set, should return E:\) - assert_eq!(drive_map.get_pwd('E'), Some("E:\\".to_string())); + // 11-21-2024 happened to start nushell from drive E:, + // run toolkit test 'toolkit check pr' then this test failed + // if a drive has not been set PWD, it will ask system to get + // current directory, so this type of test ('not set, should + // return ...') must be more careful to avoid accidentally fail. + if let Some(pwd_on_e) = get_full_path_name_w("E:") { + assert_eq!(drive_map.get_pwd('E'), Some(pwd_on_e)); + } else { + assert_eq!(drive_map.get_pwd('E'), Some("E:\\".to_string())); + } } #[test] diff --git a/crates/nu-path/src/tilde.rs b/crates/nu-path/src/tilde.rs index 0b0934f08b..262ae172e5 100644 --- a/crates/nu-path/src/tilde.rs +++ b/crates/nu-path/src/tilde.rs @@ -21,6 +21,7 @@ fn expand_tilde_with_home(path: impl AsRef, home: Option) -> Path let path = path.as_ref(); if !path.starts_with("~") { + // Try to expand relative path by PWD-per-drive if let Some(expanded_dir) = crate::expand_pwd_per_drive(path) { return expanded_dir; } diff --git a/src/main.rs b/src/main.rs index b7c5a6a173..d55940bd61 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,7 +22,7 @@ use log::{trace, Level}; use miette::Result; use nu_cli::gather_parent_env_vars; use nu_lsp::LanguageServer; -use nu_path::canonicalize_with; +use nu_path::{canonicalize_with, set_pwd_per_drive}; use nu_protocol::{ engine::EngineState, report_shell_error, ByteStream, PipelineData, ShellError, Span, Spanned, Value, @@ -78,6 +78,8 @@ fn main() -> Result<()> { // Get the current working directory from the environment. let init_cwd = current_dir_from_environment(); + // Let PWD-per-drive sync from startup + let _ = set_pwd_per_drive(init_cwd.as_ref()); // Custom additions let delta = {