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.
This commit is contained in:
Darren Schroeder 2023-02-08 20:53:46 -06:00 committed by GitHub
parent 0527f9bf0d
commit f4bf7316fe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 54 additions and 3 deletions

View file

@ -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() {

View file

@ -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());
}

View file

@ -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);

View file

@ -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;