From 1c941557c3dcf633b2c5253a86dda44af8fa4a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Sat, 3 Apr 2021 18:56:46 -0500 Subject: [PATCH] Remove unused help shell. Slight cleanup and improvement. (#3258) --- crates/nu-command/src/commands/enter.rs | 19 +- crates/nu-command/src/commands/help.rs | 66 ++++- .../src/commands/str_/to_integer.rs | 14 +- crates/nu-data/src/command.rs | 45 ---- crates/nu-data/src/lib.rs | 1 - crates/nu-engine/src/evaluate/internal.rs | 35 +-- .../src/filesystem/filesystem_shell.rs | 6 +- crates/nu-engine/src/lib.rs | 1 - crates/nu-engine/src/shell/help_shell.rs | 252 ------------------ crates/nu-engine/src/shell/mod.rs | 1 - crates/nu-protocol/src/return_value.rs | 3 - tests/fixtures/playground/config/default.toml | 2 +- tests/fixtures/playground/config/startup.toml | 3 + tests/shell/mod.rs | 17 ++ 14 files changed, 97 insertions(+), 368 deletions(-) delete mode 100644 crates/nu-data/src/command.rs delete mode 100644 crates/nu-engine/src/shell/help_shell.rs create mode 100644 tests/fixtures/playground/config/startup.toml diff --git a/crates/nu-command/src/commands/enter.rs b/crates/nu-command/src/commands/enter.rs index ead7f3290..bb2222017 100644 --- a/crates/nu-command/src/commands/enter.rs +++ b/crates/nu-command/src/commands/enter.rs @@ -89,24 +89,7 @@ async fn enter(raw_args: CommandArgs) -> Result { let location_string = location.display().to_string(); let location_clone = location_string.clone(); - if location_string.starts_with("help") { - let spec = location_string.split(':').collect::>(); - - if spec.len() == 2 { - let (_, command) = (spec[0], spec[1]); - - if scope.has_command(command) { - return Ok(OutputStream::one(ReturnSuccess::action( - CommandAction::EnterHelpShell( - UntaggedValue::string(command).into_value(Tag::unknown()), - ), - ))); - } - } - Ok(OutputStream::one(ReturnSuccess::action( - CommandAction::EnterHelpShell(UntaggedValue::nothing().into_value(Tag::unknown())), - ))) - } else if location.is_dir() { + if location.is_dir() { Ok(OutputStream::one(ReturnSuccess::action( CommandAction::EnterShell(location_clone), ))) diff --git a/crates/nu-command/src/commands/help.rs b/crates/nu-command/src/commands/help.rs index d196f681b..775a079e4 100644 --- a/crates/nu-command/src/commands/help.rs +++ b/crates/nu-command/src/commands/help.rs @@ -1,9 +1,13 @@ use crate::prelude::*; -use nu_engine::command_dict; +use crate::TaggedListBuilder; use nu_engine::documentation::generate_docs; -use nu_engine::WholeStreamCommand; +use nu_engine::{Command, WholeStreamCommand}; use nu_errors::ShellError; -use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, TaggedDictBuilder, UntaggedValue, Value}; +use nu_protocol::{ + NamedType, PositionalType, ReturnSuccess, Signature, SyntaxShape, TaggedDictBuilder, + UntaggedValue, Value, +}; +use nu_source::Tag; use nu_source::{SpannedItem, Tagged}; use nu_value_ext::ValueExt; @@ -208,6 +212,62 @@ You can also learn more at https://www.nushell.sh/book/"#; } } +fn for_spec(name: &str, ty: &str, required: bool, tag: impl Into) -> Value { + let tag = tag.into(); + + let mut spec = TaggedDictBuilder::new(tag); + + spec.insert_untagged("name", UntaggedValue::string(name)); + spec.insert_untagged("type", UntaggedValue::string(ty)); + spec.insert_untagged( + "required", + UntaggedValue::string(if required { "yes" } else { "no" }), + ); + + spec.into_value() +} + +pub fn signature_dict(signature: Signature, tag: impl Into) -> Value { + let tag = tag.into(); + let mut sig = TaggedListBuilder::new(&tag); + + for arg in signature.positional.iter() { + let is_required = matches!(arg.0, PositionalType::Mandatory(_, _)); + + sig.push_value(for_spec(arg.0.name(), "argument", is_required, &tag)); + } + + if signature.rest_positional.is_some() { + let is_required = false; + sig.push_value(for_spec("rest", "argument", is_required, &tag)); + } + + for (name, ty) in signature.named.iter() { + match ty.0 { + NamedType::Mandatory(_, _) => sig.push_value(for_spec(name, "flag", true, &tag)), + NamedType::Optional(_, _) => sig.push_value(for_spec(name, "flag", false, &tag)), + NamedType::Switch(_) => sig.push_value(for_spec(name, "switch", false, &tag)), + } + } + + sig.into_value() +} + +fn command_dict(command: Command, tag: impl Into) -> Value { + let tag = tag.into(); + + let mut cmd_dict = TaggedDictBuilder::new(&tag); + + cmd_dict.insert_untagged("name", UntaggedValue::string(command.name())); + + cmd_dict.insert_untagged("type", UntaggedValue::string("Command")); + + cmd_dict.insert_value("signature", signature_dict(command.signature(), tag)); + cmd_dict.insert_untagged("usage", UntaggedValue::string(command.usage())); + + cmd_dict.into_value() +} + #[cfg(test)] mod tests { use super::Help; diff --git a/crates/nu-command/src/commands/str_/to_integer.rs b/crates/nu-command/src/commands/str_/to_integer.rs index ea6fa3dcd..651aa8834 100644 --- a/crates/nu-command/src/commands/str_/to_integer.rs +++ b/crates/nu-command/src/commands/str_/to_integer.rs @@ -1,10 +1,9 @@ use crate::prelude::*; +use crate::utils::arguments::arguments; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; use nu_protocol::ShellTypeName; -use nu_protocol::{ - ColumnPath, Primitive, ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value, -}; +use nu_protocol::{Primitive, ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value}; use nu_source::{Tag, Tagged}; use nu_value_ext::ValueExt; @@ -13,7 +12,7 @@ use num_traits::Num; #[derive(Deserialize)] struct Arguments { - rest: Vec, + rest: Vec, radix: Option>, } @@ -29,7 +28,7 @@ impl WholeStreamCommand for SubCommand { Signature::build("str to-int") .named("radix", SyntaxShape::Number, "radix of integer", Some('r')) .rest( - SyntaxShape::ColumnPath, + SyntaxShape::Any, "optionally convert text into integer by column paths", ) } @@ -69,12 +68,11 @@ impl WholeStreamCommand for SubCommand { } async fn operate(args: CommandArgs) -> Result { - let (Arguments { rest, radix }, input) = args.process().await?; + let (Arguments { mut rest, radix }, input) = args.process().await?; + let (column_paths, _) = arguments(&mut rest)?; let radix = radix.map(|r| r.item).unwrap_or(10); - let column_paths: Vec = rest; - Ok(input .map(move |v| { if column_paths.is_empty() { diff --git a/crates/nu-data/src/command.rs b/crates/nu-data/src/command.rs deleted file mode 100644 index 259dcb026..000000000 --- a/crates/nu-data/src/command.rs +++ /dev/null @@ -1,45 +0,0 @@ -use crate::TaggedListBuilder; -use nu_source::Tag; - -use nu_protocol::{NamedType, PositionalType, Signature, TaggedDictBuilder, UntaggedValue, Value}; - -fn for_spec(name: &str, ty: &str, required: bool, tag: impl Into) -> Value { - let tag = tag.into(); - - let mut spec = TaggedDictBuilder::new(tag); - - spec.insert_untagged("name", UntaggedValue::string(name)); - spec.insert_untagged("type", UntaggedValue::string(ty)); - spec.insert_untagged( - "required", - UntaggedValue::string(if required { "yes" } else { "no" }), - ); - - spec.into_value() -} - -pub fn signature_dict(signature: Signature, tag: impl Into) -> Value { - let tag = tag.into(); - let mut sig = TaggedListBuilder::new(&tag); - - for arg in signature.positional.iter() { - let is_required = matches!(arg.0, PositionalType::Mandatory(_, _)); - - sig.push_value(for_spec(arg.0.name(), "argument", is_required, &tag)); - } - - if signature.rest_positional.is_some() { - let is_required = false; - sig.push_value(for_spec("rest", "argument", is_required, &tag)); - } - - for (name, ty) in signature.named.iter() { - match ty.0 { - NamedType::Mandatory(_, _) => sig.push_value(for_spec(name, "flag", true, &tag)), - NamedType::Optional(_, _) => sig.push_value(for_spec(name, "flag", false, &tag)), - NamedType::Switch(_) => sig.push_value(for_spec(name, "switch", false, &tag)), - } - } - - sig.into_value() -} diff --git a/crates/nu-data/src/lib.rs b/crates/nu-data/src/lib.rs index 1705e2537..feb179e5c 100644 --- a/crates/nu-data/src/lib.rs +++ b/crates/nu-data/src/lib.rs @@ -1,5 +1,4 @@ pub mod base; -pub mod command; pub mod config; pub mod dict; pub mod keybinding; diff --git a/crates/nu-engine/src/evaluate/internal.rs b/crates/nu-engine/src/evaluate/internal.rs index 7573f40b1..97cfe6795 100644 --- a/crates/nu-engine/src/evaluate/internal.rs +++ b/crates/nu-engine/src/evaluate/internal.rs @@ -2,13 +2,12 @@ use crate::call_info::UnevaluatedCallInfo; use crate::command_args::RawCommandArgs; use crate::evaluation_context::EvaluationContext; use crate::filesystem::filesystem_shell::{FilesystemShell, FilesystemShellMode}; -use crate::shell::help_shell::HelpShell; use crate::shell::value_shell::ValueShell; use futures::StreamExt; use log::{log_enabled, trace}; use nu_errors::ShellError; use nu_protocol::hir::{ExternalRedirection, InternalCommand}; -use nu_protocol::{CommandAction, Primitive, ReturnSuccess, UntaggedValue, Value}; +use nu_protocol::{CommandAction, ReturnSuccess, UntaggedValue, Value}; use nu_source::{PrettyDebug, Span, Tag}; use nu_stream::{trace_stream, InputStream, ToInputStream}; use std::sync::Arc; @@ -127,38 +126,6 @@ pub(crate) async fn run_internal_command( InputStream::one(tagged_contents) } } - CommandAction::EnterHelpShell(value) => match value { - Value { - value: UntaggedValue::Primitive(Primitive::String(cmd)), - tag, - } => { - context.shell_manager.insert_at_current(Box::new( - match HelpShell::for_command( - UntaggedValue::string(cmd).into_value(tag), - &context.scope, - ) { - Ok(v) => v, - Err(err) => { - context.error(err); - return InputStream::empty(); - } - }, - )); - InputStream::from_stream(futures::stream::iter(vec![])) - } - _ => { - context.shell_manager.insert_at_current(Box::new( - match HelpShell::index(&context.scope) { - Ok(v) => v, - Err(err) => { - context.error(err); - return InputStream::empty(); - } - }, - )); - InputStream::from_stream(futures::stream::iter(vec![])) - } - }, CommandAction::EnterValueShell(value) => { context .shell_manager diff --git a/crates/nu-engine/src/filesystem/filesystem_shell.rs b/crates/nu-engine/src/filesystem/filesystem_shell.rs index fe0ef4b6a..2432ccebf 100644 --- a/crates/nu-engine/src/filesystem/filesystem_shell.rs +++ b/crates/nu-engine/src/filesystem/filesystem_shell.rs @@ -58,6 +58,10 @@ impl Clone for FilesystemShell { } impl FilesystemShell { + fn is_cli(&self) -> bool { + matches!(&self.mode, FilesystemShellMode::Cli) + } + pub fn basic(mode: FilesystemShellMode) -> Result { let path = match std::env::current_dir() { Ok(path) => path, @@ -295,7 +299,7 @@ impl Shell for FilesystemShell { //Loading local configs in script mode, makes scripts behave different on different //filesystems and might therefore surprise users. That's why we only load them in cli mode. - if self.mode == FilesystemShellMode::Cli { + if self.is_cli() { match dunce::canonicalize(self.path()) { Err(e) => { trace!( diff --git a/crates/nu-engine/src/lib.rs b/crates/nu-engine/src/lib.rs index eb806690f..158a10b54 100644 --- a/crates/nu-engine/src/lib.rs +++ b/crates/nu-engine/src/lib.rs @@ -39,7 +39,6 @@ pub use crate::filesystem::path; pub use crate::maybe_text_codec::{MaybeTextCodec, StringOrBinary}; pub use crate::print::maybe_print_errors; pub use crate::runnable_context::RunnableContext; -pub use crate::shell::help_shell::{command_dict, HelpShell}; pub use crate::shell::painter::Painter; pub use crate::shell::palette::{DefaultPalette, Palette}; pub use crate::shell::shell_manager::ShellManager; diff --git a/crates/nu-engine/src/shell/help_shell.rs b/crates/nu-engine/src/shell/help_shell.rs deleted file mode 100644 index 9bc9a0c90..000000000 --- a/crates/nu-engine/src/shell/help_shell.rs +++ /dev/null @@ -1,252 +0,0 @@ -use crate::command_args::EvaluatedWholeStreamCommandArgs; -use crate::evaluate::scope::Scope; -use crate::maybe_text_codec::StringOrBinary; -use crate::shell::shell_args::{CdArgs, CopyArgs, LsArgs, MkdirArgs, MvArgs, RemoveArgs}; -use crate::shell::Shell; -use crate::whole_stream_command::Command; -use encoding_rs::Encoding; -use futures::stream::BoxStream; -use nu_data::command::signature_dict; -use nu_errors::ShellError; -use nu_protocol::{ - Primitive, ReturnSuccess, ShellTypeName, TaggedDictBuilder, UntaggedValue, Value, -}; -use nu_source::Tagged; -use nu_source::{Span, SpannedItem, Tag}; -use nu_stream::OutputStream; -use nu_value_ext::ValueExt; -use std::collections::VecDeque; -use std::ffi::OsStr; -use std::path::{Path, PathBuf}; -use std::sync::atomic::AtomicBool; -use std::sync::Arc; - -pub fn command_dict(command: Command, tag: impl Into) -> Value { - let tag = tag.into(); - - let mut cmd_dict = TaggedDictBuilder::new(&tag); - - cmd_dict.insert_untagged("name", UntaggedValue::string(command.name())); - - cmd_dict.insert_untagged("type", UntaggedValue::string("Command")); - - cmd_dict.insert_value("signature", signature_dict(command.signature(), tag)); - cmd_dict.insert_untagged("usage", UntaggedValue::string(command.usage())); - - cmd_dict.into_value() -} - -#[derive(Clone, Debug)] -pub struct HelpShell { - pub(crate) path: String, - pub(crate) value: Value, -} - -impl HelpShell { - pub fn index(scope: &Scope) -> Result { - let mut cmds = TaggedDictBuilder::new(Tag::unknown()); - let mut specs = Vec::new(); - - for cmd in scope.get_command_names() { - if let Some(cmd_value) = scope.get_command(&cmd) { - let mut spec = TaggedDictBuilder::new(Tag::unknown()); - let value = command_dict(cmd_value, Tag::unknown()); - - spec.insert_untagged("name", cmd); - spec.insert_untagged( - "description", - value - .get_data_by_key("usage".spanned_unknown()) - .ok_or_else(|| { - ShellError::untagged_runtime_error( - "Internal error: expected to find usage", - ) - })? - .as_string()?, - ); - spec.insert_value("details", value); - - specs.push(spec.into_value()); - } else { - } - } - - cmds.insert_untagged("help", UntaggedValue::Table(specs)); - - Ok(HelpShell { - path: "/help".to_string(), - value: cmds.into_value(), - }) - } - - pub fn for_command(cmd: Value, scope: &Scope) -> Result { - let mut sh = HelpShell::index(scope)?; - - if let Value { - value: UntaggedValue::Primitive(Primitive::String(name)), - .. - } = cmd - { - sh.set_path(format!("/help/{:}/details", name)); - } - - Ok(sh) - } - - fn commands(&self) -> VecDeque { - let mut cmds = VecDeque::new(); - let full_path = PathBuf::from(&self.path); - - let mut viewed = self.value.clone(); - let sep_string = std::path::MAIN_SEPARATOR.to_string(); - let sep = OsStr::new(&sep_string); - - for p in full_path.iter() { - match p { - x if x == sep => {} - step => { - let step: &str = &step.to_string_lossy().to_string(); - let value = viewed.get_data_by_key(step.spanned_unknown()); - if let Some(v) = value { - viewed = v.clone(); - } - } - } - } - match viewed { - Value { - value: UntaggedValue::Table(l), - .. - } => { - for item in l { - cmds.push_back(item.clone()); - } - } - x => { - cmds.push_back(x); - } - } - - cmds - } -} - -impl Shell for HelpShell { - fn name(&self) -> String { - let anchor_name = self.value.anchor_name(); - - match anchor_name { - Some(x) => format!("{{{}}}", x), - None => format!("<{}>", self.value.type_name()), - } - } - - fn homedir(&self) -> Option { - #[cfg(feature = "dirs")] - { - dirs_next::home_dir() - } - - #[cfg(not(feature = "dirs"))] - { - None - } - } - - fn path(&self) -> String { - self.path.clone() - } - - fn pwd(&self, _: EvaluatedWholeStreamCommandArgs) -> Result { - Ok(OutputStream::empty()) - } - - fn set_path(&mut self, path: String) { - let _ = std::env::set_current_dir(&path); - self.path = path; - } - - fn ls( - &self, - _args: LsArgs, - _name: Tag, - _ctrl_c: Arc, - ) -> Result { - let output = self - .commands() - .into_iter() - .map(ReturnSuccess::value) - .collect::>(); - Ok(output.into()) - } - - fn cd(&self, args: CdArgs, _name: Tag) -> Result { - let path = match args.path { - None => "/".to_string(), - Some(v) => { - let Tagged { item: target, .. } = v; - let mut cwd = PathBuf::from(&self.path); - - if target == PathBuf::from("..") { - cwd.pop(); - } else { - match target.to_str() { - Some(target) => match target.chars().next() { - Some(x) if x == '/' => cwd = PathBuf::from(target), - _ => cwd.push(target), - }, - None => cwd.push(target), - } - } - cwd.to_string_lossy().to_string() - } - }; - - let mut stream = VecDeque::new(); - stream.push_back(ReturnSuccess::change_cwd(path)); - Ok(stream.into()) - } - - fn cp(&self, _args: CopyArgs, _name: Tag, _path: &str) -> Result { - Ok(OutputStream::empty()) - } - - fn mv(&self, _args: MvArgs, _name: Tag, _path: &str) -> Result { - Ok(OutputStream::empty()) - } - - fn mkdir(&self, _args: MkdirArgs, _name: Tag, _path: &str) -> Result { - Ok(OutputStream::empty()) - } - - fn rm(&self, _args: RemoveArgs, _name: Tag, _path: &str) -> Result { - Ok(OutputStream::empty()) - } - - fn open( - &self, - _path: &Path, - _name: Span, - _with_encoding: Option<&'static Encoding>, - ) -> Result>, ShellError> { - Err(ShellError::unimplemented( - "open on help shell is not supported", - )) - } - - fn save( - &mut self, - _path: &Path, - _contents: &[u8], - _name: Span, - ) -> Result { - Err(ShellError::unimplemented( - "save on help shell is not supported", - )) - } - - fn is_interactive(&self) -> bool { - //Help shell is always interactive - true - } -} diff --git a/crates/nu-engine/src/shell/mod.rs b/crates/nu-engine/src/shell/mod.rs index 8d96ab7ca..9159619c6 100644 --- a/crates/nu-engine/src/shell/mod.rs +++ b/crates/nu-engine/src/shell/mod.rs @@ -11,7 +11,6 @@ use std::path::{Path, PathBuf}; use std::sync::atomic::AtomicBool; use std::sync::Arc; -pub(crate) mod help_shell; pub(crate) mod painter; pub(crate) mod palette; pub(crate) mod shell_args; diff --git a/crates/nu-protocol/src/return_value.rs b/crates/nu-protocol/src/return_value.rs index 65909951d..451f673d0 100644 --- a/crates/nu-protocol/src/return_value.rs +++ b/crates/nu-protocol/src/return_value.rs @@ -18,8 +18,6 @@ pub enum CommandAction { AutoConvert(Value, String), /// Enter a value shell, one that allows exploring inside of a Value EnterValueShell(Value), - /// Enter the help shell, which allows exploring the help system - EnterHelpShell(Value), /// Add plugins from path given AddPlugins(String), /// Unload the config specified by PathBuf if present @@ -50,7 +48,6 @@ impl PrettyDebug for CommandAction { DbgDocBldr::typed("enter shell", DbgDocBldr::description(s)) } CommandAction::EnterValueShell(v) => DbgDocBldr::typed("enter value shell", v.pretty()), - CommandAction::EnterHelpShell(v) => DbgDocBldr::typed("enter help shell", v.pretty()), CommandAction::AddPlugins(..) => DbgDocBldr::description("add plugins"), CommandAction::PreviousShell => DbgDocBldr::description("previous shell"), CommandAction::NextShell => DbgDocBldr::description("next shell"), diff --git a/tests/fixtures/playground/config/default.toml b/tests/fixtures/playground/config/default.toml index 4b2f5ebcc..7ae2d00bd 100644 --- a/tests/fixtures/playground/config/default.toml +++ b/tests/fixtures/playground/config/default.toml @@ -1 +1 @@ -skip_welcome_message = true \ No newline at end of file +skip_welcome_message = true diff --git a/tests/fixtures/playground/config/startup.toml b/tests/fixtures/playground/config/startup.toml new file mode 100644 index 000000000..c4b9e0678 --- /dev/null +++ b/tests/fixtures/playground/config/startup.toml @@ -0,0 +1,3 @@ +skip_welcome_message = true + +startup = ["def hello-world [] { echo 'Nu World' }"] diff --git a/tests/shell/mod.rs b/tests/shell/mod.rs index 25d3487a9..25d9c1f1c 100644 --- a/tests/shell/mod.rs +++ b/tests/shell/mod.rs @@ -1,11 +1,28 @@ +use nu_test_support::fs::AbsolutePath; +use nu_test_support::playground::{says, Playground}; use nu_test_support::{nu, pipeline}; +use hamcrest2::assert_that; +use hamcrest2::prelude::*; + #[cfg(feature = "directories-support")] #[cfg(feature = "which-support")] mod environment; mod pipeline; +#[should_panic] +#[test] +fn runs_configuration_startup_commands() { + Playground::setup("init_config_startup_commands_test", |dirs, nu| { + let file = AbsolutePath::new(dirs.test().join("startup.toml")); + + nu.with_config(&file); + + assert_that!(nu.pipeline("hello-world"), says().to_stdout("Nu World")); + }); +} + #[test] fn plugins_are_declared_with_wix() { let actual = nu!(