Revert "Feature: PWD-per-drive to facilitate working on multiple drives at Windows" (#14598)

Reverts nushell/nushell#14411
This commit is contained in:
Darren Schroeder 2024-12-16 13:52:07 -06:00 committed by GitHub
parent 39770d4197
commit e2c4ff8180
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 29 additions and 430 deletions

View File

@ -174,14 +174,6 @@ pub fn complete_item(
) -> Vec<FileSuggestion> {
let cleaned_partial = surround_remove(partial);
let isdir = cleaned_partial.ends_with(is_separator);
#[cfg(windows)]
let cleaned_partial = if let Some(absolute_partial) =
stack.pwd_per_drive.expand_pwd(Path::new(&cleaned_partial))
{
absolute_partial.display().to_string()
} else {
cleaned_partial
};
let expanded_partial = expand_ndots(Path::new(&cleaned_partial));
let should_collapse_dots = expanded_partial != Path::new(&cleaned_partial);
let mut partial = expanded_partial.to_string_lossy().to_string();

View File

@ -832,12 +832,6 @@ fn do_auto_cd(
engine_state: &mut EngineState,
span: Span,
) {
#[cfg(windows)]
let path = if let Some(abs_path) = stack.pwd_per_drive.expand_pwd(path.as_path()) {
abs_path
} else {
path
};
let path = {
if !path.exists() {
report_shell_error(

View File

@ -87,7 +87,7 @@ impl Command for Cd {
});
}
} else {
let path = stack.expand_path_with(path_no_whitespace, &cwd, true);
let path = nu_path::expand_path_with(path_no_whitespace, &cwd, true);
if !path.exists() {
return Err(ShellError::DirectoryNotFound {
dir: path_no_whitespace.to_string(),

View File

@ -156,9 +156,6 @@ pub fn env_to_strings(
}
}
#[cfg(windows)]
stack.pwd_per_drive.get_env_vars(&mut env_vars_str);
Ok(env_vars_str)
}

View File

@ -194,11 +194,6 @@ pub fn redirect_env(engine_state: &EngineState, caller_stack: &mut Stack, callee
caller_stack.add_env_var(var, value);
}
#[cfg(windows)]
{
caller_stack.pwd_per_drive = callee_stack.pwd_per_drive.clone();
}
// set config to callee config, to capture any updates to that
caller_stack.config.clone_from(&callee_stack.config);
}

View File

@ -1,6 +1,6 @@
use std::{borrow::Cow, fs::File, sync::Arc};
use nu_path::AbsolutePathBuf;
use nu_path::{expand_path_with, AbsolutePathBuf};
use nu_protocol::{
ast::{Bits, Block, Boolean, CellPath, Comparison, Math, Operator},
debugger::DebugContext,
@ -14,7 +14,7 @@ use nu_protocol::{
};
use nu_utils::IgnoreCaseExt;
use crate::{eval::is_automatic_env_var, eval_block_with_early_return, redirect_env};
use crate::{eval::is_automatic_env_var, eval_block_with_early_return};
/// Evaluate the compiled representation of a [`Block`].
pub fn eval_ir_block<D: DebugContext>(
@ -870,7 +870,7 @@ fn literal_value(
Value::string(path, span)
} else {
let cwd = ctx.engine_state.cwd(Some(ctx.stack))?;
let path = ctx.stack.expand_path_with(path, cwd, true);
let path = expand_path_with(path, cwd, true);
Value::string(path.to_string_lossy(), span)
}
@ -890,7 +890,7 @@ fn literal_value(
.cwd(Some(ctx.stack))
.map(AbsolutePathBuf::into_std_path_buf)
.unwrap_or_default();
let path = ctx.stack.expand_path_with(path, cwd, true);
let path = expand_path_with(path, cwd, true);
Value::string(path.to_string_lossy(), span)
}
@ -1405,8 +1405,7 @@ enum RedirectionStream {
/// Open a file for redirection
fn open_file(ctx: &EvalContext<'_>, path: &Value, append: bool) -> Result<Arc<File>, ShellError> {
let path_expanded =
ctx.stack
.expand_path_with(path.as_str()?, ctx.engine_state.cwd(Some(ctx.stack))?, true);
expand_path_with(path.as_str()?, ctx.engine_state.cwd(Some(ctx.stack))?, true);
let mut options = File::options();
if append {
options.append(true);
@ -1486,3 +1485,26 @@ fn eval_iterate(
eval_iterate(ctx, dst, stream, end_index)
}
}
/// Redirect environment from the callee stack to the caller stack
fn redirect_env(engine_state: &EngineState, caller_stack: &mut Stack, callee_stack: &Stack) {
// TODO: make this more efficient
// Grab all environment variables from the callee
let caller_env_vars = caller_stack.get_env_var_names(engine_state);
// remove env vars that are present in the caller but not in the callee
// (the callee hid them)
for var in caller_env_vars.iter() {
if !callee_stack.has_env_var(engine_state, var) {
caller_stack.remove_env_var(engine_state, var);
}
}
// add new env vars from callee to caller
for (var, value) in callee_stack.get_stack_env_vars() {
caller_stack.add_env_var(var, value);
}
// set config to callee config, to capture any updates to that
caller_stack.config.clone_from(&callee_stack.config);
}

View File

@ -6,8 +6,6 @@ pub mod expansions;
pub mod form;
mod helpers;
mod path;
#[cfg(windows)]
pub mod pwd_per_drive;
mod tilde;
mod trailing_slash;
@ -15,7 +13,5 @@ pub use components::components;
pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path, locate_in_dirs};
pub use helpers::{cache_dir, data_dir, home_dir, nu_config_dir};
pub use path::*;
#[cfg(windows)]
pub use pwd_per_drive::DriveToPwdMap;
pub use tilde::expand_tilde;
pub use trailing_slash::{has_trailing_slash, strip_trailing_slash};

View File

@ -1,331 +0,0 @@
/// Usage for pwd_per_drive on windows
///
/// let mut map = DriveToPwdMap::new();
///
/// Upon change PWD, call map.set_pwd() with absolute path
///
/// Call map.expand_pwd() with relative path to get absolution path
///
/// ```
/// use std::path::{Path, PathBuf};
/// use nu_path::DriveToPwdMap;
///
/// let mut map = DriveToPwdMap::new();
///
/// // Set PWD for drive C
/// assert!(map.set_pwd(Path::new(r"C:\Users\Home")).is_ok());
///
/// // Expand a relative path
/// let expanded = map.expand_pwd(Path::new("c:test"));
/// assert_eq!(expanded, Some(PathBuf::from(r"C:\Users\Home\test")));
///
/// // Will NOT expand an absolute path
/// let expanded = map.expand_pwd(Path::new(r"C:\absolute\path"));
/// assert_eq!(expanded, None);
///
/// // Expand with no drive letter
/// let expanded = map.expand_pwd(Path::new(r"\no_drive"));
/// assert_eq!(expanded, None);
///
/// // Expand with no PWD set for the drive
/// let expanded = map.expand_pwd(Path::new("D:test"));
/// assert!(expanded.is_some());
/// let abs_path = expanded.unwrap().as_path().to_str().expect("OK").to_string();
/// assert!(abs_path.starts_with(r"D:\"));
/// assert!(abs_path.ends_with(r"\test"));
///
/// // Get env vars for child process
/// use std::collections::HashMap;
/// let mut env = HashMap::<String, String>::new();
/// map.get_env_vars(&mut env);
/// assert_eq!(env.get("=C:").unwrap(), r"C:\Users\Home");
/// ```
use std::collections::HashMap;
use std::path::{Path, PathBuf};
#[derive(Debug, PartialEq)]
pub enum PathError {
InvalidDriveLetter,
InvalidPath,
}
/// Helper to check if input path is relative path
/// with drive letter, it can be expanded with PWD-per-drive.
fn need_expand(path: &Path) -> bool {
if let Some(path_str) = path.to_str() {
let chars: Vec<char> = path_str.chars().collect();
if chars.len() >= 2 {
return chars[1] == ':' && (chars.len() == 2 || (chars[2] != '/' && chars[2] != '\\'));
}
}
false
}
#[derive(Clone, Debug)]
pub struct DriveToPwdMap {
map: [Option<String>; 26], // Fixed-size array for A-Z
}
impl Default for DriveToPwdMap {
fn default() -> Self {
Self::new()
}
}
impl DriveToPwdMap {
pub fn new() -> Self {
Self {
map: Default::default(),
}
}
pub fn env_var_for_drive(drive_letter: char) -> String {
let drive_letter = drive_letter.to_ascii_uppercase();
format!("={}:", drive_letter)
}
/// Collect PWD-per-drive as env vars (for child process)
pub fn get_env_vars(&self, env: &mut HashMap<String, String>) {
for (drive_index, drive_letter) in ('A'..='Z').enumerate() {
if let Some(pwd) = self.map[drive_index].clone() {
if pwd.len() > 3 {
let env_var_for_drive = Self::env_var_for_drive(drive_letter);
env.insert(env_var_for_drive, pwd);
}
}
}
}
/// Set the PWD for the drive letter in the absolute path.
/// Return PathError for error.
pub fn set_pwd(&mut self, path: &Path) -> Result<(), PathError> {
if let (Some(drive_letter), Some(path_str)) =
(Self::extract_drive_letter(path), path.to_str())
{
if drive_letter.is_ascii_alphabetic() {
let drive_letter = drive_letter.to_ascii_uppercase();
// Make sure saved drive letter is upper case
let mut c = path_str.chars();
match c.next() {
None => Err(PathError::InvalidDriveLetter),
Some(_) => {
let drive_index = drive_letter as usize - 'A' as usize;
let normalized_pwd = drive_letter.to_string() + c.as_str();
self.map[drive_index] = Some(normalized_pwd);
Ok(())
}
}
} else {
Err(PathError::InvalidDriveLetter)
}
} else {
Err(PathError::InvalidPath)
}
}
/// Get the PWD for drive, if not yet, ask GetFullPathNameW() or omnipath,
/// or else return default r"X:\".
fn get_pwd(&self, drive_letter: char) -> Result<String, PathError> {
if drive_letter.is_ascii_alphabetic() {
let drive_letter = drive_letter.to_ascii_uppercase();
let drive_index = drive_letter as usize - 'A' as usize;
Ok(self.map[drive_index].clone().unwrap_or_else(|| {
if let Some(sys_pwd) = get_full_path_name_w(&format!("{}:", drive_letter)) {
sys_pwd
} else {
format!(r"{}:\", drive_letter)
}
}))
} else {
Err(PathError::InvalidDriveLetter)
}
}
/// Expand a relative path using the PWD-per-drive, return PathBuf
/// of absolute path.
/// Return None if path is not valid or can't get drive letter.
pub fn expand_pwd(&self, path: &Path) -> Option<PathBuf> {
if need_expand(path) {
let path_str = path.to_str()?;
if let Some(drive_letter) = Self::extract_drive_letter(path) {
if let Ok(pwd) = self.get_pwd(drive_letter) {
// Combine current PWD with the relative path
let mut base = PathBuf::from(Self::ensure_trailing_delimiter(&pwd));
// need_expand() and extract_drive_letter() all ensure path_str.len() >= 2
base.push(&path_str[2..]); // Join PWD with path parts after "C:"
return Some(base);
}
}
}
None // Invalid path or has no drive letter
}
/// Helper to extract the drive letter from a path, keep case
/// (e.g., `C:test` -> `C`, `d:\temp` -> `d`)
fn extract_drive_letter(path: &Path) -> Option<char> {
path.to_str()
.and_then(|s| s.chars().next())
.filter(|c| c.is_ascii_alphabetic())
}
/// Ensure a path has a trailing `\\` or '/'
fn ensure_trailing_delimiter(path: &str) -> String {
if !path.ends_with('\\') && !path.ends_with('/') {
format!(r"{}\", path)
} else {
path.to_string()
}
}
}
fn get_full_path_name_w(path_str: &str) -> Option<String> {
use omnipath::sys_absolute;
if let Ok(path_sys_abs) = sys_absolute(Path::new(path_str)) {
Some(path_sys_abs.to_str()?.to_string())
} else {
None
}
}
/// Test for Drive2PWD map
#[cfg(test)]
mod tests {
use super::*;
/// Test or demo usage of PWD-per-drive
/// In doctest, there's no get_full_path_name_w available so can't foresee
/// possible result, here can have more accurate test assert
#[test]
fn test_usage_for_pwd_per_drive() {
let mut map = DriveToPwdMap::new();
// Set PWD for drive E
assert!(map.set_pwd(Path::new(r"E:\Users\Home")).is_ok());
// Expand a relative path
let expanded = map.expand_pwd(Path::new("e:test"));
assert_eq!(expanded, Some(PathBuf::from(r"E:\Users\Home\test")));
// Will NOT expand an absolute path
let expanded = map.expand_pwd(Path::new(r"E:\absolute\path"));
assert_eq!(expanded, None);
// Expand with no drive letter
let expanded = map.expand_pwd(Path::new(r"\no_drive"));
assert_eq!(expanded, None);
// Expand with no PWD set for the drive
let expanded = map.expand_pwd(Path::new("F:test"));
if let Some(sys_abs) = get_full_path_name_w("F:") {
assert_eq!(
expanded,
Some(PathBuf::from(format!(
"{}test",
DriveToPwdMap::ensure_trailing_delimiter(&sys_abs)
)))
);
} else {
assert_eq!(expanded, Some(PathBuf::from(r"F:\test")));
}
}
#[test]
fn test_get_env_vars() {
let mut map = DriveToPwdMap::new();
map.set_pwd(Path::new(r"I:\Home")).unwrap();
map.set_pwd(Path::new(r"j:\User")).unwrap();
let mut env = HashMap::<String, String>::new();
map.get_env_vars(&mut env);
assert_eq!(
env.get(&DriveToPwdMap::env_var_for_drive('I')).unwrap(),
r"I:\Home"
);
assert_eq!(
env.get(&DriveToPwdMap::env_var_for_drive('J')).unwrap(),
r"J:\User"
);
}
#[test]
fn test_expand_pwd() {
let mut drive_map = DriveToPwdMap::new();
// Set PWD for drive 'M:'
assert_eq!(drive_map.set_pwd(Path::new(r"M:\Users")), Ok(()));
// or 'm:'
assert_eq!(drive_map.set_pwd(Path::new(r"m:\Users\Home")), Ok(()));
// Expand a relative path on "M:"
let expanded = drive_map.expand_pwd(Path::new(r"M:test"));
assert_eq!(expanded, Some(PathBuf::from(r"M:\Users\Home\test")));
// or on "m:"
let expanded = drive_map.expand_pwd(Path::new(r"m:test"));
assert_eq!(expanded, Some(PathBuf::from(r"M:\Users\Home\test")));
// Expand an absolute path
let expanded = drive_map.expand_pwd(Path::new(r"m:\absolute\path"));
assert_eq!(expanded, None);
// Expand with no drive letter
let expanded = drive_map.expand_pwd(Path::new(r"\no_drive"));
assert_eq!(expanded, None);
// Expand with no PWD set for the drive
let expanded = drive_map.expand_pwd(Path::new("N:test"));
if let Some(pwd_on_drive) = get_full_path_name_w("N:") {
assert_eq!(
expanded,
Some(PathBuf::from(format!(
r"{}test",
DriveToPwdMap::ensure_trailing_delimiter(&pwd_on_drive)
)))
);
} else {
assert_eq!(expanded, Some(PathBuf::from(r"N:\test")));
}
}
#[test]
fn test_set_and_get_pwd() {
let mut drive_map = DriveToPwdMap::new();
// Set PWD for drive 'O'
assert!(drive_map.set_pwd(Path::new(r"O:\Users")).is_ok());
// Or for drive 'o'
assert!(drive_map.set_pwd(Path::new(r"o:\Users\Example")).is_ok());
// Get PWD for drive 'O'
assert_eq!(drive_map.get_pwd('O'), Ok(r"O:\Users\Example".to_string()));
// or 'o'
assert_eq!(drive_map.get_pwd('o'), Ok(r"O:\Users\Example".to_string()));
// Get PWD for drive P (not set yet, but system might already
// have PWD on this drive)
if let Some(pwd_on_drive) = get_full_path_name_w("P:") {
assert_eq!(drive_map.get_pwd('P'), Ok(pwd_on_drive));
} else {
assert_eq!(drive_map.get_pwd('P'), Ok(r"P:\".to_string()));
}
}
#[test]
fn test_set_pwd_invalid_path() {
let mut drive_map = DriveToPwdMap::new();
// Invalid path (no drive letter)
let result = drive_map.set_pwd(Path::new(r"\InvalidPath"));
assert!(result.is_err());
assert_eq!(result.unwrap_err(), PathError::InvalidPath);
}
#[test]
fn test_get_pwd_invalid_drive() {
let drive_map = DriveToPwdMap::new();
// Get PWD for a drive not set (e.g., Z)
assert_eq!(drive_map.get_pwd('Z'), Ok(r"Z:\".to_string()));
// Invalid drive letter (non-alphabetic)
assert_eq!(drive_map.get_pwd('1'), Err(PathError::InvalidDriveLetter));
}
}

View File

@ -9,7 +9,6 @@ use nu_utils::IgnoreCaseExt;
use std::{
collections::{HashMap, HashSet},
fs::File,
path::{Path, PathBuf},
sync::Arc,
};
@ -54,8 +53,6 @@ pub struct Stack {
/// Locally updated config. Use [`.get_config()`](Self::get_config) to access correctly.
pub config: Option<Arc<Config>>,
pub(crate) out_dest: StackOutDest,
#[cfg(windows)]
pub pwd_per_drive: nu_path::DriveToPwdMap,
}
impl Default for Stack {
@ -85,8 +82,6 @@ impl Stack {
parent_deletions: vec![],
config: None,
out_dest: StackOutDest::new(),
#[cfg(windows)]
pwd_per_drive: nu_path::DriveToPwdMap::new(),
}
}
@ -107,8 +102,6 @@ impl Stack {
parent_deletions: vec![],
config: parent.config.clone(),
out_dest: parent.out_dest.clone(),
#[cfg(windows)]
pwd_per_drive: parent.pwd_per_drive.clone(),
parent_stack: Some(parent),
}
}
@ -135,10 +128,6 @@ impl Stack {
unique_stack.env_hidden = child.env_hidden;
unique_stack.active_overlays = child.active_overlays;
unique_stack.config = child.config;
#[cfg(windows)]
{
unique_stack.pwd_per_drive = child.pwd_per_drive.clone();
}
unique_stack
}
@ -330,8 +319,6 @@ impl Stack {
parent_deletions: vec![],
config: self.config.clone(),
out_dest: self.out_dest.clone(),
#[cfg(windows)]
pwd_per_drive: self.pwd_per_drive.clone(),
}
}
@ -365,8 +352,6 @@ impl Stack {
parent_deletions: vec![],
config: self.config.clone(),
out_dest: self.out_dest.clone(),
#[cfg(windows)]
pwd_per_drive: self.pwd_per_drive.clone(),
}
}
@ -776,29 +761,9 @@ impl Stack {
let path = nu_path::strip_trailing_slash(path);
let value = Value::string(path.to_string_lossy(), Span::unknown());
self.add_env_var("PWD".into(), value);
// Sync with PWD-per-drive
#[cfg(windows)]
{
let _ = self.pwd_per_drive.set_pwd(&path);
}
Ok(())
}
}
// Helper stub/proxy for nu_path::expand_path_with::<P, Q>(path, relative_to, expand_tilde)
// Facilitates file system commands to easily gain the ability to expand PWD-per-drive
pub fn expand_path_with<P, Q>(&self, path: P, relative_to: Q, expand_tilde: bool) -> PathBuf
where
P: AsRef<Path>,
Q: AsRef<Path>,
{
#[cfg(windows)]
if let Some(absolute_path) = self.pwd_per_drive.expand_pwd(path.as_ref()) {
return absolute_path;
}
nu_path::expand_path_with::<P, Q>(path, relative_to, expand_tilde)
}
}
#[cfg(test)]

View File

@ -12,30 +12,6 @@ use nu_protocol::{
};
use nu_utils::perf;
#[cfg(windows)]
fn init_pwd_per_drive(engine_state: &EngineState, stack: &mut Stack) {
use nu_path::DriveToPwdMap;
use std::path::Path;
// Read environment for PWD-per-drive
for drive_letter in 'A'..='Z' {
let env_var = DriveToPwdMap::env_var_for_drive(drive_letter);
if let Some(env_pwd) = engine_state.get_env_var(&env_var) {
if let Ok(pwd_str) = nu_engine::env_to_string(&env_var, env_pwd, engine_state, stack) {
trace!("Get Env({}) {}", env_var, pwd_str);
let _ = stack.pwd_per_drive.set_pwd(Path::new(&pwd_str));
stack.remove_env_var(engine_state, &env_var);
}
}
}
if let Ok(abs_pwd) = engine_state.cwd(None) {
if let Some(abs_pwd_str) = abs_pwd.to_str() {
let _ = stack.pwd_per_drive.set_pwd(Path::new(abs_pwd_str));
}
}
}
pub(crate) fn run_commands(
engine_state: &mut EngineState,
parsed_nu_cli_args: command::NushellCliArgs,
@ -50,8 +26,6 @@ pub(crate) fn run_commands(
let create_scaffold = nu_path::nu_config_dir().map_or(false, |p| !p.exists());
let mut stack = Stack::new();
#[cfg(windows)]
init_pwd_per_drive(engine_state, &mut stack);
// if the --no-config-file(-n) option is NOT passed, load the plugin file,
// load the default env file or custom (depending on parsed_nu_cli_args.env_file),
@ -141,8 +115,6 @@ pub(crate) fn run_file(
) {
trace!("run_file");
let mut stack = Stack::new();
#[cfg(windows)]
init_pwd_per_drive(engine_state, &mut stack);
// if the --no-config-file(-n) option is NOT passed, load the plugin file,
// load the default env file or custom (depending on parsed_nu_cli_args.env_file),
@ -210,9 +182,6 @@ pub(crate) fn run_repl(
) -> Result<(), miette::ErrReport> {
trace!("run_repl");
let mut stack = Stack::new();
#[cfg(windows)]
init_pwd_per_drive(engine_state, &mut stack);
let start_time = std::time::Instant::now();
if parsed_nu_cli_args.no_config_file.is_none() {