diff --git a/crates/nu-command/src/core_commands/overlay/add.rs b/crates/nu-command/src/core_commands/overlay/add.rs index 5c9b341039..3644946cf9 100644 --- a/crates/nu-command/src/core_commands/overlay/add.rs +++ b/crates/nu-command/src/core_commands/overlay/add.rs @@ -24,12 +24,11 @@ impl Command for OverlayAdd { SyntaxShape::String, "Module name to create overlay for", ) - // TODO: - // .switch( - // "prefix", - // "Prepend module name to the imported symbols", - // Some('p'), - // ) + .switch( + "prefix", + "Prepend module name to the imported commands and aliases", + Some('p'), + ) .category(Category::Core) } @@ -122,13 +121,22 @@ impl Command for OverlayAdd { Example { description: "Create an overlay from a module", example: r#"module spam { export def foo [] { "foo" } } - overlay add spam"#, + overlay add spam + foo"#, + result: None, + }, + Example { + description: "Create an overlay with a prefix", + example: r#"echo 'export def foo { "foo" }' + overlay add --prefix spam + spam foo"#, result: None, }, Example { description: "Create an overlay from a file", example: r#"echo 'export env FOO { "foo" }' | save spam.nu - overlay add spam.nu"#, + overlay add spam.nu + $env.FOO"#, result: None, }, ] diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs index 221fccc7ee..5690cef084 100644 --- a/crates/nu-parser/src/errors.rs +++ b/crates/nu-parser/src/errors.rs @@ -124,6 +124,18 @@ pub enum ParseError { #[diagnostic(code(nu::parser::active_overlay_not_found), url(docsrs))] ActiveOverlayNotFound(#[label = "not an active overlay"] Span), + #[error("Overlay prefix mismatch.")] + #[diagnostic( + code(nu::parser::overlay_prefix_mismatch), + url(docsrs), + help("Overlay {0} already exists {1} a prefix. To add it again, do it {1} the --prefix flag.") + )] + OverlayPrefixMismatch( + String, + String, + #[label = "already exists {1} a prefix"] Span, + ), + #[error("Module or overlay not found.")] #[diagnostic( code(nu::parser::module_or_overlay_not_found), @@ -327,6 +339,7 @@ impl ParseError { ParseError::ModuleNotFound(s) => *s, ParseError::ModuleOrOverlayNotFound(s) => *s, ParseError::ActiveOverlayNotFound(s) => *s, + ParseError::OverlayPrefixMismatch(_, _, s) => *s, ParseError::CantRemoveLastOverlay(s) => *s, ParseError::CantRemoveDefaultOverlay(_, s) => *s, ParseError::NotFound(s) => *s, diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 11fccb83de..d11cd33a8f 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -702,21 +702,23 @@ pub fn parse_export( let mut result = vec![]; - let decl_name = working_set.get_span_contents(spans[2]); - let decl_name = trim_quotes(decl_name); + if let Some(decl_name_span) = spans.get(2) { + let decl_name = working_set.get_span_contents(*decl_name_span); + let decl_name = trim_quotes(decl_name); - if let Some(decl_id) = working_set.find_decl(decl_name, &Type::Any) { - result.push(Exportable::Decl { - name: decl_name.to_vec(), - id: decl_id, - }); - } else { - error = error.or_else(|| { - Some(ParseError::InternalError( - "failed to find added declaration".into(), - span(&spans[1..]), - )) - }); + if let Some(decl_id) = working_set.find_decl(decl_name, &Type::Any) { + result.push(Exportable::Decl { + name: decl_name.to_vec(), + id: decl_id, + }); + } else { + error = error.or_else(|| { + Some(ParseError::InternalError( + "failed to find added declaration".into(), + span(&spans[1..]), + )) + }); + } } result @@ -2032,7 +2034,13 @@ pub fn parse_overlay_new( let module_id = working_set.add_module(&overlay_name, Module::new()); - working_set.add_overlay(overlay_name.as_bytes().to_vec(), module_id, vec![], vec![]); + working_set.add_overlay( + overlay_name.as_bytes().to_vec(), + module_id, + vec![], + vec![], + false, + ); (pipeline, None) } @@ -2118,6 +2126,8 @@ pub fn parse_overlay_add( ); }; + let has_prefix = call.has_flag("prefix"); + let pipeline = Pipeline::from_vec(vec![Expression { expr: Expr::Call(call), span: span(spans), @@ -2125,15 +2135,36 @@ pub fn parse_overlay_add( custom_completion: None, }]); - // TODO: Add support for it -- needs to play well with overlay remove - let has_prefix = false; //call.has_flag("prefix"); - let cwd = working_set.get_cwd(); let mut error = None; - let result = if let Some(module_id) = working_set.find_overlay_origin(overlay_name.as_bytes()) { + let result = if let Some(overlay_frame) = working_set.find_overlay(overlay_name.as_bytes()) { + if has_prefix && !overlay_frame.prefixed { + return ( + pipeline, + Some(ParseError::OverlayPrefixMismatch( + overlay_name, + "without".to_string(), + overlay_name_span, + )), + ); + } + + if !has_prefix && overlay_frame.prefixed { + return ( + pipeline, + Some(ParseError::OverlayPrefixMismatch( + overlay_name, + "with".to_string(), + overlay_name_span, + )), + ); + } + // 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)) @@ -2237,6 +2268,7 @@ pub fn parse_overlay_add( module_id, decls_to_lay, aliases_to_lay, + has_prefix, ); } diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 6ebf7146f5..1b7d458852 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -101,7 +101,11 @@ impl EngineState { blocks: vec![], modules: vec![Module::new()], // make sure we have some default overlay: - scope: ScopeFrame::with_empty_overlay(DEFAULT_OVERLAY_NAME.as_bytes().to_vec(), 0), + scope: ScopeFrame::with_empty_overlay( + DEFAULT_OVERLAY_NAME.as_bytes().to_vec(), + 0, + false, + ), ctrlc: None, env_vars: EnvVars::from([(DEFAULT_OVERLAY_NAME.to_string(), HashMap::new())]), previous_env_vars: HashMap::new(), @@ -832,9 +836,11 @@ pub struct StateDelta { impl StateDelta { pub fn new(engine_state: &EngineState) -> Self { + let last_overlay = engine_state.last_overlay(&[]); let scope_frame = ScopeFrame::with_empty_overlay( engine_state.last_overlay_name(&[]).to_owned(), - engine_state.last_overlay(&[]).origin, + last_overlay.origin, + last_overlay.prefixed, ); StateDelta { @@ -1726,16 +1732,16 @@ impl<'a> StateWorkingSet<'a> { self.permanent_state.has_overlay(name) } - pub fn find_overlay_origin(&self, name: &[u8]) -> Option { + pub fn find_overlay(&self, name: &[u8]) -> Option<&OverlayFrame> { for scope_frame in self.delta.scope.iter().rev() { if let Some(overlay_id) = scope_frame.find_overlay(name) { - return Some(scope_frame.get_overlay(overlay_id).origin); + return Some(scope_frame.get_overlay(overlay_id)); } } self.permanent_state .find_overlay(name) - .map(|id| self.permanent_state.get_overlay(id).origin) + .map(|id| self.permanent_state.get_overlay(id)) } pub fn last_overlay_name(&self) -> &Vec { @@ -1775,9 +1781,11 @@ impl<'a> StateWorkingSet<'a> { pub fn last_overlay_mut(&mut self) -> &mut OverlayFrame { if self.delta.last_overlay_mut().is_none() { // If there is no overlay, automatically activate the last one + let overlay_frame = self.last_overlay(); let name = self.last_overlay_name().to_vec(); - let origin = self.last_overlay().origin; - self.add_overlay(name, origin, vec![], vec![]); + let origin = overlay_frame.origin; + let prefixed = overlay_frame.prefixed; + self.add_overlay(name, origin, vec![], vec![], prefixed); } self.delta @@ -1841,6 +1849,7 @@ impl<'a> StateWorkingSet<'a> { origin: ModuleId, decls: Vec<(Vec, DeclId)>, aliases: Vec<(Vec, AliasId)>, + prefixed: bool, ) { let last_scope_frame = self.delta.last_scope_frame_mut(); @@ -1855,7 +1864,7 @@ impl<'a> StateWorkingSet<'a> { } else { last_scope_frame .overlays - .push((name, OverlayFrame::from_origin(origin))); + .push((name, OverlayFrame::from_origin(origin, prefixed))); last_scope_frame.overlays.len() - 1 }; @@ -1873,7 +1882,7 @@ impl<'a> StateWorkingSet<'a> { pub fn remove_overlay(&mut self, name: &[u8], keep_custom: bool) { let last_scope_frame = self.delta.last_scope_frame_mut(); - let removed_overlay_origin = if let Some(overlay_id) = last_scope_frame.find_overlay(name) { + let maybe_module_id = if let Some(overlay_id) = last_scope_frame.find_overlay(name) { last_scope_frame .active_overlays .retain(|id| id != &overlay_id); @@ -1885,7 +1894,7 @@ impl<'a> StateWorkingSet<'a> { .map(|id| self.permanent_state.get_overlay(id).origin) }; - if let Some(module_id) = removed_overlay_origin { + if let Some(module_id) = maybe_module_id { last_scope_frame.removed_overlays.push(name.to_owned()); if keep_custom { diff --git a/crates/nu-protocol/src/engine/overlay.rs b/crates/nu-protocol/src/engine/overlay.rs index da50e92623..ca19796cee 100644 --- a/crates/nu-protocol/src/engine/overlay.rs +++ b/crates/nu-protocol/src/engine/overlay.rs @@ -100,9 +100,9 @@ impl ScopeFrame { } } - pub fn with_empty_overlay(name: Vec, origin: ModuleId) -> Self { + pub fn with_empty_overlay(name: Vec, origin: ModuleId, prefixed: bool) -> Self { Self { - overlays: vec![(name, OverlayFrame::from_origin(origin))], + overlays: vec![(name, OverlayFrame::from_origin(origin, prefixed))], active_overlays: vec![0], removed_overlays: vec![], predecls: HashMap::new(), @@ -205,10 +205,11 @@ pub struct OverlayFrame { pub modules: HashMap, ModuleId>, pub visibility: Visibility, pub origin: ModuleId, // The original module the overlay was created from + pub prefixed: bool, // Whether the overlay has definitions prefixed with its name } impl OverlayFrame { - pub fn from_origin(origin: ModuleId) -> Self { + pub fn from_origin(origin: ModuleId, prefixed: bool) -> Self { Self { vars: HashMap::new(), predecls: HashMap::new(), @@ -217,6 +218,7 @@ impl OverlayFrame { modules: HashMap::new(), visibility: Visibility::new(), origin, + prefixed, } } diff --git a/tests/overlays/mod.rs b/tests/overlays/mod.rs index 5e666b5c94..aa534db6a8 100644 --- a/tests/overlays/mod.rs +++ b/tests/overlays/mod.rs @@ -15,6 +15,102 @@ fn add_overlay() { assert_eq!(actual_repl.out, "foo"); } +#[test] +fn add_overlay_twice() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add spam"#, + r#"overlay add spam"#, + r#"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, "foo"); + assert_eq!(actual_repl.out, "foo"); +} + +#[test] +fn add_prefixed_overlay() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add --prefix spam"#, + r#"spam 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, "foo"); + assert_eq!(actual_repl.out, "foo"); +} + +#[test] +fn add_prefixed_overlay_twice() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add --prefix spam"#, + r#"overlay add --prefix spam"#, + r#"spam 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, "foo"); + assert_eq!(actual_repl.out, "foo"); +} + +#[test] +fn add_prefixed_overlay_mismatch_1() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add --prefix spam"#, + r#"overlay add spam"#, + ]; + + let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; "))); + let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp)); + + assert!(actual.err.contains("exists with a prefix")); + // Why doesn't the REPL test work with the previous expected output + assert!(actual_repl.err.contains("overlay_prefix_mismatch")); +} + +#[test] +fn add_prefixed_overlay_mismatch_2() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add spam"#, + r#"overlay add --prefix spam"#, + ]; + + let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; "))); + let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp)); + + assert!(actual.err.contains("exists without a prefix")); + // Why doesn't the REPL test work with the previous expected output + assert!(actual_repl.err.contains("overlay_prefix_mismatch")); +} + +#[test] +fn prefixed_overlay_keeps_custom_decl() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add --prefix spam"#, + r#"def bar [] { "bar" }"#, + r#"overlay remove --keep-custom spam"#, + r#"bar"#, + ]; + + 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 add_overlay_env() { let inp = &[ @@ -30,6 +126,21 @@ fn add_overlay_env() { assert_eq!(actual_repl.out, "foo"); } +#[test] +fn add_prefixed_overlay_env_no_prefix() { + let inp = &[ + r#"module spam { export env FOO { "foo" } }"#, + r#"overlay add --prefix 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, "foo"); + assert_eq!(actual_repl.out, "foo"); +} + #[test] fn add_overlay_from_file_decl() { let inp = &[r#"overlay add samples/spam.nu"#, r#"foo"#];