From 168835ecd2a44362b47b6165ec4bc843fe57fddc Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Thu, 1 Aug 2024 15:21:39 -0400 Subject: [PATCH 01/11] `random chars` doc clarifications (#13511) # Description Clarified `random chars` help/doc: * Default string length in absence of a `--length` arg is 25 * Characters are *"uniformly distributed over ASCII letters and numbers: a-z, A-Z and 0-9"* (copied from the [`rand` crate doc](https://docs.rs/rand/latest/rand/distributions/struct.Alphanumeric.html). # User-Facing Changes Help/Doc only --- crates/nu-command/src/random/chars.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/nu-command/src/random/chars.rs b/crates/nu-command/src/random/chars.rs index fe1e6bcdc4..aea27129b9 100644 --- a/crates/nu-command/src/random/chars.rs +++ b/crates/nu-command/src/random/chars.rs @@ -19,12 +19,17 @@ impl Command for SubCommand { Signature::build("random chars") .input_output_types(vec![(Type::Nothing, Type::String)]) .allow_variants_without_examples(true) - .named("length", SyntaxShape::Int, "Number of chars", Some('l')) + .named( + "length", + SyntaxShape::Int, + "Number of chars (default 25)", + Some('l'), + ) .category(Category::Random) } fn usage(&self) -> &str { - "Generate random chars." + "Generate random chars uniformly distributed over ASCII letters and numbers: a-z, A-Z and 0-9." } fn search_terms(&self) -> Vec<&str> { @@ -44,7 +49,7 @@ impl Command for SubCommand { fn examples(&self) -> Vec { vec![ Example { - description: "Generate random chars", + description: "Generate a string with 25 random chars", example: "random chars", result: None, }, From ca8eb856e85b4a0e7a84dd6f835b6ef6737139c5 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Thu, 1 Aug 2024 16:22:25 -0400 Subject: [PATCH 02/11] Doc and examples for multi-dot directory traversal (#13513) # Description With this PR, we should be able to close https://github.com/nushell/nushell.github.io/issues/1225 Help/doc/examples updated for: * `cd` to show multi-dot traversal * `cd` to show implicit `cd` with bare directory path * Fixed/clarified another example that mentioned `$OLDPATH` while I was in there * `mv` and `cp` examples for multi-dot traversal * Updated `cp` examples to use more consistent (and clear) filenames # User-Facing Changes Help/doc only # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting N/A --- crates/nu-command/src/filesystem/cd.rs | 12 +++++++++++- crates/nu-command/src/filesystem/ucp.rs | 11 ++++++++--- crates/nu-command/src/filesystem/umv.rs | 5 +++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/nu-command/src/filesystem/cd.rs b/crates/nu-command/src/filesystem/cd.rs index ba91e6d0ca..87071d2888 100644 --- a/crates/nu-command/src/filesystem/cd.rs +++ b/crates/nu-command/src/filesystem/cd.rs @@ -134,7 +134,7 @@ impl Command for Cd { result: None, }, Example { - description: "Change to the previous working directory ($OLDPWD)", + description: r#"Change to the previous working directory (same as "cd $env.OLDPWD")"#, example: r#"cd -"#, result: None, }, @@ -143,6 +143,16 @@ impl Command for Cd { example: r#"def --env gohome [] { cd ~ }"#, result: None, }, + Example { + description: "Move two directories up in the tree (the parent directory's parent). Additional dots can be added for additional levels.", + example: r#"cd ..."#, + result: None, + }, + Example { + description: "The cd command itself is often optional. Simply entering a path to a directory will cd to it.", + example: r#"/home"#, + result: None, + }, ] } } diff --git a/crates/nu-command/src/filesystem/ucp.rs b/crates/nu-command/src/filesystem/ucp.rs index 447751220d..6077278c18 100644 --- a/crates/nu-command/src/filesystem/ucp.rs +++ b/crates/nu-command/src/filesystem/ucp.rs @@ -86,17 +86,22 @@ impl Command for UCp { }, Example { description: "Copy only if source file is newer than target file", - example: "cp -u a b", + example: "cp -u myfile newfile", result: None, }, Example { description: "Copy file preserving mode and timestamps attributes", - example: "cp --preserve [ mode timestamps ] a b", + example: "cp --preserve [ mode timestamps ] myfile newfile", result: None, }, Example { description: "Copy file erasing all attributes", - example: "cp --preserve [] a b", + example: "cp --preserve [] myfile newfile", + result: None, + }, + Example { + description: "Copy file to a directory three levels above its current location", + example: "cp myfile ....", result: None, }, ] diff --git a/crates/nu-command/src/filesystem/umv.rs b/crates/nu-command/src/filesystem/umv.rs index 12d95e015b..9f68a6697f 100644 --- a/crates/nu-command/src/filesystem/umv.rs +++ b/crates/nu-command/src/filesystem/umv.rs @@ -40,6 +40,11 @@ impl Command for UMv { example: "mv *.txt my/subdirectory", result: None, }, + Example { + description: r#"Move a file into the "my" directory two levels up in the directory tree"#, + example: "mv test.txt .../my/", + result: None, + }, ] } From d081e3386f2f741f4ec88b107ee208f9a5b52b3f Mon Sep 17 00:00:00 2001 From: Jack Wright <56345+ayax79@users.noreply.github.com> Date: Fri, 2 Aug 2024 11:01:20 -0700 Subject: [PATCH 03/11] Make pipeline metadata available to plugins (#13495) # Description Fixes an issue with pipeline metadata not being passed to plugins. --- crates/nu-plugin-core/src/interface/mod.rs | 28 ++++-- crates/nu-plugin-core/src/interface/tests.rs | 33 ++++++- .../nu-plugin-core/src/serializers/tests.rs | 18 ++-- .../nu-plugin-engine/src/interface/tests.rs | 68 ++++++------- crates/nu-plugin-protocol/src/lib.rs | 45 ++++++++- .../nu-plugin/src/plugin/interface/tests.rs | 35 +++---- crates/nu-protocol/src/pipeline/metadata.rs | 6 +- .../nu_plugin_nu_example.nu | 8 +- .../nu_plugin_python_example.py | 95 ++++++++++--------- crates/nu_plugin_stress_internals/src/main.rs | 2 +- 10 files changed, 208 insertions(+), 130 deletions(-) diff --git a/crates/nu-plugin-core/src/interface/mod.rs b/crates/nu-plugin-core/src/interface/mod.rs index 6ce33cae4c..de04bd6387 100644 --- a/crates/nu-plugin-core/src/interface/mod.rs +++ b/crates/nu-plugin-core/src/interface/mod.rs @@ -177,16 +177,19 @@ pub trait InterfaceManager { ) -> Result { self.prepare_pipeline_data(match header { PipelineDataHeader::Empty => PipelineData::Empty, - PipelineDataHeader::Value(value) => PipelineData::Value(value, None), + PipelineDataHeader::Value(value, metadata) => PipelineData::Value(value, metadata), PipelineDataHeader::ListStream(info) => { let handle = self.stream_manager().get_handle(); let reader = handle.read_stream(info.id, self.get_interface())?; - ListStream::new(reader, info.span, signals.clone()).into() + let ls = ListStream::new(reader, info.span, signals.clone()); + PipelineData::ListStream(ls, info.metadata) } PipelineDataHeader::ByteStream(info) => { let handle = self.stream_manager().get_handle(); let reader = handle.read_stream(info.id, self.get_interface())?; - ByteStream::from_result_iter(reader, info.span, signals.clone(), info.type_).into() + let bs = + ByteStream::from_result_iter(reader, info.span, signals.clone(), info.type_); + PipelineData::ByteStream(bs, info.metadata) } }) } @@ -248,26 +251,33 @@ pub trait Interface: Clone + Send { Ok::<_, ShellError>((id, writer)) }; match self.prepare_pipeline_data(data, context)? { - PipelineData::Value(value, ..) => { - Ok((PipelineDataHeader::Value(value), PipelineDataWriter::None)) - } + PipelineData::Value(value, metadata) => Ok(( + PipelineDataHeader::Value(value, metadata), + PipelineDataWriter::None, + )), PipelineData::Empty => Ok((PipelineDataHeader::Empty, PipelineDataWriter::None)), - PipelineData::ListStream(stream, ..) => { + PipelineData::ListStream(stream, metadata) => { let (id, writer) = new_stream(LIST_STREAM_HIGH_PRESSURE)?; Ok(( PipelineDataHeader::ListStream(ListStreamInfo { id, span: stream.span(), + metadata, }), PipelineDataWriter::ListStream(writer, stream), )) } - PipelineData::ByteStream(stream, ..) => { + PipelineData::ByteStream(stream, metadata) => { let span = stream.span(); let type_ = stream.type_(); if let Some(reader) = stream.reader() { let (id, writer) = new_stream(RAW_STREAM_HIGH_PRESSURE)?; - let header = PipelineDataHeader::ByteStream(ByteStreamInfo { id, span, type_ }); + let header = PipelineDataHeader::ByteStream(ByteStreamInfo { + id, + span, + type_, + metadata, + }); Ok((header, PipelineDataWriter::ByteStream(writer, reader))) } else { Ok((PipelineDataHeader::Empty, PipelineDataWriter::None)) diff --git a/crates/nu-plugin-core/src/interface/tests.rs b/crates/nu-plugin-core/src/interface/tests.rs index cb28692706..6b4ad85d7f 100644 --- a/crates/nu-plugin-core/src/interface/tests.rs +++ b/crates/nu-plugin-core/src/interface/tests.rs @@ -137,10 +137,16 @@ fn read_pipeline_data_empty() -> Result<(), ShellError> { fn read_pipeline_data_value() -> Result<(), ShellError> { let manager = TestInterfaceManager::new(&TestCase::new()); let value = Value::test_int(4); - let header = PipelineDataHeader::Value(value.clone()); - + let metadata = Some(PipelineMetadata { + data_source: DataSource::FilePath("/test/path".into()), + content_type: None, + }); + let header = PipelineDataHeader::Value(value.clone(), metadata.clone()); match manager.read_pipeline_data(header, &Signals::empty())? { - PipelineData::Value(read_value, ..) => assert_eq!(value, read_value), + PipelineData::Value(read_value, read_metadata) => { + assert_eq!(value, read_value); + assert_eq!(metadata, read_metadata); + } PipelineData::ListStream(..) => panic!("unexpected ListStream"), PipelineData::ByteStream(..) => panic!("unexpected ByteStream"), PipelineData::Empty => panic!("unexpected Empty"), @@ -161,9 +167,15 @@ fn read_pipeline_data_list_stream() -> Result<(), ShellError> { } test.add(StreamMessage::End(7)); + let metadata = Some(PipelineMetadata { + data_source: DataSource::None, + content_type: Some("foobar".into()), + }); + let header = PipelineDataHeader::ListStream(ListStreamInfo { id: 7, span: Span::test_data(), + metadata, }); let pipe = manager.read_pipeline_data(header, &Signals::empty())?; @@ -204,10 +216,17 @@ fn read_pipeline_data_byte_stream() -> Result<(), ShellError> { test.add(StreamMessage::End(12)); let test_span = Span::new(10, 13); + + let metadata = Some(PipelineMetadata { + data_source: DataSource::None, + content_type: Some("foobar".into()), + }); + let header = PipelineDataHeader::ByteStream(ByteStreamInfo { id: 12, span: test_span, type_: ByteStreamType::Unknown, + metadata, }); let pipe = manager.read_pipeline_data(header, &Signals::empty())?; @@ -251,9 +270,15 @@ fn read_pipeline_data_byte_stream() -> Result<(), ShellError> { #[test] fn read_pipeline_data_prepared_properly() -> Result<(), ShellError> { let manager = TestInterfaceManager::new(&TestCase::new()); + let metadata = Some(PipelineMetadata { + data_source: DataSource::None, + content_type: Some("foobar".into()), + }); + let header = PipelineDataHeader::ListStream(ListStreamInfo { id: 0, span: Span::test_data(), + metadata, }); match manager.read_pipeline_data(header, &Signals::empty())? { PipelineData::ListStream(_, meta) => match meta { @@ -301,7 +326,7 @@ fn write_pipeline_data_value() -> Result<(), ShellError> { interface.init_write_pipeline_data(PipelineData::Value(value.clone(), None), &())?; match header { - PipelineDataHeader::Value(read_value) => assert_eq!(value, read_value), + PipelineDataHeader::Value(read_value, _) => assert_eq!(value, read_value), _ => panic!("unexpected header: {header:?}"), } diff --git a/crates/nu-plugin-core/src/serializers/tests.rs b/crates/nu-plugin-core/src/serializers/tests.rs index 06d47ffc14..5d0c07cceb 100644 --- a/crates/nu-plugin-core/src/serializers/tests.rs +++ b/crates/nu-plugin-core/src/serializers/tests.rs @@ -6,7 +6,8 @@ macro_rules! generate_tests { StreamData, }; use nu_protocol::{ - LabeledError, PluginSignature, Signature, Span, Spanned, SyntaxShape, Value, + DataSource, LabeledError, PipelineMetadata, PluginSignature, Signature, Span, Spanned, + SyntaxShape, Value, }; #[test] @@ -123,10 +124,15 @@ macro_rules! generate_tests { )], }; + let metadata = Some(PipelineMetadata { + data_source: DataSource::None, + content_type: Some("foobar".into()), + }); + let plugin_call = PluginCall::Run(CallInfo { name: name.clone(), call: call.clone(), - input: PipelineDataHeader::Value(input.clone()), + input: PipelineDataHeader::Value(input.clone(), metadata.clone()), }); let plugin_input = PluginInput::Call(1, plugin_call); @@ -144,7 +150,7 @@ macro_rules! generate_tests { match returned { PluginInput::Call(1, PluginCall::Run(call_info)) => { assert_eq!(name, call_info.name); - assert_eq!(PipelineDataHeader::Value(input), call_info.input); + assert_eq!(PipelineDataHeader::Value(input, metadata), call_info.input); assert_eq!(call.head, call_info.call.head); assert_eq!(call.positional.len(), call_info.call.positional.len()); @@ -305,7 +311,7 @@ macro_rules! generate_tests { match returned { PluginOutput::CallResponse( 4, - PluginCallResponse::PipelineData(PipelineDataHeader::Value(returned_value)), + PluginCallResponse::PipelineData(PipelineDataHeader::Value(returned_value, _)), ) => { assert_eq!(value, returned_value) } @@ -325,7 +331,7 @@ macro_rules! generate_tests { span, ); - let response = PluginCallResponse::PipelineData(PipelineDataHeader::Value(value)); + let response = PluginCallResponse::PipelineData(PipelineDataHeader::value(value)); let output = PluginOutput::CallResponse(5, response); let encoder = $encoder; @@ -341,7 +347,7 @@ macro_rules! generate_tests { match returned { PluginOutput::CallResponse( 5, - PluginCallResponse::PipelineData(PipelineDataHeader::Value(returned_value)), + PluginCallResponse::PipelineData(PipelineDataHeader::Value(returned_value, _)), ) => { assert_eq!(span, returned_value.span()); diff --git a/crates/nu-plugin-engine/src/interface/tests.rs b/crates/nu-plugin-engine/src/interface/tests.rs index 2bab67f25f..31828973d3 100644 --- a/crates/nu-plugin-engine/src/interface/tests.rs +++ b/crates/nu-plugin-engine/src/interface/tests.rs @@ -17,8 +17,9 @@ use nu_plugin_protocol::{ use nu_protocol::{ ast::{Math, Operator}, engine::Closure, - ByteStreamType, CustomValue, IntoInterruptiblePipelineData, IntoSpanned, PipelineData, - PluginMetadata, PluginSignature, ShellError, Signals, Span, Spanned, Value, + ByteStreamType, CustomValue, DataSource, IntoInterruptiblePipelineData, IntoSpanned, + PipelineData, PipelineMetadata, PluginMetadata, PluginSignature, ShellError, Signals, Span, + Spanned, Value, }; use serde::{Deserialize, Serialize}; use std::{ @@ -52,10 +53,7 @@ fn manager_consume_all_exits_after_streams_and_interfaces_are_dropped() -> Resul // Create a stream... let stream = manager.read_pipeline_data( - PipelineDataHeader::ListStream(ListStreamInfo { - id: 0, - span: Span::test_data(), - }), + PipelineDataHeader::list_stream(ListStreamInfo::new(0, Span::test_data())), &Signals::empty(), )?; @@ -108,10 +106,7 @@ fn manager_consume_all_propagates_io_error_to_readers() -> Result<(), ShellError test.set_read_error(test_io_error()); let stream = manager.read_pipeline_data( - PipelineDataHeader::ListStream(ListStreamInfo { - id: 0, - span: Span::test_data(), - }), + PipelineDataHeader::list_stream(ListStreamInfo::new(0, Span::test_data())), &Signals::empty(), )?; @@ -154,11 +149,11 @@ fn manager_consume_all_propagates_message_error_to_readers() -> Result<(), Shell test.add(invalid_output()); let stream = manager.read_pipeline_data( - PipelineDataHeader::ByteStream(ByteStreamInfo { - id: 0, - span: Span::test_data(), - type_: ByteStreamType::Unknown, - }), + PipelineDataHeader::byte_stream(ByteStreamInfo::new( + 0, + Span::test_data(), + ByteStreamType::Unknown, + )), &Signals::empty(), )?; @@ -331,10 +326,10 @@ fn manager_consume_call_response_forwards_to_subscriber_with_pipeline_data( manager.consume(PluginOutput::CallResponse( 0, - PluginCallResponse::PipelineData(PipelineDataHeader::ListStream(ListStreamInfo { - id: 0, - span: Span::test_data(), - })), + PluginCallResponse::PipelineData(PipelineDataHeader::list_stream(ListStreamInfo::new( + 0, + Span::test_data(), + ))), ))?; for i in 0..2 { @@ -375,18 +370,18 @@ fn manager_consume_call_response_registers_streams() -> Result<(), ShellError> { // Check list streams, byte streams manager.consume(PluginOutput::CallResponse( 0, - PluginCallResponse::PipelineData(PipelineDataHeader::ListStream(ListStreamInfo { - id: 0, - span: Span::test_data(), - })), + PluginCallResponse::PipelineData(PipelineDataHeader::list_stream(ListStreamInfo::new( + 0, + Span::test_data(), + ))), ))?; manager.consume(PluginOutput::CallResponse( 1, - PluginCallResponse::PipelineData(PipelineDataHeader::ByteStream(ByteStreamInfo { - id: 1, - span: Span::test_data(), - type_: ByteStreamType::Unknown, - })), + PluginCallResponse::PipelineData(PipelineDataHeader::byte_stream(ByteStreamInfo::new( + 1, + Span::test_data(), + ByteStreamType::Unknown, + ))), ))?; // ListStream should have one @@ -442,10 +437,7 @@ fn manager_consume_engine_call_forwards_to_subscriber_with_pipeline_data() -> Re span: Span::test_data(), }, positional: vec![], - input: PipelineDataHeader::ListStream(ListStreamInfo { - id: 2, - span: Span::test_data(), - }), + input: PipelineDataHeader::list_stream(ListStreamInfo::new(2, Span::test_data())), redirect_stdout: false, redirect_stderr: false, }, @@ -806,6 +798,11 @@ fn interface_write_plugin_call_writes_run_with_value_input() -> Result<(), Shell let manager = test.plugin("test"); let interface = manager.get_interface(); + let metadata0 = PipelineMetadata { + data_source: DataSource::None, + content_type: Some("baz".into()), + }; + let result = interface.write_plugin_call( PluginCall::Run(CallInfo { name: "foo".into(), @@ -814,7 +811,7 @@ fn interface_write_plugin_call_writes_run_with_value_input() -> Result<(), Shell positional: vec![], named: vec![], }, - input: PipelineData::Value(Value::test_int(-1), None), + input: PipelineData::Value(Value::test_int(-1), Some(metadata0.clone())), }), None, )?; @@ -826,7 +823,10 @@ fn interface_write_plugin_call_writes_run_with_value_input() -> Result<(), Shell PluginCall::Run(CallInfo { name, input, .. }) => { assert_eq!("foo", name); match input { - PipelineDataHeader::Value(value) => assert_eq!(-1, value.as_int()?), + PipelineDataHeader::Value(value, metadata) => { + assert_eq!(-1, value.as_int()?); + assert_eq!(metadata0, metadata.expect("there should be metadata")); + } _ => panic!("unexpected input header: {input:?}"), } } diff --git a/crates/nu-plugin-protocol/src/lib.rs b/crates/nu-plugin-protocol/src/lib.rs index 1a2bbd99ff..00a18beca9 100644 --- a/crates/nu-plugin-protocol/src/lib.rs +++ b/crates/nu-plugin-protocol/src/lib.rs @@ -23,7 +23,7 @@ pub mod test_util; use nu_protocol::{ ast::Operator, engine::Closure, ByteStreamType, Config, DeclId, LabeledError, PipelineData, - PluginMetadata, PluginSignature, ShellError, Span, Spanned, Value, + PipelineMetadata, PluginMetadata, PluginSignature, ShellError, Span, Spanned, Value, }; use nu_utils::SharedCow; use serde::{Deserialize, Serialize}; @@ -78,7 +78,7 @@ pub enum PipelineDataHeader { /// No input Empty, /// A single value - Value(Value), + Value(Value, Option), /// Initiate [`nu_protocol::PipelineData::ListStream`]. /// /// Items are sent via [`StreamData`] @@ -94,11 +94,23 @@ impl PipelineDataHeader { pub fn stream_id(&self) -> Option { match self { PipelineDataHeader::Empty => None, - PipelineDataHeader::Value(_) => None, + PipelineDataHeader::Value(_, _) => None, PipelineDataHeader::ListStream(info) => Some(info.id), PipelineDataHeader::ByteStream(info) => Some(info.id), } } + + pub fn value(value: Value) -> Self { + PipelineDataHeader::Value(value, None) + } + + pub fn list_stream(info: ListStreamInfo) -> Self { + PipelineDataHeader::ListStream(info) + } + + pub fn byte_stream(info: ByteStreamInfo) -> Self { + PipelineDataHeader::ByteStream(info) + } } /// Additional information about list (value) streams @@ -106,6 +118,18 @@ impl PipelineDataHeader { pub struct ListStreamInfo { pub id: StreamId, pub span: Span, + pub metadata: Option, +} + +impl ListStreamInfo { + /// Create a new `ListStreamInfo` with a unique ID + pub fn new(id: StreamId, span: Span) -> Self { + ListStreamInfo { + id, + span, + metadata: None, + } + } } /// Additional information about byte streams @@ -115,6 +139,19 @@ pub struct ByteStreamInfo { pub span: Span, #[serde(rename = "type")] pub type_: ByteStreamType, + pub metadata: Option, +} + +impl ByteStreamInfo { + /// Create a new `ByteStreamInfo` with a unique ID + pub fn new(id: StreamId, span: Span, type_: ByteStreamType) -> Self { + ByteStreamInfo { + id, + span, + type_, + metadata: None, + } + } } /// Calls that a plugin can execute. The type parameter determines the input type. @@ -344,7 +381,7 @@ impl PluginCallResponse { if value.is_nothing() { PluginCallResponse::PipelineData(PipelineDataHeader::Empty) } else { - PluginCallResponse::PipelineData(PipelineDataHeader::Value(value)) + PluginCallResponse::PipelineData(PipelineDataHeader::value(value)) } } } diff --git a/crates/nu-plugin/src/plugin/interface/tests.rs b/crates/nu-plugin/src/plugin/interface/tests.rs index 7065c6f64d..f8002b2c6d 100644 --- a/crates/nu-plugin/src/plugin/interface/tests.rs +++ b/crates/nu-plugin/src/plugin/interface/tests.rs @@ -55,10 +55,7 @@ fn manager_consume_all_exits_after_streams_and_interfaces_are_dropped() -> Resul // Create a stream... let stream = manager.read_pipeline_data( - PipelineDataHeader::ListStream(ListStreamInfo { - id: 0, - span: Span::test_data(), - }), + PipelineDataHeader::list_stream(ListStreamInfo::new(0, Span::test_data())), &Signals::empty(), )?; @@ -111,10 +108,7 @@ fn manager_consume_all_propagates_io_error_to_readers() -> Result<(), ShellError test.set_read_error(test_io_error()); let stream = manager.read_pipeline_data( - PipelineDataHeader::ListStream(ListStreamInfo { - id: 0, - span: Span::test_data(), - }), + PipelineDataHeader::list_stream(ListStreamInfo::new(0, Span::test_data())), &Signals::empty(), )?; @@ -157,11 +151,11 @@ fn manager_consume_all_propagates_message_error_to_readers() -> Result<(), Shell test.add(invalid_input()); let stream = manager.read_pipeline_data( - PipelineDataHeader::ByteStream(ByteStreamInfo { - id: 0, - span: Span::test_data(), - type_: ByteStreamType::Unknown, - }), + PipelineDataHeader::byte_stream(ByteStreamInfo::new( + 0, + Span::test_data(), + ByteStreamType::Unknown, + )), &Signals::empty(), )?; @@ -414,10 +408,7 @@ fn manager_consume_call_run_forwards_to_receiver_with_pipeline_data() -> Result< positional: vec![], named: vec![], }, - input: PipelineDataHeader::ListStream(ListStreamInfo { - id: 6, - span: Span::test_data(), - }), + input: PipelineDataHeader::list_stream(ListStreamInfo::new(6, Span::test_data())), }), ))?; @@ -556,10 +547,10 @@ fn manager_consume_engine_call_response_forwards_to_subscriber_with_pipeline_dat manager.consume(PluginInput::EngineCallResponse( 0, - EngineCallResponse::PipelineData(PipelineDataHeader::ListStream(ListStreamInfo { - id: 0, - span: Span::test_data(), - })), + EngineCallResponse::PipelineData(PipelineDataHeader::list_stream(ListStreamInfo::new( + 0, + Span::test_data(), + ))), ))?; for i in 0..2 { @@ -707,7 +698,7 @@ fn interface_write_response_with_value() -> Result<(), ShellError> { assert_eq!(33, id, "id"); match response { PluginCallResponse::PipelineData(header) => match header { - PipelineDataHeader::Value(value) => assert_eq!(6, value.as_int()?), + PipelineDataHeader::Value(value, _) => assert_eq!(6, value.as_int()?), _ => panic!("unexpected pipeline data header: {header:?}"), }, _ => panic!("unexpected response: {response:?}"), diff --git a/crates/nu-protocol/src/pipeline/metadata.rs b/crates/nu-protocol/src/pipeline/metadata.rs index 1ed9222f46..c1f00769b3 100644 --- a/crates/nu-protocol/src/pipeline/metadata.rs +++ b/crates/nu-protocol/src/pipeline/metadata.rs @@ -1,7 +1,9 @@ use std::path::PathBuf; +use serde::{Deserialize, Serialize}; + /// Metadata that is valid for the whole [`PipelineData`](crate::PipelineData) -#[derive(Debug, Default, Clone)] +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct PipelineMetadata { pub data_source: DataSource, pub content_type: Option, @@ -27,7 +29,7 @@ impl PipelineMetadata { /// /// This can either be a particular family of commands (useful so downstream commands can adjust /// the presentation e.g. `Ls`) or the opened file to protect against overwrite-attempts properly. -#[derive(Debug, Default, Clone)] +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub enum DataSource { Ls, HtmlThemes, diff --git a/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu b/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu index 68f87eaca0..3bbf5b965c 100755 --- a/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu +++ b/crates/nu_plugin_nu_example/nu_plugin_nu_example.nu @@ -7,7 +7,7 @@ # language without adding any extra dependencies to our tests. const NUSHELL_VERSION = "0.96.2" -const PLUGIN_VERSION = "0.1.0" # bump if you change commands! +const PLUGIN_VERSION = "0.1.1" # bump if you change commands! def main [--stdio] { if ($stdio) { @@ -133,7 +133,7 @@ def process_call [ # Create a Value of type List that will be encoded and sent to Nushell let value = { - Value: { + Value: [{ List: { vals: (0..9 | each { |x| { @@ -157,7 +157,7 @@ def process_call [ }), span: $span } - } + }, null] } write_response $id { PipelineData: $value } @@ -265,4 +265,4 @@ def start_plugin [] { }) | each { from json | handle_input } | ignore -} \ No newline at end of file +} diff --git a/crates/nu_plugin_python/nu_plugin_python_example.py b/crates/nu_plugin_python/nu_plugin_python_example.py index 5a0357e9ca..55d3204a34 100755 --- a/crates/nu_plugin_python/nu_plugin_python_example.py +++ b/crates/nu_plugin_python/nu_plugin_python_example.py @@ -28,7 +28,7 @@ import json NUSHELL_VERSION = "0.96.2" -PLUGIN_VERSION = "0.1.0" # bump if you change commands! +PLUGIN_VERSION = "0.1.1" # bump if you change commands! def signatures(): @@ -125,31 +125,31 @@ def process_call(id, plugin_call): span = plugin_call["call"]["head"] # Creates a Value of type List that will be encoded and sent to Nushell - def f(x, y): return { - "Int": { - "val": x * y, - "span": span - } - } + def f(x, y): + return {"Int": {"val": x * y, "span": span}} value = { - "Value": { - "List": { - "vals": [ - { - "Record": { - "val": { - "one": f(x, 0), - "two": f(x, 1), - "three": f(x, 2), - }, - "span": span + "Value": [ + { + "List": { + "vals": [ + { + "Record": { + "val": { + "one": f(x, 0), + "two": f(x, 1), + "three": f(x, 2), + }, + "span": span, + } } - } for x in range(0, 10) - ], - "span": span - } - } + for x in range(0, 10) + ], + "span": span, + } + }, + None, + ] } write_response(id, {"PipelineData": value}) @@ -172,7 +172,7 @@ def tell_nushell_hello(): "Hello": { "protocol": "nu-plugin", # always this value "version": NUSHELL_VERSION, - "features": [] + "features": [], } } sys.stdout.write(json.dumps(hello)) @@ -200,22 +200,26 @@ def write_error(id, text, span=None): Use this error format to send errors to nushell in response to a plugin call. The ID of the plugin call is required. """ - error = { - "Error": { - "msg": "ERROR from plugin", - "labels": [ - { - "text": text, - "span": span, - } - ], + error = ( + { + "Error": { + "msg": "ERROR from plugin", + "labels": [ + { + "text": text, + "span": span, + } + ], + } } - } if span is not None else { - "Error": { - "msg": "ERROR from plugin", - "help": text, + if span is not None + else { + "Error": { + "msg": "ERROR from plugin", + "help": text, + } } - } + ) write_response(id, error) @@ -230,11 +234,14 @@ def handle_input(input): elif "Call" in input: [id, plugin_call] = input["Call"] if plugin_call == "Metadata": - write_response(id, { - "Metadata": { - "version": PLUGIN_VERSION, - } - }) + write_response( + id, + { + "Metadata": { + "version": PLUGIN_VERSION, + } + }, + ) elif plugin_call == "Signature": write_response(id, signatures()) elif "Run" in plugin_call: @@ -258,4 +265,4 @@ if __name__ == "__main__": if len(sys.argv) == 2 and sys.argv[1] == "--stdio": plugin() else: - print("Run me from inside nushell!") \ No newline at end of file + print("Run me from inside nushell!") diff --git a/crates/nu_plugin_stress_internals/src/main.rs b/crates/nu_plugin_stress_internals/src/main.rs index 96023b44b7..79cbb92549 100644 --- a/crates/nu_plugin_stress_internals/src/main.rs +++ b/crates/nu_plugin_stress_internals/src/main.rs @@ -178,7 +178,7 @@ fn handle_message( id, { "PipelineData": { - "Value": return_value + "Value": [return_value, null] } } ] From ed82f9ee18f382557135336459b1d098e0406d39 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Fri, 2 Aug 2024 15:23:25 -0400 Subject: [PATCH 04/11] Clarify `default` command help (#13519) # Description Updates `default` command description to be more clear and adds an example for a missing values in a list-of-records. # User-Facing Changes Help/doc only # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting - Update `nothing` doc in Book to reference `default` per https://github.com/nushell/nushell.github.io/issues/1073 - This was a bit of a rabbit trail on the path to that update. ;-) --------- Co-authored-by: Stefan Holderbach --- crates/nu-command/src/filters/default.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/nu-command/src/filters/default.rs b/crates/nu-command/src/filters/default.rs index 9b08ce6331..3f7f93c3f6 100644 --- a/crates/nu-command/src/filters/default.rs +++ b/crates/nu-command/src/filters/default.rs @@ -27,7 +27,7 @@ impl Command for Default { } fn usage(&self) -> &str { - "Sets a default row's column if missing." + "Sets a default value if a row's column is missing or null." } fn run( @@ -66,6 +66,20 @@ impl Command for Default { Span::test_data(), )), }, + Example { + description: r#"Replace the missing value in the "a" column of a list"#, + example: "[{a:1 b:2} {b:1}] | default 'N/A' a", + result: Some(Value::test_list(vec![ + Value::test_record(record! { + "a" => Value::test_int(1), + "b" => Value::test_int(2), + }), + Value::test_record(record! { + "a" => Value::test_string("N/A"), + "b" => Value::test_int(1), + }), + ])), + }, ] } } From af34d5c062bc8fc9d6169ce4457c05d951bbd771 Mon Sep 17 00:00:00 2001 From: Embers-of-the-Fire Date: Sat, 3 Aug 2024 03:47:18 +0800 Subject: [PATCH 05/11] Fix internal panic for `query web` (#13507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Original implementation contains multiple `expect` which can cause internal runtime panic. This pr forks the `css` selector impl and make it return an error that nushell can recognize. **Note:** The original impl is still used in pre-defined selector implementations, but they should never fail, so the `css` fn is preserved. Closes #13496. # User-Facing Changes Now `query web` will not panic when the `query` parameter is not given or has syntax error. ```plain ❯ .\target\debug\nu.exe -c "http get https://www.rust-lang.org | query web" Error: × CSS query parse error ╭─[source:1:38] 1 │ http get https://www.rust-lang.org | query web · ────┬──── · ╰─┤ Unexpected error occurred. Please report this to the developer · │ EmptySelector ╰──── help: cannot parse query as a valid CSS selector ``` # Tests + Formatting - [x] `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - [x] `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - [x] `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the tests for the standard library # After Submitting - [x] Impl change, no doc update. --------- Co-authored-by: Stefan Holderbach --- crates/nu_plugin_query/src/query_web.rs | 161 +++++++++++++----------- 1 file changed, 91 insertions(+), 70 deletions(-) diff --git a/crates/nu_plugin_query/src/query_web.rs b/crates/nu_plugin_query/src/query_web.rs index 2d8b9e4aac..2d57af6021 100644 --- a/crates/nu_plugin_query/src/query_web.rs +++ b/crates/nu_plugin_query/src/query_web.rs @@ -99,29 +99,11 @@ pub fn web_examples() -> Vec> { } pub struct Selector { - pub query: String, + pub query: Spanned, pub as_html: bool, pub attribute: Value, pub as_table: Value, - pub inspect: bool, -} - -impl Selector { - pub fn new() -> Selector { - Selector { - query: String::new(), - as_html: false, - attribute: Value::string("".to_string(), Span::unknown()), - as_table: Value::string("".to_string(), Span::unknown()), - inspect: false, - } - } -} - -impl Default for Selector { - fn default() -> Self { - Self::new() - } + pub inspect: Spanned, } pub fn parse_selector_params(call: &EvaluatedCall, input: &Value) -> Result { @@ -136,43 +118,46 @@ pub fn parse_selector_params(call: &EvaluatedCall, input: &Value) -> Result Ok(begin_selector_query(val.to_string(), selector, span)), + Value::String { val, .. } => begin_selector_query(val.to_string(), selector, span), _ => Err(LabeledError::new("Requires text input") .with_label("expected text from pipeline", span)), } } -fn begin_selector_query(input_html: String, selector: Selector, span: Span) -> Value { +fn begin_selector_query( + input_html: String, + selector: Selector, + span: Span, +) -> Result { if let Value::List { .. } = selector.as_table { - return retrieve_tables( + retrieve_tables( input_html.as_str(), &selector.as_table, - selector.inspect, + selector.inspect.item, span, - ); + ) } else if selector.attribute.is_empty() { execute_selector_query( input_html.as_str(), - selector.query.as_str(), + selector.query, selector.as_html, selector.inspect, span, @@ -180,7 +165,7 @@ fn begin_selector_query(input_html: String, selector: Selector, span: Span) -> V } else if let Value::List { .. } = selector.attribute { execute_selector_query_with_attributes( input_html.as_str(), - selector.query.as_str(), + selector.query, &selector.attribute, selector.inspect, span, @@ -188,7 +173,7 @@ fn begin_selector_query(input_html: String, selector: Selector, span: Span) -> V } else { execute_selector_query_with_attribute( input_html.as_str(), - selector.query.as_str(), + selector.query, selector.attribute.as_str().unwrap_or(""), selector.inspect, span, @@ -201,7 +186,7 @@ pub fn retrieve_tables( columns: &Value, inspect_mode: bool, span: Span, -) -> Value { +) -> Result { let html = input_string; let mut cols: Vec = Vec::new(); if let Value::List { vals, .. } = &columns { @@ -228,11 +213,15 @@ pub fn retrieve_tables( }; if tables.len() == 1 { - return retrieve_table( - tables.into_iter().next().expect("Error retrieving table"), + return Ok(retrieve_table( + tables.into_iter().next().ok_or_else(|| { + LabeledError::new("Cannot retrieve table") + .with_label("Error retrieving table.", span) + .with_help("No table found.") + })?, columns, span, - ); + )); } let vals = tables @@ -240,7 +229,7 @@ pub fn retrieve_tables( .map(move |table| retrieve_table(table, columns, span)) .collect(); - Value::list(vals, span) + Ok(Value::list(vals, span)) } fn retrieve_table(mut table: WebTable, columns: &Value, span: Span) -> Value { @@ -323,15 +312,15 @@ fn retrieve_table(mut table: WebTable, columns: &Value, span: Span) -> Value { fn execute_selector_query_with_attribute( input_string: &str, - query_string: &str, + query_string: Spanned, attribute: &str, - inspect: bool, + inspect: Spanned, span: Span, -) -> Value { +) -> Result { let doc = Html::parse_fragment(input_string); let vals: Vec = doc - .select(&css(query_string, inspect)) + .select(&fallible_css(query_string, inspect)?) .map(|selection| { Value::string( selection.value().attr(attribute).unwrap_or("").to_string(), @@ -339,16 +328,16 @@ fn execute_selector_query_with_attribute( ) }) .collect(); - Value::list(vals, span) + Ok(Value::list(vals, span)) } fn execute_selector_query_with_attributes( input_string: &str, - query_string: &str, + query_string: Spanned, attributes: &Value, - inspect: bool, + inspect: Spanned, span: Span, -) -> Value { +) -> Result { let doc = Html::parse_fragment(input_string); let mut attrs: Vec = Vec::new(); @@ -361,7 +350,7 @@ fn execute_selector_query_with_attributes( } let vals: Vec = doc - .select(&css(query_string, inspect)) + .select(&fallible_css(query_string, inspect)?) .map(|selection| { let mut record = Record::new(); for attr in &attrs { @@ -373,25 +362,25 @@ fn execute_selector_query_with_attributes( Value::record(record, span) }) .collect(); - Value::list(vals, span) + Ok(Value::list(vals, span)) } fn execute_selector_query( input_string: &str, - query_string: &str, + query_string: Spanned, as_html: bool, - inspect: bool, + inspect: Spanned, span: Span, -) -> Value { +) -> Result { let doc = Html::parse_fragment(input_string); let vals: Vec = match as_html { true => doc - .select(&css(query_string, inspect)) + .select(&fallible_css(query_string, inspect)?) .map(|selection| Value::string(selection.html(), span)) .collect(), false => doc - .select(&css(query_string, inspect)) + .select(&fallible_css(query_string, inspect)?) .map(|selection| { Value::list( selection @@ -404,7 +393,28 @@ fn execute_selector_query( .collect(), }; - Value::list(vals, span) + Ok(Value::list(vals, span)) +} + +fn fallible_css( + selector: Spanned, + inspect: Spanned, +) -> Result { + if inspect.item { + ScraperSelector::parse("html").map_err(|e| { + LabeledError::new("CSS query parse error") + .with_label(e.to_string(), inspect.span) + .with_help( + "cannot parse query `html` as a valid CSS selector, possibly an internal error", + ) + }) + } else { + ScraperSelector::parse(&selector.item).map_err(|e| { + LabeledError::new("CSS query parse error") + .with_label(e.to_string(), selector.span) + .with_help("cannot parse query as a valid CSS selector") + }) + } } pub fn css(selector: &str, inspect: bool) -> ScraperSelector { @@ -433,15 +443,23 @@ mod tests { Example "#; + fn null_spanned(input: &T) -> Spanned { + Spanned { + item: input.to_owned(), + span: Span::unknown(), + } + } + #[test] fn test_first_child_is_not_empty() { assert!(!execute_selector_query( SIMPLE_LIST, - "li:first-child", - false, + null_spanned("li:first-child"), false, + null_spanned(&false), Span::test_data() ) + .unwrap() .is_empty()) } @@ -449,11 +467,12 @@ mod tests { fn test_first_child() { let item = execute_selector_query( SIMPLE_LIST, - "li:first-child", - false, + null_spanned("li:first-child"), false, + null_spanned(&false), Span::test_data(), - ); + ) + .unwrap(); let config = nu_protocol::Config::default(); let out = item.to_expanded_string("\n", &config); assert_eq!("[[Coffee]]".to_string(), out) @@ -463,11 +482,12 @@ mod tests { fn test_nested_text_nodes() { let item = execute_selector_query( NESTED_TEXT, - "p:first-child", - false, + null_spanned("p:first-child"), false, + null_spanned(&false), Span::test_data(), - ); + ) + .unwrap(); let out = item .into_list() .unwrap() @@ -492,7 +512,7 @@ mod tests { fn test_multiple_attributes() { let item = execute_selector_query_with_attributes( MULTIPLE_ATTRIBUTES, - "a", + null_spanned("a"), &Value::list( vec![ Value::string("href".to_string(), Span::unknown()), @@ -500,9 +520,10 @@ mod tests { ], Span::unknown(), ), - false, + null_spanned(&false), Span::test_data(), - ); + ) + .unwrap(); let out = item .into_list() .unwrap() From ff1ad77130b4a5baa6bbc6728e99023340f8842a Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sat, 3 Aug 2024 00:26:35 +0200 Subject: [PATCH 06/11] Simplify column look-up in `default` (#13522) # Description Since we make the promise that record keys/columns are exclusice we don't have to go through all columns after we have found the first one. Should permit some short-circuiting if the column is found early. # User-Facing Changes (-) # Tests + Formatting (-) --- crates/nu-command/src/filters/default.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/crates/nu-command/src/filters/default.rs b/crates/nu-command/src/filters/default.rs index 3f7f93c3f6..e269c58ccd 100644 --- a/crates/nu-command/src/filters/default.rs +++ b/crates/nu-command/src/filters/default.rs @@ -102,19 +102,13 @@ fn default( val: ref mut record, .. } => { - let mut found = false; - - for (col, val) in record.to_mut().iter_mut() { - if *col == column.item { - found = true; - if matches!(val, Value::Nothing { .. }) { - *val = value.clone(); - } + let record = record.to_mut(); + if let Some(val) = record.get_mut(&column.item) { + if matches!(val, Value::Nothing { .. }) { + *val = value.clone(); } - } - - if !found { - record.to_mut().push(column.item.clone(), value.clone()); + } else { + record.push(column.item.clone(), value.clone()); } item From 63f00e78d1ad5b75afbc3c7c883cd1e0e9084632 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sat, 3 Aug 2024 00:26:48 +0200 Subject: [PATCH 07/11] Lift `SharedCow::to_mut` out of if let branches (#13524) In some `if let`s we ran the `SharedCow::to_mut` for the test and to get access to a mutable reference in the happy path. Internally `Arc::into_mut` has to read atomics and if necessary clone. For else branches, where we still want to modify the record we previously called this again (not just in rust, confirmed in the asm). This would have introduced a `call` instruction and its cost (even if it would be guaranteed to take the short path in `Arc::into_mut`). Lifting it get's rid of this. --- crates/nu-protocol/src/value/mod.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index fc618d6341..edd51d70a6 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1223,7 +1223,8 @@ impl Value { for val in vals.iter_mut() { match val { Value::Record { val: record, .. } => { - if let Some(val) = record.to_mut().get_mut(col_name) { + let record = record.to_mut(); + if let Some(val) = record.get_mut(col_name) { val.upsert_data_at_cell_path(path, new_val.clone())?; } else { let new_col = if path.is_empty() { @@ -1235,7 +1236,7 @@ impl Value { .upsert_data_at_cell_path(path, new_val.clone())?; new_col }; - record.to_mut().push(col_name, new_col); + record.push(col_name, new_col); } } Value::Error { error, .. } => return Err(*error.clone()), @@ -1250,7 +1251,8 @@ impl Value { } } Value::Record { val: record, .. } => { - if let Some(val) = record.to_mut().get_mut(col_name) { + let record = record.to_mut(); + if let Some(val) = record.get_mut(col_name) { val.upsert_data_at_cell_path(path, new_val)?; } else { let new_col = if path.is_empty() { @@ -1260,7 +1262,7 @@ impl Value { new_col.upsert_data_at_cell_path(path, new_val)?; new_col }; - record.to_mut().push(col_name, new_col); + record.push(col_name, new_col); } } Value::Error { error, .. } => return Err(*error.clone()), @@ -1591,7 +1593,8 @@ impl Value { let v_span = val.span(); match val { Value::Record { val: record, .. } => { - if let Some(val) = record.to_mut().get_mut(col_name) { + let record = record.to_mut(); + if let Some(val) = record.get_mut(col_name) { if path.is_empty() { return Err(ShellError::ColumnAlreadyExists { col_name: col_name.clone(), @@ -1618,7 +1621,7 @@ impl Value { )?; new_col }; - record.to_mut().push(col_name, new_col); + record.push(col_name, new_col); } } Value::Error { error, .. } => return Err(*error.clone()), @@ -1634,7 +1637,8 @@ impl Value { } } Value::Record { val: record, .. } => { - if let Some(val) = record.to_mut().get_mut(col_name) { + let record = record.to_mut(); + if let Some(val) = record.get_mut(col_name) { if path.is_empty() { return Err(ShellError::ColumnAlreadyExists { col_name: col_name.clone(), @@ -1652,7 +1656,7 @@ impl Value { new_col.insert_data_at_cell_path(path, new_val, head_span)?; new_col }; - record.to_mut().push(col_name, new_col); + record.push(col_name, new_col); } } other => { From 85b06b22d99942062f1dccd201616396211721a4 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sat, 3 Aug 2024 01:14:44 +0200 Subject: [PATCH 08/11] Replace manual `Record::get` implementation (#13525) Let's simplify here --- crates/nu-command/src/charting/histogram.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/nu-command/src/charting/histogram.rs b/crates/nu-command/src/charting/histogram.rs index 29515c96c5..1162259409 100755 --- a/crates/nu-command/src/charting/histogram.rs +++ b/crates/nu-command/src/charting/histogram.rs @@ -177,11 +177,9 @@ fn run_histogram( match v { // parse record, and fill valid value to actual input. Value::Record { val, .. } => { - for (c, v) in val.iter() { - if c == col_name { - if let Ok(v) = HashableValue::from_value(v.clone(), head_span) { - inputs.push(v); - } + if let Some(v) = val.get(col_name) { + if let Ok(v) = HashableValue::from_value(v.clone(), head_span) { + inputs.push(v); } } } From f4c0d9d45b14c3eb0c02339fae1f04aaf2305953 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Sat, 3 Aug 2024 08:09:13 +0000 Subject: [PATCH 09/11] Path migration part 4: various tests (#13373) # Description Part 4 of replacing std::path types with nu_path types added in https://github.com/nushell/nushell/pull/13115. This PR migrates various tests throughout the code base. --- crates/nu-cli/src/repl.rs | 75 ++++++++++++------- crates/nu-command/tests/commands/cd.rs | 29 ++++--- .../tests/commands/database/into_sqlite.rs | 8 +- crates/nu-command/tests/commands/mktemp.rs | 9 +-- crates/nu-command/tests/commands/move_/umv.rs | 36 ++++----- .../nu-command/tests/commands/path/expand.rs | 17 ++--- crates/nu-command/tests/commands/rm.rs | 31 +++----- crates/nu-command/tests/commands/touch.rs | 5 +- crates/nu-command/tests/commands/ucp.rs | 72 +++++++----------- crates/nu-command/tests/commands/umkdir.rs | 15 +--- crates/nu-protocol/src/engine/engine_state.rs | 55 ++++++++------ crates/nu-test-support/src/fs.rs | 2 +- tests/repl/test_config_path.rs | 71 +++++++++++------- 13 files changed, 213 insertions(+), 212 deletions(-) diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 817950a445..28a3e2cec7 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -1337,20 +1337,26 @@ fn are_session_ids_in_sync() { #[cfg(test)] mod test_auto_cd { use super::{do_auto_cd, parse_operation, ReplOperation}; + use nu_path::AbsolutePath; use nu_protocol::engine::{EngineState, Stack}; - use std::path::Path; use tempfile::tempdir; /// Create a symlink. Works on both Unix and Windows. #[cfg(any(unix, windows))] - fn symlink(original: impl AsRef, link: impl AsRef) -> std::io::Result<()> { + fn symlink( + original: impl AsRef, + link: impl AsRef, + ) -> std::io::Result<()> { + let original = original.as_ref(); + let link = link.as_ref(); + #[cfg(unix)] { std::os::unix::fs::symlink(original, link) } #[cfg(windows)] { - if original.as_ref().is_dir() { + if original.is_dir() { std::os::windows::fs::symlink_dir(original, link) } else { std::os::windows::fs::symlink_file(original, link) @@ -1362,11 +1368,11 @@ mod test_auto_cd { /// `before`, and after `input` is parsed and evaluated, PWD should be /// changed to `after`. #[track_caller] - fn check(before: impl AsRef, input: &str, after: impl AsRef) { + fn check(before: impl AsRef, input: &str, after: impl AsRef) { // Setup EngineState and Stack. let mut engine_state = EngineState::new(); let mut stack = Stack::new(); - stack.set_cwd(before).unwrap(); + stack.set_cwd(before.as_ref()).unwrap(); // Parse the input. It must be an auto-cd operation. let op = parse_operation(input.to_string(), &engine_state, &stack).unwrap(); @@ -1382,54 +1388,66 @@ mod test_auto_cd { // don't have to be byte-wise equal (on Windows, the 8.3 filename // conversion messes things up), let updated_cwd = std::fs::canonicalize(updated_cwd).unwrap(); - let after = std::fs::canonicalize(after).unwrap(); + let after = std::fs::canonicalize(after.as_ref()).unwrap(); assert_eq!(updated_cwd, after); } #[test] fn auto_cd_root() { let tempdir = tempdir().unwrap(); - let root = if cfg!(windows) { r"C:\" } else { "/" }; - check(&tempdir, root, root); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + + let input = if cfg!(windows) { r"C:\" } else { "/" }; + let root = AbsolutePath::try_new(input).unwrap(); + check(tempdir, input, root); } #[test] fn auto_cd_tilde() { let tempdir = tempdir().unwrap(); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + let home = nu_path::home_dir().unwrap(); - check(&tempdir, "~", home); + check(tempdir, "~", home); } #[test] fn auto_cd_dot() { let tempdir = tempdir().unwrap(); - check(&tempdir, ".", &tempdir); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + + check(tempdir, ".", tempdir); } #[test] fn auto_cd_double_dot() { let tempdir = tempdir().unwrap(); - let dir = tempdir.path().join("foo"); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + + let dir = tempdir.join("foo"); std::fs::create_dir_all(&dir).unwrap(); - check(dir, "..", &tempdir); + check(dir, "..", tempdir); } #[test] fn auto_cd_triple_dot() { let tempdir = tempdir().unwrap(); - let dir = tempdir.path().join("foo").join("bar"); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + + let dir = tempdir.join("foo").join("bar"); std::fs::create_dir_all(&dir).unwrap(); - check(dir, "...", &tempdir); + check(dir, "...", tempdir); } #[test] fn auto_cd_relative() { let tempdir = tempdir().unwrap(); - let foo = tempdir.path().join("foo"); - let bar = tempdir.path().join("bar"); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + + let foo = tempdir.join("foo"); + let bar = tempdir.join("bar"); std::fs::create_dir_all(&foo).unwrap(); std::fs::create_dir_all(&bar).unwrap(); - let input = if cfg!(windows) { r"..\bar" } else { "../bar" }; check(foo, input, bar); } @@ -1437,32 +1455,35 @@ mod test_auto_cd { #[test] fn auto_cd_trailing_slash() { let tempdir = tempdir().unwrap(); - let dir = tempdir.path().join("foo"); - std::fs::create_dir_all(&dir).unwrap(); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + let dir = tempdir.join("foo"); + std::fs::create_dir_all(&dir).unwrap(); let input = if cfg!(windows) { r"foo\" } else { "foo/" }; - check(&tempdir, input, dir); + check(tempdir, input, dir); } #[test] fn auto_cd_symlink() { let tempdir = tempdir().unwrap(); - let dir = tempdir.path().join("foo"); - std::fs::create_dir_all(&dir).unwrap(); - let link = tempdir.path().join("link"); - symlink(&dir, &link).unwrap(); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + let dir = tempdir.join("foo"); + std::fs::create_dir_all(&dir).unwrap(); + let link = tempdir.join("link"); + symlink(&dir, &link).unwrap(); let input = if cfg!(windows) { r".\link" } else { "./link" }; - check(&tempdir, input, link); + check(tempdir, input, link); } #[test] #[should_panic(expected = "was not parsed into an auto-cd operation")] fn auto_cd_nonexistent_directory() { let tempdir = tempdir().unwrap(); - let dir = tempdir.path().join("foo"); + let tempdir = AbsolutePath::try_new(tempdir.path()).unwrap(); + let dir = tempdir.join("foo"); let input = if cfg!(windows) { r"foo\" } else { "foo/" }; - check(&tempdir, input, dir); + check(tempdir, input, dir); } } diff --git a/crates/nu-command/tests/commands/cd.rs b/crates/nu-command/tests/commands/cd.rs index f58638cc35..1b8e2dad5a 100644 --- a/crates/nu-command/tests/commands/cd.rs +++ b/crates/nu-command/tests/commands/cd.rs @@ -1,7 +1,7 @@ +use nu_path::Path; use nu_test_support::fs::Stub::EmptyFile; use nu_test_support::nu; use nu_test_support::playground::Playground; -use std::path::PathBuf; #[test] fn cd_works_with_in_var() { @@ -22,7 +22,7 @@ fn filesystem_change_from_current_directory_using_relative_path() { Playground::setup("cd_test_1", |dirs, _| { let actual = nu!( cwd: dirs.root(), "cd cd_test_1; $env.PWD"); - assert_eq!(PathBuf::from(actual.out), *dirs.test()); + assert_eq!(Path::new(&actual.out), dirs.test()); }) } @@ -32,7 +32,7 @@ fn filesystem_change_from_current_directory_using_relative_path_with_trailing_sl // Intentionally not using correct path sep because this should work on Windows let actual = nu!( cwd: dirs.root(), "cd cd_test_1_slash/; $env.PWD"); - assert_eq!(PathBuf::from(actual.out), *dirs.test()); + assert_eq!(Path::new(&actual.out), *dirs.test()); }) } @@ -48,7 +48,7 @@ fn filesystem_change_from_current_directory_using_absolute_path() { dirs.formats().display() ); - assert_eq!(PathBuf::from(actual.out), dirs.formats()); + assert_eq!(Path::new(&actual.out), dirs.formats()); }) } @@ -65,7 +65,7 @@ fn filesystem_change_from_current_directory_using_absolute_path_with_trailing_sl std::path::MAIN_SEPARATOR_STR, ); - assert_eq!(PathBuf::from(actual.out), dirs.formats()); + assert_eq!(Path::new(&actual.out), dirs.formats()); }) } @@ -84,7 +84,7 @@ fn filesystem_switch_back_to_previous_working_directory() { dirs.test().display() ); - assert_eq!(PathBuf::from(actual.out), dirs.test().join("odin")); + assert_eq!(Path::new(&actual.out), dirs.test().join("odin")); }) } @@ -101,10 +101,7 @@ fn filesystem_change_from_current_directory_using_relative_path_and_dash() { " ); - assert_eq!( - PathBuf::from(actual.out), - dirs.test().join("odin").join("-") - ); + assert_eq!(Path::new(&actual.out), dirs.test().join("odin").join("-")); }) } @@ -119,7 +116,7 @@ fn filesystem_change_current_directory_to_parent_directory() { " ); - assert_eq!(PathBuf::from(actual.out), *dirs.root()); + assert_eq!(Path::new(&actual.out), *dirs.root()); }) } @@ -136,7 +133,7 @@ fn filesystem_change_current_directory_to_two_parents_up_using_multiple_dots() { " ); - assert_eq!(PathBuf::from(actual.out), *dirs.test()); + assert_eq!(Path::new(&actual.out), *dirs.test()); }) } @@ -151,7 +148,7 @@ fn filesystem_change_to_home_directory() { " ); - assert_eq!(Some(PathBuf::from(actual.out)), dirs::home_dir()); + assert_eq!(Path::new(&actual.out), dirs::home_dir().unwrap()); }) } @@ -169,7 +166,7 @@ fn filesystem_change_to_a_directory_containing_spaces() { ); assert_eq!( - PathBuf::from(actual.out), + Path::new(&actual.out), dirs.test().join("robalino turner katz") ); }) @@ -234,7 +231,7 @@ fn filesystem_change_directory_to_symlink_relative() { $env.PWD " ); - assert_eq!(PathBuf::from(actual.out), dirs.test().join("foo_link")); + assert_eq!(Path::new(&actual.out), dirs.test().join("foo_link")); let actual = nu!( cwd: dirs.test().join("boo"), @@ -243,7 +240,7 @@ fn filesystem_change_directory_to_symlink_relative() { $env.PWD " ); - assert_eq!(PathBuf::from(actual.out), dirs.test().join("foo")); + assert_eq!(Path::new(&actual.out), dirs.test().join("foo")); }) } diff --git a/crates/nu-command/tests/commands/database/into_sqlite.rs b/crates/nu-command/tests/commands/database/into_sqlite.rs index faa45040b3..3de7973588 100644 --- a/crates/nu-command/tests/commands/database/into_sqlite.rs +++ b/crates/nu-command/tests/commands/database/into_sqlite.rs @@ -1,6 +1,5 @@ -use std::{io::Write, path::PathBuf}; - use chrono::{DateTime, FixedOffset}; +use nu_path::AbsolutePathBuf; use nu_protocol::{ast::PathMember, record, Span, Value}; use nu_test_support::{ fs::{line_ending, Stub}, @@ -13,6 +12,7 @@ use rand::{ rngs::StdRng, Rng, SeedableRng, }; +use std::io::Write; #[test] fn into_sqlite_schema() { @@ -453,7 +453,7 @@ impl Distribution for Standard { } } -fn make_sqlite_db(dirs: &Dirs, nu_table: &str) -> PathBuf { +fn make_sqlite_db(dirs: &Dirs, nu_table: &str) -> AbsolutePathBuf { let testdir = dirs.test(); let testdb_path = testdir.join(testdir.file_name().unwrap().to_str().unwrap().to_owned() + ".db"); @@ -465,7 +465,7 @@ fn make_sqlite_db(dirs: &Dirs, nu_table: &str) -> PathBuf { ); assert!(nucmd.status.success()); - testdb_path.into() + testdb_path } fn insert_test_rows(dirs: &Dirs, nu_table: &str, sql_query: Option<&str>, expected: Vec) { diff --git a/crates/nu-command/tests/commands/mktemp.rs b/crates/nu-command/tests/commands/mktemp.rs index cda3f59978..30de3beb33 100644 --- a/crates/nu-command/tests/commands/mktemp.rs +++ b/crates/nu-command/tests/commands/mktemp.rs @@ -1,6 +1,6 @@ +use nu_path::AbsolutePath; use nu_test_support::nu; use nu_test_support::playground::Playground; -use std::path::PathBuf; #[test] fn creates_temp_file() { @@ -9,7 +9,7 @@ fn creates_temp_file() { cwd: dirs.test(), "mktemp" ); - let loc = PathBuf::from(output.out.clone()); + let loc = AbsolutePath::try_new(&output.out).unwrap(); println!("{:?}", loc); assert!(loc.exists()); }) @@ -22,7 +22,7 @@ fn creates_temp_file_with_suffix() { cwd: dirs.test(), "mktemp --suffix .txt tempfileXXX" ); - let loc = PathBuf::from(output.out.clone()); + let loc = AbsolutePath::try_new(&output.out).unwrap(); assert!(loc.exists()); assert!(loc.is_file()); assert!(output.out.ends_with(".txt")); @@ -37,8 +37,7 @@ fn creates_temp_directory() { cwd: dirs.test(), "mktemp -d" ); - - let loc = PathBuf::from(output.out); + let loc = AbsolutePath::try_new(&output.out).unwrap(); assert!(loc.exists()); assert!(loc.is_dir()); }) diff --git a/crates/nu-command/tests/commands/move_/umv.rs b/crates/nu-command/tests/commands/move_/umv.rs index 0c96c88a36..b193d5b7cb 100644 --- a/crates/nu-command/tests/commands/move_/umv.rs +++ b/crates/nu-command/tests/commands/move_/umv.rs @@ -2,7 +2,6 @@ use nu_test_support::fs::{files_exist_at, Stub::EmptyFile, Stub::FileWithContent use nu_test_support::nu; use nu_test_support::playground::Playground; use rstest::rstest; -use std::path::Path; #[test] fn moves_a_file() { @@ -96,7 +95,7 @@ fn moves_the_directory_inside_directory_if_path_to_move_is_existing_directory() assert!(!original_dir.exists()); assert!(expected.exists()); - assert!(files_exist_at(vec!["jttxt"], expected)) + assert!(files_exist_at(&["jttxt"], expected)) }) } @@ -125,7 +124,7 @@ fn moves_using_path_with_wildcard() { nu!(cwd: work_dir, "mv ../originals/*.ini ../expected"); assert!(files_exist_at( - vec!["yehuda.ini", "jt.ini", "sample.ini", "andres.ini",], + &["yehuda.ini", "jt.ini", "sample.ini", "andres.ini",], expected )); }) @@ -152,7 +151,7 @@ fn moves_using_a_glob() { assert!(meal_dir.exists()); assert!(files_exist_at( - vec!["arepa.txt", "empanada.txt", "taquiza.txt",], + &["arepa.txt", "empanada.txt", "taquiza.txt",], expected )); }) @@ -184,7 +183,7 @@ fn moves_a_directory_with_files() { assert!(!original_dir.exists()); assert!(expected_dir.exists()); assert!(files_exist_at( - vec![ + &[ "car/car1.txt", "car/car2.txt", "bicycle/bicycle1.txt", @@ -322,7 +321,7 @@ fn move_files_using_glob_two_parents_up_using_multiple_dots() { "# ); - let files = vec![ + let files = &[ "yehuda.yaml", "jtjson", "andres.xml", @@ -333,7 +332,7 @@ fn move_files_using_glob_two_parents_up_using_multiple_dots() { let original_dir = dirs.test().join("foo/bar"); let destination_dir = dirs.test(); - assert!(files_exist_at(files.clone(), destination_dir)); + assert!(files_exist_at(files, destination_dir)); assert!(!files_exist_at(files, original_dir)) }) } @@ -440,10 +439,7 @@ fn mv_change_case_of_directory() { ); #[cfg(any(target_os = "linux", target_os = "freebsd"))] - assert!(files_exist_at( - vec!["somefile.txt",], - dirs.test().join(new_dir) - )); + assert!(files_exist_at(&["somefile.txt"], dirs.test().join(new_dir))); #[cfg(not(any(target_os = "linux", target_os = "freebsd")))] _actual.err.contains("to a subdirectory of itself"); @@ -647,10 +643,10 @@ fn test_cp_inside_glob_metachars_dir() { assert!(actual.err.is_empty()); assert!(!files_exist_at( - vec!["test_file.txt"], + &["test_file.txt"], dirs.test().join(sub_dir) )); - assert!(files_exist_at(vec!["test_file.txt"], dirs.test())); + assert!(files_exist_at(&["test_file.txt"], dirs.test())); }); } @@ -667,19 +663,13 @@ fn mv_with_tilde() { // mv file let actual = nu!(cwd: dirs.test(), "mv '~tilde/f1.txt' ./"); assert!(actual.err.is_empty()); - assert!(!files_exist_at( - vec![Path::new("f1.txt")], - dirs.test().join("~tilde") - )); - assert!(files_exist_at(vec![Path::new("f1.txt")], dirs.test())); + assert!(!files_exist_at(&["f1.txt"], dirs.test().join("~tilde"))); + assert!(files_exist_at(&["f1.txt"], dirs.test())); // pass variable let actual = nu!(cwd: dirs.test(), "let f = '~tilde/f2.txt'; mv $f ./"); assert!(actual.err.is_empty()); - assert!(!files_exist_at( - vec![Path::new("f2.txt")], - dirs.test().join("~tilde") - )); - assert!(files_exist_at(vec![Path::new("f1.txt")], dirs.test())); + assert!(!files_exist_at(&["f2.txt"], dirs.test().join("~tilde"))); + assert!(files_exist_at(&["f1.txt"], dirs.test())); }) } diff --git a/crates/nu-command/tests/commands/path/expand.rs b/crates/nu-command/tests/commands/path/expand.rs index 71f2e03284..4c18c5fd4e 100644 --- a/crates/nu-command/tests/commands/path/expand.rs +++ b/crates/nu-command/tests/commands/path/expand.rs @@ -1,9 +1,8 @@ +use nu_path::Path; use nu_test_support::fs::Stub::EmptyFile; use nu_test_support::playground::Playground; use nu_test_support::{nu, pipeline}; -use std::path::PathBuf; - #[test] fn expands_path_with_dot() { Playground::setup("path_expand_1", |dirs, sandbox| { @@ -18,7 +17,7 @@ fn expands_path_with_dot() { )); let expected = dirs.test.join("menu").join("spam.txt"); - assert_eq!(PathBuf::from(actual.out), expected); + assert_eq!(Path::new(&actual.out), expected); }) } @@ -38,7 +37,7 @@ fn expands_path_without_follow_symlink() { )); let expected = dirs.test.join("menu").join("spam_link.ln"); - assert_eq!(PathBuf::from(actual.out), expected); + assert_eq!(Path::new(&actual.out), expected); }) } @@ -56,7 +55,7 @@ fn expands_path_with_double_dot() { )); let expected = dirs.test.join("menu").join("spam.txt"); - assert_eq!(PathBuf::from(actual.out), expected); + assert_eq!(Path::new(&actual.out), expected); }) } @@ -74,7 +73,7 @@ fn const_path_expand() { )); let expected = dirs.test.join("menu").join("spam.txt"); - assert_eq!(PathBuf::from(actual.out), expected); + assert_eq!(Path::new(&actual.out), expected); }) } @@ -92,7 +91,7 @@ mod windows { "# )); - assert!(!PathBuf::from(actual.out).starts_with("~")); + assert!(!Path::new(&actual.out).starts_with("~")); }) } @@ -106,7 +105,7 @@ mod windows { "# )); - assert!(!PathBuf::from(actual.out).starts_with("~")); + assert!(!Path::new(&actual.out).starts_with("~")); }) } @@ -131,7 +130,7 @@ mod windows { )); let expected = dirs.test.join("menu").join("spam_link.ln"); - assert_eq!(PathBuf::from(actual.out), expected); + assert_eq!(Path::new(&actual.out), expected); }) } } diff --git a/crates/nu-command/tests/commands/rm.rs b/crates/nu-command/tests/commands/rm.rs index fbea7edaf1..fdd7258bd4 100644 --- a/crates/nu-command/tests/commands/rm.rs +++ b/crates/nu-command/tests/commands/rm.rs @@ -6,7 +6,6 @@ use nu_test_support::playground::Playground; use rstest::rstest; #[cfg(not(windows))] use std::fs; -use std::path::Path; #[test] fn removes_a_file() { @@ -50,7 +49,7 @@ fn removes_files_with_wildcard() { ); assert!(!files_exist_at( - vec![ + &[ "src/parser/parse/token_tree.rs", "src/parser/hir/baseline_parse.rs", "src/parser/hir/baseline_parse_tokens.rs" @@ -91,7 +90,7 @@ fn removes_deeply_nested_directories_with_wildcard_and_recursive_flag() { ); assert!(!files_exist_at( - vec!["src/parser/parse", "src/parser/hir"], + &["src/parser/parse", "src/parser/hir"], dirs.test() )); }) @@ -277,7 +276,7 @@ fn remove_files_from_two_parents_up_using_multiple_dots_and_glob() { ); assert!(!files_exist_at( - vec!["yehuda.txt", "jttxt", "kevin.txt"], + &["yehuda.txt", "jttxt", "kevin.txt"], dirs.test() )); }) @@ -305,8 +304,8 @@ fn rm_wildcard_keeps_dotfiles() { r#"rm *"# ); - assert!(!files_exist_at(vec!["foo"], dirs.test())); - assert!(files_exist_at(vec![".bar"], dirs.test())); + assert!(!files_exist_at(&["foo"], dirs.test())); + assert!(files_exist_at(&[".bar"], dirs.test())); }) } @@ -320,8 +319,8 @@ fn rm_wildcard_leading_dot_deletes_dotfiles() { "rm .*" ); - assert!(files_exist_at(vec!["foo"], dirs.test())); - assert!(!files_exist_at(vec![".bar"], dirs.test())); + assert!(files_exist_at(&["foo"], dirs.test())); + assert!(!files_exist_at(&[".bar"], dirs.test())); }) } @@ -453,7 +452,7 @@ fn rm_prints_filenames_on_error() { // This rm is expected to fail, and stderr output indicating so is also expected. let actual = nu!(cwd: test_dir, "rm test*.txt"); - assert!(files_exist_at(file_names.clone(), test_dir)); + assert!(files_exist_at(&file_names, test_dir)); for file_name in file_names { let path = test_dir.join(file_name); let substr = format!("Could not delete {}", path.to_string_lossy()); @@ -482,7 +481,7 @@ fn rm_files_inside_glob_metachars_dir() { assert!(actual.err.is_empty()); assert!(!files_exist_at( - vec!["test_file.txt"], + &["test_file.txt"], dirs.test().join(sub_dir) )); }); @@ -556,22 +555,16 @@ fn rm_with_tilde() { let actual = nu!(cwd: dirs.test(), "rm '~tilde/f1.txt'"); assert!(actual.err.is_empty()); - assert!(!files_exist_at( - vec![Path::new("f1.txt")], - dirs.test().join("~tilde") - )); + assert!(!files_exist_at(&["f1.txt"], dirs.test().join("~tilde"))); // pass variable let actual = nu!(cwd: dirs.test(), "let f = '~tilde/f2.txt'; rm $f"); assert!(actual.err.is_empty()); - assert!(!files_exist_at( - vec![Path::new("f2.txt")], - dirs.test().join("~tilde") - )); + assert!(!files_exist_at(&["f2.txt"], dirs.test().join("~tilde"))); // remove directory let actual = nu!(cwd: dirs.test(), "let f = '~tilde'; rm -r $f"); assert!(actual.err.is_empty()); - assert!(!files_exist_at(vec![Path::new("~tilde")], dirs.test())); + assert!(!files_exist_at(&["~tilde"], dirs.test())); }) } diff --git a/crates/nu-command/tests/commands/touch.rs b/crates/nu-command/tests/commands/touch.rs index 5f454cdec4..10ae299a0f 100644 --- a/crates/nu-command/tests/commands/touch.rs +++ b/crates/nu-command/tests/commands/touch.rs @@ -2,7 +2,6 @@ use chrono::{DateTime, Local}; use nu_test_support::fs::{files_exist_at, Stub}; use nu_test_support::nu; use nu_test_support::playground::Playground; -use std::path::Path; // Use 1 instead of 0 because 0 has a special meaning in Windows const TIME_ONE: filetime::FileTime = filetime::FileTime::from_unix_time(1, 0); @@ -494,12 +493,12 @@ fn create_a_file_with_tilde() { Playground::setup("touch with tilde", |dirs, _| { let actual = nu!(cwd: dirs.test(), "touch '~tilde'"); assert!(actual.err.is_empty()); - assert!(files_exist_at(vec![Path::new("~tilde")], dirs.test())); + assert!(files_exist_at(&["~tilde"], dirs.test())); // pass variable let actual = nu!(cwd: dirs.test(), "let f = '~tilde2'; touch $f"); assert!(actual.err.is_empty()); - assert!(files_exist_at(vec![Path::new("~tilde2")], dirs.test())); + assert!(files_exist_at(&["~tilde2"], dirs.test())); }) } diff --git a/crates/nu-command/tests/commands/ucp.rs b/crates/nu-command/tests/commands/ucp.rs index 2c2b30f252..eb2363e68c 100644 --- a/crates/nu-command/tests/commands/ucp.rs +++ b/crates/nu-command/tests/commands/ucp.rs @@ -7,7 +7,6 @@ use nu_test_support::nu; use nu_test_support::playground::Playground; use rstest::rstest; -use std::path::Path; #[cfg(not(target_os = "windows"))] const PATH_SEPARATOR: &str = "/"; @@ -131,11 +130,7 @@ fn copies_the_directory_inside_directory_if_path_to_copy_is_directory_and_with_r assert!(expected_dir.exists()); assert!(files_exist_at( - vec![ - Path::new("yehuda.txt"), - Path::new("jttxt"), - Path::new("andres.txt") - ], + &["yehuda.txt", "jttxt", "andres.txt"], &expected_dir )); }) @@ -181,15 +176,15 @@ fn deep_copies_with_recursive_flag_impl(progress: bool) { assert!(expected_dir.exists()); assert!(files_exist_at( - vec![Path::new("errors.txt"), Path::new("multishells.txt")], + &["errors.txt", "multishells.txt"], jts_expected_copied_dir )); assert!(files_exist_at( - vec![Path::new("coverage.txt"), Path::new("commands.txt")], + &["coverage.txt", "commands.txt"], andres_expected_copied_dir )); assert!(files_exist_at( - vec![Path::new("defer-evaluation.txt")], + &["defer-evaluation.txt"], yehudas_expected_copied_dir )); }) @@ -220,13 +215,13 @@ fn copies_using_path_with_wildcard_impl(progress: bool) { ); assert!(files_exist_at( - vec![ - Path::new("caco3_plastics.csv"), - Path::new("cargo_sample.toml"), - Path::new("jt.xml"), - Path::new("sample.ini"), - Path::new("sgml_description.json"), - Path::new("utf16.ini"), + &[ + "caco3_plastics.csv", + "cargo_sample.toml", + "jt.xml", + "sample.ini", + "sgml_description.json", + "utf16.ini", ], dirs.test() )); @@ -265,13 +260,13 @@ fn copies_using_a_glob_impl(progress: bool) { ); assert!(files_exist_at( - vec![ - Path::new("caco3_plastics.csv"), - Path::new("cargo_sample.toml"), - Path::new("jt.xml"), - Path::new("sample.ini"), - Path::new("sgml_description.json"), - Path::new("utf16.ini"), + &[ + "caco3_plastics.csv", + "cargo_sample.toml", + "jt.xml", + "sample.ini", + "sgml_description.json", + "utf16.ini", ], dirs.test() )); @@ -341,7 +336,7 @@ fn copy_files_using_glob_two_parents_up_using_multiple_dots_imp(progress: bool) ); assert!(files_exist_at( - vec![ + &[ "yehuda.yaml", "jtjson", "andres.xml", @@ -377,7 +372,7 @@ fn copy_file_and_dir_from_two_parents_up_using_multiple_dots_to_current_dir_recu let expected = dirs.test().join("foo/bar"); - assert!(files_exist_at(vec!["hello_there", "hello_again"], expected)); + assert!(files_exist_at(&["hello_there", "hello_again"], expected)); }) } @@ -428,7 +423,7 @@ fn copy_dir_contains_symlink_ignored_impl(progress: bool) { // check hello_there exists inside `tmp_dir_2`, and `dangle_symlink` don't exists inside `tmp_dir_2`. let expected = sandbox.cwd().join("tmp_dir_2"); - assert!(files_exist_at(vec!["hello_there"], expected)); + assert!(files_exist_at(&["hello_there"], expected)); // GNU cp will copy the broken symlink, so following their behavior // thus commenting out below // let path = expected.join("dangle_symlink"); @@ -461,7 +456,7 @@ fn copy_dir_contains_symlink_impl(progress: bool) { // check hello_there exists inside `tmp_dir_2`, and `dangle_symlink` also exists inside `tmp_dir_2`. let expected = sandbox.cwd().join("tmp_dir_2"); - assert!(files_exist_at(vec!["hello_there"], expected.clone())); + assert!(files_exist_at(&["hello_there"], expected.clone())); let path = expected.join("dangle_symlink"); assert!(path.is_symlink()); }); @@ -1151,10 +1146,10 @@ fn test_cp_inside_glob_metachars_dir() { assert!(actual.err.is_empty()); assert!(files_exist_at( - vec!["test_file.txt"], + &["test_file.txt"], dirs.test().join(sub_dir) )); - assert!(files_exist_at(vec!["test_file.txt"], dirs.test())); + assert!(files_exist_at(&["test_file.txt"], dirs.test())); }); } @@ -1167,10 +1162,7 @@ fn test_cp_to_customized_home_directory() { let actual = nu!(cwd: dirs.test(), "mkdir test; cp test_file.txt ~/test/"); assert!(actual.err.is_empty()); - assert!(files_exist_at( - vec!["test_file.txt"], - dirs.test().join("test") - )); + assert!(files_exist_at(&["test_file.txt"], dirs.test().join("test"))); }) } @@ -1193,20 +1185,14 @@ fn cp_with_tilde() { // cp file let actual = nu!(cwd: dirs.test(), "cp '~tilde/f1.txt' ./"); assert!(actual.err.is_empty()); - assert!(files_exist_at( - vec![Path::new("f1.txt")], - dirs.test().join("~tilde") - )); - assert!(files_exist_at(vec![Path::new("f1.txt")], dirs.test())); + assert!(files_exist_at(&["f1.txt"], dirs.test().join("~tilde"))); + assert!(files_exist_at(&["f1.txt"], dirs.test())); // pass variable let actual = nu!(cwd: dirs.test(), "let f = '~tilde/f2.txt'; cp $f ./"); assert!(actual.err.is_empty()); - assert!(files_exist_at( - vec![Path::new("f2.txt")], - dirs.test().join("~tilde") - )); - assert!(files_exist_at(vec![Path::new("f1.txt")], dirs.test())); + assert!(files_exist_at(&["f2.txt"], dirs.test().join("~tilde"))); + assert!(files_exist_at(&["f1.txt"], dirs.test())); }) } diff --git a/crates/nu-command/tests/commands/umkdir.rs b/crates/nu-command/tests/commands/umkdir.rs index 3ca1b3bdfb..fdaaf4f721 100644 --- a/crates/nu-command/tests/commands/umkdir.rs +++ b/crates/nu-command/tests/commands/umkdir.rs @@ -1,7 +1,6 @@ use nu_test_support::fs::files_exist_at; use nu_test_support::playground::Playground; use nu_test_support::{nu, pipeline}; -use std::path::Path; #[test] fn creates_directory() { @@ -25,10 +24,7 @@ fn accepts_and_creates_directories() { "mkdir dir_1 dir_2 dir_3" ); - assert!(files_exist_at( - vec![Path::new("dir_1"), Path::new("dir_2"), Path::new("dir_3")], - dirs.test() - )); + assert!(files_exist_at(&["dir_1", "dir_2", "dir_3"], dirs.test())); }) } @@ -70,10 +66,7 @@ fn print_created_paths() { pipeline("mkdir -v dir_1 dir_2 dir_3") ); - assert!(files_exist_at( - vec![Path::new("dir_1"), Path::new("dir_2"), Path::new("dir_3")], - dirs.test() - )); + assert!(files_exist_at(&["dir_1", "dir_2", "dir_3"], dirs.test())); assert!(actual.out.contains("dir_1")); assert!(actual.out.contains("dir_2")); @@ -165,11 +158,11 @@ fn mkdir_with_tilde() { Playground::setup("mkdir with tilde", |dirs, _| { let actual = nu!(cwd: dirs.test(), "mkdir '~tilde'"); assert!(actual.err.is_empty()); - assert!(files_exist_at(vec![Path::new("~tilde")], dirs.test())); + assert!(files_exist_at(&["~tilde"], dirs.test())); // pass variable let actual = nu!(cwd: dirs.test(), "let f = '~tilde2'; mkdir $f"); assert!(actual.err.is_empty()); - assert!(files_exist_at(vec![Path::new("~tilde2")], dirs.test())); + assert!(files_exist_at(&["~tilde2"], dirs.test())); }) } diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 7c7cfb9bb5..bd049bbb2d 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -1169,20 +1169,25 @@ mod test_cwd { engine::{EngineState, Stack}, Span, Value, }; - use nu_path::assert_path_eq; - use std::path::Path; + use nu_path::{assert_path_eq, AbsolutePath, Path}; use tempfile::{NamedTempFile, TempDir}; /// Creates a symlink. Works on both Unix and Windows. #[cfg(any(unix, windows))] - fn symlink(original: impl AsRef, link: impl AsRef) -> std::io::Result<()> { + fn symlink( + original: impl AsRef, + link: impl AsRef, + ) -> std::io::Result<()> { + let original = original.as_ref(); + let link = link.as_ref(); + #[cfg(unix)] { std::os::unix::fs::symlink(original, link) } #[cfg(windows)] { - if original.as_ref().is_dir() { + if original.is_dir() { std::os::windows::fs::symlink_dir(original, link) } else { std::os::windows::fs::symlink_file(original, link) @@ -1195,10 +1200,7 @@ mod test_cwd { let mut engine_state = EngineState::new(); engine_state.add_env_var( "PWD".into(), - Value::String { - val: path.as_ref().to_string_lossy().to_string(), - internal_span: Span::unknown(), - }, + Value::test_string(path.as_ref().to_str().unwrap()), ); engine_state } @@ -1208,10 +1210,7 @@ mod test_cwd { let mut stack = Stack::new(); stack.add_env_var( "PWD".into(), - Value::String { - val: path.as_ref().to_string_lossy().to_string(), - internal_span: Span::unknown(), - }, + Value::test_string(path.as_ref().to_str().unwrap()), ); stack } @@ -1289,9 +1288,12 @@ mod test_cwd { #[test] fn pwd_points_to_symlink_to_file() { let file = NamedTempFile::new().unwrap(); + let temp_file = AbsolutePath::try_new(file.path()).unwrap(); let dir = TempDir::new().unwrap(); - let link = dir.path().join("link"); - symlink(file.path(), &link).unwrap(); + let temp = AbsolutePath::try_new(dir.path()).unwrap(); + + let link = temp.join("link"); + symlink(temp_file, &link).unwrap(); let engine_state = engine_state_with_pwd(&link); engine_state.cwd(None).unwrap_err(); @@ -1300,8 +1302,10 @@ mod test_cwd { #[test] fn pwd_points_to_symlink_to_directory() { let dir = TempDir::new().unwrap(); - let link = dir.path().join("link"); - symlink(dir.path(), &link).unwrap(); + let temp = AbsolutePath::try_new(dir.path()).unwrap(); + + let link = temp.join("link"); + symlink(temp, &link).unwrap(); let engine_state = engine_state_with_pwd(&link); let cwd = engine_state.cwd(None).unwrap(); @@ -1311,10 +1315,15 @@ mod test_cwd { #[test] fn pwd_points_to_broken_symlink() { let dir = TempDir::new().unwrap(); - let link = dir.path().join("link"); - symlink(TempDir::new().unwrap().path(), &link).unwrap(); + let temp = AbsolutePath::try_new(dir.path()).unwrap(); + let other_dir = TempDir::new().unwrap(); + let other_temp = AbsolutePath::try_new(other_dir.path()).unwrap(); + + let link = temp.join("link"); + symlink(other_temp, &link).unwrap(); let engine_state = engine_state_with_pwd(&link); + drop(other_dir); engine_state.cwd(None).unwrap_err(); } @@ -1357,12 +1366,14 @@ mod test_cwd { #[test] fn stack_pwd_points_to_normal_directory_with_symlink_components() { - // `/tmp/dir/link` points to `/tmp/dir`, then we set PWD to `/tmp/dir/link/foo` let dir = TempDir::new().unwrap(); - let link = dir.path().join("link"); - symlink(dir.path(), &link).unwrap(); + let temp = AbsolutePath::try_new(dir.path()).unwrap(); + + // `/tmp/dir/link` points to `/tmp/dir`, then we set PWD to `/tmp/dir/link/foo` + let link = temp.join("link"); + symlink(temp, &link).unwrap(); let foo = link.join("foo"); - std::fs::create_dir(dir.path().join("foo")).unwrap(); + std::fs::create_dir(temp.join("foo")).unwrap(); let engine_state = EngineState::new(); let stack = stack_with_pwd(&foo); diff --git a/crates/nu-test-support/src/fs.rs b/crates/nu-test-support/src/fs.rs index 230e5eafe3..d591a7de18 100644 --- a/crates/nu-test-support/src/fs.rs +++ b/crates/nu-test-support/src/fs.rs @@ -35,7 +35,7 @@ pub fn line_ending() -> String { } } -pub fn files_exist_at(files: Vec>, path: impl AsRef) -> bool { +pub fn files_exist_at(files: &[impl AsRef], path: impl AsRef) -> bool { let path = path.as_ref(); files.iter().all(|f| path.join(f.as_ref()).exists()) } diff --git a/tests/repl/test_config_path.rs b/tests/repl/test_config_path.rs index b66633c879..6bee043765 100644 --- a/tests/repl/test_config_path.rs +++ b/tests/repl/test_config_path.rs @@ -1,9 +1,8 @@ -use nu_path::canonicalize_with; +use nu_path::{AbsolutePath, AbsolutePathBuf, Path}; use nu_test_support::nu; use nu_test_support::playground::{Executable, Playground}; use pretty_assertions::assert_eq; use std::fs::{self, File}; -use std::path::{Path, PathBuf}; #[cfg(not(target_os = "windows"))] fn adjust_canonicalization>(p: P) -> String { @@ -24,21 +23,26 @@ fn adjust_canonicalization>(p: P) -> String { /// Make the config directory a symlink that points to a temporary folder, and also makes /// the nushell directory inside a symlink. /// Returns the path to the `nushell` config folder inside, via the symlink. -fn setup_fake_config(playground: &mut Playground) -> PathBuf { - let config_dir = "config_real"; +fn setup_fake_config(playground: &mut Playground) -> AbsolutePathBuf { + let config_real = "config_real"; let config_link = "config_link"; let nushell_real = "nushell_real"; - let nushell_config_dir = Path::new(config_dir).join("nushell").display().to_string(); + let nushell_link = Path::new(config_real) + .join("nushell") + .into_os_string() + .into_string() + .unwrap(); + + let config_home = playground.cwd().join(config_link); + playground.mkdir(nushell_real); - playground.mkdir(config_dir); - playground.symlink(nushell_real, &nushell_config_dir); - playground.symlink(config_dir, config_link); - playground.with_env( - "XDG_CONFIG_HOME", - &playground.cwd().join(config_link).display().to_string(), - ); - let path = Path::new(config_link).join("nushell"); - canonicalize_with(&path, playground.cwd()).unwrap_or(path) + playground.mkdir(config_real); + playground.symlink(nushell_real, &nushell_link); + playground.symlink(config_real, config_link); + playground.with_env("XDG_CONFIG_HOME", config_home.to_str().unwrap()); + + let path = config_home.join("nushell"); + path.canonicalize().map(Into::into).unwrap_or(path) } fn run(playground: &mut Playground, command: &str) -> String { @@ -79,47 +83,55 @@ fn run_interactive_stderr(xdg_config_home: impl AsRef) -> String { .to_string(); } -fn test_config_path_helper(playground: &mut Playground, config_dir_nushell: PathBuf) { +fn test_config_path_helper( + playground: &mut Playground, + config_dir_nushell: impl AsRef, +) { + let config_dir_nushell = config_dir_nushell.as_ref(); + // Create the config dir folder structure if it does not already exist if !config_dir_nushell.exists() { - let _ = fs::create_dir_all(&config_dir_nushell); + let _ = fs::create_dir_all(config_dir_nushell); } - let config_dir_nushell = - std::fs::canonicalize(&config_dir_nushell).expect("canonicalize config dir failed"); + let config_dir_nushell = config_dir_nushell + .canonicalize() + .expect("canonicalize config dir failed"); let actual = run(playground, "$nu.default-config-dir"); assert_eq!(actual, adjust_canonicalization(&config_dir_nushell)); let config_path = config_dir_nushell.join("config.nu"); // We use canonicalize here in case the config or env is symlinked since $nu.config-path is returning the canonicalized path in #8653 let canon_config_path = - adjust_canonicalization(std::fs::canonicalize(&config_path).unwrap_or(config_path)); + adjust_canonicalization(std::fs::canonicalize(&config_path).unwrap_or(config_path.into())); let actual = run(playground, "$nu.config-path"); assert_eq!(actual, canon_config_path); let env_path = config_dir_nushell.join("env.nu"); let canon_env_path = - adjust_canonicalization(std::fs::canonicalize(&env_path).unwrap_or(env_path)); + adjust_canonicalization(std::fs::canonicalize(&env_path).unwrap_or(env_path.into())); let actual = run(playground, "$nu.env-path"); assert_eq!(actual, canon_env_path); let history_path = config_dir_nushell.join("history.txt"); - let canon_history_path = - adjust_canonicalization(std::fs::canonicalize(&history_path).unwrap_or(history_path)); + let canon_history_path = adjust_canonicalization( + std::fs::canonicalize(&history_path).unwrap_or(history_path.into()), + ); let actual = run(playground, "$nu.history-path"); assert_eq!(actual, canon_history_path); let login_path = config_dir_nushell.join("login.nu"); let canon_login_path = - adjust_canonicalization(std::fs::canonicalize(&login_path).unwrap_or(login_path)); + adjust_canonicalization(std::fs::canonicalize(&login_path).unwrap_or(login_path.into())); let actual = run(playground, "$nu.loginshell-path"); assert_eq!(actual, canon_login_path); #[cfg(feature = "plugin")] { let plugin_path = config_dir_nushell.join("plugin.msgpackz"); - let canon_plugin_path = - adjust_canonicalization(std::fs::canonicalize(&plugin_path).unwrap_or(plugin_path)); + let canon_plugin_path = adjust_canonicalization( + std::fs::canonicalize(&plugin_path).unwrap_or(plugin_path.into()), + ); let actual = run(playground, "$nu.plugin-path"); assert_eq!(actual, canon_plugin_path); } @@ -129,7 +141,7 @@ fn test_config_path_helper(playground: &mut Playground, config_dir_nushell: Path fn test_default_config_path() { Playground::setup("default_config_path", |_, playground| { let config_dir = nu_path::config_dir().expect("Could not get config directory"); - test_config_path_helper(playground, config_dir.join("nushell").into()); + test_config_path_helper(playground, config_dir.join("nushell")); }); } @@ -152,8 +164,9 @@ fn test_default_symlink_config_path_broken_symlink_config_files() { |_, playground| { let fake_config_dir_nushell = setup_fake_config(playground); - let fake_dir = PathBuf::from("fake"); - playground.mkdir(&fake_dir.display().to_string()); + let fake_dir = "fake"; + playground.mkdir(fake_dir); + let fake_dir = Path::new(fake_dir); for config_file in [ "config.nu", @@ -172,7 +185,7 @@ fn test_default_symlink_config_path_broken_symlink_config_files() { // Windows doesn't allow creating a symlink without the file existing, // so we first create original files for the symlinks, then delete them // to break the symlinks - std::fs::remove_dir_all(playground.cwd().join(&fake_dir)).unwrap(); + std::fs::remove_dir_all(playground.cwd().join(fake_dir)).unwrap(); test_config_path_helper(playground, fake_config_dir_nushell); }, From 20b53067cd3f8da2af930370659172e11db81161 Mon Sep 17 00:00:00 2001 From: Embers-of-the-Fire Date: Sat, 3 Aug 2024 16:55:35 +0800 Subject: [PATCH 10/11] Fix overflow table display in command documentation (#13526) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Check and set table width beforehand. Closes #13520. # User-Facing Changes Before: ```plain ❯ help net cmd1 network Usage: > net cmd1 Flags: -h, --help - Display the help message for this command Input/output types: ╭───┬─────────┬─────────────────────────────────────────────────────────╮ │ # │ input │ output │ ├───┼─────────┼─────────────────────────────────────────────────────────┤ │ 0 │ nothing │ table, │ │ │ │ flags: record> │ ╰───┴─────────┴─────────────────────────────────────────────────────────╯ ``` After: ```plain ❯ help net cmd1 network Usage: > net cmd1 Flags: -h, --help - Display the help message for this command Input/output types: ╭───┬─────────┬───────────────────────────────────────────────────────╮ │ # │ input │ output │ ├───┼─────────┼───────────────────────────────────────────────────────┤ │ 0 │ nothing │ table, │ │ │ │ flags: record> │ ╰───┴─────────┴───────────────────────────────────────────────────────╯ ``` # Tests + Formatting - [x] `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - [x] `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - [x] `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the tests for the standard library # After Submitting - [x] Bug fix, no doc update. --- Cargo.lock | 1 + crates/nu-engine/Cargo.toml | 3 ++- crates/nu-engine/src/documentation.rs | 35 ++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1c0fd9a954..f4495d6bfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3157,6 +3157,7 @@ dependencies = [ "nu-path", "nu-protocol", "nu-utils", + "terminal_size", ] [[package]] diff --git a/crates/nu-engine/Cargo.toml b/crates/nu-engine/Cargo.toml index be544265fe..2324eb5676 100644 --- a/crates/nu-engine/Cargo.toml +++ b/crates/nu-engine/Cargo.toml @@ -16,6 +16,7 @@ nu-path = { path = "../nu-path", version = "0.96.2" } nu-glob = { path = "../nu-glob", version = "0.96.2" } nu-utils = { path = "../nu-utils", version = "0.96.2" } log = { workspace = true } +terminal_size = { workspace = true } [features] -plugin = [] \ No newline at end of file +plugin = [] diff --git a/crates/nu-engine/src/documentation.rs b/crates/nu-engine/src/documentation.rs index a0dee3d343..0a7d742d57 100644 --- a/crates/nu-engine/src/documentation.rs +++ b/crates/nu-engine/src/documentation.rs @@ -7,6 +7,7 @@ use nu_protocol::{ Spanned, SyntaxShape, Type, Value, }; use std::{collections::HashMap, fmt::Write}; +use terminal_size::{Height, Width}; pub fn get_full_help( command: &dyn Command, @@ -234,6 +235,14 @@ fn get_documentation( } } + fn get_term_width() -> usize { + if let Some((Width(w), Height(_))) = terminal_size::terminal_size() { + w as usize + } else { + 80 + } + } + if !is_parser_keyword && !sig.input_output_types.is_empty() { if let Some(decl_id) = engine_state.find_decl(b"table", &[]) { // FIXME: we may want to make this the span of the help command in the future @@ -256,7 +265,18 @@ fn get_documentation( &Call { decl_id, head: span, - arguments: vec![], + arguments: vec![Argument::Named(( + Spanned { + item: "width".to_string(), + span: Span::unknown(), + }, + None, + Some(Expression::new_unknown( + Expr::Int(get_term_width() as i64 - 2), // padding, see below + Span::unknown(), + Type::Int, + )), + ))], parser_info: HashMap::new(), }, PipelineData::Value(Value::list(vals, span), None), @@ -334,6 +354,19 @@ fn get_documentation( None, )) } + table_call.add_named(( + Spanned { + item: "expand".to_string(), + span: Span::unknown(), + }, + None, + Some(Expression::new_unknown( + Expr::Int(get_term_width() as i64 - 2), + Span::unknown(), + Type::Int, + )), + )); + let table = engine_state .find_decl("table".as_bytes(), &[]) .and_then(|decl_id| { From 07e7c8c81f32481b44c0a55b9b0fc37c56f5fa16 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sat, 3 Aug 2024 16:42:30 +0200 Subject: [PATCH 11/11] Fixup #13526 width flag for example (#13529) # Description This seems to be a minor copy paste mistake. cc @Embers-of-the-Fire Followup to #13526 # User-Facing Changes (-) # Tests + Formatting (-) --- crates/nu-engine/src/documentation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-engine/src/documentation.rs b/crates/nu-engine/src/documentation.rs index 0a7d742d57..ed432d848f 100644 --- a/crates/nu-engine/src/documentation.rs +++ b/crates/nu-engine/src/documentation.rs @@ -356,7 +356,7 @@ fn get_documentation( } table_call.add_named(( Spanned { - item: "expand".to_string(), + item: "width".to_string(), span: Span::unknown(), }, None,