diff --git a/crates/nu-command/src/core_commands/overlay/list.rs b/crates/nu-command/src/core_commands/overlay/list.rs index 6464efdfbf..883cec5774 100644 --- a/crates/nu-command/src/core_commands/overlay/list.rs +++ b/crates/nu-command/src/core_commands/overlay/list.rs @@ -4,8 +4,6 @@ use nu_protocol::{ Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Value, }; -use log::trace; - #[derive(Clone)] pub struct OverlayList; @@ -28,41 +26,17 @@ impl Command for OverlayList { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, stack: &mut Stack, call: &Call, _input: PipelineData, ) -> Result { - let active_overlays_parser: Vec = engine_state - .active_overlay_names(&[]) - .iter() - .map(|s| Value::string(String::from_utf8_lossy(s), call.head)) - .collect(); - let active_overlays_engine: Vec = stack .active_overlays .iter() .map(|s| Value::string(s, call.head)) .collect(); - // Check if the overlays in the engine match the overlays in the parser - if (active_overlays_parser.len() != active_overlays_engine.len()) - || active_overlays_parser - .iter() - .zip(active_overlays_engine.iter()) - .any(|(op, oe)| op != oe) - { - trace!("parser overlays: {:?}", active_overlays_parser); - trace!("engine overlays: {:?}", active_overlays_engine); - - return Err(ShellError::NushellFailedSpannedHelp( - "Overlay mismatch".into(), - "Active overlays do not match between the engine and the parser.".into(), - call.head, - "Run Nushell with --log-level=trace to see what went wrong.".into(), - )); - } - Ok(Value::List { vals: active_overlays_engine, span: call.head, diff --git a/crates/nu-command/src/core_commands/overlay/use_.rs b/crates/nu-command/src/core_commands/overlay/use_.rs index e8f617b58f..4793cd0195 100644 --- a/crates/nu-command/src/core_commands/overlay/use_.rs +++ b/crates/nu-command/src/core_commands/overlay/use_.rs @@ -1,5 +1,5 @@ use nu_engine::{eval_block, find_in_dirs_env, redirect_env, CallExt}; -use nu_protocol::ast::Call; +use nu_protocol::ast::{Call, Expr}; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ Category, Example, PipelineData, ShellError, Signature, Spanned, SyntaxShape, Value, @@ -57,12 +57,29 @@ impl Command for OverlayUse { ) -> Result { let name_arg: Spanned = call.req(engine_state, caller_stack, 0)?; - let (overlay_name, overlay_name_span) = if let Some(kw_expression) = call.positional_nth(1) - { + let origin_module_id = if let Some(overlay_expr) = call.positional_nth(0) { + if let Expr::Overlay(module_id) = overlay_expr.expr { + module_id + } else { + return Err(ShellError::NushellFailedSpanned( + "Not an overlay".to_string(), + "requires an overlay (path or a string)".to_string(), + overlay_expr.span, + )); + } + } else { + return Err(ShellError::NushellFailedSpanned( + "Missing positional".to_string(), + "missing required overlay".to_string(), + call.head, + )); + }; + + let overlay_name = if let Some(kw_expression) = call.positional_nth(1) { // If renamed via the 'as' keyword, use the new name as the overlay name if let Some(new_name_expression) = kw_expression.as_keyword() { if let Some(new_name) = new_name_expression.as_string() { - (new_name, new_name_expression.span) + new_name } else { return Err(ShellError::NushellFailedSpanned( "Wrong keyword type".to_string(), @@ -81,10 +98,10 @@ impl Command for OverlayUse { .find_overlay(name_arg.item.as_bytes()) .is_some() { - (name_arg.item.clone(), name_arg.span) + name_arg.item.clone() } else if let Some(os_str) = Path::new(&name_arg.item).file_stem() { if let Some(name) = os_str.to_str() { - (name.to_string(), name_arg.span) + name.to_string() } else { return Err(ShellError::NonUtf8(name_arg.span)); } @@ -95,87 +112,73 @@ impl Command for OverlayUse { )); }; - if let Some(overlay_id) = engine_state.find_overlay(overlay_name.as_bytes()) { - let old_module_id = engine_state.get_overlay(overlay_id).origin; + caller_stack.add_overlay(overlay_name); - caller_stack.add_overlay(overlay_name.clone()); + if let Some(module_id) = origin_module_id { + // Add environment variables only if: + // a) adding a new overlay + // b) refreshing an active overlay (the origin module changed) + let module = engine_state.get_module(module_id); - if let Some(new_module_id) = engine_state.find_module(overlay_name.as_bytes(), &[]) { - if !caller_stack.has_env_overlay(&overlay_name, engine_state) - || (old_module_id != new_module_id) - { - // Add environment variables only if: - // a) adding a new overlay - // b) refreshing an active overlay (the origin module changed) - let module = engine_state.get_module(new_module_id); + for (name, block_id) in module.env_vars() { + let name = if let Ok(s) = String::from_utf8(name.clone()) { + s + } else { + return Err(ShellError::NonUtf8(call.head)); + }; - for (name, block_id) in module.env_vars() { - let name = if let Ok(s) = String::from_utf8(name.clone()) { - s - } else { - return Err(ShellError::NonUtf8(call.head)); - }; + let block = engine_state.get_block(block_id); - let block = engine_state.get_block(block_id); + let val = eval_block( + engine_state, + caller_stack, + block, + PipelineData::new(call.head), + false, + true, + )? + .into_value(call.head); - let val = eval_block( - engine_state, - caller_stack, - block, - PipelineData::new(call.head), - false, - true, - )? - .into_value(call.head); + caller_stack.add_env_var(name, val); + } - caller_stack.add_env_var(name, val); - } + // Evaluate the export-env block (if any) and keep its environment + if let Some(block_id) = module.env_block { + let maybe_path = find_in_dirs_env(&name_arg.item, engine_state, caller_stack)?; - // Evaluate the export-env block (if any) and keep its environment - if let Some(block_id) = module.env_block { - let maybe_path = - find_in_dirs_env(&name_arg.item, engine_state, caller_stack)?; + if let Some(path) = &maybe_path { + // Set the currently evaluated directory, if the argument is a valid path + let mut parent = path.clone(); + parent.pop(); - if let Some(path) = &maybe_path { - // Set the currently evaluated directory, if the argument is a valid path - let mut parent = path.clone(); - parent.pop(); + let file_pwd = Value::String { + val: parent.to_string_lossy().to_string(), + span: call.head, + }; - let file_pwd = Value::String { - val: parent.to_string_lossy().to_string(), - span: call.head, - }; + caller_stack.add_env_var("FILE_PWD".to_string(), file_pwd); + } - caller_stack.add_env_var("FILE_PWD".to_string(), file_pwd); - } + let block = engine_state.get_block(block_id); + let mut callee_stack = caller_stack.gather_captures(&block.captures); - let block = engine_state.get_block(block_id); - let mut callee_stack = caller_stack.gather_captures(&block.captures); + let _ = eval_block( + engine_state, + &mut callee_stack, + block, + input, + call.redirect_stdout, + call.redirect_stderr, + ); - let _ = eval_block( - engine_state, - &mut callee_stack, - block, - input, - call.redirect_stdout, - call.redirect_stderr, - ); + // Merge the block's environment to the current stack + redirect_env(engine_state, caller_stack, &callee_stack); - // Merge the block's environment to the current stack - redirect_env(engine_state, caller_stack, &callee_stack); - - if maybe_path.is_some() { - // Remove the file-relative PWD, if the argument is a valid path - caller_stack.remove_env_var(engine_state, "FILE_PWD"); - } - } + if maybe_path.is_some() { + // Remove the file-relative PWD, if the argument is a valid path + caller_stack.remove_env_var(engine_state, "FILE_PWD"); } } - } else { - return Err(ShellError::OverlayNotFoundAtRuntime( - overlay_name, - overlay_name_span, - )); } Ok(PipelineData::new(call.head)) diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 1f26de1366..5c64603c55 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -265,6 +265,12 @@ fn convert_to_value( "imports not supported in nuon".into(), expr.span, )), + Expr::Overlay(..) => Err(ShellError::OutsideSpannedLabeledError( + original_text.to_string(), + "Error when loading".into(), + "overlays not supported in nuon".into(), + expr.span, + )), Expr::Int(val) => Ok(Value::Int { val, span }), Expr::Keyword(kw, ..) => Err(ShellError::OutsideSpannedLabeledError( original_text.to_string(), diff --git a/crates/nu-command/tests/commands/source_env.rs b/crates/nu-command/tests/commands/source_env.rs index 571a47e045..9b6ca50d0a 100644 --- a/crates/nu-command/tests/commands/source_env.rs +++ b/crates/nu-command/tests/commands/source_env.rs @@ -141,3 +141,17 @@ fn sources_unicode_file_in_unicode_dir_with_spaces_2() { fn sources_unicode_file_in_non_utf8_dir() { // How do I create non-UTF-8 path??? } + +#[test] +fn can_source_dynamic_path() { + Playground::setup("can_source_dynamic_path", |dirs, sandbox| { + let foo_file = "foo.nu"; + + sandbox.with_files(vec![FileWithContent(&foo_file, "echo foo")]); + + let cmd = format!("let file = `{}`; source-env $file", foo_file); + let actual = nu!(cwd: dirs.test(), &cmd); + + assert_eq!(actual.out, "foo"); + }); +} diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index f3e9b2c14f..c53da6af3f 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -355,6 +355,15 @@ pub fn eval_expression( value.follow_cell_path(&cell_path.tail, false) } Expr::ImportPattern(_) => Ok(Value::Nothing { span: expr.span }), + Expr::Overlay(_) => { + let name = + String::from_utf8_lossy(engine_state.get_span_contents(&expr.span)).to_string(); + + Ok(Value::String { + val: name, + span: expr.span, + }) + } Expr::Call(call) => { // FIXME: protect this collect with ctrl-c Ok( diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index bdc8692e47..dddc789828 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -260,6 +260,9 @@ pub fn flatten_expression( output } + Expr::Overlay(_) => { + vec![(expr.span, FlatShape::String)] + } Expr::Range(from, next, to, op) => { let mut output = vec![]; if let Some(f) = from { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 50ee95fd1f..a84c2bc67f 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -2422,7 +2422,7 @@ pub fn parse_overlay_use( let has_prefix = call.has_flag("prefix"); let pipeline = Pipeline::from_vec(vec![Expression { - expr: Expr::Call(call), + expr: Expr::Call(call.clone()), span: span(spans), ty: Type::Any, custom_completion: None, @@ -2432,7 +2432,14 @@ pub fn parse_overlay_use( let mut error = None; - let result = if let Some(overlay_frame) = working_set.find_overlay(overlay_name.as_bytes()) { + let (final_overlay_name, origin_module, origin_module_id, is_module_updated) = if let Some( + overlay_frame, + ) = + working_set.find_overlay(overlay_name.as_bytes()) + { + // Activate existing overlay + + // First, check for errors if has_prefix && !overlay_frame.prefixed { return ( pipeline, @@ -2467,22 +2474,22 @@ pub fn parse_overlay_use( } } - // Activate existing overlay let module_id = overlay_frame.origin; if let Some(new_module_id) = working_set.find_module(overlay_name.as_bytes()) { if module_id == new_module_id { - Some((overlay_name, Module::new(), module_id)) + (overlay_name, Module::new(), module_id, false) } else { // The origin module of an overlay changed => update it - Some(( + ( overlay_name, working_set.get_module(new_module_id).clone(), new_module_id, - )) + true, + ) } } else { - Some((overlay_name, Module::new(), module_id)) + (overlay_name, Module::new(), module_id, true) } } else { // Create a new overlay from a module @@ -2490,11 +2497,12 @@ pub fn parse_overlay_use( // the name is a module working_set.find_module(overlay_name.as_bytes()) { - Some(( + ( new_name.map(|spanned| spanned.item).unwrap_or(overlay_name), working_set.get_module(module_id).clone(), module_id, - )) + true, + ) } else { // try if the name is a file if let Ok(module_filename) = @@ -2541,11 +2549,12 @@ pub fn parse_overlay_use( let _ = working_set.add_block(block); let module_id = working_set.add_module(&overlay_name, module.clone()); - Some(( + ( new_name.map(|spanned| spanned.item).unwrap_or(overlay_name), module, module_id, - )) + true, + ) } else { return ( pipeline, @@ -2553,8 +2562,10 @@ pub fn parse_overlay_use( ); } } else { - error = error.or(Some(ParseError::ModuleOrOverlayNotFound(overlay_name_span))); - None + return ( + pipeline, + Some(ParseError::ModuleOrOverlayNotFound(overlay_name_span)), + ); } } else { return (garbage_pipeline(spans), Some(ParseError::NonUtf8(spans[1]))); @@ -2562,24 +2573,39 @@ pub fn parse_overlay_use( } }; - if let Some((name, module, module_id)) = result { - let (decls_to_lay, aliases_to_lay) = if has_prefix { - ( - module.decls_with_head(name.as_bytes()), - module.aliases_with_head(name.as_bytes()), - ) - } else { - (module.decls(), module.aliases()) - }; + let (decls_to_lay, aliases_to_lay) = if has_prefix { + ( + origin_module.decls_with_head(final_overlay_name.as_bytes()), + origin_module.aliases_with_head(final_overlay_name.as_bytes()), + ) + } else { + (origin_module.decls(), origin_module.aliases()) + }; - working_set.add_overlay( - name.as_bytes().to_vec(), - module_id, - decls_to_lay, - aliases_to_lay, - has_prefix, - ); - } + working_set.add_overlay( + final_overlay_name.as_bytes().to_vec(), + origin_module_id, + decls_to_lay, + aliases_to_lay, + has_prefix, + ); + + // Change the call argument to include the Overlay expression with the module ID + let mut call = call; + if let Some(overlay_expr) = call.positional_nth_mut(0) { + overlay_expr.expr = Expr::Overlay(if is_module_updated { + Some(origin_module_id) + } else { + None + }); + } // no need to check for else since it was already checked + + let pipeline = Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: span(spans), + ty: Type::Any, + custom_completion: None, + }]); (pipeline, error) } diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 799e046087..177fa2f1e1 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -5221,6 +5221,7 @@ pub fn discover_captures_in_expr( output.extend(&result); } Expr::ImportPattern(_) => {} + Expr::Overlay(_) => {} Expr::Garbage => {} Expr::Nothing => {} Expr::GlobPattern(_) => {} diff --git a/crates/nu-protocol/src/ast/call.rs b/crates/nu-protocol/src/ast/call.rs index 5398207ff8..897f4be892 100644 --- a/crates/nu-protocol/src/ast/call.rs +++ b/crates/nu-protocol/src/ast/call.rs @@ -82,6 +82,10 @@ impl Call { self.positional_iter().nth(i) } + pub fn positional_nth_mut(&mut self, i: usize) -> Option<&mut Expression> { + self.positional_iter_mut().nth(i) + } + pub fn positional_len(&self) -> usize { self.positional_iter().count() } diff --git a/crates/nu-protocol/src/ast/expr.rs b/crates/nu-protocol/src/ast/expr.rs index d3640aff45..63e3907659 100644 --- a/crates/nu-protocol/src/ast/expr.rs +++ b/crates/nu-protocol/src/ast/expr.rs @@ -39,6 +39,7 @@ pub enum Expr { CellPath(CellPath), FullCellPath(Box), ImportPattern(ImportPattern), + Overlay(Option), // block ID of the overlay's origin module Signature(Box), StringInterpolation(Vec), Nothing, diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 121a4337ee..f1803ac2f9 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -169,6 +169,7 @@ impl Expression { false } Expr::ImportPattern(_) => false, + Expr::Overlay(_) => false, Expr::Filepath(_) => false, Expr::Directory(_) => false, Expr::Float(_) => false, @@ -337,6 +338,7 @@ impl Expression { .replace_in_variable(working_set, new_var_id); } Expr::ImportPattern(_) => {} + Expr::Overlay(_) => {} Expr::Garbage => {} Expr::Nothing => {} Expr::GlobPattern(_) => {} @@ -485,6 +487,7 @@ impl Expression { .replace_span(working_set, replaced, new_span); } Expr::ImportPattern(_) => {} + Expr::Overlay(_) => {} Expr::Garbage => {} Expr::Nothing => {} Expr::GlobPattern(_) => {} diff --git a/tests/overlays/mod.rs b/tests/overlays/mod.rs index 82dbd54ff8..3bc33371d9 100644 --- a/tests/overlays/mod.rs +++ b/tests/overlays/mod.rs @@ -238,6 +238,24 @@ fn update_overlay_from_module_env() { assert_eq!(actual_repl.out, "bar"); } +#[test] +fn overlay_use_do_not_eval_twice() { + let inp = &[ + r#"module spam { export env FOO { "foo" } }"#, + r#"overlay use spam"#, + r#"let-env FOO = "bar""#, + r#"overlay hide spam"#, + r#"overlay use spam"#, + r#"$env.FOO"#, + ]; + + let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; "))); + let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp)); + + assert_eq!(actual.out, "bar"); + assert_eq!(actual_repl.out, "bar"); +} + #[test] fn remove_overlay() { let inp = &[ @@ -843,23 +861,20 @@ fn overlay_use_dont_cd_overlay() { }) } -#[ignore] #[test] -fn overlay_use_find_module_scoped() { +fn overlay_use_find_scoped_module() { Playground::setup("overlay_use_find_module_scoped", |dirs, _| { - let inp = &[r#" + let inp = r#" do { - module spam { export def foo [] { 'foo' } } + module spam { } overlay use spam - foo + overlay list | last } - "#]; + "#; - let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; "))); - let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp)); + let actual = nu!(cwd: dirs.test(), inp); - assert_eq!(actual.out, "foo"); - assert_eq!(actual_repl.out, "foo"); + assert_eq!(actual.out, "spam"); }) }