From a35ecb48378db7d14c5034b45c0303d795b44826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 22 Oct 2023 14:06:53 +0300 Subject: [PATCH] Finish removing `profile` command and related data (#10807) --- .../nu-cmd-extra/src/extra/formats/to/html.rs | 8 +- crates/nu-command/src/debug/metadata.rs | 28 ++--- crates/nu-command/src/debug/mod.rs | 2 - crates/nu-command/src/debug/profile.rs | 102 ------------------ crates/nu-command/src/default_context.rs | 1 - crates/nu-command/src/filesystem/ls.rs | 4 +- crates/nu-command/src/filters/uniq.rs | 2 +- crates/nu-command/src/viewers/table.rs | 4 +- crates/nu-explore/src/nu_common/value.rs | 2 +- crates/nu-protocol/src/pipeline_data.rs | 25 ++--- 10 files changed, 28 insertions(+), 150 deletions(-) delete mode 100644 crates/nu-command/src/debug/profile.rs diff --git a/crates/nu-cmd-extra/src/extra/formats/to/html.rs b/crates/nu-cmd-extra/src/extra/formats/to/html.rs index 5c203899a..f724f32f3 100644 --- a/crates/nu-cmd-extra/src/extra/formats/to/html.rs +++ b/crates/nu-cmd-extra/src/extra/formats/to/html.rs @@ -289,11 +289,9 @@ fn to_html( }) .collect(); return Ok( - Value::list(result, head).into_pipeline_data_with_metadata(Box::new( - PipelineMetadata { - data_source: DataSource::HtmlThemes, - }, - )), + Value::list(result, head).into_pipeline_data_with_metadata(PipelineMetadata { + data_source: DataSource::HtmlThemes, + }), ); } else { let theme_span = match &theme { diff --git a/crates/nu-command/src/debug/metadata.rs b/crates/nu-command/src/debug/metadata.rs index 9e3231d0d..56b56dcc1 100644 --- a/crates/nu-command/src/debug/metadata.rs +++ b/crates/nu-command/src/debug/metadata.rs @@ -55,36 +55,30 @@ impl Command for Metadata { let origin = stack.get_var_with_origin(*var_id, *span)?; Ok( - build_metadata_record(&origin, input.metadata().as_deref(), head) + build_metadata_record(&origin, input.metadata().as_ref(), head) .into_pipeline_data(), ) } _ => { let val: Value = call.req(engine_state, stack, 0)?; - Ok( - build_metadata_record(&val, input.metadata().as_deref(), head) - .into_pipeline_data(), - ) + Ok(build_metadata_record(&val, input.metadata().as_ref(), head) + .into_pipeline_data()) } } } else { let val: Value = call.req(engine_state, stack, 0)?; - Ok( - build_metadata_record(&val, input.metadata().as_deref(), head) - .into_pipeline_data(), - ) + Ok(build_metadata_record(&val, input.metadata().as_ref(), head) + .into_pipeline_data()) } } Some(_) => { let val: Value = call.req(engine_state, stack, 0)?; - Ok( - build_metadata_record(&val, input.metadata().as_deref(), head) - .into_pipeline_data(), - ) + Ok(build_metadata_record(&val, input.metadata().as_ref(), head) + .into_pipeline_data()) } None => { let mut record = Record::new(); - if let Some(x) = input.metadata().as_deref() { + if let Some(x) = input.metadata().as_ref() { match x { PipelineMetadata { data_source: DataSource::Ls, @@ -92,9 +86,6 @@ impl Command for Metadata { PipelineMetadata { data_source: DataSource::HtmlThemes, } => record.push("source", Value::string("into html --list", head)), - PipelineMetadata { - data_source: DataSource::Profiling(values), - } => record.push("profiling", Value::list(values.clone(), head)), } } @@ -142,9 +133,6 @@ fn build_metadata_record(arg: &Value, metadata: Option<&PipelineMetadata>, head: PipelineMetadata { data_source: DataSource::HtmlThemes, } => record.push("source", Value::string("into html --list", head)), - PipelineMetadata { - data_source: DataSource::Profiling(values), - } => record.push("profiling", Value::list(values.clone(), head)), } } diff --git a/crates/nu-command/src/debug/mod.rs b/crates/nu-command/src/debug/mod.rs index 32c09da46..aa8e84e61 100644 --- a/crates/nu-command/src/debug/mod.rs +++ b/crates/nu-command/src/debug/mod.rs @@ -5,7 +5,6 @@ mod info; mod inspect; mod inspect_table; mod metadata; -mod profile; mod timeit; mod view; mod view_files; @@ -19,7 +18,6 @@ pub use info::DebugInfo; pub use inspect::Inspect; pub use inspect_table::build_table; pub use metadata::Metadata; -pub use profile::Profile; pub use timeit::TimeIt; pub use view::View; pub use view_files::ViewFiles; diff --git a/crates/nu-command/src/debug/profile.rs b/crates/nu-command/src/debug/profile.rs deleted file mode 100644 index efef90101..000000000 --- a/crates/nu-command/src/debug/profile.rs +++ /dev/null @@ -1,102 +0,0 @@ -use nu_engine::{eval_block, CallExt}; -use nu_protocol::ast::Call; -use nu_protocol::engine::{Closure, Command, EngineState, Stack}; -use nu_protocol::{ - Category, DataSource, Example, IntoPipelineData, PipelineData, PipelineMetadata, Signature, - Spanned, SyntaxShape, Type, Value, -}; - -#[derive(Clone)] -pub struct Profile; - -impl Command for Profile { - fn name(&self) -> &str { - "profile" - } - - fn usage(&self) -> &str { - "Profile each pipeline element in a closure." - } - - fn extra_usage(&self) -> &str { - r#"The command collects run time of every pipeline element, recursively stepping into child closures -until a maximum depth. Optionally, it also collects the source code and intermediate values. - -Current known limitations are: -* profiling data from subexpressions is not tracked -* it does not step into loop iterations"# - } - - fn signature(&self) -> nu_protocol::Signature { - Signature::build("profile") - .required( - "closure", - SyntaxShape::Closure(Some(vec![SyntaxShape::Any])), - "the closure to run", - ) - .switch("source", "Collect source code in the report", Some('s')) - .switch("values", "Collect values in the report", Some('v')) - .named( - "max-depth", - SyntaxShape::Int, - "How many levels of blocks to step into (default: 1)", - Some('d'), - ) - .input_output_types(vec![(Type::Any, Type::Table(vec![]))]) - .allow_variants_without_examples(true) - .category(Category::Debug) - } - - fn run( - &self, - engine_state: &EngineState, - stack: &mut Stack, - call: &Call, - input: PipelineData, - ) -> Result { - let capture_block: Spanned = call.req(engine_state, stack, 0)?; - let block = engine_state.get_block(capture_block.item.block_id); - - let redirect_stdout = call.redirect_stdout; - let redirect_stderr = call.redirect_stderr; - - let mut stack = stack.captures_to_stack(&capture_block.item.captures); - - let input_val = input.into_value(call.head); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, input_val.clone()); - } - } - - let result = if let Some(PipelineMetadata { - data_source: DataSource::Profiling(values), - }) = eval_block( - engine_state, - &mut stack, - block, - input_val.into_pipeline_data(), - redirect_stdout, - redirect_stderr, - )? - .metadata() - .map(|m| *m) - { - Value::list(values, call.head) - } else { - Value::nothing(call.head) - }; - - Ok(result.into_pipeline_data()) - } - - fn examples(&self) -> Vec { - vec![Example { - description: - "Profile some code, stepping into the `spam` command and collecting source.", - example: r#"def spam [] { "spam" }; profile {|| spam | str length } --max-depth 2 --source"#, - result: None, - }] - } -} diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index 1029cff26..0bd8cc41a 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -138,7 +138,6 @@ pub fn add_shell_command_context(mut engine_state: EngineState) -> EngineState { Explain, Inspect, Metadata, - Profile, TimeIt, View, ViewFiles, diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 8d356b5c3..5587a4617 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -266,9 +266,9 @@ impl Command for Ls { _ => Some(Value::nothing(call_span)), }) .into_pipeline_data_with_metadata( - Box::new(PipelineMetadata { + PipelineMetadata { data_source: DataSource::Ls, - }), + }, engine_state.ctrlc.clone(), )) } diff --git a/crates/nu-command/src/filters/uniq.rs b/crates/nu-command/src/filters/uniq.rs index 964d3e7c4..c2d5ed495 100644 --- a/crates/nu-command/src/filters/uniq.rs +++ b/crates/nu-command/src/filters/uniq.rs @@ -258,7 +258,7 @@ pub fn uniq( call: &Call, input: Vec, item_mapper: Box ValueCounter>, - metadata: Option>, + metadata: Option, ) -> Result { let ctrlc = engine_state.ctrlc.clone(); let head = call.head; diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 16b387f88..143b46e47 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -444,11 +444,11 @@ fn handle_row_stream( input: CmdInput<'_>, cfg: TableConfig, stream: ListStream, - metadata: Option>, + metadata: Option, ) -> Result { let ctrlc = input.engine_state.ctrlc.clone(); - let stream = match metadata.as_deref() { + let stream = match metadata.as_ref() { // First, `ls` sources: Some(PipelineMetadata { data_source: DataSource::Ls, diff --git a/crates/nu-explore/src/nu_common/value.rs b/crates/nu-explore/src/nu_common/value.rs index cecab073d..0538b25b1 100644 --- a/crates/nu-explore/src/nu_common/value.rs +++ b/crates/nu-explore/src/nu_common/value.rs @@ -19,7 +19,7 @@ pub fn collect_pipeline(input: PipelineData) -> (Vec, Vec>) { metadata, span, .. - } => collect_external_stream(stdout, stderr, exit_code, metadata.map(|m| *m), span), + } => collect_external_stream(stdout, stderr, exit_code, metadata, span), } } diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index cf96eca13..183bf8b29 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -40,16 +40,14 @@ const LINE_ENDING_PATTERN: &[char] = &['\r', '\n']; /// Nushell. #[derive(Debug)] pub enum PipelineData { - // Note: the PipelineMetadata is boxed everywhere because the DataSource::Profiling caused - // stack overflow on Windows CI when testing virtualenv - Value(Value, Option>), - ListStream(ListStream, Option>), + Value(Value, Option), + ListStream(ListStream, Option), ExternalStream { stdout: Option, stderr: Option, exit_code: Option, span: Span, - metadata: Option>, + metadata: Option, trim_end_newline: bool, }, Empty, @@ -64,11 +62,10 @@ pub struct PipelineMetadata { pub enum DataSource { Ls, HtmlThemes, - Profiling(Vec), } impl PipelineData { - pub fn new_with_metadata(metadata: Option>, span: Span) -> PipelineData { + pub fn new_with_metadata(metadata: Option, span: Span) -> PipelineData { PipelineData::Value(Value::nothing(span), metadata) } @@ -93,7 +90,7 @@ impl PipelineData { PipelineData::Empty } - pub fn metadata(&self) -> Option> { + pub fn metadata(&self) -> Option { match self { PipelineData::ListStream(_, x) => x.clone(), PipelineData::ExternalStream { metadata: x, .. } => x.clone(), @@ -102,7 +99,7 @@ impl PipelineData { } } - pub fn set_metadata(mut self, metadata: Option>) -> Self { + pub fn set_metadata(mut self, metadata: Option) -> Self { match &mut self { PipelineData::ListStream(_, x) => *x = metadata, PipelineData::ExternalStream { metadata: x, .. } => *x = metadata, @@ -354,7 +351,7 @@ impl PipelineData { pub fn collect_string_strict( self, span: Span, - ) -> Result<(String, Span, Option>), ShellError> { + ) -> Result<(String, Span, Option), ShellError> { match self { PipelineData::Empty => Ok((String::new(), span, None)), PipelineData::Value(Value::String { val, .. }, metadata) => Ok((val, span, metadata)), @@ -926,7 +923,7 @@ pub trait IntoPipelineData { fn into_pipeline_data_with_metadata( self, - metadata: impl Into>>, + metadata: impl Into>, ) -> PipelineData; } @@ -940,7 +937,7 @@ where fn into_pipeline_data_with_metadata( self, - metadata: impl Into>>, + metadata: impl Into>, ) -> PipelineData { PipelineData::Value(self.into(), metadata.into()) } @@ -950,7 +947,7 @@ pub trait IntoInterruptiblePipelineData { fn into_pipeline_data(self, ctrlc: Option>) -> PipelineData; fn into_pipeline_data_with_metadata( self, - metadata: impl Into>>, + metadata: impl Into>, ctrlc: Option>, ) -> PipelineData; } @@ -973,7 +970,7 @@ where fn into_pipeline_data_with_metadata( self, - metadata: impl Into>>, + metadata: impl Into>, ctrlc: Option>, ) -> PipelineData { PipelineData::ListStream(