Follow review comments, and adjust expectation on test results for windows. Hide env_vars for PWD-per-drive from being collected by auto completion.

Cargo.lock

@@ -3256,6 +3256,7 @@ dependencies = [
  "nu-utils",
  "percent-encoding",
  "reedline",
+ "regex",
  "rstest",
  "sysinfo 0.32.0",
  "tempfile",

crates/nu-cli/Cargo.toml

@@ -42,6 +42,8 @@ sysinfo = { workspace = true }
 unicode-segmentation = { workspace = true }
 uuid = { workspace = true, features = ["v4"] }
 which = { workspace = true }
+#[cfg(windows)]
+regex = { workspace = true }
 
 [features]
 plugin = ["nu-plugin-engine"]

crates/nu-cli/src/completions/variable_completions.rs

@@ -46,7 +46,17 @@ impl Completer for VariableCompletion {
         if !var_str.is_empty() {
             // Completion for $env.<tab>
             if var_str == "$env" {
+                #[cfg(not(windows))]
                 let env_vars = stack.get_env_vars(working_set.permanent_state);
+                #[cfg(windows)]
+                let mut env_vars = stack.get_env_vars(working_set.permanent_state);
+                #[cfg(windows)]
+                {
+                    // Hide env_vars for PWD-per-drive from being listed in completion selection
+                    if let Ok(pattern) = regex::Regex::new(r"^=[a-zA-Z]:$") {
+                        env_vars.retain(|key, _| !pattern.is_match(key));
+                    }
+                }
 
                 // Return nested values
                 if sublevels_count > 0 {

crates/nu-cli/tests/completions/mod.rs

@@ -1414,10 +1414,7 @@ fn variables_completions() {
     // Test completions for $env
     let suggestions = completer.complete("$env.", 5);
 
-    #[cfg(not(windows))]
     assert_eq!(3, suggestions.len());
-    #[cfg(windows)]
-    assert_eq!(4, suggestions.len());
 
     #[cfg(windows)]
     let expected: Vec<String> = vec!["Path".into(), "PWD".into(), "TEST".into()];
@@ -1425,14 +1422,7 @@ fn variables_completions() {
     let expected: Vec<String> = vec!["PATH".into(), "PWD".into(), "TEST".into()];
 
     // Match results
-    #[cfg(not(windows))]
     match_suggestions(&expected, &suggestions);
-    #[cfg(windows)]
-    {
-        let mut suggestions = suggestions;
-        let _ = suggestions.remove(0);
-        match_suggestions(&expected, &suggestions);
-    }
 
     // Test completions for $env
     let suggestions = completer.complete("$env.T", 6);

crates/nu-protocol/src/engine/pwd_per_drive.rs

@@ -1,4 +1,6 @@
 use crate::engine::{EngineState, Stack};
+#[cfg(windows)]
+use omnipath::sys_absolute;
 use std::path::{Path, PathBuf};
 
 // For file system command usage
@@ -40,28 +42,34 @@ pub mod windows {
         fn maintain(&mut self, key: String, value: Value);
     }
 
-    macro_rules! impl_env_maintainer {
+    impl EnvMaintainer for EngineState {
-        ($type:ty) => {
+        fn maintain(&mut self, key: String, value: Value) {
-            impl EnvMaintainer for $type {
+            self.add_env_var(key, value);
-                fn maintain(&mut self, key: String, value: Value) {
+        }
-                    self.add_env_var(key, value);
-                }
-            }
-        };
     }
 
-    impl_env_maintainer!(Stack);
+    impl EnvMaintainer for Stack {
-    impl_env_maintainer!(EngineState);
+        fn maintain(&mut self, key: String, value: Value) {
+            self.add_env_var(key, value);
+        }
+    }
 
     /// For maintainer to notify PWD
     /// When user changes current directory, maintainer calls set_pwd()
     /// and PWD-per-drive call maintainer back to store it for the
     /// env_var_for_drive.
+    /// TBD: If value can't be converted to String or the value is not valid for
+    /// windows path on a drive, should 'cd' or 'auto_cd' get warning message
+    /// that PWD-per-drive can't process the path value?
     pub fn set_pwd<T: EnvMaintainer>(maintainer: &mut T, value: Value) {
         if let Ok(path_string) = String::from_value(value.clone()) {
             if let Some(drive) = extract_drive_letter(&path_string) {
                 maintainer.maintain(env_var_for_drive(drive), value);
+            } else {
+                log::trace!("PWD-per-drive can't find drive of {}", path_string);
             }
+        } else if let Err(e) = value.into_string() {
+            log::trace!("PWD-per-drive can't keep this path value: {}", e);
         }
     }
 
@@ -154,9 +162,6 @@ pub mod windows {
     /// Call Windows system API (via omnipath crate) to expand
     /// absolute path
     fn get_full_path_name_w(path_str: &str) -> Option<String> {
-        use omnipath::sys_absolute;
-        use std::path::Path;
-
         if let Ok(path_sys_abs) = sys_absolute(Path::new(path_str)) {
             Some(path_sys_abs.to_str()?.to_string())
         } else {

tests/path/canonicalize.rs

@@ -100,6 +100,13 @@ fn canonicalize_dot() {
 
     let actual = canonicalize_with(".", expected.as_path()).expect("Failed to canonicalize");
 
+    // Once my windows gives the current directory for different case
+    // Actual "E:\\Study", got "E:\\study"
+    #[cfg(windows)]
+    let actual = std::path::PathBuf::from(actual.to_str().unwrap().to_ascii_uppercase());
+    #[cfg(windows)]
+    let expected = std::path::PathBuf::from(expected.to_str().unwrap().to_ascii_uppercase());
+
     assert_eq!(actual, expected);
 }
 
@@ -110,6 +117,13 @@ fn canonicalize_many_dots() {
     let actual = canonicalize_with("././/.//////./././//.///", expected.as_path())
         .expect("Failed to canonicalize");
 
+    // Once my windows gives the current directory for different case
+    // Actual "E:\\Study", got "E:\\study"
+    #[cfg(windows)]
+    let actual = std::path::PathBuf::from(actual.to_str().unwrap().to_ascii_uppercase());
+    #[cfg(windows)]
+    let expected = std::path::PathBuf::from(expected.to_str().unwrap().to_ascii_uppercase());
+
     assert_eq!(actual, expected);
 }
 
@@ -148,6 +162,13 @@ fn canonicalize_double_dot() {
         .parent()
         .expect("Could not get parent of current directory");
 
+    // Once my windows gives the current directory for different case
+    // Actual "E:\\Study", got "E:\\study"
+    #[cfg(windows)]
+    let actual = std::path::PathBuf::from(actual.to_str().unwrap().to_ascii_uppercase());
+    #[cfg(windows)]
+    let expected = std::path::PathBuf::from(expected.to_str().unwrap().to_ascii_uppercase());
+
     assert_eq!(actual, expected);
 }
 

tests/plugins/env.rs

@@ -49,7 +49,14 @@ fn get_current_dir() {
             cwd.chars().next().unwrap().to_ascii_uppercase(),
             result.out.chars().next().unwrap().to_ascii_uppercase()
         );
-        assert_eq!(cwd[1..], result.out[1..]);
+        // Once my windows gives the current directory for different case
+        // Actual "E:\\Study", got "E:\\study"
+        //left: ":\\study\\nushell\\tests"
+        //right: ":\\Study\\nushell\\tests"
+        assert_eq!(
+            cwd[1..].to_ascii_uppercase(),
+            result.out[1..].to_ascii_uppercase()
+        );
     }
 }