From f4bf7316fe96a3158b293df7eca24b4c2ea2d838 Mon Sep 17 00:00:00 2001
From: Darren Schroeder <343840+fdncred@users.noreply.github.com>
Date: Wed, 8 Feb 2023 20:53:46 -0600
Subject: [PATCH] fix completions PATH vs Path (#8003)

# Description

This PR attempts to fix the completions issue where, on Windows,
completions wouldn't get generated from items in your path environment
variable. This seemed to be down to `PATH` vs `Path`. So, I tried to add
a new function that we can use anywhere to avoid this problem.

# User-Facing Changes

_(List of all changes that impact the user experience here. This helps
us keep track of breaking changes.)_

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
---
 .../src/completions/command_completions.rs    |  3 ++-
 crates/nu-cli/tests/completions.rs            | 17 +++++++++++++--
 .../tests/support/completions_helpers.rs      | 16 ++++++++++++++
 crates/nu-protocol/src/engine/engine_state.rs | 21 +++++++++++++++++++
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs
index 73de7ac8cd..746dcb9ec3 100644
--- a/crates/nu-cli/src/completions/command_completions.rs
+++ b/crates/nu-cli/src/completions/command_completions.rs
@@ -37,7 +37,8 @@ impl CommandCompletion {
     ) -> Vec<String> {
         let mut executables = vec![];
 
-        let paths = self.engine_state.get_env_var("PATH");
+        // os agnostic way to get the PATH env var
+        let paths = self.engine_state.get_path_env_var();
 
         if let Some(paths) = paths {
             if let Ok(paths) = paths.as_list() {
diff --git a/crates/nu-cli/tests/completions.rs b/crates/nu-cli/tests/completions.rs
index 219015f54e..976083e4be 100644
--- a/crates/nu-cli/tests/completions.rs
+++ b/crates/nu-cli/tests/completions.rs
@@ -574,9 +574,12 @@ fn variables_completions() {
     // Test completions for $env
     let suggestions = completer.complete("$env.", 5);
 
-    assert_eq!(2, suggestions.len());
+    assert_eq!(3, suggestions.len());
 
-    let expected: Vec<String> = vec!["PWD".into(), "TEST".into()];
+    #[cfg(windows)]
+    let expected: Vec<String> = vec!["PWD".into(), "Path".into(), "TEST".into()];
+    #[cfg(not(windows))]
+    let expected: Vec<String> = vec!["PATH".into(), "PWD".into(), "TEST".into()];
 
     // Match results
     match_suggestions(expected, suggestions);
@@ -853,3 +856,13 @@ fn alias_offset_bug_7754() {
 
     //println!(" --------- suggestions: {:?}", suggestions);
 }
+
+#[test]
+fn get_path_env_var_8003() {
+    // Create a new engine
+    let (_, _, engine, _) = new_engine();
+    // Get the path env var in a platform agnostic way
+    let the_path = engine.get_path_env_var();
+    // Make sure it's not empty
+    assert!(the_path.is_some());
+}
diff --git a/crates/nu-cli/tests/support/completions_helpers.rs b/crates/nu-cli/tests/support/completions_helpers.rs
index 7b50edf7a5..2ae8b2b121 100644
--- a/crates/nu-cli/tests/support/completions_helpers.rs
+++ b/crates/nu-cli/tests/support/completions_helpers.rs
@@ -43,6 +43,22 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) {
             span: nu_protocol::Span::new(0, dir_str.len()),
         },
     );
+    #[cfg(windows)]
+    stack.add_env_var(
+        "Path".to_string(),
+        Value::String {
+            val: "c:\\some\\path;c:\\some\\other\\path".to_string(),
+            span: nu_protocol::Span::new(0, dir_str.len()),
+        },
+    );
+    #[cfg(not(windows))]
+    stack.add_env_var(
+        "PATH".to_string(),
+        Value::String {
+            val: "/some/path:/some/other/path".to_string(),
+            span: nu_protocol::Span::new(0, dir_str.len()),
+        },
+    );
 
     // Merge environment into the permanent state
     let merge_result = engine_state.merge_env(&mut stack, &dir);
diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs
index 27a9908c98..e41e44a926 100644
--- a/crates/nu-protocol/src/engine/engine_state.rs
+++ b/crates/nu-protocol/src/engine/engine_state.rs
@@ -449,6 +449,27 @@ impl EngineState {
         None
     }
 
+    // Get the path environment variable in a platform agnostic way
+    pub fn get_path_env_var(&self) -> Option<&Value> {
+        let env_path_name_windows: &str = "Path";
+        let env_path_name_nix: &str = "PATH";
+
+        for overlay_id in self.scope.active_overlays.iter().rev() {
+            let overlay_name = String::from_utf8_lossy(self.get_overlay_name(*overlay_id));
+            if let Some(env_vars) = self.env_vars.get(overlay_name.as_ref()) {
+                if let Some(val) = env_vars.get(env_path_name_nix) {
+                    return Some(val);
+                } else if let Some(val) = env_vars.get(env_path_name_windows) {
+                    return Some(val);
+                } else {
+                    return None;
+                }
+            }
+        }
+
+        None
+    }
+
     #[cfg(feature = "plugin")]
     pub fn update_plugin_file(&self) -> Result<(), ShellError> {
         use std::io::Write;