Change environment variables to be case-preserving (#12701)

This PR changes `$env` to be **case-preserving** instead of
case-sensitive. That is, it preserves the case of the environment
variable when it is first assigned, but subsequent retrieval and update
ignores the case.

Notably, both `$env.PATH` and `$env.Path` can now be used to read or set
the environment variable, but child processes will always see the
correct case based on the platform.

Fixes #11268.

---

This feature was surprising simple to implement, because most of the
infrastructure to support case-insensitive cell path access already
exists. The `get` command extracts data using a cell path in a
case-insensitive way (!), but accepts a `--sensitive` flag. (I think
this should be flipped around?)
This commit is contained in:
YizhePKU 2024-05-02 06:22:34 +08:00 committed by GitHub
parent 21ebdfe8d7
commit 52d99cc60c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 70 additions and 26 deletions

View File

@ -11,6 +11,7 @@ use nu_protocol::{
Config, FromValue, IntoPipelineData, OutDest, PipelineData, ShellError, Span, Spanned, Type,
Value, VarId, ENV_VARIABLE_ID,
};
use nu_utils::IgnoreCaseExt;
use std::{borrow::Cow, fs::OpenOptions, path::PathBuf};
pub fn eval_call<D: DebugContext>(
@ -769,40 +770,48 @@ impl Eval for EvalRuntime {
if is_env || engine_state.get_var(*var_id).mutable {
let mut lhs =
eval_expression::<D>(engine_state, stack, &cell_path.head)?;
lhs.upsert_data_at_cell_path(&cell_path.tail, rhs)?;
if is_env {
// Reject attempts to assign to the entire $env
if cell_path.tail.is_empty() {
return Err(ShellError::CannotReplaceEnv {
span: cell_path.head.span,
});
}
// The special $env treatment: for something like $env.config.history.max_size = 2000,
// get $env.config (or whichever one it is) AFTER the above mutation, and set it
// as the "config" environment variable.
let vardata =
lhs.follow_cell_path(&[cell_path.tail[0].clone()], false)?;
match &cell_path.tail[0] {
PathMember::String { val, span, .. } => {
if val == "FILE_PWD"
|| val == "CURRENT_FILE"
|| val == "PWD"
{
// Updating environment variables should be case-preserving,
// so we need to figure out the original key before we do anything.
let (key, span) = match &cell_path.tail[0] {
PathMember::String { val, span, .. } => (val.to_string(), span),
PathMember::Int { val, span, .. } => (val.to_string(), span),
};
let original_key = if let Value::Record { val: record, .. } = &lhs {
record
.iter()
.rev()
.map(|(k, _)| k)
.find(|x| x.eq_ignore_case(&key))
.cloned()
.unwrap_or(key)
} else {
key
};
// Retrieve the updated environment value.
lhs.upsert_data_at_cell_path(&cell_path.tail, rhs)?;
let value =
lhs.follow_cell_path(&[cell_path.tail[0].clone()], true)?;
// Reject attempts to set automatic environment variables.
if is_automatic_env_var(&original_key) {
return Err(ShellError::AutomaticEnvVarSetManually {
envvar_name: val.to_string(),
envvar_name: original_key,
span: *span,
});
}
stack.add_env_var(original_key, value);
} else {
stack.add_env_var(val.to_string(), vardata);
}
}
// In case someone really wants an integer env-var
PathMember::Int { val, .. } => {
stack.add_env_var(val.to_string(), vardata);
}
}
} else {
lhs.upsert_data_at_cell_path(&cell_path.tail, rhs)?;
stack.add_var(*var_id, lhs);
}
Ok(Value::nothing(cell_path.head.span))
@ -854,3 +863,19 @@ impl Eval for EvalRuntime {
Ok(Value::nothing(expr.span))
}
}
/// Returns whether a string, when used as the name of an environment variable,
/// is considered an automatic environment variable.
///
/// An automatic environment variable cannot be assigned to by user code.
/// Current there are three of them: $env.PWD, $env.FILE_PWD, $env.CURRENT_FILE
fn is_automatic_env_var(var: &str) -> bool {
let names = ["PWD", "FILE_PWD", "CURRENT_FILE"];
names.iter().any(|&name| {
if cfg!(windows) {
name.eq_ignore_case(var)
} else {
name.eq(var)
}
})
}

View File

@ -5,6 +5,7 @@ use crate::{
},
debugger::DebugContext,
Config, IntoInterruptiblePipelineData, Range, Record, ShellError, Span, Value, VarId,
ENV_VARIABLE_ID,
};
use std::{borrow::Cow, collections::HashMap};
@ -37,8 +38,14 @@ pub trait Eval {
Expr::FullCellPath(cell_path) => {
let value = Self::eval::<D>(state, mut_state, &cell_path.head)?;
// Cell paths are usually case-sensitive, but we give $env
// special treatment.
if cell_path.head.expr == Expr::Var(ENV_VARIABLE_ID) {
value.follow_cell_path(&cell_path.tail, true)
} else {
value.follow_cell_path(&cell_path.tail, false)
}
}
Expr::DateTime(dt) => Ok(Value::date(*dt, expr.span)),
Expr::List(list) => {
let mut output = vec![];

View File

@ -194,3 +194,15 @@ fn env_var_not_var() {
");
assert!(actual.err.contains("use $env.PWD instead of $PWD"));
}
#[test]
fn env_var_case_insensitive() {
let actual = nu!("
$env.foo = 111
print $env.Foo
$env.FOO = 222
print $env.foo
");
assert!(actual.out.contains("111"));
assert!(actual.out.contains("222"));
}