diff --git a/crates/nu-cli/src/nu_highlight.rs b/crates/nu-cli/src/nu_highlight.rs index 2c4c221a41..02dc64e9cb 100644 --- a/crates/nu-cli/src/nu_highlight.rs +++ b/crates/nu-cli/src/nu_highlight.rs @@ -1,7 +1,7 @@ use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, Type, Value}; -use reedline::Highlighter; +use reedline::{Highlighter, StyledText}; #[derive(Clone)] pub struct NuHighlight; @@ -64,3 +64,16 @@ impl Command for NuHighlight { }] } } + +/// A highlighter that does nothing +/// +/// Used to remove highlighting from a reedline instance +/// (letting NuHighlighter structs be dropped) +#[derive(Default)] +pub struct NoOpHighlighter {} + +impl Highlighter for NoOpHighlighter { + fn highlight(&self, _line: &str, _cursor: usize) -> reedline::StyledText { + StyledText::new() + } +} diff --git a/crates/nu-cli/src/prompt_update.rs b/crates/nu-cli/src/prompt_update.rs index 4784c6693c..83fa75b0da 100644 --- a/crates/nu-cli/src/prompt_update.rs +++ b/crates/nu-cli/src/prompt_update.rs @@ -102,12 +102,10 @@ fn get_prompt_string( pub(crate) fn update_prompt( config: &Config, engine_state: &EngineState, - stack: &Stack, + stack: &mut Stack, nu_prompt: &mut NushellPrompt, ) { - let mut stack = stack.clone(); - - let left_prompt_string = get_prompt_string(PROMPT_COMMAND, config, engine_state, &mut stack); + let left_prompt_string = get_prompt_string(PROMPT_COMMAND, config, engine_state, stack); // Now that we have the prompt string lets ansify it. // <133 A><133 B><133 C> @@ -123,20 +121,18 @@ pub(crate) fn update_prompt( left_prompt_string }; - let right_prompt_string = - get_prompt_string(PROMPT_COMMAND_RIGHT, config, engine_state, &mut stack); + let right_prompt_string = get_prompt_string(PROMPT_COMMAND_RIGHT, config, engine_state, stack); - let prompt_indicator_string = - get_prompt_string(PROMPT_INDICATOR, config, engine_state, &mut stack); + let prompt_indicator_string = get_prompt_string(PROMPT_INDICATOR, config, engine_state, stack); let prompt_multiline_string = - get_prompt_string(PROMPT_MULTILINE_INDICATOR, config, engine_state, &mut stack); + get_prompt_string(PROMPT_MULTILINE_INDICATOR, config, engine_state, stack); let prompt_vi_insert_string = - get_prompt_string(PROMPT_INDICATOR_VI_INSERT, config, engine_state, &mut stack); + get_prompt_string(PROMPT_INDICATOR_VI_INSERT, config, engine_state, stack); let prompt_vi_normal_string = - get_prompt_string(PROMPT_INDICATOR_VI_NORMAL, config, engine_state, &mut stack); + get_prompt_string(PROMPT_INDICATOR_VI_NORMAL, config, engine_state, stack); // apply the other indicators nu_prompt.update_all_prompt_strings( diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 01df9a77b2..e1825e0cbc 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -1,5 +1,6 @@ use crate::{ completions::NuCompleter, + nu_highlight::NoOpHighlighter, prompt_update, reedline_config::{add_menus, create_keybindings, KeybindingsMode}, util::eval_source, @@ -22,8 +23,8 @@ use nu_protocol::{ }; use nu_utils::utils::perf; use reedline::{ - CursorConfig, CwdAwareHinter, EditCommand, Emacs, FileBackedHistory, HistorySessionId, - Reedline, SqliteBackedHistory, Vi, + CursorConfig, CwdAwareHinter, DefaultCompleter, EditCommand, Emacs, FileBackedHistory, + HistorySessionId, Reedline, SqliteBackedHistory, Vi, }; use std::{ collections::HashMap, @@ -32,7 +33,7 @@ use std::{ panic::{catch_unwind, AssertUnwindSafe}, path::Path, path::PathBuf, - sync::atomic::Ordering, + sync::{atomic::Ordering, Arc}, time::{Duration, Instant}, }; use sysinfo::System; @@ -47,17 +48,21 @@ const PRE_EXECUTE_MARKER: &str = "\x1b]133;C\x1b\\"; // const CMD_FINISHED_MARKER: &str = "\x1b]133;D;{}\x1b\\"; const RESET_APPLICATION_MODE: &str = "\x1b[?1l"; -/// /// The main REPL loop, including spinning up the prompt itself. -/// pub fn evaluate_repl( engine_state: &mut EngineState, - stack: &mut Stack, + stack: Stack, nushell_path: &str, prerun_command: Option>, load_std_lib: Option>, entire_start_time: Instant, ) -> Result<()> { + // throughout this code, we hold this stack uniquely. + // During the main REPL loop, we hand ownership of this value to an Arc, + // so that it may be read by various reedline plugins. During this, we + // can't modify the stack, but at the end of the loop we take back ownership + // from the Arc. This lets us avoid copying stack variables needlessly + let mut unique_stack = stack; let config = engine_state.get_config(); let use_color = config.use_ansi_coloring; @@ -69,7 +74,7 @@ pub fn evaluate_repl( let start_time = std::time::Instant::now(); // Translate environment variables from Strings to Values - if let Some(e) = convert_env_values(engine_state, stack) { + if let Some(e) = convert_env_values(engine_state, &unique_stack) { report_error_new(engine_state, &e); } perf( @@ -82,12 +87,12 @@ pub fn evaluate_repl( ); // seed env vars - stack.add_env_var( + unique_stack.add_env_var( "CMD_DURATION_MS".into(), Value::string("0823", Span::unknown()), ); - stack.add_env_var("LAST_EXIT_CODE".into(), Value::int(0, Span::unknown())); + unique_stack.add_env_var("LAST_EXIT_CODE".into(), Value::int(0, Span::unknown())); let mut line_editor = get_line_editor(engine_state, nushell_path, use_color)?; let temp_file = temp_dir().join(format!("{}.nu", uuid::Uuid::new_v4())); @@ -95,13 +100,14 @@ pub fn evaluate_repl( if let Some(s) = prerun_command { eval_source( engine_state, - stack, + &mut unique_stack, s.item.as_bytes(), &format!("entry #{entry_num}"), PipelineData::empty(), false, ); - engine_state.merge_env(stack, get_guaranteed_cwd(engine_state, stack))?; + let cwd = get_guaranteed_cwd(engine_state, &unique_stack); + engine_state.merge_env(&mut unique_stack, cwd)?; } engine_state.set_startup_time(entire_start_time.elapsed().as_nanos() as i64); @@ -113,7 +119,7 @@ pub fn evaluate_repl( if load_std_lib.is_none() && engine_state.get_config().show_banner { eval_source( engine_state, - stack, + &mut unique_stack, r#"use std banner; banner"#.as_bytes(), "show_banner", PipelineData::empty(), @@ -125,20 +131,22 @@ pub fn evaluate_repl( // Setup initial engine_state and stack state let mut previous_engine_state = engine_state.clone(); - let mut previous_stack = stack.clone(); + let mut previous_stack_arc = Arc::new(unique_stack); loop { // clone these values so that they can be moved by AssertUnwindSafe // If there is a panic within this iteration the last engine_state and stack // will be used let mut current_engine_state = previous_engine_state.clone(); - let mut current_stack = previous_stack.clone(); + // for the stack, we are going to hold to create a child stack instead, + // avoiding an expensive copy + let current_stack = Stack::with_parent(previous_stack_arc.clone()); let temp_file_cloned = temp_file.clone(); let mut nu_prompt_cloned = nu_prompt.clone(); match catch_unwind(AssertUnwindSafe(move || { - let (continue_loop, line_editor) = loop_iteration(LoopContext { + let (continue_loop, current_stack, line_editor) = loop_iteration(LoopContext { engine_state: &mut current_engine_state, - stack: &mut current_stack, + stack: current_stack, line_editor, nu_prompt: &mut nu_prompt_cloned, temp_file: &temp_file_cloned, @@ -157,7 +165,9 @@ pub fn evaluate_repl( Ok((continue_loop, es, s, le)) => { // setup state for the next iteration of the repl loop previous_engine_state = es; - previous_stack = s; + // we apply the changes from the updated stack back onto our previous stack + previous_stack_arc = + Arc::new(Stack::with_changes_from_child(previous_stack_arc, s)); line_editor = le; if !continue_loop { break; @@ -211,7 +221,7 @@ fn get_line_editor( struct LoopContext<'a> { engine_state: &'a mut EngineState, - stack: &'a mut Stack, + stack: Stack, line_editor: Reedline, nu_prompt: &'a mut NushellPrompt, temp_file: &'a Path, @@ -222,14 +232,14 @@ struct LoopContext<'a> { /// Perform one iteration of the REPL loop /// Result is bool: continue loop, current reedline #[inline] -fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { +fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { use nu_cmd_base::hook; use reedline::Signal; let loop_start_time = std::time::Instant::now(); let LoopContext { engine_state, - stack, + mut stack, line_editor, nu_prompt, temp_file, @@ -237,12 +247,12 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { entry_num, } = ctx; - let cwd = get_guaranteed_cwd(engine_state, stack); + let cwd = get_guaranteed_cwd(engine_state, &stack); let mut start_time = std::time::Instant::now(); // Before doing anything, merge the environment from the previous REPL iteration into the // permanent state. - if let Err(err) = engine_state.merge_env(stack, cwd) { + if let Err(err) = engine_state.merge_env(&mut stack, cwd) { report_error_new(engine_state, &err); } perf( @@ -289,6 +299,10 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { ); start_time = std::time::Instant::now(); + // at this line we have cloned the state for the completer and the transient prompt + // until we drop those, we cannot use the stack in the REPL loop itself + // See STACK-REFERENCE to see where we have taken a reference + let mut stack_arc = Arc::new(stack); let mut line_editor = line_editor .use_kitty_keyboard_enhancement(config.use_kitty_protocol) @@ -297,7 +311,8 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { .use_bracketed_paste(cfg!(not(target_os = "windows")) && config.bracketed_paste) .with_highlighter(Box::new(NuHighlighter { engine_state: engine_reference.clone(), - stack: std::sync::Arc::new(stack.clone()), + // STACK-REFERENCE 1 + stack: stack_arc.clone(), config: config.clone(), })) .with_validator(Box::new(NuValidator { @@ -305,7 +320,8 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { })) .with_completer(Box::new(NuCompleter::new( engine_reference.clone(), - stack.clone(), + // STACK-REFERENCE 2 + Stack::with_parent(stack_arc.clone()), ))) .with_quick_completions(config.quick_completions) .with_partial_completions(config.partial_completions) @@ -320,7 +336,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { use_color, ); - let style_computer = StyleComputer::from_config(engine_state, stack); + let style_computer = StyleComputer::from_config(engine_state, &stack_arc); start_time = std::time::Instant::now(); line_editor = if config.use_ansi_coloring { @@ -342,10 +358,11 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { ); start_time = std::time::Instant::now(); - line_editor = add_menus(line_editor, engine_reference, stack, config).unwrap_or_else(|e| { - report_error_new(engine_state, &e); - Reedline::create() - }); + line_editor = + add_menus(line_editor, engine_reference, &stack_arc, config).unwrap_or_else(|e| { + report_error_new(engine_state, &e); + Reedline::create() + }); perf( "reedline menus", start_time, @@ -356,11 +373,11 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { ); start_time = std::time::Instant::now(); - let buffer_editor = get_editor(engine_state, stack, Span::unknown()); + let buffer_editor = get_editor(engine_state, &stack_arc, Span::unknown()); line_editor = if let Ok((cmd, args)) = buffer_editor { let mut command = std::process::Command::new(cmd); - let envs = env_to_strings(engine_state, stack).unwrap_or_else(|e| { + let envs = env_to_strings(engine_state, &stack_arc).unwrap_or_else(|e| { warn!("Couldn't convert environment variable values to strings: {e}"); HashMap::default() }); @@ -411,7 +428,14 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { // Right before we start our prompt and take input from the user, // fire the "pre_prompt" hook if let Some(hook) = config.hooks.pre_prompt.clone() { - if let Err(err) = eval_hook(engine_state, stack, None, vec![], &hook, "pre_prompt") { + if let Err(err) = eval_hook( + engine_state, + &mut Stack::with_parent(stack_arc.clone()), + None, + vec![], + &hook, + "pre_prompt", + ) { report_error_new(engine_state, &err); } } @@ -428,9 +452,11 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { // Next, check all the environment variables they ask for // fire the "env_change" hook let config = engine_state.get_config(); - if let Err(error) = - hook::eval_env_change_hook(config.hooks.env_change.clone(), engine_state, stack) - { + if let Err(error) = hook::eval_env_change_hook( + config.hooks.env_change.clone(), + engine_state, + &mut Stack::with_parent(stack_arc.clone()), + ) { report_error_new(engine_state, &error) } perf( @@ -444,9 +470,18 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { start_time = std::time::Instant::now(); let config = &engine_state.get_config().clone(); - prompt_update::update_prompt(config, engine_state, stack, nu_prompt); - let transient_prompt = - prompt_update::make_transient_prompt(config, engine_state, stack, nu_prompt); + prompt_update::update_prompt( + config, + engine_state, + &mut Stack::with_parent(stack_arc.clone()), + nu_prompt, + ); + let transient_prompt = prompt_update::make_transient_prompt( + config, + engine_state, + &mut Stack::with_parent(stack_arc.clone()), + nu_prompt, + ); perf( "update_prompt", start_time, @@ -461,6 +496,13 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { start_time = std::time::Instant::now(); line_editor = line_editor.with_transient_prompt(transient_prompt); let input = line_editor.read_line(nu_prompt); + // we got our inputs, we can now drop our stack references + // This lists all of the stack references that we have cleaned up + line_editor = line_editor + // CLEAR STACK-REFERENCE 1 + .with_highlighter(Box::::default()) + // CLEAR STACK-REFERENCE 2 + .with_completer(Box::::default()); let shell_integration = config.shell_integration; match input { @@ -483,9 +525,14 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { repl.buffer = s.to_string(); drop(repl); - if let Err(err) = - eval_hook(engine_state, stack, None, vec![], &hook, "pre_execution") - { + if let Err(err) = eval_hook( + engine_state, + &mut Stack::with_parent(stack_arc.clone()), + None, + vec![], + &hook, + "pre_execution", + ) { report_error_new(engine_state, &err); } } @@ -502,15 +549,17 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { // Actual command execution logic starts from here let start_time = Instant::now(); - match parse_operation(s.clone(), engine_state, stack) { + let mut stack = Stack::unwrap_unique(stack_arc); + + match parse_operation(s.clone(), engine_state, &stack) { Ok(operation) => match operation { ReplOperation::AutoCd { cwd, target, span } => { - do_auto_cd(target, cwd, stack, engine_state, span); + do_auto_cd(target, cwd, &mut stack, engine_state, span); } ReplOperation::RunCommand(cmd) => { line_editor = do_run_cmd( &cmd, - stack, + &mut stack, engine_state, line_editor, shell_integration, @@ -522,7 +571,6 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { }, Err(ref e) => error!("Error parsing operation: {e}"), } - let cmd_duration = start_time.elapsed(); stack.add_env_var( @@ -535,7 +583,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { &s, engine_state, cmd_duration, - stack, + &mut stack, &mut line_editor, ) { warn!("Could not fill in result related history metadata: {e}"); @@ -543,24 +591,26 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { } if shell_integration { - do_shell_integration_finalize_command(hostname, engine_state, stack); + do_shell_integration_finalize_command(hostname, engine_state, &mut stack); } flush_engine_state_repl_buffer(engine_state, &mut line_editor); + // put the stack back into the arc + stack_arc = Arc::new(stack); } Ok(Signal::CtrlC) => { // `Reedline` clears the line content. New prompt is shown if shell_integration { - run_ansi_sequence(&get_command_finished_marker(stack, engine_state)); + run_ansi_sequence(&get_command_finished_marker(&stack_arc, engine_state)); } } Ok(Signal::CtrlD) => { // When exiting clear to a new line if shell_integration { - run_ansi_sequence(&get_command_finished_marker(stack, engine_state)); + run_ansi_sequence(&get_command_finished_marker(&stack_arc, engine_state)); } println!(); - return (false, line_editor); + return (false, Stack::unwrap_unique(stack_arc), line_editor); } Err(err) => { let message = err.to_string(); @@ -572,7 +622,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { // Alternatively only allow that expected failures let the REPL loop } if shell_integration { - run_ansi_sequence(&get_command_finished_marker(stack, engine_state)); + run_ansi_sequence(&get_command_finished_marker(&stack_arc, engine_state)); } } } @@ -594,7 +644,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Reedline) { use_color, ); - (true, line_editor) + (true, Stack::unwrap_unique(stack_arc), line_editor) } /// diff --git a/crates/nu-cmd-base/src/util.rs b/crates/nu-cmd-base/src/util.rs index 120b6feae5..da1e6dbfb7 100644 --- a/crates/nu-cmd-base/src/util.rs +++ b/crates/nu-cmd-base/src/util.rs @@ -99,7 +99,7 @@ fn get_editor_commandline( pub fn get_editor( engine_state: &EngineState, - stack: &mut Stack, + stack: &Stack, span: Span, ) -> Result<(String, Vec), ShellError> { let config = engine_state.get_config(); diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index da0f5bfc2f..4c336d55d1 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -1,4 +1,5 @@ use std::collections::{HashMap, HashSet}; +use std::sync::Arc; use crate::engine::EngineState; use crate::engine::DEFAULT_OVERLAY_NAME; @@ -36,6 +37,10 @@ pub struct Stack { /// List of active overlays pub active_overlays: Vec, pub recursion_count: u64, + + pub parent_stack: Option>, + /// Variables that have been deleted (this is used to hide values from parent stack lookups) + pub parent_deletions: Vec, } impl Stack { @@ -46,9 +51,66 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], recursion_count: 0, + parent_stack: None, + parent_deletions: vec![], } } + /// Unwrap a uniquely-owned stack. + /// + /// In debug mode, this panics if there are multiple references. + /// In production this will instead clone the underlying stack. + pub fn unwrap_unique(stack_arc: Arc) -> Stack { + // If you hit an error here, it's likely that you created an extra + // Arc pointing to the stack somewhere. Make sure that it gets dropped before + // getting here! + Arc::try_unwrap(stack_arc).unwrap_or_else(|arc| { + // in release mode, we clone the stack, but this can lead to + // major performance issues, so we should avoid it + debug_assert!(false, "More than one stack reference remaining!"); + (*arc).clone() + }) + } + /// Create a new child stack from a parent. + /// + /// Changes from this child can be merged back into the parent with + /// Stack::with_changes_from_child + pub fn with_parent(parent: Arc) -> Stack { + Stack { + // here we are still cloning environment variable-related information + env_vars: parent.env_vars.clone(), + env_hidden: parent.env_hidden.clone(), + active_overlays: parent.active_overlays.clone(), + recursion_count: parent.recursion_count, + parent_stack: Some(parent), + vars: vec![], + parent_deletions: vec![], + } + } + + /// Take an Arc of a parent (assumed to be unique), and a child, and apply + /// all the changes from a child back to the parent. + /// + /// Here it is assumed that child was created with a call to Stack::with_parent + /// with parent + pub fn with_changes_from_child(parent: Arc, mut child: Stack) -> Stack { + // we're going to drop the link to the parent stack on our new stack + // so that we can unwrap the Arc as a unique reference + // + // This makes the new_stack be in a bit of a weird state, so we shouldn't call + // any structs + drop(child.parent_stack); + let mut unique_stack = Stack::unwrap_unique(parent); + + unique_stack.vars.append(&mut child.vars); + unique_stack.env_vars = child.env_vars; + unique_stack.env_hidden = child.env_hidden; + unique_stack.active_overlays = child.active_overlays; + for item in child.parent_deletions.into_iter() { + unique_stack.vars.remove(item); + } + unique_stack + } pub fn with_env( &mut self, env_vars: &[EnvVars], @@ -64,34 +126,52 @@ impl Stack { } } - pub fn get_var(&self, var_id: VarId, span: Span) -> Result { + /// Lookup a variable, returning None if it is not present + fn lookup_var(&self, var_id: VarId) -> Option { for (id, val) in &self.vars { if var_id == *id { - return Ok(val.clone().with_span(span)); + return Some(val.clone()); } } - Err(ShellError::VariableNotFoundAtRuntime { span }) + if let Some(stack) = &self.parent_stack { + if !self.parent_deletions.contains(&var_id) { + return stack.lookup_var(var_id); + } + } + None } + /// Lookup a variable, erroring if it is not found + /// + /// The passed-in span will be used to tag the value + pub fn get_var(&self, var_id: VarId, span: Span) -> Result { + match self.lookup_var(var_id) { + Some(v) => Ok(v.with_span(span)), + None => Err(ShellError::VariableNotFoundAtRuntime { span }), + } + } + + /// Lookup a variable, erroring if it is not found + /// + /// While the passed-in span will be used for errors, the returned value + /// has the span from where it was originally defined pub fn get_var_with_origin(&self, var_id: VarId, span: Span) -> Result { - for (id, val) in &self.vars { - if var_id == *id { - return Ok(val.clone()); + match self.lookup_var(var_id) { + Some(v) => Ok(v), + None => { + if var_id == NU_VARIABLE_ID || var_id == ENV_VARIABLE_ID { + return Err(ShellError::GenericError { + error: "Built-in variables `$env` and `$nu` have no metadata".into(), + msg: "no metadata available".into(), + span: Some(span), + help: None, + inner: vec![], + }); + } + Err(ShellError::VariableNotFoundAtRuntime { span }) } } - - if var_id == NU_VARIABLE_ID || var_id == ENV_VARIABLE_ID { - return Err(ShellError::GenericError { - error: "Built-in variables `$env` and `$nu` have no metadata".into(), - msg: "no metadata available".into(), - span: Some(span), - help: None, - inner: vec![], - }); - } - - Err(ShellError::VariableNotFoundAtRuntime { span }) } pub fn add_var(&mut self, var_id: VarId, value: Value) { @@ -109,9 +189,14 @@ impl Stack { for (idx, (id, _)) in self.vars.iter().enumerate() { if *id == var_id { self.vars.remove(idx); - return; + break; } } + // even if we did have it in the original layer, we need to make sure to remove it here + // as well (since the previous update might have simply hid the parent value) + if self.parent_stack.is_some() { + self.parent_deletions.push(var_id); + } } pub fn add_env_var(&mut self, var: String, value: Value) { @@ -160,6 +245,8 @@ impl Stack { env_hidden: self.env_hidden.clone(), active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count, + parent_stack: None, + parent_deletions: vec![], } } @@ -187,6 +274,8 @@ impl Stack { env_hidden: self.env_hidden.clone(), active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count, + parent_stack: None, + parent_deletions: vec![], } } @@ -308,7 +397,6 @@ impl Stack { } } } - None } @@ -400,3 +488,107 @@ impl Default for Stack { Self::new() } } + +#[cfg(test)] +mod test { + use std::sync::Arc; + + use crate::{engine::EngineState, Span, Value}; + + use super::Stack; + + const ZERO_SPAN: Span = Span { start: 0, end: 0 }; + fn string_value(s: &str) -> Value { + Value::String { + val: s.to_string(), + internal_span: ZERO_SPAN, + } + } + + #[test] + fn test_children_see_inner_values() { + let mut original = Stack::new(); + original.add_var(0, string_value("hello")); + + let cloned = Stack::with_parent(Arc::new(original)); + assert_eq!(cloned.get_var(0, ZERO_SPAN), Ok(string_value("hello"))); + } + + #[test] + fn test_children_dont_see_deleted_values() { + let mut original = Stack::new(); + original.add_var(0, string_value("hello")); + + let mut cloned = Stack::with_parent(Arc::new(original)); + cloned.remove_var(0); + + assert_eq!( + cloned.get_var(0, ZERO_SPAN), + Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) + ); + } + + #[test] + fn test_children_changes_override_parent() { + let mut original = Stack::new(); + original.add_var(0, string_value("hello")); + + let mut cloned = Stack::with_parent(Arc::new(original)); + cloned.add_var(0, string_value("there")); + assert_eq!(cloned.get_var(0, ZERO_SPAN), Ok(string_value("there"))); + + cloned.remove_var(0); + // the underlying value shouldn't magically re-appear + assert_eq!( + cloned.get_var(0, ZERO_SPAN), + Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) + ); + } + #[test] + fn test_children_changes_persist_in_offspring() { + let mut original = Stack::new(); + original.add_var(0, string_value("hello")); + + let mut cloned = Stack::with_parent(Arc::new(original)); + cloned.add_var(1, string_value("there")); + + cloned.remove_var(0); + let cloned = Stack::with_parent(Arc::new(cloned)); + + assert_eq!( + cloned.get_var(0, ZERO_SPAN), + Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) + ); + + assert_eq!(cloned.get_var(1, ZERO_SPAN), Ok(string_value("there"))); + } + + #[test] + fn test_merging_children_back_to_parent() { + let mut original = Stack::new(); + let engine_state = EngineState::new(); + original.add_var(0, string_value("hello")); + + let original_arc = Arc::new(original); + let mut cloned = Stack::with_parent(original_arc.clone()); + cloned.add_var(1, string_value("there")); + + cloned.remove_var(0); + + cloned.add_env_var("ADDED_IN_CHILD".to_string(), string_value("New Env Var")); + + let original = Stack::with_changes_from_child(original_arc, cloned); + + assert_eq!( + original.get_var(0, ZERO_SPAN), + Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) + ); + + assert_eq!(original.get_var(1, ZERO_SPAN), Ok(string_value("there"))); + + assert_eq!( + original.get_env_var(&engine_state, "ADDED_IN_CHILD"), + Some(string_value("New Env Var")), + ); + } +} diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index c94613ecd2..1cc87a27cd 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -9,7 +9,7 @@ use crate::{ /// The fundamental error type for the evaluation engine. These cases represent different kinds of errors /// the evaluator might face, along with helpful spans to label. An error renderer will take this error value /// and pass it into an error viewer to display to the user. -#[derive(Debug, Clone, Error, Diagnostic, Serialize, Deserialize)] +#[derive(Debug, Clone, Error, Diagnostic, Serialize, Deserialize, PartialEq)] pub enum ShellError { /// An operator received two arguments of incompatible types. /// diff --git a/src/run.rs b/src/run.rs index 58986ba89b..1329cf64d0 100644 --- a/src/run.rs +++ b/src/run.rs @@ -272,7 +272,7 @@ pub(crate) fn run_repl( let start_time = std::time::Instant::now(); let ret_val = evaluate_repl( engine_state, - &mut stack, + stack, config_files::NUSHELL_FOLDER, parsed_nu_cli_args.execute, parsed_nu_cli_args.no_std_lib,