Update config directly at assignment (#13332)

# Description

Allows `Stack` to have a modified local `Config`, which is updated
immediately when `$env.config` is assigned to. This means that even
within a script, commands that come after `$env.config` changes will
always see those changes in `Stack::get_config()`.

Also fixed a lot of cases where `engine_state.get_config()` was used
even when `Stack` was available.

Closes #13324.

# User-Facing Changes
- Config changes apply immediately after the assignment is executed,
rather than whenever config is read by a command that needs it.
- Potentially slower performance when executing a lot of lines that
change `$env.config` one after another. Recommended to get `$env.config`
into a `mut` variable first and do modifications, then assign it back.
- Much faster performance when executing a script that made
modifications to `$env.config`, as the changes are only parsed once.

# Tests + Formatting
All passing.

# After Submitting
- [ ] release notes
This commit is contained in:
Devyn Cairns
2024-07-11 06:09:33 -07:00
committed by GitHub
parent deaa711ca6
commit f65bc97a54
46 changed files with 327 additions and 222 deletions

View File

@ -301,28 +301,11 @@ impl EngineState {
stack: &mut Stack,
cwd: impl AsRef<Path>,
) -> Result<(), ShellError> {
let mut config_updated = false;
for mut scope in stack.env_vars.drain(..) {
for (overlay_name, mut env) in Arc::make_mut(&mut scope).drain() {
if let Some(env_vars) = Arc::make_mut(&mut self.env_vars).get_mut(&overlay_name) {
// Updating existing overlay
for (k, v) in env.drain() {
if k == "config" {
// Don't insert the record as the "config" env var as-is.
// Instead, mutate a clone of it with into_config(), and put THAT in env_vars.
let mut new_record = v.clone();
let (config, error) = new_record.parse_as_config(&self.config);
self.config = Arc::new(config);
config_updated = true;
env_vars.insert(k, new_record);
if let Some(e) = error {
return Err(e);
}
} else {
env_vars.insert(k, v);
}
}
env_vars.extend(env.drain());
} else {
// Pushing a new overlay
Arc::make_mut(&mut self.env_vars).insert(overlay_name, env);
@ -333,7 +316,10 @@ impl EngineState {
// TODO: better error
std::env::set_current_dir(cwd)?;
if config_updated {
if let Some(config) = stack.config.take() {
// If config was updated in the stack, replace it.
self.config = config;
// Make plugin GC config changes take effect immediately.
#[cfg(feature = "plugin")]
self.update_plugin_gc_configs(&self.config.plugin_gc);
@ -738,18 +724,24 @@ impl EngineState {
&[0u8; 0]
}
pub fn get_config(&self) -> &Config {
/// Get the global config from the engine state.
///
/// Use [`Stack::get_config()`] instead whenever the `Stack` is available, as it takes into
/// account local changes to `$env.config`.
pub fn get_config(&self) -> &Arc<Config> {
&self.config
}
pub fn set_config(&mut self, conf: Config) {
pub fn set_config(&mut self, conf: impl Into<Arc<Config>>) {
let conf = conf.into();
#[cfg(feature = "plugin")]
if conf.plugin_gc != self.config.plugin_gc {
// Make plugin GC config changes take effect immediately.
self.update_plugin_gc_configs(&conf.plugin_gc);
}
self.config = Arc::new(conf);
self.config = conf;
}
/// Fetch the configuration for a plugin
@ -1137,7 +1129,7 @@ mod engine_state_tests {
let mut plugins = HashMap::new();
plugins.insert("example".into(), Value::string("value", Span::test_data()));
let mut config = engine_state.get_config().clone();
let mut config = Config::clone(engine_state.get_config());
config.plugins = plugins;
engine_state.set_config(config);

View File

@ -3,7 +3,7 @@ use crate::{
ArgumentStack, EngineState, ErrorHandlerStack, Redirection, StackCallArgGuard,
StackCaptureGuard, StackIoGuard, StackOutDest, DEFAULT_OVERLAY_NAME,
},
OutDest, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, NU_VARIABLE_ID,
Config, OutDest, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, NU_VARIABLE_ID,
};
use std::{
collections::{HashMap, HashSet},
@ -51,6 +51,8 @@ pub struct Stack {
pub parent_stack: Option<Arc<Stack>>,
/// Variables that have been deleted (this is used to hide values from parent stack lookups)
pub parent_deletions: Vec<VarId>,
/// Locally updated config. Use [`.get_config()`] to access correctly.
pub config: Option<Arc<Config>>,
pub(crate) out_dest: StackOutDest,
}
@ -80,6 +82,7 @@ impl Stack {
recursion_count: 0,
parent_stack: None,
parent_deletions: vec![],
config: None,
out_dest: StackOutDest::new(),
}
}
@ -100,6 +103,7 @@ impl Stack {
recursion_count: parent.recursion_count,
vars: vec![],
parent_deletions: vec![],
config: parent.config.clone(),
out_dest: parent.out_dest.clone(),
parent_stack: Some(parent),
}
@ -126,6 +130,7 @@ impl Stack {
unique_stack.env_vars = child.env_vars;
unique_stack.env_hidden = child.env_hidden;
unique_stack.active_overlays = child.active_overlays;
unique_stack.config = child.config;
unique_stack
}
@ -192,6 +197,36 @@ impl Stack {
}
}
/// Get the local config if set, otherwise the config from the engine state.
///
/// This is the canonical way to get [`Config`] when [`Stack`] is available.
pub fn get_config(&self, engine_state: &EngineState) -> Arc<Config> {
self.config
.clone()
.unwrap_or_else(|| engine_state.config.clone())
}
/// Update the local config with the config stored in the `config` environment variable. Run
/// this after assigning to `$env.config`.
///
/// The config will be updated with successfully parsed values even if an error occurs.
pub fn update_config(&mut self, engine_state: &EngineState) -> Result<(), ShellError> {
if let Some(mut config) = self.get_env_var(engine_state, "config") {
let existing_config = self.get_config(engine_state);
let (new_config, error) = config.parse_as_config(&existing_config);
self.config = Some(new_config.into());
// The config value is modified by the update, so we should add it again
self.add_env_var("config".into(), config);
match error {
None => Ok(()),
Some(err) => Err(err),
}
} else {
self.config = None;
Ok(())
}
}
pub fn add_var(&mut self, var_id: VarId, value: Value) {
//self.vars.insert(var_id, value);
for (id, val) in &mut self.vars {
@ -272,6 +307,7 @@ impl Stack {
recursion_count: self.recursion_count,
parent_stack: None,
parent_deletions: vec![],
config: self.config.clone(),
out_dest: self.out_dest.clone(),
}
}
@ -305,6 +341,7 @@ impl Stack {
recursion_count: self.recursion_count,
parent_stack: None,
parent_deletions: vec![],
config: self.config.clone(),
out_dest: self.out_dest.clone(),
}
}

View File

@ -627,9 +627,9 @@ impl<'a> StateWorkingSet<'a> {
/// Returns a reference to the config stored at permanent state
///
/// At runtime, you most likely want to call nu_engine::env::get_config because this method
/// does not capture environment updates during runtime.
pub fn get_config(&self) -> &Config {
/// At runtime, you most likely want to call [`Stack::get_config()`][super::Stack::get_config()]
/// because this method does not capture environment updates during runtime.
pub fn get_config(&self) -> &Arc<Config> {
&self.permanent_state.config
}

View File

@ -7,7 +7,7 @@ use crate::{
debugger::DebugContext,
Config, GetSpan, Range, Record, ShellError, Span, Value, VarId, ENV_VARIABLE_ID,
};
use std::{borrow::Cow, collections::HashMap};
use std::{collections::HashMap, sync::Arc};
/// To share implementations for regular eval and const eval
pub trait Eval {
@ -316,7 +316,7 @@ pub trait Eval {
}
}
fn get_config<'a>(state: Self::State<'a>, mut_state: &mut Self::MutState) -> Cow<'a, Config>;
fn get_config(state: Self::State<'_>, mut_state: &mut Self::MutState) -> Arc<Config>;
fn eval_filepath(
state: Self::State<'_>,

View File

@ -11,8 +11,8 @@ use crate::{
};
use nu_system::os_info::{get_kernel_version, get_os_arch, get_os_family, get_os_name};
use std::{
borrow::Cow,
path::{Path, PathBuf},
sync::Arc,
};
/// Create a Value for `$nu`.
@ -364,8 +364,8 @@ impl Eval for EvalConst {
type MutState = ();
fn get_config<'a>(state: Self::State<'a>, _: &mut ()) -> Cow<'a, Config> {
Cow::Borrowed(state.get_config())
fn get_config(state: Self::State<'_>, _: &mut ()) -> Arc<Config> {
state.get_config().clone()
}
fn eval_filepath(