From 8783742060c59db6a98577e3f033a010d2dc4c7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sat, 13 Aug 2022 17:28:18 +0300 Subject: [PATCH] Add 'as' keyword to 'overlay add' (#6314) --- .../src/core_commands/overlay/add.rs | 124 +++++++++++------- crates/nu-parser/src/errors.rs | 5 + crates/nu-parser/src/parse_keywords.rs | 52 +++++++- tests/overlays/mod.rs | 103 +++++++++++++++ 4 files changed, 231 insertions(+), 53 deletions(-) diff --git a/crates/nu-command/src/core_commands/overlay/add.rs b/crates/nu-command/src/core_commands/overlay/add.rs index 3644946cf9..fa5be88c2c 100644 --- a/crates/nu-command/src/core_commands/overlay/add.rs +++ b/crates/nu-command/src/core_commands/overlay/add.rs @@ -24,6 +24,11 @@ impl Command for OverlayAdd { SyntaxShape::String, "Module name to create overlay for", ) + .optional( + "as", + SyntaxShape::Keyword(b"as".to_vec(), Box::new(SyntaxShape::String)), + "as keyword followed by a new name", + ) .switch( "prefix", "Prepend module name to the imported commands and aliases", @@ -50,67 +55,86 @@ impl Command for OverlayAdd { ) -> Result { let name_arg: Spanned = call.req(engine_state, stack, 0)?; - let maybe_overlay_name = if engine_state + let (overlay_name, overlay_name_span) = 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) + } else { + return Err(ShellError::NushellFailedSpanned( + "Wrong keyword type".to_string(), + "keyword argument not a string".to_string(), + new_name_expression.span, + )); + } + } else { + return Err(ShellError::NushellFailedSpanned( + "Wrong keyword type".to_string(), + "keyword argument not a keyword".to_string(), + kw_expression.span, + )); + } + } else if engine_state .find_overlay(name_arg.item.as_bytes()) .is_some() { - Some(name_arg.item.clone()) + (name_arg.item, name_arg.span) } else if let Some(os_str) = Path::new(&name_arg.item).file_stem() { - os_str.to_str().map(|name| name.to_string()) - } else { - None - }; - - if let Some(overlay_name) = maybe_overlay_name { - if let Some(overlay_id) = engine_state.find_overlay(overlay_name.as_bytes()) { - let old_module_id = engine_state.get_overlay(overlay_id).origin; - - stack.add_overlay(overlay_name.clone()); - - if let Some(new_module_id) = engine_state.find_module(overlay_name.as_bytes(), &[]) - { - if !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)); - }; - - let block = engine_state.get_block(block_id); - - let val = eval_block( - engine_state, - stack, - block, - PipelineData::new(call.head), - false, - true, - )? - .into_value(call.head); - - stack.add_env_var(name, val); - } - } - } + if let Some(name) = os_str.to_str() { + (name.to_string(), name_arg.span) } else { - return Err(ShellError::OverlayNotFoundAtRuntime( - name_arg.item, - name_arg.span, - )); + return Err(ShellError::NonUtf8(name_arg.span)); } } else { return Err(ShellError::OverlayNotFoundAtRuntime( name_arg.item, name_arg.span, )); + }; + + if let Some(overlay_id) = engine_state.find_overlay(overlay_name.as_bytes()) { + let old_module_id = engine_state.get_overlay(overlay_id).origin; + + stack.add_overlay(overlay_name.clone()); + + if let Some(new_module_id) = engine_state.find_module(overlay_name.as_bytes(), &[]) { + if !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)); + }; + + let block = engine_state.get_block(block_id); + + let val = eval_block( + engine_state, + stack, + block, + PipelineData::new(call.head), + false, + true, + )? + .into_value(call.head); + + stack.add_env_var(name, val); + } + } + } + } else { + return Err(ShellError::OverlayNotFoundAtRuntime( + overlay_name, + overlay_name_span, + )); } Ok(PipelineData::new(call.head)) diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs index 5690cef084..d9586884a9 100644 --- a/crates/nu-parser/src/errors.rs +++ b/crates/nu-parser/src/errors.rs @@ -160,6 +160,10 @@ pub enum ParseError { )] CantRemoveDefaultOverlay(String, #[label = "can't remove overlay"] Span), + #[error("Cannot add overlay.")] + #[diagnostic(code(nu::parser::cant_add_overlay_help), url(docsrs), help("{0}"))] + CantAddOverlayHelp(String, #[label = "cannot add this overlay"] Span), + #[error("Not found.")] #[diagnostic(code(nu::parser::not_found), url(docsrs))] NotFound(#[label = "did not find anything under this name"] Span), @@ -342,6 +346,7 @@ impl ParseError { ParseError::OverlayPrefixMismatch(_, _, s) => *s, ParseError::CantRemoveLastOverlay(s) => *s, ParseError::CantRemoveDefaultOverlay(_, s) => *s, + ParseError::CantAddOverlayHelp(_, s) => *s, ParseError::NotFound(s) => *s, ParseError::DuplicateCommandDef(s) => *s, ParseError::UnknownCommand(s) => *s, diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index d11cd33a8f..6ccfb7699e 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -5,7 +5,7 @@ use nu_protocol::{ ImportPatternMember, Pipeline, }, engine::{StateWorkingSet, DEFAULT_OVERLAY_NAME}, - span, Exportable, Module, PositionalArg, Span, SyntaxShape, Type, + span, Exportable, Module, PositionalArg, Span, Spanned, SyntaxShape, Type, }; use std::collections::HashSet; use std::path::{Path, PathBuf}; @@ -2126,6 +2126,36 @@ pub fn parse_overlay_add( ); }; + let new_name = if let Some(kw_expression) = call.positional_nth(1) { + if let Some(new_name_expression) = kw_expression.as_keyword() { + if let Some(new_name) = new_name_expression.as_string() { + Some(Spanned { + item: new_name, + span: new_name_expression.span, + }) + } else { + return ( + garbage_pipeline(spans), + Some(ParseError::TypeMismatch( + Type::String, + new_name_expression.ty.clone(), + new_name_expression.span, + )), + ); + } + } else { + return ( + garbage_pipeline(spans), + Some(ParseError::ExpectedKeyword( + "as keyword".to_string(), + kw_expression.span, + )), + ); + } + } else { + None + }; + let has_prefix = call.has_flag("prefix"); let pipeline = Pipeline::from_vec(vec![Expression { @@ -2162,6 +2192,18 @@ pub fn parse_overlay_add( ); } + if let Some(new_name) = new_name { + if new_name.item != overlay_name { + return ( + pipeline, + Some(ParseError::CantAddOverlayHelp( + format!("Cannot add overlay as '{}' because it already exsits under the name '{}'", new_name.item, overlay_name), + new_name.span, + )), + ); + } + } + // Activate existing overlay let module_id = overlay_frame.origin; @@ -2186,7 +2228,7 @@ pub fn parse_overlay_add( working_set.find_module(overlay_name.as_bytes()) { Some(( - overlay_name, + new_name.map(|spanned| spanned.item).unwrap_or(overlay_name), working_set.get_module(module_id).clone(), module_id, )) @@ -2236,7 +2278,11 @@ pub fn parse_overlay_add( let _ = working_set.add_block(block); let module_id = working_set.add_module(&overlay_name, module.clone()); - Some((overlay_name, module, module_id)) + Some(( + new_name.map(|spanned| spanned.item).unwrap_or(overlay_name), + module, + module_id, + )) } else { return ( pipeline, diff --git a/tests/overlays/mod.rs b/tests/overlays/mod.rs index aa534db6a8..10ba04077e 100644 --- a/tests/overlays/mod.rs +++ b/tests/overlays/mod.rs @@ -633,3 +633,106 @@ fn overlay_keep_pwd() { assert_eq!(actual.out, "samples"); assert_eq!(actual_repl.out, "samples"); } + +#[test] +fn overlay_wrong_rename_type() { + let inp = &[r#"module spam {}"#, r#"overlay add spam as { echo foo }"#]; + + let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; "))); + + assert!(actual.err.contains("parse_mismatch")); +} + +#[test] +fn overlay_add_renamed() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add spam as eggs --prefix"#, + r#"eggs 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 overlay_add_renamed_from_file() { + let inp = &[ + r#"overlay add samples/spam.nu as eggs --prefix"#, + r#"eggs 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 overlay_cant_rename_existing_overlay() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add spam"#, + r#"overlay remove spam"#, + r#"overlay add spam as eggs"#, + ]; + + let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; "))); + let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp)); + + assert!(actual.err.contains("cant_add_overlay_help")); + assert!(actual_repl.err.contains("cant_add_overlay_help")); +} + +#[test] +fn overlay_can_add_renamed_overlay() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add spam as eggs --prefix"#, + r#"overlay add spam --prefix"#, + r#"(spam foo) + (eggs 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, "foofoo"); + assert_eq!(actual_repl.out, "foofoo"); +} + +#[test] +fn overlay_remove_renamed_overlay() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add spam as eggs"#, + r#"overlay remove eggs"#, + r#"foo"#, + ]; + + let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; "))); + let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp)); + + assert!(actual.err.contains("did you mean")); + assert!(actual_repl.err.contains("did you mean")); +} + +#[test] +fn overlay_remove_and_add_renamed_overlay() { + let inp = &[ + r#"module spam { export def foo [] { "foo" } }"#, + r#"overlay add spam as eggs"#, + r#"overlay remove eggs"#, + r#"overlay add eggs"#, + 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"); +}