From 2d0a60ac678543a964fc578df3ff552c22127b86 Mon Sep 17 00:00:00 2001 From: Andrej Kolchin Date: Fri, 7 Jun 2024 15:43:30 +0300 Subject: [PATCH 01/38] Use native toml datetime type in `to toml` (#13018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Makes `to toml` use the `toml::value::Datetime` type, so that `to toml` serializes dates properly. # User-Facing Changes `to toml` will now encode dates differently, in a native format instead of a string. This could, in theory, break some workflows: ```Nushell # Before: ~> {datetime: 2024-05-31} | to toml | from toml | get datetime | into datetime Fri, 31 May 2024 00:00:00 +0000 (10 hours ago) # After: ~> {datetime: 2024-05-31} | to toml | from toml | get datetime | into datetime Error: nu::shell::only_supports_this_input_type × Input type not supported. ╭─[entry #13:1:36] 1 │ {datetime: 2024-05-31} | to toml | from toml | get datetime | into datetime · ────┬──── ──────┬────── · │ ╰── only string and int input data is supported · ╰── input type: date ╰──── ``` Fix #11751 --- crates/nu-command/src/formats/to/toml.rs | 58 ++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/nu-command/src/formats/to/toml.rs b/crates/nu-command/src/formats/to/toml.rs index 187304a7c0..ed1490231c 100644 --- a/crates/nu-command/src/formats/to/toml.rs +++ b/crates/nu-command/src/formats/to/toml.rs @@ -1,4 +1,4 @@ -use chrono::SecondsFormat; +use chrono::{DateTime, Datelike, FixedOffset, Timelike}; use nu_engine::command_prelude::*; use nu_protocol::ast::PathMember; @@ -49,9 +49,7 @@ fn helper(engine_state: &EngineState, v: &Value) -> Result toml::Value::Integer(*val), Value::Filesize { val, .. } => toml::Value::Integer(*val), Value::Duration { val, .. } => toml::Value::String(val.to_string()), - Value::Date { val, .. } => { - toml::Value::String(val.to_rfc3339_opts(SecondsFormat::AutoSi, false)) - } + Value::Date { val, .. } => toml::Value::Datetime(to_toml_datetime(val)), Value::Range { .. } => toml::Value::String("".to_string()), Value::Float { val, .. } => toml::Value::Float(*val), Value::String { val, .. } | Value::Glob { val, .. } => toml::Value::String(val.clone()), @@ -157,6 +155,43 @@ fn to_toml( } } +/// Convert chrono datetime into a toml::Value datetime. The latter uses its +/// own ad-hoc datetime types, which makes this somewhat convoluted. +fn to_toml_datetime(datetime: &DateTime) -> toml::value::Datetime { + let date = toml::value::Date { + // TODO: figure out what to do with BC dates, because the toml + // crate doesn't support them. Same for large years, which + // don't fit in u16. + year: datetime.year_ce().1 as u16, + // Panic: this is safe, because chrono guarantees that the month + // value will be between 1 and 12 and the day will be between 1 + // and 31 + month: datetime.month() as u8, + day: datetime.day() as u8, + }; + + let time = toml::value::Time { + // Panic: same as before, chorono guarantees that all of the following 3 + // methods return values less than 65'000 + hour: datetime.hour() as u8, + minute: datetime.minute() as u8, + second: datetime.second() as u8, + nanosecond: datetime.nanosecond(), + }; + + let offset = toml::value::Offset::Custom { + // Panic: minute timezone offset fits into i16 (that's more than + // 1000 hours) + minutes: (-datetime.timezone().utc_minus_local() / 60) as i16, + }; + + toml::value::Datetime { + date: Some(date), + time: Some(time), + offset: Some(offset), + } +} + #[cfg(test)] mod tests { use super::*; @@ -181,7 +216,20 @@ mod tests { Span::test_data(), ); - let reference_date = toml::Value::String(String::from("1980-10-12T10:12:44+02:00")); + let reference_date = toml::Value::Datetime(toml::value::Datetime { + date: Some(toml::value::Date { + year: 1980, + month: 10, + day: 12, + }), + time: Some(toml::value::Time { + hour: 10, + minute: 12, + second: 44, + nanosecond: 0, + }), + offset: Some(toml::value::Offset::Custom { minutes: 120 }), + }); let result = helper(&engine_state, &test_date); From a246a19387594cea15618a89a4bcd9d4c439d16a Mon Sep 17 00:00:00 2001 From: Access Date: Fri, 7 Jun 2024 21:02:52 +0800 Subject: [PATCH 02/38] fix: coredump without any messages (#13034) # Description - this PR should close https://github.com/nushell/nushell/issues/12874 - fixes https://github.com/nushell/nushell/issues/12874 I want to fix the issue which is induced by the fix for https://github.com/nushell/nushell/issues/12369. after this pr. This pr induced a new error for unix system, in order to show coredump messages # User-Facing Changes after fix for 12874, coredump message is messing, so I want to fix it # Tests + Formatting # After Submitting ![image](https://github.com/nushell/nushell/assets/60290287/6d8ab756-3031-4212-a5f5-5f71be3857f9) --------- Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com> --- crates/nu-protocol/src/errors/shell_error.rs | 15 +++++++++++++++ crates/nu-protocol/src/process/child.rs | 16 +++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index a8ca52142c..e201915fcf 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -876,6 +876,21 @@ pub enum ShellError { span: Span, }, + #[cfg(unix)] + /// An I/O operation failed. + /// + /// ## Resolution + /// + /// This is a generic error. Refer to the specific error message for further details. + #[error("program coredump error")] + #[diagnostic(code(nu::shell::coredump_error))] + CoredumpErrorSpanned { + msg: String, + signal: i32, + #[label("{msg}")] + span: Span, + }, + /// Tried to `cd` to a path that isn't a directory. /// /// ## Resolution diff --git a/crates/nu-protocol/src/process/child.rs b/crates/nu-protocol/src/process/child.rs index cc74b40fc1..08da7e704c 100644 --- a/crates/nu-protocol/src/process/child.rs +++ b/crates/nu-protocol/src/process/child.rs @@ -23,7 +23,21 @@ impl ExitStatusFuture { ExitStatusFuture::Finished(Err(err)) => Err(err.as_ref().clone()), ExitStatusFuture::Running(receiver) => { let code = match receiver.recv() { - Ok(Ok(status)) => Ok(status), + Ok(Ok(status)) => { + #[cfg(unix)] + if let ExitStatus::Signaled { + signal, + core_dumped: true, + } = status + { + return Err(ShellError::CoredumpErrorSpanned { + msg: format!("coredump detected. received signal: {signal}"), + signal, + span, + }); + } + Ok(status) + } Ok(Err(err)) => Err(ShellError::IOErrorSpanned { msg: format!("failed to get exit code: {err:?}"), span, From d6a9fb0e4013ba0e2d5f36e6bbf0b39f1221acab Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Fri, 7 Jun 2024 13:03:31 +0000 Subject: [PATCH 03/38] Fix display formatting for command type in `help commands` (#12996) # Description Related to #12832, this PR changes the way `help commands` displays the command type to be consistent with `scope commands` and `which`. # User-Facing Changes Technically a breaking change since the `help commands` output can now be different. --- crates/nu-command/src/help/help_commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-command/src/help/help_commands.rs b/crates/nu-command/src/help/help_commands.rs index f2440cc36f..f067a9fdcb 100644 --- a/crates/nu-command/src/help/help_commands.rs +++ b/crates/nu-command/src/help/help_commands.rs @@ -122,7 +122,7 @@ fn build_help_commands(engine_state: &EngineState, span: Span) -> Vec { let usage = sig.usage; let search_terms = sig.search_terms; - let command_type = format!("{:?}", decl.command_type()).to_ascii_lowercase(); + let command_type = decl.command_type().to_string(); // Build table of parameters let param_table = { From cfe13397edc6fde92744907fd3c4ae068543fdbf Mon Sep 17 00:00:00 2001 From: ymcx <89810988+ymcx@users.noreply.github.com> Date: Fri, 7 Jun 2024 16:07:23 +0300 Subject: [PATCH 04/38] Fix the colors when completing using a relative path (#12898) # Description Fixes a bug where the autocompletion menu has the wrong colors due to 'std::fs::symlink_metadata' not being able to handle relative paths. Attached below are screenshots before and after applying the commit, in which the colors are wrong when autocompleting on a path prefixed by a tilde, whereas the same directory is highlighted correctly when prefixed by a dot. BEFORE: ![1715982514020](https://github.com/nushell/nushell/assets/89810988/62051f03-0846-430d-b493-e6f3cd6d0e04) ![1715982873231](https://github.com/nushell/nushell/assets/89810988/28c647ab-3b2a-47ef-9967-5d09927e299d) AFTER: ![1715982490585](https://github.com/nushell/nushell/assets/89810988/7a370138-50af-42fd-9724-a34cc605bede) ![1715982894748](https://github.com/nushell/nushell/assets/89810988/e884f69f-f757-426e-98c4-bc9f7f6fc561) --- crates/nu-cli/src/completions/completion_common.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs index 4a8b0d57ac..26d48a8675 100644 --- a/crates/nu-cli/src/completions/completion_common.rs +++ b/crates/nu-cli/src/completions/completion_common.rs @@ -1,7 +1,7 @@ use crate::completions::{matches, CompletionOptions}; use nu_ansi_term::Style; use nu_engine::env_to_string; -use nu_path::home_dir; +use nu_path::{expand_to_real_path, home_dir}; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, Span, @@ -185,9 +185,14 @@ pub fn complete_item( .map(|p| { let path = original_cwd.apply(p); let style = ls_colors.as_ref().map(|lsc| { - lsc.style_for_path_with_metadata(&path, std::fs::symlink_metadata(&path).ok().as_ref()) - .map(lscolors::Style::to_nu_ansi_term_style) - .unwrap_or_default() + lsc.style_for_path_with_metadata( + &path, + std::fs::symlink_metadata(expand_to_real_path(&path)) + .ok() + .as_ref(), + ) + .map(lscolors::Style::to_nu_ansi_term_style) + .unwrap_or_default() }); (span, escape_path(path, want_directory), style) }) From d2121a155eb5b1ee06c2b1922327b5fba52d51a9 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Fri, 7 Jun 2024 10:33:48 -0400 Subject: [PATCH 05/38] Fixes #13093 - Erroneous example in 'touch' help (#13095) # Description Fixes #13093 by: * Removing the mentioned help example * Updating the `--accessed` and `--modified` flag descriptions to remove mention of "timestamp/date" # User-Facing Changes Help changes # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` --- crates/nu-command/src/filesystem/touch.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index 0253e7e317..2a1e128b04 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -35,12 +35,12 @@ impl Command for Touch { ) .switch( "modified", - "change the modification time of the file or directory. If no timestamp, date or reference file/directory is given, the current time is used", + "change the modification time of the file or directory. If no reference file/directory is given, the current time is used", Some('m'), ) .switch( "access", - "change the access time of the file or directory. If no timestamp, date or reference file/directory is given, the current time is used", + "change the access time of the file or directory. If no reference file/directory is given, the current time is used", Some('a'), ) .switch( @@ -189,11 +189,6 @@ impl Command for Touch { example: r#"touch -m -r fixture.json d e"#, result: None, }, - Example { - description: r#"Changes the last accessed time of "fixture.json" to a date"#, - example: r#"touch -a -d "August 24, 2019; 12:30:30" fixture.json"#, - result: None, - }, ] } } From 48a34ffe6d39bc72d8971e40dc54ef09ec331e49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sat, 8 Jun 2024 15:11:49 +0300 Subject: [PATCH 06/38] Fix test failure when running tests with nextest (#13101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Test was failing with “did you mean” due to the `NEXTEST` env var being present when running tests via `cargo nextest run`. # User-Facing Changes # Tests + Formatting # After Submitting --- tests/shell/environment/env.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/shell/environment/env.rs b/tests/shell/environment/env.rs index 74736415a3..1a9c14b18a 100644 --- a/tests/shell/environment/env.rs +++ b/tests/shell/environment/env.rs @@ -132,7 +132,7 @@ fn hides_environment_from_child() { $env.TEST = 1; ^$nu.current-exe -c "hide-env TEST; ^$nu.current-exe -c '$env.TEST'" "#); assert!(actual.out.is_empty()); - assert!(actual.err.contains("cannot find column")); + assert!(actual.err.contains("column_not_found") || actual.err.contains("name_not_found")); } #[test] From e52d7bc58541e38634fa5499396903f11b7e3f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 9 Jun 2024 12:15:53 +0300 Subject: [PATCH 07/38] Span ID Refactor (Step 2): Use SpanId of expressions in some places (#13102) # Description Part of https://github.com/nushell/nushell/issues/12963, step 2. This PR refactors changes the use of `expression.span` to `expression.span_id` via a new helper `Expression::span()`. A new `GetSpan` is added to abstract getting the span from both `EngineState` and `StateWorkingSet`. # User-Facing Changes `format pattern` loses the ability to use variables in the pattern, e.g., `... | format pattern 'value of {$it.name} is {$it.value}'`. This is because the command did a custom parse-eval cycle, creating spans that are not merged into the main engine state. We could clone the engine state, add Clone trait to StateDelta and merge the cloned delta to the cloned state, but IMO there is not much value from having this ability, since we have string interpolation nowadays: `... | $"value of ($in.name) is ($in.value)"`. # Tests + Formatting # After Submitting --- .../src/extra/strings/format/command.rs | 68 ++------ crates/nu-command/tests/commands/format.rs | 7 +- crates/nu-engine/src/eval.rs | 34 ++-- crates/nu-protocol/src/ast/expression.rs | 8 +- crates/nu-protocol/src/engine/engine_state.rs | 16 +- .../src/engine/state_working_set.rs | 8 +- crates/nu-protocol/src/eval_base.rs | 153 +++++++++--------- crates/nu-protocol/src/eval_const.rs | 8 +- crates/nu-protocol/src/span.rs | 5 + 9 files changed, 148 insertions(+), 159 deletions(-) diff --git a/crates/nu-cmd-extra/src/extra/strings/format/command.rs b/crates/nu-cmd-extra/src/extra/strings/format/command.rs index 1c72627779..50e89e61e3 100644 --- a/crates/nu-cmd-extra/src/extra/strings/format/command.rs +++ b/crates/nu-cmd-extra/src/extra/strings/format/command.rs @@ -1,5 +1,4 @@ -use nu_engine::{command_prelude::*, get_eval_expression}; -use nu_parser::parse_expression; +use nu_engine::command_prelude::*; use nu_protocol::{ast::PathMember, engine::StateWorkingSet, ListStream}; #[derive(Clone)] @@ -57,14 +56,7 @@ impl Command for FormatPattern { string_span.start + 1, )?; - format( - input_val, - &ops, - engine_state, - &mut working_set, - stack, - call.head, - ) + format(input_val, &ops, engine_state, call.head) } } } @@ -100,8 +92,6 @@ enum FormatOperation { FixedText(String), // raw input is something like {column1.column2} ValueFromColumn(String, Span), - // raw input is something like {$it.column1.column2} or {$var}. - ValueNeedEval(String, Span), } /// Given a pattern that is fed into the Format command, we can process it and subdivide it @@ -110,7 +100,6 @@ enum FormatOperation { /// there without any further processing. /// FormatOperation::ValueFromColumn contains the name of a column whose values will be /// formatted according to the input pattern. -/// FormatOperation::ValueNeedEval contains expression which need to eval, it has the following form: /// "$it.column1.column2" or "$variable" fn extract_formatting_operations( input: String, @@ -161,10 +150,17 @@ fn extract_formatting_operations( if !column_name.is_empty() { if column_need_eval { - output.push(FormatOperation::ValueNeedEval( - column_name.clone(), - Span::new(span_start + column_span_start, span_start + column_span_end), - )); + return Err(ShellError::GenericError { + error: "Removed functionality".into(), + msg: "The ability to use variables ($it) in `format pattern` has been removed." + .into(), + span: Some(error_span), + help: Some( + "You can use other formatting options, such as string interpolation." + .into(), + ), + inner: vec![], + }); } else { output.push(FormatOperation::ValueFromColumn( column_name.clone(), @@ -185,8 +181,6 @@ fn format( input_data: Value, format_operations: &[FormatOperation], engine_state: &EngineState, - working_set: &mut StateWorkingSet, - stack: &mut Stack, head_span: Span, ) -> Result { let data_as_value = input_data; @@ -194,13 +188,7 @@ fn format( // We can only handle a Record or a List of Records match data_as_value { Value::Record { .. } => { - match format_record( - format_operations, - &data_as_value, - engine_state, - working_set, - stack, - ) { + match format_record(format_operations, &data_as_value, engine_state) { Ok(value) => Ok(PipelineData::Value(Value::string(value, head_span), None)), Err(value) => Err(value), } @@ -211,13 +199,7 @@ fn format( for val in vals.iter() { match val { Value::Record { .. } => { - match format_record( - format_operations, - val, - engine_state, - working_set, - stack, - ) { + match format_record(format_operations, val, engine_state) { Ok(value) => { list.push(Value::string(value, head_span)); } @@ -256,12 +238,9 @@ fn format_record( format_operations: &[FormatOperation], data_as_value: &Value, engine_state: &EngineState, - working_set: &mut StateWorkingSet, - stack: &mut Stack, ) -> Result { let config = engine_state.get_config(); let mut output = String::new(); - let eval_expression = get_eval_expression(engine_state); for op in format_operations { match op { @@ -283,23 +262,6 @@ fn format_record( Err(se) => return Err(se), } } - FormatOperation::ValueNeedEval(_col_name, span) => { - let exp = parse_expression(working_set, &[*span]); - match working_set.parse_errors.first() { - None => { - let parsed_result = eval_expression(engine_state, stack, &exp); - if let Ok(val) = parsed_result { - output.push_str(&val.to_abbreviated_string(config)) - } - } - Some(err) => { - return Err(ShellError::TypeMismatch { - err_message: format!("expression is invalid, detail message: {err:?}"), - span: *span, - }) - } - } - } } } Ok(output) diff --git a/crates/nu-command/tests/commands/format.rs b/crates/nu-command/tests/commands/format.rs index 6b1649c8b5..6084faf1e4 100644 --- a/crates/nu-command/tests/commands/format.rs +++ b/crates/nu-command/tests/commands/format.rs @@ -37,7 +37,7 @@ fn given_fields_can_be_column_paths() { } #[test] -fn can_use_variables() { +fn cant_use_variables() { let actual = nu!( cwd: "tests/fixtures/formats", pipeline( r#" @@ -46,7 +46,8 @@ fn can_use_variables() { "# )); - assert_eq!(actual.out, "nu is a new type of shell"); + // TODO SPAN: This has been removed during SpanId refactor + assert!(actual.err.contains("Removed functionality")); } #[test] @@ -55,7 +56,7 @@ fn error_unmatched_brace() { cwd: "tests/fixtures/formats", pipeline( r#" open cargo_sample.toml - | format pattern "{$it.package.name" + | format pattern "{package.name" "# )); diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index af051a1dc7..044bf86980 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -208,11 +208,13 @@ fn eval_external( ) -> Result { let decl_id = engine_state .find_decl("run-external".as_bytes(), &[]) - .ok_or(ShellError::ExternalNotSupported { span: head.span })?; + .ok_or(ShellError::ExternalNotSupported { + span: head.span(&engine_state), + })?; let command = engine_state.get_decl(decl_id); - let mut call = Call::new(head.span); + let mut call = Call::new(head.span(&engine_state)); call.add_positional(head.clone()); @@ -712,7 +714,7 @@ impl Eval for EvalRuntime { args: &[ExternalArgument], _: Span, ) -> Result { - let span = head.span; + let span = head.span(&engine_state); // FIXME: protect this collect with ctrl-c eval_external(engine_state, stack, head, args, PipelineData::empty())?.into_value(span) } @@ -779,9 +781,11 @@ impl Eval for EvalRuntime { let var_info = engine_state.get_var(*var_id); if var_info.mutable { stack.add_var(*var_id, rhs); - Ok(Value::nothing(lhs.span)) + Ok(Value::nothing(lhs.span(&engine_state))) } else { - Err(ShellError::AssignmentRequiresMutableVar { lhs_span: lhs.span }) + Err(ShellError::AssignmentRequiresMutableVar { + lhs_span: lhs.span(&engine_state), + }) } } Expr::FullCellPath(cell_path) => { @@ -797,7 +801,7 @@ impl Eval for EvalRuntime { // Reject attempts to assign to the entire $env if cell_path.tail.is_empty() { return Err(ShellError::CannotReplaceEnv { - span: cell_path.head.span, + span: cell_path.head.span(&engine_state), }); } @@ -837,15 +841,21 @@ impl Eval for EvalRuntime { lhs.upsert_data_at_cell_path(&cell_path.tail, rhs)?; stack.add_var(*var_id, lhs); } - Ok(Value::nothing(cell_path.head.span)) + Ok(Value::nothing(cell_path.head.span(&engine_state))) } else { - Err(ShellError::AssignmentRequiresMutableVar { lhs_span: lhs.span }) + Err(ShellError::AssignmentRequiresMutableVar { + lhs_span: lhs.span(&engine_state), + }) } } - _ => Err(ShellError::AssignmentRequiresVar { lhs_span: lhs.span }), + _ => Err(ShellError::AssignmentRequiresVar { + lhs_span: lhs.span(&engine_state), + }), } } - _ => Err(ShellError::AssignmentRequiresVar { lhs_span: lhs.span }), + _ => Err(ShellError::AssignmentRequiresVar { + lhs_span: lhs.span(&engine_state), + }), } } @@ -882,8 +892,8 @@ impl Eval for EvalRuntime { Ok(Value::string(name, span)) } - fn unreachable(expr: &Expression) -> Result { - Ok(Value::nothing(expr.span)) + fn unreachable(engine_state: &EngineState, expr: &Expression) -> Result { + Ok(Value::nothing(expr.span(&engine_state))) } } diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 832bc1f438..08ecc59aaf 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -1,7 +1,7 @@ use crate::{ ast::{Argument, Block, Expr, ExternalArgument, ImportPattern, MatchPattern, RecordItem}, - engine::{EngineState, StateWorkingSet}, - BlockId, DeclId, Signature, Span, SpanId, Type, VarId, IN_VARIABLE_ID, + engine::StateWorkingSet, + BlockId, DeclId, GetSpan, Signature, Span, SpanId, Type, VarId, IN_VARIABLE_ID, }; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -516,7 +516,7 @@ impl Expression { } } - pub fn span(&self, engine_state: &EngineState) -> Span { - engine_state.get_span(self.span_id) + pub fn span(&self, state: &impl GetSpan) -> Span { + state.get_span(self.span_id) } } diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index e81d3360ab..3960f6d9d0 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -7,7 +7,7 @@ use crate::{ Variable, Visibility, DEFAULT_OVERLAY_NAME, }, eval_const::create_nu_constant, - BlockId, Category, Config, DeclId, FileId, HistoryConfig, Module, ModuleId, OverlayId, + BlockId, Category, Config, DeclId, FileId, GetSpan, HistoryConfig, Module, ModuleId, OverlayId, ShellError, Signature, Span, SpanId, Type, Value, VarId, VirtualPathId, }; use fancy_regex::Regex; @@ -1035,18 +1035,20 @@ impl EngineState { SpanId(self.num_spans() - 1) } + /// Find ID of a span (should be avoided if possible) + pub fn find_span_id(&self, span: Span) -> Option { + self.spans.iter().position(|sp| sp == &span).map(SpanId) + } +} + +impl<'a> GetSpan for &'a EngineState { /// Get existing span - pub fn get_span(&self, span_id: SpanId) -> Span { + fn get_span(&self, span_id: SpanId) -> Span { *self .spans .get(span_id.0) .expect("internal error: missing span") } - - /// Find ID of a span (should be avoided if possible) - pub fn find_span_id(&self, span: Span) -> Option { - self.spans.iter().position(|sp| sp == &span).map(SpanId) - } } impl Default for EngineState { diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs index 31600b5eb9..af950b8321 100644 --- a/crates/nu-protocol/src/engine/state_working_set.rs +++ b/crates/nu-protocol/src/engine/state_working_set.rs @@ -4,8 +4,8 @@ use crate::{ usage::build_usage, CachedFile, Command, CommandType, EngineState, OverlayFrame, StateDelta, Variable, VirtualPath, Visibility, }, - BlockId, Category, Config, DeclId, FileId, Module, ModuleId, ParseError, ParseWarning, Span, - SpanId, Type, Value, VarId, VirtualPathId, + BlockId, Category, Config, DeclId, FileId, GetSpan, Module, ModuleId, ParseError, ParseWarning, + Span, SpanId, Type, Value, VarId, VirtualPathId, }; use core::panic; use std::{ @@ -1019,8 +1019,10 @@ impl<'a> StateWorkingSet<'a> { self.delta.spans.push(span); SpanId(num_permanent_spans + self.delta.spans.len() - 1) } +} - pub fn get_span(&self, span_id: SpanId) -> Span { +impl<'a> GetSpan for &'a StateWorkingSet<'a> { + fn get_span(&self, span_id: SpanId) -> Span { let num_permanent_spans = self.permanent_state.num_spans(); if span_id.0 < num_permanent_spans { self.permanent_state.get_span(span_id) diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index 6a1c0d302f..2317149460 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -4,7 +4,7 @@ use crate::{ ExternalArgument, ListItem, Math, Operator, RecordItem, }, debugger::DebugContext, - Config, Range, Record, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, + Config, GetSpan, Range, Record, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, }; use std::{borrow::Cow, collections::HashMap}; @@ -12,7 +12,7 @@ use std::{borrow::Cow, collections::HashMap}; pub trait Eval { /// State that doesn't need to be mutated. /// EngineState for regular eval and StateWorkingSet for const eval - type State<'a>: Copy; + type State<'a>: Copy + GetSpan; /// State that needs to be mutated. /// This is the stack for regular eval, and unused by const eval @@ -23,17 +23,19 @@ pub trait Eval { mut_state: &mut Self::MutState, expr: &Expression, ) -> Result { + let expr_span = expr.span(&state); + match &expr.expr { - Expr::Bool(b) => Ok(Value::bool(*b, expr.span)), - Expr::Int(i) => Ok(Value::int(*i, expr.span)), - Expr::Float(f) => Ok(Value::float(*f, expr.span)), - Expr::Binary(b) => Ok(Value::binary(b.clone(), expr.span)), - Expr::Filepath(path, quoted) => Self::eval_filepath(state, mut_state, path.clone(), *quoted, expr.span), + Expr::Bool(b) => Ok(Value::bool(*b, expr_span)), + Expr::Int(i) => Ok(Value::int(*i, expr_span)), + Expr::Float(f) => Ok(Value::float(*f, expr_span)), + Expr::Binary(b) => Ok(Value::binary(b.clone(), expr_span)), + Expr::Filepath(path, quoted) => Self::eval_filepath(state, mut_state, path.clone(), *quoted, expr_span), Expr::Directory(path, quoted) => { - Self::eval_directory(state, mut_state, path.clone(), *quoted, expr.span) + Self::eval_directory(state, mut_state, path.clone(), *quoted, expr_span) } - Expr::Var(var_id) => Self::eval_var(state, mut_state, *var_id, expr.span), - Expr::CellPath(cell_path) => Ok(Value::cell_path(cell_path.clone(), expr.span)), + Expr::Var(var_id) => Self::eval_var(state, mut_state, *var_id, expr_span), + Expr::CellPath(cell_path) => Ok(Value::cell_path(cell_path.clone(), expr_span)), Expr::FullCellPath(cell_path) => { let value = Self::eval::(state, mut_state, &cell_path.head)?; @@ -45,7 +47,7 @@ pub trait Eval { value.follow_cell_path(&cell_path.tail, false) } } - Expr::DateTime(dt) => Ok(Value::date(*dt, expr.span)), + Expr::DateTime(dt) => Ok(Value::date(*dt, expr_span)), Expr::List(list) => { let mut output = vec![]; for item in list { @@ -53,11 +55,11 @@ pub trait Eval { ListItem::Item(expr) => output.push(Self::eval::(state, mut_state, expr)?), ListItem::Spread(_, expr) => match Self::eval::(state, mut_state, expr)? { Value::List { vals, .. } => output.extend(vals), - _ => return Err(ShellError::CannotSpreadAsList { span: expr.span }), + _ => return Err(ShellError::CannotSpreadAsList { span: expr_span }), }, } } - Ok(Value::list(output, expr.span)) + Ok(Value::list(output, expr_span)) } Expr::Record(items) => { let mut record = Record::new(); @@ -67,36 +69,38 @@ pub trait Eval { RecordItem::Pair(col, val) => { // avoid duplicate cols let col_name = Self::eval::(state, mut_state, col)?.coerce_into_string()?; + let col_span = col.span(&state); if let Some(orig_span) = col_names.get(&col_name) { return Err(ShellError::ColumnDefinedTwice { col_name, - second_use: col.span, + second_use: col_span, first_use: *orig_span, }); } else { - col_names.insert(col_name.clone(), col.span); + col_names.insert(col_name.clone(), col_span); record.push(col_name, Self::eval::(state, mut_state, val)?); } } RecordItem::Spread(_, inner) => { + let inner_span = inner.span(&state); match Self::eval::(state, mut_state, inner)? { Value::Record { val: inner_val, .. } => { for (col_name, val) in inner_val.into_owned() { if let Some(orig_span) = col_names.get(&col_name) { return Err(ShellError::ColumnDefinedTwice { col_name, - second_use: inner.span, + second_use: inner_span, first_use: *orig_span, }); } else { - col_names.insert(col_name.clone(), inner.span); + col_names.insert(col_name.clone(), inner_span); record.push(col_name, val); } } } _ => { return Err(ShellError::CannotSpreadAsRecord { - span: inner.span, + span: inner_span, }) } } @@ -104,7 +108,7 @@ pub trait Eval { } } - Ok(Value::record(record, expr.span)) + Ok(Value::record(record, expr_span)) } Expr::Table(table) => { let mut output_headers = vec![]; @@ -114,10 +118,11 @@ pub trait Eval { .iter() .position(|existing| existing == &header) { + let first_use = table.columns[idx].span(&state); return Err(ShellError::ColumnDefinedTwice { col_name: header, - second_use: expr.span, - first_use: table.columns[idx].span, + second_use: expr_span, + first_use, }); } else { output_headers.push(header); @@ -132,66 +137,66 @@ pub trait Eval { output_rows.push(Value::record( record, - expr.span, + expr_span, )); } - Ok(Value::list(output_rows, expr.span)) + Ok(Value::list(output_rows, expr_span)) } Expr::Keyword(kw) => Self::eval::(state, mut_state, &kw.expr), - Expr::String(s) | Expr::RawString(s) => Ok(Value::string(s.clone(), expr.span)), - Expr::Nothing => Ok(Value::nothing(expr.span)), + Expr::String(s) | Expr::RawString(s) => Ok(Value::string(s.clone(), expr_span)), + Expr::Nothing => Ok(Value::nothing(expr_span)), Expr::ValueWithUnit(value) => match Self::eval::(state, mut_state, &value.expr)? { Value::Int { val, .. } => value.unit.item.build_value(val, value.unit.span), x => Err(ShellError::CantConvert { to_type: "unit value".into(), from_type: x.get_type().to_string(), - span: value.expr.span, + span: value.expr.span(&state), help: None, }), }, - Expr::Call(call) => Self::eval_call::(state, mut_state, call, expr.span), + Expr::Call(call) => Self::eval_call::(state, mut_state, call, expr_span), Expr::ExternalCall(head, args) => { - Self::eval_external_call(state, mut_state, head, args, expr.span) + Self::eval_external_call(state, mut_state, head, args, expr_span) } Expr::Subexpression(block_id) => { - Self::eval_subexpression::(state, mut_state, *block_id, expr.span) + Self::eval_subexpression::(state, mut_state, *block_id, expr_span) } Expr::Range(range) => { let from = if let Some(f) = &range.from { Self::eval::(state, mut_state, f)? } else { - Value::nothing(expr.span) + Value::nothing(expr_span) }; let next = if let Some(s) = &range.next { Self::eval::(state, mut_state, s)? } else { - Value::nothing(expr.span) + Value::nothing(expr_span) }; let to = if let Some(t) = &range.to { Self::eval::(state, mut_state, t)? } else { - Value::nothing(expr.span) + Value::nothing(expr_span) }; Ok(Value::range( - Range::new(from, next, to, range.operator.inclusion, expr.span)?, - expr.span, + Range::new(from, next, to, range.operator.inclusion, expr_span)?, + expr_span, )) } Expr::UnaryNot(expr) => { let lhs = Self::eval::(state, mut_state, expr)?; match lhs { - Value::Bool { val, .. } => Ok(Value::bool(!val, expr.span)), + Value::Bool { val, .. } => Ok(Value::bool(!val, expr_span)), other => Err(ShellError::TypeMismatch { err_message: format!("expected bool, found {}", other.get_type()), - span: expr.span, + span: expr_span, }), } } Expr::BinaryOp(lhs, op, rhs) => { - let op_span = op.span; + let op_span = op.span(&state); let op = eval_operator(op)?; match op { @@ -200,23 +205,23 @@ pub trait Eval { match boolean { Boolean::And => { if lhs.is_false() { - Ok(Value::bool(false, expr.span)) + Ok(Value::bool(false, expr_span)) } else { let rhs = Self::eval::(state, mut_state, rhs)?; - lhs.and(op_span, &rhs, expr.span) + lhs.and(op_span, &rhs, expr_span) } } Boolean::Or => { if lhs.is_true() { - Ok(Value::bool(true, expr.span)) + Ok(Value::bool(true, expr_span)) } else { let rhs = Self::eval::(state, mut_state, rhs)?; - lhs.or(op_span, &rhs, expr.span) + lhs.or(op_span, &rhs, expr_span) } } Boolean::Xor => { let rhs = Self::eval::(state, mut_state, rhs)?; - lhs.xor(op_span, &rhs, expr.span) + lhs.xor(op_span, &rhs, expr_span) } } } @@ -225,35 +230,35 @@ pub trait Eval { let rhs = Self::eval::(state, mut_state, rhs)?; match math { - Math::Plus => lhs.add(op_span, &rhs, expr.span), - Math::Minus => lhs.sub(op_span, &rhs, expr.span), - Math::Multiply => lhs.mul(op_span, &rhs, expr.span), - Math::Divide => lhs.div(op_span, &rhs, expr.span), - Math::Append => lhs.append(op_span, &rhs, expr.span), - Math::Modulo => lhs.modulo(op_span, &rhs, expr.span), - Math::FloorDivision => lhs.floor_div(op_span, &rhs, expr.span), - Math::Pow => lhs.pow(op_span, &rhs, expr.span), + Math::Plus => lhs.add(op_span, &rhs, expr_span), + Math::Minus => lhs.sub(op_span, &rhs, expr_span), + Math::Multiply => lhs.mul(op_span, &rhs, expr_span), + Math::Divide => lhs.div(op_span, &rhs, expr_span), + Math::Append => lhs.append(op_span, &rhs, expr_span), + Math::Modulo => lhs.modulo(op_span, &rhs, expr_span), + Math::FloorDivision => lhs.floor_div(op_span, &rhs, expr_span), + Math::Pow => lhs.pow(op_span, &rhs, expr_span), } } Operator::Comparison(comparison) => { let lhs = Self::eval::(state, mut_state, lhs)?; let rhs = Self::eval::(state, mut_state, rhs)?; match comparison { - Comparison::LessThan => lhs.lt(op_span, &rhs, expr.span), - Comparison::LessThanOrEqual => lhs.lte(op_span, &rhs, expr.span), - Comparison::GreaterThan => lhs.gt(op_span, &rhs, expr.span), - Comparison::GreaterThanOrEqual => lhs.gte(op_span, &rhs, expr.span), - Comparison::Equal => lhs.eq(op_span, &rhs, expr.span), - Comparison::NotEqual => lhs.ne(op_span, &rhs, expr.span), - Comparison::In => lhs.r#in(op_span, &rhs, expr.span), - Comparison::NotIn => lhs.not_in(op_span, &rhs, expr.span), - Comparison::StartsWith => lhs.starts_with(op_span, &rhs, expr.span), - Comparison::EndsWith => lhs.ends_with(op_span, &rhs, expr.span), + Comparison::LessThan => lhs.lt(op_span, &rhs, expr_span), + Comparison::LessThanOrEqual => lhs.lte(op_span, &rhs, expr_span), + Comparison::GreaterThan => lhs.gt(op_span, &rhs, expr_span), + Comparison::GreaterThanOrEqual => lhs.gte(op_span, &rhs, expr_span), + Comparison::Equal => lhs.eq(op_span, &rhs, expr_span), + Comparison::NotEqual => lhs.ne(op_span, &rhs, expr_span), + Comparison::In => lhs.r#in(op_span, &rhs, expr_span), + Comparison::NotIn => lhs.not_in(op_span, &rhs, expr_span), + Comparison::StartsWith => lhs.starts_with(op_span, &rhs, expr_span), + Comparison::EndsWith => lhs.ends_with(op_span, &rhs, expr_span), Comparison::RegexMatch => { - Self::regex_match(state, op_span, &lhs, &rhs, false, expr.span) + Self::regex_match(state, op_span, &lhs, &rhs, false, expr_span) } Comparison::NotRegexMatch => { - Self::regex_match(state, op_span, &lhs, &rhs, true, expr.span) + Self::regex_match(state, op_span, &lhs, &rhs, true, expr_span) } } } @@ -261,20 +266,20 @@ pub trait Eval { let lhs = Self::eval::(state, mut_state, lhs)?; let rhs = Self::eval::(state, mut_state, rhs)?; match bits { - Bits::BitAnd => lhs.bit_and(op_span, &rhs, expr.span), - Bits::BitOr => lhs.bit_or(op_span, &rhs, expr.span), - Bits::BitXor => lhs.bit_xor(op_span, &rhs, expr.span), - Bits::ShiftLeft => lhs.bit_shl(op_span, &rhs, expr.span), - Bits::ShiftRight => lhs.bit_shr(op_span, &rhs, expr.span), + Bits::BitAnd => lhs.bit_and(op_span, &rhs, expr_span), + Bits::BitOr => lhs.bit_or(op_span, &rhs, expr_span), + Bits::BitXor => lhs.bit_xor(op_span, &rhs, expr_span), + Bits::ShiftLeft => lhs.bit_shl(op_span, &rhs, expr_span), + Bits::ShiftRight => lhs.bit_shr(op_span, &rhs, expr_span), } } Operator::Assignment(assignment) => Self::eval_assignment::( - state, mut_state, lhs, rhs, assignment, op_span, expr.span + state, mut_state, lhs, rhs, assignment, op_span, expr_span ), } } Expr::RowCondition(block_id) | Expr::Closure(block_id) => { - Self::eval_row_condition_or_closure(state, mut_state, *block_id, expr.span) + Self::eval_row_condition_or_closure(state, mut_state, *block_id, expr_span) } Expr::StringInterpolation(exprs) => { let config = Self::get_config(state, mut_state); @@ -283,13 +288,13 @@ pub trait Eval { .map(|expr| Self::eval::(state, mut_state, expr).map(|v| v.to_expanded_string(", ", &config))) .collect::>()?; - Ok(Value::string(str, expr.span)) + Ok(Value::string(str, expr_span)) } - Expr::Overlay(_) => Self::eval_overlay(state, expr.span), + Expr::Overlay(_) => Self::eval_overlay(state, expr_span), Expr::GlobPattern(pattern, quoted) => { // GlobPattern is similar to Filepath // But we don't want to expand path during eval time, it's required for `nu_engine::glob_from` to run correctly - Ok(Value::glob(pattern, *quoted, expr.span)) + Ok(Value::glob(pattern, *quoted, expr_span)) } Expr::MatchBlock(_) // match blocks are handled by `match` | Expr::Block(_) // blocks are handled directly by core commands @@ -297,7 +302,7 @@ pub trait Eval { | Expr::ImportPattern(_) | Expr::Signature(_) | Expr::Operator(_) - | Expr::Garbage => Self::unreachable(expr), + | Expr::Garbage => Self::unreachable(state, expr), } } @@ -378,5 +383,5 @@ pub trait Eval { fn eval_overlay(state: Self::State<'_>, span: Span) -> Result; /// For expressions that should never actually be evaluated - fn unreachable(expr: &Expression) -> Result; + fn unreachable(state: Self::State<'_>, expr: &Expression) -> Result; } diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 4cc7e25324..08ff6fa391 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -251,7 +251,7 @@ pub fn eval_constant_with_input( Expr::Call(call) => eval_const_call(working_set, call, input), Expr::Subexpression(block_id) => { let block = working_set.get_block(*block_id); - eval_const_subexpression(working_set, block, input, expr.span) + eval_const_subexpression(working_set, block, input, expr.span(&working_set)) } _ => eval_constant(working_set, expr).map(|v| PipelineData::Value(v, None)), } @@ -379,7 +379,9 @@ impl Eval for EvalConst { Err(ShellError::NotAConstant { span }) } - fn unreachable(expr: &Expression) -> Result { - Err(ShellError::NotAConstant { span: expr.span }) + fn unreachable(working_set: &StateWorkingSet, expr: &Expression) -> Result { + Err(ShellError::NotAConstant { + span: expr.span(&working_set), + }) } } diff --git a/crates/nu-protocol/src/span.rs b/crates/nu-protocol/src/span.rs index 3d32aa4ddf..0d280eaa9d 100644 --- a/crates/nu-protocol/src/span.rs +++ b/crates/nu-protocol/src/span.rs @@ -1,7 +1,12 @@ +use crate::SpanId; use miette::SourceSpan; use serde::{Deserialize, Serialize}; use std::ops::Deref; +pub trait GetSpan { + fn get_span(&self, span_id: SpanId) -> Span; +} + /// A spanned area of interest, generic over what kind of thing is of interest #[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct Spanned { From 5ac3ad61c42c9c97952a5df11d79e69ffcde73db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 9 Jun 2024 14:03:52 +0300 Subject: [PATCH 08/38] Extend functionality of tango benchmark helpers (#13107) # Description Refactors the tango helpers in the toolkit and makes them more flexible (e.g., being able to benchmark any branch against any branch, not just current and main). # User-Facing Changes # Tests + Formatting # After Submitting --- toolkit.nu | 78 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 17 deletions(-) diff --git a/toolkit.nu b/toolkit.nu index 6f19900fdb..0ef56d1077 100644 --- a/toolkit.nu +++ b/toolkit.nu @@ -491,29 +491,73 @@ export def cov [] { print $"Coverage generation took ($end - $start)." } - -# Benchmark the current branch against the main branch +# Benchmark a target revision (default: current branch) against a reference revision (default: main branch) +# +# Results are saved in a `./tango` directory # Ensure you have `cargo-export` installed to generate separate artifacts for each branch. -export def "benchmark-current-branch-with-main" [] { - let main = "main" - let current_branch = (git branch --show-current) +export def benchmark-compare [ + target?: string # which branch to compare (default: current branch) + reference?: string # the reference to compare against (default: main branch) +] { + let reference = $reference | default "main" + let current = git branch --show-current + let target = $target | default $current - cargo export $"target/($current_branch)" -- bench - git checkout $main - cargo export $"target/($main)" -- bench - git checkout $current_branch - ^$"./target/($current_branch)/benchmarks" compare $"./target/($main)/benchmarks" -o -s 50 + print $'-- Benchmarking ($target) against ($reference)' + + let export_dir = $env.PWD | path join "tango" + let ref_bin_dir = $export_dir | path join bin $reference + let tgt_bin_dir = $export_dir | path join bin $target + + # benchmark the target revision + print $'-- Running benchmarks for ($target)' + git checkout $target + cargo export $tgt_bin_dir -- bench + + # benchmark the comparison reference revision + print $'-- Running benchmarks for ($reference)' + git checkout $reference + cargo export $ref_bin_dir -- bench + + # return back to the whatever revision before benchmarking + print '-- Done' + git checkout $current + + # report results + let reference_bin = $ref_bin_dir | path join benchmarks + let target_bin = $tgt_bin_dir | path join benchmarks + ^$target_bin compare $reference_bin -o -s 50 --dump ($export_dir | path join samples) } -# Benchmark the current branch and logs the result in `./target/samples` +# Benchmark the current branch and logs the result in `./tango/samples` +# +# Results are saved in a `./tango` directory # Ensure you have `cargo-export` installed to generate separate artifacts for each branch. -export def "benchmark-and-log-result" [] { - let current_branch = (git branch --show-current) - let current_dir = "./" | path expand - let res_path = $"($current_dir)/target/samples" +export def benchmark-log [ + target?: string # which branch to compare (default: current branch) +] { + let current = git branch --show-current + let target = $target | default $current + print $'-- Benchmarking ($target)' - cargo export $"target/($current_branch)" -- bench - ^$"./target/($current_branch)/benchmarks" compare -o -s 50 --dump $res_path + let export_dir = $env.PWD | path join "tango" + let bin_dir = ($export_dir | path join bin $target) + + # benchmark the target revision + if $target != $current { + git checkout $target + } + cargo export $bin_dir -- bench + + # return back to the whatever revision before benchmarking + print '-- Done' + if $target != $current { + git checkout $current + } + + # report results + let bench_bin = ($bin_dir | path join benchmarks) + ^$bench_bin compare -o -s 50 --dump ($export_dir | path join samples) } # Build all Windows archives and MSIs for release manually From dc76183cd535d44eb184d6e428a5a9f33c88dfc4 Mon Sep 17 00:00:00 2001 From: Sang-Heon Jeon Date: Mon, 10 Jun 2024 11:43:17 +0900 Subject: [PATCH 09/38] fix wrong casting with into filesize (#13110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Fix wrong casting which is related to https://github.com/nushell/nushell/pull/12974#discussion_r1618598336 # User-Facing Changes AS-IS (before fixing) ``` $ "-10000PiB" | into filesize 6.2 EiB <--- Wrong casted value $ "10000PiB" | into filesize -6.2 EiB <--- Wrong casted value ``` TO-BE (after fixing) ``` $ "-10000PiB" | into filesize Error: nu::shell::cant_convert × Can't convert to filesize. ╭─[entry #6:1:1] 1 │ "-10000PiB" | into filesize · ─────┬───── · ╰── can't convert string to filesize ╰──── $ "10000PiB" | into filesize Error: nu::shell::cant_convert × Can't convert to filesize. ╭─[entry #7:1:1] 1 │ "10000PiB" | into filesize · ─────┬──── · ╰── can't convert string to filesize ╰──── ``` --- .../src/conversions/into/filesize.rs | 21 ++++++++++++++----- .../tests/commands/into_filesize.rs | 14 +++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/crates/nu-command/src/conversions/into/filesize.rs b/crates/nu-command/src/conversions/into/filesize.rs index 101a9fe6f0..010c031b2a 100644 --- a/crates/nu-command/src/conversions/into/filesize.rs +++ b/crates/nu-command/src/conversions/into/filesize.rs @@ -122,7 +122,7 @@ pub fn action(input: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { Value::Filesize { .. } => input.clone(), Value::Int { val, .. } => Value::filesize(*val, value_span), Value::Float { val, .. } => Value::filesize(*val as i64, value_span), - Value::String { val, .. } => match int_from_string(val, value_span) { + Value::String { val, .. } => match i64_from_string(val, value_span) { Ok(val) => Value::filesize(val, value_span), Err(error) => Value::error(error, value_span), }, @@ -139,7 +139,7 @@ pub fn action(input: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value { } } -fn int_from_string(a_string: &str, span: Span) -> Result { +fn i64_from_string(a_string: &str, span: Span) -> Result { // Get the Locale so we know what the thousands separator is let locale = get_system_locale(); @@ -151,24 +151,35 @@ fn int_from_string(a_string: &str, span: Span) -> Result { // Hadle negative file size if let Some(stripped_negative_string) = clean_string.strip_prefix('-') { match stripped_negative_string.parse::() { - Ok(n) => Ok(-(n.as_u64() as i64)), + Ok(n) => i64_from_byte_size(n, true, span), Err(_) => Err(string_convert_error(span)), } } else if let Some(stripped_positive_string) = clean_string.strip_prefix('+') { match stripped_positive_string.parse::() { Ok(n) if stripped_positive_string.starts_with(|c: char| c.is_ascii_digit()) => { - Ok(n.as_u64() as i64) + i64_from_byte_size(n, false, span) } _ => Err(string_convert_error(span)), } } else { match clean_string.parse::() { - Ok(n) => Ok(n.as_u64() as i64), + Ok(n) => i64_from_byte_size(n, false, span), Err(_) => Err(string_convert_error(span)), } } } +fn i64_from_byte_size( + byte_size: bytesize::ByteSize, + is_negative: bool, + span: Span, +) -> Result { + match i64::try_from(byte_size.as_u64()) { + Ok(n) => Ok(if is_negative { -n } else { n }), + Err(_) => Err(string_convert_error(span)), + } +} + fn string_convert_error(span: Span) -> ShellError { ShellError::CantConvert { to_type: "filesize".into(), diff --git a/crates/nu-command/tests/commands/into_filesize.rs b/crates/nu-command/tests/commands/into_filesize.rs index d8ba88021c..6a72c76b5d 100644 --- a/crates/nu-command/tests/commands/into_filesize.rs +++ b/crates/nu-command/tests/commands/into_filesize.rs @@ -76,6 +76,13 @@ fn into_filesize_wrong_negative_str_filesize() { assert!(actual.err.contains("can't convert string to filesize")); } +#[test] +fn into_filesize_large_negative_str_filesize() { + let actual = nu!("'-10000PiB' | into filesize"); + + assert!(actual.err.contains("can't convert string to filesize")); +} + #[test] fn into_filesize_negative_str() { let actual = nu!("'-1' | into filesize"); @@ -104,6 +111,13 @@ fn into_filesize_wrong_positive_str_filesize() { assert!(actual.err.contains("can't convert string to filesize")); } +#[test] +fn into_filesize_large_positive_str_filesize() { + let actual = nu!("'+10000PiB' | into filesize"); + + assert!(actual.err.contains("can't convert string to filesize")); +} + #[test] fn into_filesize_positive_str() { let actual = nu!("'+1' | into filesize"); From 650ae537c3a7346e9ce899e41634f611786399dd Mon Sep 17 00:00:00 2001 From: Jack Wright <56345+ayax79@users.noreply.github.com> Date: Sun, 9 Jun 2024 19:44:04 -0700 Subject: [PATCH 10/38] Fix the use of right hand expressions in operations (#13096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As reported by @maxim-uvarov and pyz in the dataframes discord channel: ```nushell [[a b]; [1 1] [1 2] [2 1] [2 2] [3 1] [3 2]] | polars into-df | polars with-column ((polars col a) / (polars col b)) --name c × Type mismatch. ╭─[entry #45:1:102] 1 │ [[a b]; [1 1] [1 2] [2 1] [2 2] [3 1] [3 2]] | polars into-df | polars with-column ((polars col a) / (polars col b)) --name c · ───────┬────── · ╰── Right hand side not a dataframe expression ╰──── ``` This pull request corrects the type casting on the right hand side and allows more than just polars literal expressions. --- .../values/nu_expression/custom_value.rs | 49 ++++--------------- 1 file changed, 10 insertions(+), 39 deletions(-) diff --git a/crates/nu_plugin_polars/src/dataframe/values/nu_expression/custom_value.rs b/crates/nu_plugin_polars/src/dataframe/values/nu_expression/custom_value.rs index ea751b0255..a9371f03f2 100644 --- a/crates/nu_plugin_polars/src/dataframe/values/nu_expression/custom_value.rs +++ b/crates/nu_plugin_polars/src/dataframe/values/nu_expression/custom_value.rs @@ -63,45 +63,16 @@ fn compute_with_value( op: Span, right: &Value, ) -> Result { - let rhs_span = right.span(); - match right { - Value::Custom { val: rhs, .. } => { - let rhs = rhs.as_any().downcast_ref::().ok_or_else(|| { - ShellError::TypeMismatch { - err_message: "Right hand side not a dataframe expression".into(), - span: rhs_span, - } - })?; - - match rhs.as_ref() { - polars::prelude::Expr::Literal(..) => with_operator( - (plugin, engine), - operator, - left, - rhs, - lhs_span, - right.span(), - op, - ), - _ => Err(ShellError::TypeMismatch { - err_message: "Only literal expressions or number".into(), - span: right.span(), - }), - } - } - _ => { - let rhs = NuExpression::try_from_value(plugin, right)?; - with_operator( - (plugin, engine), - operator, - left, - &rhs, - lhs_span, - right.span(), - op, - ) - } - } + let rhs = NuExpression::try_from_value(plugin, right)?; + with_operator( + (plugin, engine), + operator, + left, + &rhs, + lhs_span, + right.span(), + op, + ) } fn with_operator( From 021b8633cb8f776ca1a30db2597d107aa92fbf6f Mon Sep 17 00:00:00 2001 From: Jack Wright <56345+ayax79@users.noreply.github.com> Date: Sun, 9 Jun 2024 19:45:25 -0700 Subject: [PATCH 11/38] Allow the addition of an index column to be optional (#13097) Per discussion on discord dataframes channel with @maxim-uvarov and pyz. When converting a dataframe to an nushell value via `polars into-nu`, the index column should not be added by default and should only be added when specifying `--index` --- .../src/dataframe/eager/to_nu.rs | 14 ++++---- .../src/dataframe/values/nu_dataframe/mod.rs | 32 +++++++++++++------ 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/crates/nu_plugin_polars/src/dataframe/eager/to_nu.rs b/crates/nu_plugin_polars/src/dataframe/eager/to_nu.rs index 8e3cdffa24..72aff7a728 100644 --- a/crates/nu_plugin_polars/src/dataframe/eager/to_nu.rs +++ b/crates/nu_plugin_polars/src/dataframe/eager/to_nu.rs @@ -35,6 +35,7 @@ impl PluginCommand for ToNu { Some('n'), ) .switch("tail", "shows tail rows", Some('t')) + .switch("index", "add an index column", Some('i')) .input_output_types(vec![ (Type::Custom("expression".into()), Type::Any), (Type::Custom("dataframe".into()), Type::table()), @@ -62,18 +63,18 @@ impl PluginCommand for ToNu { vec![ Example { description: "Shows head rows from dataframe", - example: "[[a b]; [1 2] [3 4]] | polars into-df | polars into-nu", + example: "[[a b]; [1 2] [3 4]] | polars into-df | polars into-nu --index", result: Some(Value::list(vec![rec_1, rec_2], Span::test_data())), }, Example { description: "Shows tail rows from dataframe", example: - "[[a b]; [1 2] [5 6] [3 4]] | polars into-df | polars into-nu --tail --rows 1", + "[[a b]; [1 2] [5 6] [3 4]] | polars into-df | polars into-nu --tail --rows 1 --index", result: Some(Value::list(vec![rec_3], Span::test_data())), }, Example { description: "Convert a col expression into a nushell value", - example: "polars col a | polars into-nu", + example: "polars col a | polars into-nu --index", result: Some(Value::test_record(record! { "expr" => Value::test_string("column"), "value" => Value::test_string("a"), @@ -106,17 +107,18 @@ fn dataframe_command( ) -> Result { let rows: Option = call.get_flag("rows")?; let tail: bool = call.has_flag("tail")?; + let index: bool = call.has_flag("index")?; let df = NuDataFrame::try_from_value_coerce(plugin, &input, call.head)?; let values = if tail { - df.tail(rows, call.head)? + df.tail(rows, index, call.head)? } else { // if rows is specified, return those rows, otherwise return everything if rows.is_some() { - df.head(rows, call.head)? + df.head(rows, index, call.head)? } else { - df.head(Some(df.height()), call.head)? + df.head(Some(df.height()), index, call.head)? } }; diff --git a/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/mod.rs b/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/mod.rs index 1c322579dd..89a8c862e7 100644 --- a/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/mod.rs +++ b/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/mod.rs @@ -326,22 +326,22 @@ impl NuDataFrame { } // Print is made out a head and if the dataframe is too large, then a tail - pub fn print(&self, span: Span) -> Result, ShellError> { + pub fn print(&self, include_index: bool, span: Span) -> Result, ShellError> { let df = &self.df; let size: usize = 20; if df.height() > size { let sample_size = size / 2; - let mut values = self.head(Some(sample_size), span)?; + let mut values = self.head(Some(sample_size), include_index, span)?; conversion::add_separator(&mut values, df, self.has_index(), span); let remaining = df.height() - sample_size; let tail_size = remaining.min(sample_size); - let mut tail_values = self.tail(Some(tail_size), span)?; + let mut tail_values = self.tail(Some(tail_size), include_index, span)?; values.append(&mut tail_values); Ok(values) } else { - Ok(self.head(Some(size), span)?) + Ok(self.head(Some(size), include_index, span)?) } } @@ -349,26 +349,38 @@ impl NuDataFrame { self.df.height() } - pub fn head(&self, rows: Option, span: Span) -> Result, ShellError> { + pub fn head( + &self, + rows: Option, + include_index: bool, + span: Span, + ) -> Result, ShellError> { let to_row = rows.unwrap_or(5); - let values = self.to_rows(0, to_row, span)?; + let values = self.to_rows(0, to_row, include_index, span)?; Ok(values) } - pub fn tail(&self, rows: Option, span: Span) -> Result, ShellError> { + pub fn tail( + &self, + rows: Option, + include_index: bool, + span: Span, + ) -> Result, ShellError> { let df = &self.df; let to_row = df.height(); let size = rows.unwrap_or(DEFAULT_ROWS); let from_row = to_row.saturating_sub(size); - let values = self.to_rows(from_row, to_row, span)?; + let values = self.to_rows(from_row, to_row, include_index, span)?; Ok(values) } + /// Converts the dataframe to a nushell list of values pub fn to_rows( &self, from_row: usize, to_row: usize, + include_index: bool, span: Span, ) -> Result, ShellError> { let df = &self.df; @@ -400,7 +412,7 @@ impl NuDataFrame { .map(|i| { let mut record = Record::new(); - if !has_index { + if !has_index && include_index { record.push("index", Value::int((i + from_row) as i64, span)); } @@ -602,7 +614,7 @@ impl CustomValueSupport for NuDataFrame { } fn base_value(self, span: Span) -> Result { - let vals = self.print(span)?; + let vals = self.print(true, span)?; Ok(Value::list(vals, span)) } From 5b7e8bf1d8022b2aef50d8a0ad7cbd8e3c65f707 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Sun, 9 Jun 2024 23:01:22 -0400 Subject: [PATCH 12/38] Deprecate `--numbered` from `for` (#13112) # Description #7777 removed the `--numbered` flag from `each`, `par-each`, `reduce`, and `each while`. It was suggested at the time that it should be removed from `for` as well, but for several reasons it wasn't. This PR deprecates `--numbered` in anticipation of removing it in 0.96. Note: Please review carefully, as this is my first "real" Rust/Nushell code. I was hoping that some prior commit would be useful as a template, but since this was an argument on a parser keyword, I didn't find too much useful. So I had to actually find the relevant helpers in the code and `nu_protocol` doc and learn how to use them - oh darn ;-) But please make sure I did it correctly. # User-Facing Changes * Use of `--numbered` will result in a deprecation warning. * Changed help example to demonstrate the new syntax. * Help shows deprecation notice on the flag --- crates/nu-cmd-lang/src/core_commands/for_.rs | 20 ++++++++++++++++--- .../nu-protocol/src/errors/parse_warning.rs | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/for_.rs b/crates/nu-cmd-lang/src/core_commands/for_.rs index 387af45282..0f2434d162 100644 --- a/crates/nu-cmd-lang/src/core_commands/for_.rs +++ b/crates/nu-cmd-lang/src/core_commands/for_.rs @@ -1,5 +1,6 @@ use nu_engine::{command_prelude::*, get_eval_block, get_eval_expression}; use nu_protocol::engine::CommandType; +use nu_protocol::ParseWarning; #[derive(Clone)] pub struct For; @@ -30,7 +31,7 @@ impl Command for For { .required("block", SyntaxShape::Block, "The block to run.") .switch( "numbered", - "return a numbered item ($it.index and $it.item)", + "DEPRECATED: return a numbered item ($it.index and $it.item)", Some('n'), ) .creates_scope() @@ -78,6 +79,20 @@ impl Command for For { let value = eval_expression(engine_state, stack, keyword_expr)?; let numbered = call.has_flag(engine_state, stack, "numbered")?; + if numbered { + nu_protocol::report_error_new( + engine_state, + &ParseWarning::DeprecatedWarning { + old_command: "--numbered/-n".into(), + new_suggestion: "use `enumerate`".into(), + span: call + .get_named_arg("numbered") + .expect("`get_named_arg` found `--numbered` but still failed") + .span, + url: "See `help for` examples".into(), + }, + ); + } let ctrlc = engine_state.ctrlc.clone(); let engine_state = engine_state.clone(); @@ -198,8 +213,7 @@ impl Command for For { }, Example { description: "Number each item and print a message", - example: - "for $it in ['bob' 'fred'] --numbered { print $\"($it.index) is ($it.item)\" }", + example: r#"for $it in (['bob' 'fred'] | enumerate) { print $"($it.index) is ($it.item)" }"#, result: None, }, ] diff --git a/crates/nu-protocol/src/errors/parse_warning.rs b/crates/nu-protocol/src/errors/parse_warning.rs index 0213d6889f..a30bc731d8 100644 --- a/crates/nu-protocol/src/errors/parse_warning.rs +++ b/crates/nu-protocol/src/errors/parse_warning.rs @@ -10,7 +10,7 @@ pub enum ParseWarning { DeprecatedWarning { old_command: String, new_suggestion: String, - #[label("`{old_command}` is deprecated and will be removed in 0.94. Please {new_suggestion} instead")] + #[label("`{old_command}` is deprecated and will be removed in a future release. Please {new_suggestion} instead")] span: Span, url: String, }, From af22bb8d52a3e011cff9f0fb5766749206822dc3 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Mon, 10 Jun 2024 11:31:47 +0000 Subject: [PATCH 13/38] Remove old `sys` command behavior (#13114) # Description Removes the old, deprecated behavior of the `sys` command. That is, it will no longer return the full system information record. # User-Facing Changes Breaking change: `sys` no longer outputs anything and will instead display help text. --- crates/nu-command/src/system/sys/cpu.rs | 43 ++++- crates/nu-command/src/system/sys/disks.rs | 28 ++- crates/nu-command/src/system/sys/host.rs | 56 +++++- crates/nu-command/src/system/sys/mem.rs | 20 ++- crates/nu-command/src/system/sys/mod.rs | 198 +--------------------- crates/nu-command/src/system/sys/net.rs | 21 ++- crates/nu-command/src/system/sys/sys_.rs | 32 +--- crates/nu-command/src/system/sys/temp.rs | 24 ++- crates/nu-command/src/system/sys/users.rs | 26 ++- 9 files changed, 215 insertions(+), 233 deletions(-) diff --git a/crates/nu-command/src/system/sys/cpu.rs b/crates/nu-command/src/system/sys/cpu.rs index b2f6d6afa6..d24197bf70 100644 --- a/crates/nu-command/src/system/sys/cpu.rs +++ b/crates/nu-command/src/system/sys/cpu.rs @@ -1,4 +1,6 @@ +use super::trim_cstyle_null; use nu_engine::command_prelude::*; +use sysinfo::{CpuRefreshKind, System, MINIMUM_CPU_UPDATE_INTERVAL}; #[derive(Clone)] pub struct SysCpu; @@ -26,7 +28,7 @@ impl Command for SysCpu { call: &Call, _input: PipelineData, ) -> Result { - Ok(super::cpu(call.head).into_pipeline_data()) + Ok(cpu(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -37,3 +39,42 @@ impl Command for SysCpu { }] } } + +fn cpu(span: Span) -> Value { + let mut sys = System::new(); + sys.refresh_cpu_specifics(CpuRefreshKind::everything()); + // We must refresh the CPU twice a while apart to get valid usage data. + // In theory we could just sleep MINIMUM_CPU_UPDATE_INTERVAL, but I've noticed that + // that gives poor results (error of ~5%). Decided to wait 2x that long, somewhat arbitrarily + std::thread::sleep(MINIMUM_CPU_UPDATE_INTERVAL * 2); + sys.refresh_cpu_specifics(CpuRefreshKind::new().with_cpu_usage()); + + let cpus = sys + .cpus() + .iter() + .map(|cpu| { + // sysinfo CPU usage numbers are not very precise unless you wait a long time between refreshes. + // Round to 1DP (chosen somewhat arbitrarily) so people aren't misled by high-precision floats. + let rounded_usage = (cpu.cpu_usage() * 10.0).round() / 10.0; + + let load_avg = System::load_average(); + let load_avg = format!( + "{:.2}, {:.2}, {:.2}", + load_avg.one, load_avg.five, load_avg.fifteen + ); + + let record = record! { + "name" => Value::string(trim_cstyle_null(cpu.name()), span), + "brand" => Value::string(trim_cstyle_null(cpu.brand()), span), + "freq" => Value::int(cpu.frequency() as i64, span), + "cpu_usage" => Value::float(rounded_usage.into(), span), + "load_average" => Value::string(load_avg, span), + "vendor_id" => Value::string(trim_cstyle_null(cpu.vendor_id()), span), + }; + + Value::record(record, span) + }) + .collect(); + + Value::list(cpus, span) +} diff --git a/crates/nu-command/src/system/sys/disks.rs b/crates/nu-command/src/system/sys/disks.rs index 66991c9087..0d9ce09db3 100644 --- a/crates/nu-command/src/system/sys/disks.rs +++ b/crates/nu-command/src/system/sys/disks.rs @@ -1,4 +1,6 @@ +use super::trim_cstyle_null; use nu_engine::command_prelude::*; +use sysinfo::Disks; #[derive(Clone)] pub struct SysDisks; @@ -26,7 +28,7 @@ impl Command for SysDisks { call: &Call, _input: PipelineData, ) -> Result { - Ok(super::disks(call.head).into_pipeline_data()) + Ok(disks(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -37,3 +39,27 @@ impl Command for SysDisks { }] } } + +fn disks(span: Span) -> Value { + let disks = Disks::new_with_refreshed_list() + .iter() + .map(|disk| { + let device = trim_cstyle_null(disk.name().to_string_lossy()); + let typ = trim_cstyle_null(disk.file_system().to_string_lossy()); + + let record = record! { + "device" => Value::string(device, span), + "type" => Value::string(typ, span), + "mount" => Value::string(disk.mount_point().to_string_lossy(), span), + "total" => Value::filesize(disk.total_space() as i64, span), + "free" => Value::filesize(disk.available_space() as i64, span), + "removable" => Value::bool(disk.is_removable(), span), + "kind" => Value::string(disk.kind().to_string(), span), + }; + + Value::record(record, span) + }) + .collect(); + + Value::list(disks, span) +} diff --git a/crates/nu-command/src/system/sys/host.rs b/crates/nu-command/src/system/sys/host.rs index 969f59ef99..64dd43424d 100644 --- a/crates/nu-command/src/system/sys/host.rs +++ b/crates/nu-command/src/system/sys/host.rs @@ -1,4 +1,7 @@ +use super::trim_cstyle_null; +use chrono::{DateTime, FixedOffset, Local}; use nu_engine::command_prelude::*; +use sysinfo::System; #[derive(Clone)] pub struct SysHost; @@ -26,8 +29,7 @@ impl Command for SysHost { call: &Call, _input: PipelineData, ) -> Result { - let host = super::host(call.head); - Ok(Value::record(host, call.head).into_pipeline_data()) + Ok(host(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -38,3 +40,53 @@ impl Command for SysHost { }] } } + +fn host(span: Span) -> Value { + let mut record = Record::new(); + + if let Some(name) = System::name() { + record.push("name", Value::string(trim_cstyle_null(name), span)); + } + if let Some(version) = System::os_version() { + record.push("os_version", Value::string(trim_cstyle_null(version), span)); + } + if let Some(long_version) = System::long_os_version() { + record.push( + "long_os_version", + Value::string(trim_cstyle_null(long_version), span), + ); + } + if let Some(version) = System::kernel_version() { + record.push( + "kernel_version", + Value::string(trim_cstyle_null(version), span), + ); + } + if let Some(hostname) = System::host_name() { + record.push("hostname", Value::string(trim_cstyle_null(hostname), span)); + } + + let uptime = System::uptime() + .saturating_mul(1_000_000_000) + .try_into() + .unwrap_or(i64::MAX); + + record.push("uptime", Value::duration(uptime, span)); + + let boot_time = boot_time() + .map(|time| Value::date(time, span)) + .unwrap_or(Value::nothing(span)); + + record.push("boot_time", boot_time); + + Value::record(record, span) +} + +fn boot_time() -> Option> { + // Broken systems can apparently return really high values. + // See: https://github.com/nushell/nushell/issues/10155 + // First, try to convert u64 to i64, and then try to create a `DateTime`. + let secs = System::boot_time().try_into().ok()?; + let time = DateTime::from_timestamp(secs, 0)?; + Some(time.with_timezone(&Local).fixed_offset()) +} diff --git a/crates/nu-command/src/system/sys/mem.rs b/crates/nu-command/src/system/sys/mem.rs index e5dc36e1b7..89527807d7 100644 --- a/crates/nu-command/src/system/sys/mem.rs +++ b/crates/nu-command/src/system/sys/mem.rs @@ -1,4 +1,5 @@ use nu_engine::command_prelude::*; +use sysinfo::System; #[derive(Clone)] pub struct SysMem; @@ -26,7 +27,7 @@ impl Command for SysMem { call: &Call, _input: PipelineData, ) -> Result { - Ok(super::mem(call.head).into_pipeline_data()) + Ok(mem(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -37,3 +38,20 @@ impl Command for SysMem { }] } } + +fn mem(span: Span) -> Value { + let mut sys = System::new(); + sys.refresh_memory(); + + let record = record! { + "total" => Value::filesize(sys.total_memory() as i64, span), + "free" => Value::filesize(sys.free_memory() as i64, span), + "used" => Value::filesize(sys.used_memory() as i64, span), + "available" => Value::filesize(sys.available_memory() as i64, span), + "swap total" => Value::filesize(sys.total_swap() as i64, span), + "swap free" => Value::filesize(sys.free_swap() as i64, span), + "swap used" => Value::filesize(sys.used_swap() as i64, span), + }; + + Value::record(record, span) +} diff --git a/crates/nu-command/src/system/sys/mod.rs b/crates/nu-command/src/system/sys/mod.rs index fcb40fd37d..02d271537e 100644 --- a/crates/nu-command/src/system/sys/mod.rs +++ b/crates/nu-command/src/system/sys/mod.rs @@ -16,202 +16,6 @@ pub use sys_::Sys; pub use temp::SysTemp; pub use users::SysUsers; -use chrono::{DateTime, FixedOffset, Local}; -use nu_protocol::{record, Record, Span, Value}; -use sysinfo::{ - Components, CpuRefreshKind, Disks, Networks, System, Users, MINIMUM_CPU_UPDATE_INTERVAL, -}; - -pub fn trim_cstyle_null(s: impl AsRef) -> String { +fn trim_cstyle_null(s: impl AsRef) -> String { s.as_ref().trim_matches('\0').into() } - -pub fn disks(span: Span) -> Value { - let disks = Disks::new_with_refreshed_list() - .iter() - .map(|disk| { - let device = trim_cstyle_null(disk.name().to_string_lossy()); - let typ = trim_cstyle_null(disk.file_system().to_string_lossy()); - - let record = record! { - "device" => Value::string(device, span), - "type" => Value::string(typ, span), - "mount" => Value::string(disk.mount_point().to_string_lossy(), span), - "total" => Value::filesize(disk.total_space() as i64, span), - "free" => Value::filesize(disk.available_space() as i64, span), - "removable" => Value::bool(disk.is_removable(), span), - "kind" => Value::string(disk.kind().to_string(), span), - }; - - Value::record(record, span) - }) - .collect(); - - Value::list(disks, span) -} - -pub fn net(span: Span) -> Value { - let networks = Networks::new_with_refreshed_list() - .iter() - .map(|(iface, data)| { - let record = record! { - "name" => Value::string(trim_cstyle_null(iface), span), - "sent" => Value::filesize(data.total_transmitted() as i64, span), - "recv" => Value::filesize(data.total_received() as i64, span), - }; - - Value::record(record, span) - }) - .collect(); - - Value::list(networks, span) -} - -pub fn cpu(span: Span) -> Value { - let mut sys = System::new(); - sys.refresh_cpu_specifics(CpuRefreshKind::everything()); - // We must refresh the CPU twice a while apart to get valid usage data. - // In theory we could just sleep MINIMUM_CPU_UPDATE_INTERVAL, but I've noticed that - // that gives poor results (error of ~5%). Decided to wait 2x that long, somewhat arbitrarily - std::thread::sleep(MINIMUM_CPU_UPDATE_INTERVAL * 2); - sys.refresh_cpu_specifics(CpuRefreshKind::new().with_cpu_usage()); - - let cpus = sys - .cpus() - .iter() - .map(|cpu| { - // sysinfo CPU usage numbers are not very precise unless you wait a long time between refreshes. - // Round to 1DP (chosen somewhat arbitrarily) so people aren't misled by high-precision floats. - let rounded_usage = (cpu.cpu_usage() * 10.0).round() / 10.0; - - let load_avg = System::load_average(); - let load_avg = format!( - "{:.2}, {:.2}, {:.2}", - load_avg.one, load_avg.five, load_avg.fifteen - ); - - let record = record! { - "name" => Value::string(trim_cstyle_null(cpu.name()), span), - "brand" => Value::string(trim_cstyle_null(cpu.brand()), span), - "freq" => Value::int(cpu.frequency() as i64, span), - "cpu_usage" => Value::float(rounded_usage.into(), span), - "load_average" => Value::string(load_avg, span), - "vendor_id" => Value::string(trim_cstyle_null(cpu.vendor_id()), span), - }; - - Value::record(record, span) - }) - .collect(); - - Value::list(cpus, span) -} - -pub fn mem(span: Span) -> Value { - let mut sys = System::new(); - sys.refresh_memory(); - - let record = record! { - "total" => Value::filesize(sys.total_memory() as i64, span), - "free" => Value::filesize(sys.free_memory() as i64, span), - "used" => Value::filesize(sys.used_memory() as i64, span), - "available" => Value::filesize(sys.available_memory() as i64, span), - "swap total" => Value::filesize(sys.total_swap() as i64, span), - "swap free" => Value::filesize(sys.free_swap() as i64, span), - "swap used" => Value::filesize(sys.used_swap() as i64, span), - }; - - Value::record(record, span) -} - -pub fn users(span: Span) -> Value { - let users = Users::new_with_refreshed_list() - .iter() - .map(|user| { - let groups = user - .groups() - .iter() - .map(|group| Value::string(trim_cstyle_null(group.name()), span)) - .collect(); - - let record = record! { - "name" => Value::string(trim_cstyle_null(user.name()), span), - "groups" => Value::list(groups, span), - }; - - Value::record(record, span) - }) - .collect(); - - Value::list(users, span) -} - -pub fn host(span: Span) -> Record { - let mut record = Record::new(); - - if let Some(name) = System::name() { - record.push("name", Value::string(trim_cstyle_null(name), span)); - } - if let Some(version) = System::os_version() { - record.push("os_version", Value::string(trim_cstyle_null(version), span)); - } - if let Some(long_version) = System::long_os_version() { - record.push( - "long_os_version", - Value::string(trim_cstyle_null(long_version), span), - ); - } - if let Some(version) = System::kernel_version() { - record.push( - "kernel_version", - Value::string(trim_cstyle_null(version), span), - ); - } - if let Some(hostname) = System::host_name() { - record.push("hostname", Value::string(trim_cstyle_null(hostname), span)); - } - - let uptime = System::uptime() - .saturating_mul(1_000_000_000) - .try_into() - .unwrap_or(i64::MAX); - - record.push("uptime", Value::duration(uptime, span)); - - let boot_time = boot_time() - .map(|time| Value::date(time, span)) - .unwrap_or(Value::nothing(span)); - - record.push("boot_time", boot_time); - - record -} - -fn boot_time() -> Option> { - // Broken systems can apparently return really high values. - // See: https://github.com/nushell/nushell/issues/10155 - // First, try to convert u64 to i64, and then try to create a `DateTime`. - let secs = System::boot_time().try_into().ok()?; - let time = DateTime::from_timestamp(secs, 0)?; - Some(time.with_timezone(&Local).fixed_offset()) -} - -pub fn temp(span: Span) -> Value { - let components = Components::new_with_refreshed_list() - .iter() - .map(|component| { - let mut record = record! { - "unit" => Value::string(component.label(), span), - "temp" => Value::float(component.temperature().into(), span), - "high" => Value::float(component.max().into(), span), - }; - - if let Some(critical) = component.critical() { - record.push("critical", Value::float(critical.into(), span)); - } - - Value::record(record, span) - }) - .collect(); - - Value::list(components, span) -} diff --git a/crates/nu-command/src/system/sys/net.rs b/crates/nu-command/src/system/sys/net.rs index 98dca1bc2f..ef1c595800 100644 --- a/crates/nu-command/src/system/sys/net.rs +++ b/crates/nu-command/src/system/sys/net.rs @@ -1,4 +1,6 @@ +use super::trim_cstyle_null; use nu_engine::command_prelude::*; +use sysinfo::Networks; #[derive(Clone)] pub struct SysNet; @@ -26,7 +28,7 @@ impl Command for SysNet { call: &Call, _input: PipelineData, ) -> Result { - Ok(super::net(call.head).into_pipeline_data()) + Ok(net(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -37,3 +39,20 @@ impl Command for SysNet { }] } } + +fn net(span: Span) -> Value { + let networks = Networks::new_with_refreshed_list() + .iter() + .map(|(iface, data)| { + let record = record! { + "name" => Value::string(trim_cstyle_null(iface), span), + "sent" => Value::filesize(data.total_transmitted() as i64, span), + "recv" => Value::filesize(data.total_received() as i64, span), + }; + + Value::record(record, span) + }) + .collect(); + + Value::list(networks, span) +} diff --git a/crates/nu-command/src/system/sys/sys_.rs b/crates/nu-command/src/system/sys/sys_.rs index 2886836be9..5369064f50 100644 --- a/crates/nu-command/src/system/sys/sys_.rs +++ b/crates/nu-command/src/system/sys/sys_.rs @@ -1,4 +1,4 @@ -use nu_engine::command_prelude::*; +use nu_engine::{command_prelude::*, get_full_help}; #[derive(Clone)] pub struct Sys; @@ -20,41 +20,17 @@ impl Command for Sys { } fn extra_usage(&self) -> &str { - "Note that this command may take a noticeable amount of time to run. To reduce the time taken, you can use the various `sys` sub commands to get the subset of information you are interested in." + "You must use one of the following subcommands. Using this command as-is will only produce this help message." } fn run( &self, engine_state: &EngineState, - _stack: &mut Stack, + stack: &mut Stack, call: &Call, _input: PipelineData, ) -> Result { - nu_protocol::report_error_new( - engine_state, - &ShellError::GenericError { - error: "Deprecated command".into(), - msg: "the `sys` command is deprecated, please use the new subcommands (`sys host`, `sys mem`, etc.)." - .into(), - span: Some(call.head), - help: None, - inner: vec![], - }, - ); - - let head = call.head; - - let mut host = super::host(head); - host.push("sessions", super::users(head)); - let record = record! { - "host" => Value::record(host, head), - "cpu" => super::cpu(head), - "disks" => super::disks(head), - "mem" => super::mem(head), - "temp" => super::temp(head), - "net" => super::net(head), - }; - Ok(Value::record(record, head).into_pipeline_data()) + Ok(Value::string(get_full_help(self, engine_state, stack), call.head).into_pipeline_data()) } fn examples(&self) -> Vec { diff --git a/crates/nu-command/src/system/sys/temp.rs b/crates/nu-command/src/system/sys/temp.rs index eaf1ed05db..088471f0fd 100644 --- a/crates/nu-command/src/system/sys/temp.rs +++ b/crates/nu-command/src/system/sys/temp.rs @@ -1,4 +1,5 @@ use nu_engine::command_prelude::*; +use sysinfo::Components; #[derive(Clone)] pub struct SysTemp; @@ -30,7 +31,7 @@ impl Command for SysTemp { call: &Call, _input: PipelineData, ) -> Result { - Ok(super::temp(call.head).into_pipeline_data()) + Ok(temp(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -41,3 +42,24 @@ impl Command for SysTemp { }] } } + +fn temp(span: Span) -> Value { + let components = Components::new_with_refreshed_list() + .iter() + .map(|component| { + let mut record = record! { + "unit" => Value::string(component.label(), span), + "temp" => Value::float(component.temperature().into(), span), + "high" => Value::float(component.max().into(), span), + }; + + if let Some(critical) = component.critical() { + record.push("critical", Value::float(critical.into(), span)); + } + + Value::record(record, span) + }) + .collect(); + + Value::list(components, span) +} diff --git a/crates/nu-command/src/system/sys/users.rs b/crates/nu-command/src/system/sys/users.rs index 9aab2b9b7b..7d4a43cae5 100644 --- a/crates/nu-command/src/system/sys/users.rs +++ b/crates/nu-command/src/system/sys/users.rs @@ -1,4 +1,6 @@ +use super::trim_cstyle_null; use nu_engine::command_prelude::*; +use sysinfo::Users; #[derive(Clone)] pub struct SysUsers; @@ -25,7 +27,7 @@ impl Command for SysUsers { call: &Call, _input: PipelineData, ) -> Result { - Ok(super::users(call.head).into_pipeline_data()) + Ok(users(call.head).into_pipeline_data()) } fn examples(&self) -> Vec { @@ -36,3 +38,25 @@ impl Command for SysUsers { }] } } + +fn users(span: Span) -> Value { + let users = Users::new_with_refreshed_list() + .iter() + .map(|user| { + let groups = user + .groups() + .iter() + .map(|group| Value::string(trim_cstyle_null(group.name()), span)) + .collect(); + + let record = record! { + "name" => Value::string(trim_cstyle_null(user.name()), span), + "groups" => Value::list(groups, span), + }; + + Value::record(record, span) + }) + .collect(); + + Value::list(users, span) +} From a55a48529d57868b4ccc3ca7e19299289bca9b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 10 Jun 2024 22:33:22 +0300 Subject: [PATCH 14/38] Fix delta not being merged when evaluating menus (#13120) # Description After parsing menu code, the changes weren't merged into the engine state, which didn't produce any errors (somehow?) until the recent span ID refactors. With this PR, menus get a new cloned engine state with the parsed changes correctly merged in. Hopefully fixes https://github.com/nushell/nushell/issues/13118 # User-Facing Changes # Tests + Formatting # After Submitting --- crates/nu-cli/src/reedline_config.rs | 37 +++++++++++++++++++++------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/crates/nu-cli/src/reedline_config.rs b/crates/nu-cli/src/reedline_config.rs index 9c08265dfa..dd5a3199dc 100644 --- a/crates/nu-cli/src/reedline_config.rs +++ b/crates/nu-cli/src/reedline_config.rs @@ -75,7 +75,7 @@ const DEFAULT_HELP_MENU: &str = r#" // Adds all menus to line editor pub(crate) fn add_menus( mut line_editor: Reedline, - engine_state: Arc, + engine_state_ref: Arc, stack: &Stack, config: &Config, ) -> Result { @@ -83,7 +83,7 @@ pub(crate) fn add_menus( line_editor = line_editor.clear_menus(); for menu in &config.menus { - line_editor = add_menu(line_editor, menu, engine_state.clone(), stack, config)? + line_editor = add_menu(line_editor, menu, engine_state_ref.clone(), stack, config)? } // Checking if the default menus have been added from the config file @@ -93,13 +93,16 @@ pub(crate) fn add_menus( ("help_menu", DEFAULT_HELP_MENU), ]; + let mut engine_state = (*engine_state_ref).clone(); + let mut menu_eval_results = vec![]; + for (name, definition) in default_menus { if !config .menus .iter() .any(|menu| menu.name.to_expanded_string("", config) == name) { - let (block, _) = { + let (block, delta) = { let mut working_set = StateWorkingSet::new(&engine_state); let output = parse( &mut working_set, @@ -111,15 +114,31 @@ pub(crate) fn add_menus( (output, working_set.render()) }; + engine_state.merge_delta(delta)?; + let mut temp_stack = Stack::new().capture(); let input = PipelineData::Empty; - let res = eval_block::(&engine_state, &mut temp_stack, &block, input)?; + menu_eval_results.push(eval_block::( + &engine_state, + &mut temp_stack, + &block, + input, + )?); + } + } - if let PipelineData::Value(value, None) = res { - for menu in create_menus(&value)? { - line_editor = - add_menu(line_editor, &menu, engine_state.clone(), stack, config)?; - } + let new_engine_state_ref = Arc::new(engine_state); + + for res in menu_eval_results.into_iter() { + if let PipelineData::Value(value, None) = res { + for menu in create_menus(&value)? { + line_editor = add_menu( + line_editor, + &menu, + new_engine_state_ref.clone(), + stack, + config, + )?; } } } From 944d941dec0459a910a6cbf0f6f60688e94554e6 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Mon, 10 Jun 2024 21:40:09 +0000 Subject: [PATCH 15/38] `path type` error and not found changes (#13007) # Description Instead of an empty string, this PR changes `path type` to return null if the path does not exist. If some other IO error is encountered, then that error is bubbled up instead of treating it as a "not found" case. # User-Facing Changes - `path type` will now return null instead of an empty string, which is technically a breaking change. In most cases though, I think this shouldn't affect the behavior of scripts too much. - `path type` can now error instead of returning an empty string if some other IO error besides a "not found" error occurs. Since this PR introduces breaking changes, it should be merged after the 0.94.1 patch. --- crates/nu-command/src/path/type.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/nu-command/src/path/type.rs b/crates/nu-command/src/path/type.rs index 0897593109..e58e697604 100644 --- a/crates/nu-command/src/path/type.rs +++ b/crates/nu-command/src/path/type.rs @@ -1,7 +1,10 @@ use super::PathSubcommandArguments; use nu_engine::command_prelude::*; use nu_protocol::engine::StateWorkingSet; -use std::path::{Path, PathBuf}; +use std::{ + io, + path::{Path, PathBuf}, +}; struct Arguments { pwd: PathBuf, @@ -36,7 +39,7 @@ impl Command for SubCommand { fn extra_usage(&self) -> &str { r#"This checks the file system to confirm the path's object type. -If nothing is found, an empty string will be returned."# +If the path does not exist, null will be returned."# } fn is_const(&self) -> bool { @@ -104,9 +107,11 @@ If nothing is found, an empty string will be returned."# fn path_type(path: &Path, span: Span, args: &Arguments) -> Value { let path = nu_path::expand_path_with(path, &args.pwd, true); - let meta = path.symlink_metadata(); - let ty = meta.as_ref().map(get_file_type).unwrap_or(""); - Value::string(ty, span) + match path.symlink_metadata() { + Ok(metadata) => Value::string(get_file_type(&metadata), span), + Err(err) if err.kind() == io::ErrorKind::NotFound => Value::nothing(span), + Err(err) => Value::error(err.into_spanned(span).into(), span), + } } fn get_file_type(md: &std::fs::Metadata) -> &str { From c09488f51580ac4208e300b27365ff3c4095ba4a Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Mon, 10 Jun 2024 20:12:54 -0400 Subject: [PATCH 16/38] Fix multiple issues with `def --wrapped` help example (#13123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description I've noticed this several times but kept forgetting to fix it: The example given for `help def` for the `--wrapped` flag is: ```nu Define a custom wrapper for an external command > def --wrapped my-echo [...rest] { echo $rest }; my-echo spam ╭───┬──────╮ │ 0 │ spam │ ╰───┴──────╯ ``` That's ... odd, since (a) it specifically says *"for an external"* command, and yet uses (and shows the output from) the builtin `echo`. Also, (b) I believe `--wrapped` is *only* applicable to external commands. Finally, (c) the `my-echo spam` doesn't even demonstrate a wrapped argument. Unless I'm truly missing something, the example just makes no sense. This updates the example to really demonstrate `def --wrapped` with the *external* version of `^echo`. It uses the `-e` command to interpret the escape-tab character in the string. ```nu > def --wrapped my-echo [...rest] { ^echo ...$rest }; my-echo -e 'spam\tspam' spam spam ``` # User-Facing Changes Help example only. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting --- crates/nu-cmd-lang/src/core_commands/def.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/def.rs b/crates/nu-cmd-lang/src/core_commands/def.rs index e4a169ffcc..913f74803c 100644 --- a/crates/nu-cmd-lang/src/core_commands/def.rs +++ b/crates/nu-cmd-lang/src/core_commands/def.rs @@ -67,8 +67,8 @@ impl Command for Def { }, Example { description: "Define a custom wrapper for an external command", - example: r#"def --wrapped my-echo [...rest] { echo $rest }; my-echo spam"#, - result: Some(Value::test_list(vec![Value::test_string("spam")])), + example: r#"def --wrapped my-echo [...rest] { ^echo ...$rest }; my-echo -e 'spam\tspam'"#, + result: Some(Value::test_string("spam\tspam")), }, ] } From a8376fad4093ce957ee6cef32d5793d1599811b8 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Tue, 11 Jun 2024 13:44:13 -0500 Subject: [PATCH 17/38] update uutils crates (#13130) # Description This PR updates the uutils/coreutils crates to the latest released version. # User-Facing Changes # Tests + Formatting # After Submitting --- Cargo.lock | 54 +++++++++---------------- Cargo.toml | 14 +++---- crates/nu-command/tests/commands/ucp.rs | 2 +- 3 files changed, 27 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c95f0d1435..e0c8b048ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3011,7 +3011,7 @@ dependencies = [ "uu_mv", "uu_uname", "uu_whoami", - "uucore 0.0.25", + "uucore", "uuid", "v_htmlescape", "wax", @@ -6352,93 +6352,77 @@ checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" [[package]] name = "uu_cp" -version = "0.0.25" +version = "0.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcbe045dc92209114afdfd366bd18f7b95dbf999f3eaa85ad6dca910b0be3d56" +checksum = "c31fc5c95f7668999e129464a29e9080f69ba01ccf7a0ae43ff2cfdb15baa340" dependencies = [ "clap", "filetime", "indicatif", "libc", "quick-error 2.0.1", - "uucore 0.0.26", + "uucore", "walkdir", "xattr", ] [[package]] name = "uu_mkdir" -version = "0.0.25" +version = "0.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "040aa4584036b2f65e05387b0ea9ac468afce1db325743ce5f350689fd9ce4ae" +checksum = "496d95e0e3121e4d424ba62019eb84a6f1102213ca8ca16c0a2f8c652c7236c3" dependencies = [ "clap", - "uucore 0.0.26", + "uucore", ] [[package]] name = "uu_mktemp" -version = "0.0.25" +version = "0.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f240a99c36d768153874d198c43605a45c86996b576262689a0f18248cc3bc57" +checksum = "a28a0d9744bdc28ceaf13f70b959bacded91aedfd008402d72fa1e3224158653" dependencies = [ "clap", "rand", "tempfile", - "uucore 0.0.26", + "uucore", ] [[package]] name = "uu_mv" -version = "0.0.25" +version = "0.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c99fd7c75e6e85553c92537314be3d9a64b4927051aa1608513feea2f933022" +checksum = "53680908b01c5ac3cc0ee8a376de3e51a36dde2c5a5227a115a3d0977cc4539b" dependencies = [ "clap", "fs_extra", "indicatif", - "uucore 0.0.26", + "uucore", ] [[package]] name = "uu_uname" -version = "0.0.25" +version = "0.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5951832d73199636bde6c0d61cf960932b3c4450142c290375bc10c7abed6db5" +checksum = "a7f4125fb4f286313bca8f222abaefe39db54d65179ea788c91ebd3162345f4e" dependencies = [ "clap", "platform-info", - "uucore 0.0.26", + "uucore", ] [[package]] name = "uu_whoami" -version = "0.0.25" +version = "0.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3b44166eb6335aeac42744ea368cc4c32d3f2287a4ff765a5ce44d927ab8bb4" +checksum = "7f7b313901a15cfde2d88f434fcd077903d690f73cc36d1cec20f47906960aec" dependencies = [ "clap", "libc", - "uucore 0.0.26", + "uucore", "windows-sys 0.48.0", ] -[[package]] -name = "uucore" -version = "0.0.25" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23994a722acb43dbc56877e271c9723f167ae42c4c089f909b2d7dd106c3a9b4" -dependencies = [ - "clap", - "glob", - "libc", - "nix", - "once_cell", - "os_display", - "uucore_procs", - "wild", -] - [[package]] name = "uucore" version = "0.0.26" diff --git a/Cargo.toml b/Cargo.toml index 7e5f436bbf..f7f911a0ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -159,13 +159,13 @@ unicode-segmentation = "1.11" unicode-width = "0.1" ureq = { version = "2.9", default-features = false } url = "2.2" -uu_cp = "0.0.25" -uu_mkdir = "0.0.25" -uu_mktemp = "0.0.25" -uu_mv = "0.0.25" -uu_whoami = "0.0.25" -uu_uname = "0.0.25" -uucore = "0.0.25" +uu_cp = "0.0.26" +uu_mkdir = "0.0.26" +uu_mktemp = "0.0.26" +uu_mv = "0.0.26" +uu_whoami = "0.0.26" +uu_uname = "0.0.26" +uucore = "0.0.26" uuid = "1.8.0" v_htmlescape = "0.15.0" wax = "0.6" diff --git a/crates/nu-command/tests/commands/ucp.rs b/crates/nu-command/tests/commands/ucp.rs index 453786ad4e..728cf1dcb2 100644 --- a/crates/nu-command/tests/commands/ucp.rs +++ b/crates/nu-command/tests/commands/ucp.rs @@ -906,7 +906,7 @@ fn test_cp_debug_default() { #[cfg(any(target_os = "linux", target_os = "freebsd"))] if !actual .out - .contains("copy offload: unknown, reflink: unsupported, sparse detection: no") + .contains("copy offload: yes, reflink: unsupported, sparse detection: no") { panic!("{}", format!("Failure: stdout was \n{}", actual.out)); } From b0d1b4b1827d3d90867ca22d6095a009a7c7ae52 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Tue, 11 Jun 2024 19:00:00 +0000 Subject: [PATCH 18/38] Remove deprecated `--not` flag on `str contains` (#13124) # Description Removes the `str contains --not` flag that was deprecated in the last minor release. # User-Facing Changes Breaking change since a flag was removed. --- .../nu-command/src/strings/str_/contains.rs | 41 +++---------------- 1 file changed, 5 insertions(+), 36 deletions(-) diff --git a/crates/nu-command/src/strings/str_/contains.rs b/crates/nu-command/src/strings/str_/contains.rs index 63b9366b09..ff9cb85fcb 100644 --- a/crates/nu-command/src/strings/str_/contains.rs +++ b/crates/nu-command/src/strings/str_/contains.rs @@ -10,7 +10,6 @@ struct Arguments { substring: String, cell_paths: Option>, case_insensitive: bool, - not_contain: bool, } impl CmdArgument for Arguments { @@ -40,7 +39,6 @@ impl Command for SubCommand { "For a data structure input, check strings at the given cell paths, and replace with result.", ) .switch("ignore-case", "search is case insensitive", Some('i')) - .switch("not", "DEPRECATED OPTION: does not contain", Some('n')) .category(Category::Strings) } @@ -63,27 +61,12 @@ impl Command for SubCommand { call: &Call, input: PipelineData, ) -> Result { - if call.has_flag(engine_state, stack, "not")? { - nu_protocol::report_error_new( - engine_state, - &ShellError::GenericError { - error: "Deprecated option".into(), - msg: "`str contains --not {string}` is deprecated and will be removed in 0.95." - .into(), - span: Some(call.head), - help: Some("Please use the `not` operator instead.".into()), - inner: vec![], - }, - ); - } - let cell_paths: Vec = call.rest(engine_state, stack, 1)?; let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths); let args = Arguments { substring: call.req::(engine_state, stack, 0)?, cell_paths, case_insensitive: call.has_flag(engine_state, stack, "ignore-case")?, - not_contain: call.has_flag(engine_state, stack, "not")?, }; operate(action, args, input, call.head, engine_state.ctrlc.clone()) } @@ -114,7 +97,6 @@ impl Command for SubCommand { substring: call.req_const::(working_set, 0)?, cell_paths, case_insensitive: call.has_flag_const(working_set, "ignore-case")?, - not_contain: call.has_flag_const(working_set, "not")?, }; operate( action, @@ -183,7 +165,6 @@ fn action( input: &Value, Arguments { case_insensitive, - not_contain, substring, .. }: &Arguments, @@ -191,23 +172,11 @@ fn action( ) -> Value { match input { Value::String { val, .. } => Value::bool( - match case_insensitive { - true => { - if *not_contain { - !val.to_folded_case() - .contains(substring.to_folded_case().as_str()) - } else { - val.to_folded_case() - .contains(substring.to_folded_case().as_str()) - } - } - false => { - if *not_contain { - !val.contains(substring) - } else { - val.contains(substring) - } - } + if *case_insensitive { + val.to_folded_case() + .contains(substring.to_folded_case().as_str()) + } else { + val.contains(substring) }, head, ), From 0372e8c53c085d4e7d9109fda104cfa68d846e71 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Tue, 11 Jun 2024 14:10:31 -0500 Subject: [PATCH 19/38] add `$nu.data-dir` for completions and `$nu.cache-dir` for other uses (#13122) # Description This PR is an attempt to add a standard location for people to put completions in. I saw this topic come up again recently and IIRC we decided to create a standard location. I used the dirs-next crate to dictate where these locations are. I know some people won't like that but at least this gets the ball rolling in a direction that has a standard directory. This is what the default NU_LIB_DIRS looks like now in the default_env.nu. It should also be like this when starting nushell with `nu -n` ```nushell $env.NU_LIB_DIRS = [ ($nu.default-config-dir | path join 'scripts') # add /scripts ($nu.data-dir | path join 'completions') # default home for nushell completions ] ``` I also added these default folders to the `$nu` variable so now there is `$nu.data-path` and `$nu.cache-path`. ## Data Dir Default ![image](https://github.com/nushell/nushell/assets/343840/aeeb7cd6-17b4-43e8-bb6f-986a0c7fce23) While I was in there, I also decided to add a cache dir ## Cache Dir Default ![image](https://github.com/nushell/nushell/assets/343840/87dead66-4911-4f67-bfb2-acb16f386674) ### This is what the default looks like in Ubuntu. ![image](https://github.com/nushell/nushell/assets/343840/bca8eae8-8c18-47e8-b64f-3efe34f0004f) ### This is what it looks like with XDG_CACHE_HOME and XDG_DATA_HOME overridden ```nushell XDG_DATA_HOME=/tmp/data_home XDG_CACHE_HOME=/tmp/cache_home cargo r ``` ![image](https://github.com/nushell/nushell/assets/343840/fae86d50-9821-41f1-868e-3814eca3730b) ### This is what the defaults look like in Windows (username scrubbed to protect the innocent) ![image](https://github.com/nushell/nushell/assets/343840/3ebdb5cd-0150-448c-aff5-c57053e4788a) How my NU_LIB_DIRS is set in the images above ```nushell $env.NU_LIB_DIRS = [ ($nu.default-config-dir | path join 'scripts') # add /scripts '/Users/fdncred/src/nu_scripts' ($nu.config-path | path dirname) ($nu.data-dir | path join 'completions') # default home for nushell completions ] ``` Let the debate begin. # User-Facing Changes # Tests + Formatting # After Submitting --- Cargo.toml | 1 + crates/nu-cli/tests/completions/mod.rs | 4 ++- crates/nu-path/src/helpers.rs | 26 ++++++++++++--- crates/nu-path/src/lib.rs | 2 +- crates/nu-protocol/src/eval_const.rs | 32 +++++++++++++++++++ .../nu-utils/src/sample_config/default_env.nu | 1 + src/main.rs | 19 ++++++++--- 7 files changed, 75 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f7f911a0ed..3442e96ff5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -195,6 +195,7 @@ reedline = { workspace = true, features = ["bashisms", "sqlite"] } crossterm = { workspace = true } ctrlc = { workspace = true } +dirs-next = { workspace = true } log = { workspace = true } miette = { workspace = true, features = ["fancy-no-backtrace", "fancy"] } mimalloc = { version = "0.1.42", default-features = false, optional = true } diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs index a5b0b13aa8..107de98c80 100644 --- a/crates/nu-cli/tests/completions/mod.rs +++ b/crates/nu-cli/tests/completions/mod.rs @@ -763,11 +763,13 @@ fn variables_completions() { // Test completions for $nu let suggestions = completer.complete("$nu.", 4); - assert_eq!(15, suggestions.len()); + assert_eq!(17, suggestions.len()); let expected: Vec = vec![ + "cache-dir".into(), "config-path".into(), "current-exe".into(), + "data-dir".into(), "default-config-dir".into(), "env-path".into(), "history-enabled".into(), diff --git a/crates/nu-path/src/helpers.rs b/crates/nu-path/src/helpers.rs index aaa53eab71..5b389410e4 100644 --- a/crates/nu-path/src/helpers.rs +++ b/crates/nu-path/src/helpers.rs @@ -6,18 +6,36 @@ pub fn home_dir() -> Option { dirs_next::home_dir() } +/// Return the data directory for the current platform or XDG_DATA_HOME if specified. +pub fn data_dir() -> Option { + match std::env::var("XDG_DATA_HOME").map(PathBuf::from) { + Ok(xdg_data) if xdg_data.is_absolute() => Some(canonicalize(&xdg_data).unwrap_or(xdg_data)), + _ => get_canonicalized_path(dirs_next::data_dir()), + } +} + +/// Return the cache directory for the current platform or XDG_CACHE_HOME if specified. +pub fn cache_dir() -> Option { + match std::env::var("XDG_CACHE_HOME").map(PathBuf::from) { + Ok(xdg_cache) if xdg_cache.is_absolute() => { + Some(canonicalize(&xdg_cache).unwrap_or(xdg_cache)) + } + _ => get_canonicalized_path(dirs_next::cache_dir()), + } +} + +/// Return the config directory for the current platform or XDG_CONFIG_HOME if specified. pub fn config_dir() -> Option { match std::env::var("XDG_CONFIG_HOME").map(PathBuf::from) { Ok(xdg_config) if xdg_config.is_absolute() => { Some(canonicalize(&xdg_config).unwrap_or(xdg_config)) } - _ => config_dir_old(), + _ => get_canonicalized_path(dirs_next::config_dir()), } } -/// Get the old default config directory. Outside of Linux, this will ignore `XDG_CONFIG_HOME` -pub fn config_dir_old() -> Option { - let path = dirs_next::config_dir()?; +pub fn get_canonicalized_path(path: Option) -> Option { + let path = path?; Some(canonicalize(&path).unwrap_or(path)) } diff --git a/crates/nu-path/src/lib.rs b/crates/nu-path/src/lib.rs index 6c064d4b57..13640acd2f 100644 --- a/crates/nu-path/src/lib.rs +++ b/crates/nu-path/src/lib.rs @@ -8,6 +8,6 @@ mod trailing_slash; pub use components::components; pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path, locate_in_dirs}; -pub use helpers::{config_dir, config_dir_old, home_dir}; +pub use helpers::{cache_dir, config_dir, data_dir, get_canonicalized_path, home_dir}; pub use tilde::expand_tilde; pub use trailing_slash::{has_trailing_slash, strip_trailing_slash}; diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 08ff6fa391..9f81a6f38b 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -149,6 +149,38 @@ pub(crate) fn create_nu_constant(engine_state: &EngineState, span: Span) -> Valu }, ); + record.push( + "data-dir", + if let Some(path) = nu_path::data_dir() { + let mut canon_data_path = canonicalize_path(engine_state, &path); + canon_data_path.push("nushell"); + Value::string(canon_data_path.to_string_lossy(), span) + } else { + Value::error( + ShellError::IOError { + msg: "Could not get data path".into(), + }, + span, + ) + }, + ); + + record.push( + "cache-dir", + if let Some(path) = nu_path::cache_dir() { + let mut canon_cache_path = canonicalize_path(engine_state, &path); + canon_cache_path.push("nushell"); + Value::string(canon_cache_path.to_string_lossy(), span) + } else { + Value::error( + ShellError::IOError { + msg: "Could not get cache path".into(), + }, + span, + ) + }, + ); + record.push("temp-path", { let canon_temp_path = canonicalize_path(engine_state, &std::env::temp_dir()); Value::string(canon_temp_path.to_string_lossy(), span) diff --git a/crates/nu-utils/src/sample_config/default_env.nu b/crates/nu-utils/src/sample_config/default_env.nu index 57e255ed20..effe76c98a 100644 --- a/crates/nu-utils/src/sample_config/default_env.nu +++ b/crates/nu-utils/src/sample_config/default_env.nu @@ -77,6 +77,7 @@ $env.ENV_CONVERSIONS = { # The default for this is $nu.default-config-dir/scripts $env.NU_LIB_DIRS = [ ($nu.default-config-dir | path join 'scripts') # add /scripts + ($nu.data-dir | path join 'completions') # default home for nushell completions ] # Directories to search for plugin binaries when calling register diff --git a/src/main.rs b/src/main.rs index f9fdff3ccf..41d5534bb0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -104,7 +104,9 @@ fn main() -> Result<()> { default: nushell_config_path.display().to_string(), }, ); - } else if let Some(old_config) = nu_path::config_dir_old().map(|p| p.join("nushell")) { + } else if let Some(old_config) = + nu_path::get_canonicalized_path(dirs_next::config_dir()).map(|p| p.join("nushell")) + { let xdg_config_empty = nushell_config_path .read_dir() .map_or(true, |mut dir| dir.next().is_none()); @@ -125,13 +127,22 @@ fn main() -> Result<()> { } } + let default_nushell_completions_path = if let Some(mut path) = nu_path::data_dir() { + path.push("nushell"); + path.push("completions"); + path + } else { + std::path::PathBuf::new() + }; + let mut default_nu_lib_dirs_path = nushell_config_path.clone(); default_nu_lib_dirs_path.push("scripts"); engine_state.add_env_var( "NU_LIB_DIRS".to_string(), - Value::test_list(vec![Value::test_string( - default_nu_lib_dirs_path.to_string_lossy(), - )]), + Value::test_list(vec![ + Value::test_string(default_nu_lib_dirs_path.to_string_lossy()), + Value::test_string(default_nushell_completions_path.to_string_lossy()), + ]), ); let mut default_nu_plugin_dirs_path = nushell_config_path; From 1e430d155ee67f9ffb0944e00b78b3be572e2f6d Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Tue, 11 Jun 2024 14:36:13 -0500 Subject: [PATCH 20/38] make packaging status 3 columns --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0bfb7fc0c3..0f1114c992 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ To use `Nu` in GitHub Action, check [setup-nu](https://github.com/marketplace/ac Detailed installation instructions can be found in the [installation chapter of the book](https://www.nushell.sh/book/installation.html). Nu is available via many package managers: -[![Packaging status](https://repology.org/badge/vertical-allrepos/nushell.svg)](https://repology.org/project/nushell/versions) +[![Packaging status](https://repology.org/badge/vertical-allrepos/nushell.svg?columns=3)](https://repology.org/project/nushell/versions) For details about which platforms the Nushell team actively supports, see [our platform support policy](devdocs/PLATFORM_SUPPORT.md). From 17daf783b266ab121dc9c2bb4ee8c4cab95e49c4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 12 Jun 2024 00:04:35 +0000 Subject: [PATCH 21/38] Bump crate-ci/typos from 1.22.1 to 1.22.4 Bumps [crate-ci/typos](https://github.com/crate-ci/typos) from 1.22.1 to 1.22.4. - [Release notes](https://github.com/crate-ci/typos/releases) - [Changelog](https://github.com/crate-ci/typos/blob/master/CHANGELOG.md) - [Commits](https://github.com/crate-ci/typos/compare/v1.22.1...v1.22.4) --- updated-dependencies: - dependency-name: crate-ci/typos dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- .github/workflows/typos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/typos.yml b/.github/workflows/typos.yml index 1142b2dbaf..f81f5c89a5 100644 --- a/.github/workflows/typos.yml +++ b/.github/workflows/typos.yml @@ -10,4 +10,4 @@ jobs: uses: actions/checkout@v4.1.6 - name: Check spelling - uses: crate-ci/typos@v1.22.1 + uses: crate-ci/typos@v1.22.4 From 63c863c81b5eb1f5318c6784bbd5528158cf7675 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 12 Jun 2024 09:37:22 +0800 Subject: [PATCH 22/38] Bump actions-rust-lang/setup-rust-toolchain from 1.8.0 to 1.9.0 (#13132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [actions-rust-lang/setup-rust-toolchain](https://github.com/actions-rust-lang/setup-rust-toolchain) from 1.8.0 to 1.9.0.
Release notes

Sourced from actions-rust-lang/setup-rust-toolchain's releases.

v1.9.0

  • Add extra argument cache-on-failure and forward it to Swatinem/rust-cache. (#39 by @​samuelhnrq)
    Set the default the value to true. This will result in more caching than previously. This helps when large dependencies are compiled only for testing to fail.
Changelog

Sourced from actions-rust-lang/setup-rust-toolchain's changelog.

[1.9.0] - 2024-06-08

  • Add extra argument cache-on-failure and forward it to Swatinem/rust-cache. (#39 by @​samuelhnrq)
    Set the default the value to true. This will result in more caching than previously. This helps when large dependencies are compiled only for testing to fail.
Commits
  • 1fbea72 Merge pull request #40 from actions-rust-lang/prepare-release
  • 46dca5d Add changelog entry
  • 1734e14 Switch default of cache-on-failure to true
  • 74e1b40 Merge pull request #39 from samuelhnrq/main
  • d60b90d feat: adds cache-on-failure propagation
  • See full diff in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=actions-rust-lang/setup-rust-toolchain&package-manager=github_actions&previous-version=1.8.0&new-version=1.9.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 8 ++++---- .github/workflows/nightly-build.yml | 2 +- .github/workflows/release.yml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9cb92b0902..cb94e5fe8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: - uses: actions/checkout@v4.1.6 - name: Setup Rust toolchain and cache - uses: actions-rust-lang/setup-rust-toolchain@v1.8.0 + uses: actions-rust-lang/setup-rust-toolchain@v1.9.0 - name: cargo fmt run: cargo fmt --all -- --check @@ -69,7 +69,7 @@ jobs: - uses: actions/checkout@v4.1.6 - name: Setup Rust toolchain and cache - uses: actions-rust-lang/setup-rust-toolchain@v1.8.0 + uses: actions-rust-lang/setup-rust-toolchain@v1.9.0 - name: Tests run: cargo test --workspace --profile ci --exclude nu_plugin_* ${{ matrix.default-flags }} @@ -98,7 +98,7 @@ jobs: - uses: actions/checkout@v4.1.6 - name: Setup Rust toolchain and cache - uses: actions-rust-lang/setup-rust-toolchain@v1.8.0 + uses: actions-rust-lang/setup-rust-toolchain@v1.9.0 - name: Install Nushell run: cargo install --path . --locked --no-default-features @@ -149,7 +149,7 @@ jobs: - uses: actions/checkout@v4.1.6 - name: Setup Rust toolchain and cache - uses: actions-rust-lang/setup-rust-toolchain@v1.8.0 + uses: actions-rust-lang/setup-rust-toolchain@v1.9.0 - name: Clippy run: cargo clippy --package nu_plugin_* -- $CLIPPY_OPTIONS diff --git a/.github/workflows/nightly-build.yml b/.github/workflows/nightly-build.yml index 251cffd3a1..4f6dccbcf9 100644 --- a/.github/workflows/nightly-build.yml +++ b/.github/workflows/nightly-build.yml @@ -122,7 +122,7 @@ jobs: echo "targets = ['${{matrix.target}}']" >> rust-toolchain.toml - name: Setup Rust toolchain and cache - uses: actions-rust-lang/setup-rust-toolchain@v1.8.0 + uses: actions-rust-lang/setup-rust-toolchain@v1.9.0 # WARN: Keep the rustflags to prevent from the winget submission error: `CAQuietExec: Error 0xc0000135` with: rustflags: '' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6aca9578d4..a71792e8c8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -69,7 +69,7 @@ jobs: echo "targets = ['${{matrix.target}}']" >> rust-toolchain.toml - name: Setup Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1.8.0 + uses: actions-rust-lang/setup-rust-toolchain@v1.9.0 # WARN: Keep the rustflags to prevent from the winget submission error: `CAQuietExec: Error 0xc0000135` with: cache: false From bdbb09652631bfa924c934b4013ce8230585a16f Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Wed, 12 Jun 2024 15:26:58 -0400 Subject: [PATCH 23/38] Fixed help for `banner` (#13138) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description `help banner` had several issues: * It used a Markdown link to an Asciinema recording, but Markdown links aren't rendered as Markdown links by the help system (and can't be, since (most?) terminals don't support that) * Minor grammatical issues * The Asciinema recording is out of date anyway. It still uses `use stdn.nu banner` which isn't valid syntax any longer. Since everyone at this point knows exactly what `banner` does 😉, I chose to simply remove the link to the recording. Also tweaked the text (initial caps and removed comma). # User-Facing Changes Help only # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` --- crates/nu-std/std/mod.nu | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/nu-std/std/mod.nu b/crates/nu-std/std/mod.nu index a20bdb4b78..15bad0e8e5 100644 --- a/crates/nu-std/std/mod.nu +++ b/crates/nu-std/std/mod.nu @@ -161,10 +161,7 @@ export def bench [ } } -# print a banner for nushell, with information about the project -# -# Example: -# an example can be found in [this asciinema recording](https://asciinema.org/a/566513) +# Print a banner for nushell with information about the project export def banner [] { let dt = (datetime-diff (date now) 2019-05-10T09:59:12-07:00) $"(ansi green) __ ,(ansi reset) From 634361b2d12e5cc3fecfcfec1f71532914c72d77 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Thu, 13 Jun 2024 01:04:12 +0000 Subject: [PATCH 24/38] Make `which-support` feature non-optional (#13125) # Description Removes the `which-support` cargo feature and makes all of its feature-gated code enabled by default in all builds. I'm not sure why this one command is gated behind a feature. It seems to be a relic of older code where we had features for what seems like every command. --- Cargo.toml | 2 -- crates/nu-cmd-extra/Cargo.toml | 4 ---- crates/nu-cmd-lang/Cargo.toml | 1 - .../nu-cmd-lang/src/core_commands/version.rs | 5 ----- crates/nu-command/Cargo.toml | 1 - crates/nu-command/src/default_context.rs | 5 +---- crates/nu-command/src/system/which_.rs | 22 ------------------- crates/nu-command/tests/commands/mod.rs | 1 - tests/shell/environment/nu_env.rs | 5 ----- tests/shell/mod.rs | 2 -- tests/shell/pipeline/commands/external.rs | 3 --- tests/shell/pipeline/commands/internal.rs | 1 - 12 files changed, 1 insertion(+), 51 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3442e96ff5..d59d7cdbff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -245,7 +245,6 @@ default = ["default-no-clipboard", "system-clipboard"] # See https://github.com/nushell/nushell/pull/11535 default-no-clipboard = [ "plugin", - "which-support", "trash-support", "sqlite", "mimalloc", @@ -265,7 +264,6 @@ system-clipboard = [ ] # Stable (Default) -which-support = ["nu-command/which-support", "nu-cmd-lang/which-support"] trash-support = ["nu-command/trash-support", "nu-cmd-lang/trash-support"] # SQLite commands for nushell diff --git a/crates/nu-cmd-extra/Cargo.toml b/crates/nu-cmd-extra/Cargo.toml index 39a705f47c..038c9421e8 100644 --- a/crates/nu-cmd-extra/Cargo.toml +++ b/crates/nu-cmd-extra/Cargo.toml @@ -32,10 +32,6 @@ serde_urlencoded = { workspace = true } v_htmlescape = { workspace = true } itertools = { workspace = true } -[features] -extra = ["default"] -default = [] - [dev-dependencies] nu-cmd-lang = { path = "../nu-cmd-lang", version = "0.94.3" } nu-command = { path = "../nu-command", version = "0.94.3" } diff --git a/crates/nu-cmd-lang/Cargo.toml b/crates/nu-cmd-lang/Cargo.toml index 22e1cad374..dcd940ba58 100644 --- a/crates/nu-cmd-lang/Cargo.toml +++ b/crates/nu-cmd-lang/Cargo.toml @@ -25,7 +25,6 @@ shadow-rs = { version = "0.28", default-features = false } [features] mimalloc = [] -which-support = [] trash-support = [] sqlite = [] static-link-openssl = [] diff --git a/crates/nu-cmd-lang/src/core_commands/version.rs b/crates/nu-cmd-lang/src/core_commands/version.rs index 22ef1f2a08..77283105a9 100644 --- a/crates/nu-cmd-lang/src/core_commands/version.rs +++ b/crates/nu-cmd-lang/src/core_commands/version.rs @@ -160,11 +160,6 @@ fn features_enabled() -> Vec { // NOTE: There should be another way to know features on. - #[cfg(feature = "which-support")] - { - names.push("which".to_string()); - } - #[cfg(feature = "trash-support")] { names.push("trash".to_string()); diff --git a/crates/nu-command/Cargo.toml b/crates/nu-command/Cargo.toml index 4d69ce089a..f16d62d836 100644 --- a/crates/nu-command/Cargo.toml +++ b/crates/nu-command/Cargo.toml @@ -134,7 +134,6 @@ workspace = true plugin = ["nu-parser/plugin"] sqlite = ["rusqlite"] trash-support = ["trash"] -which-support = [] [dev-dependencies] nu-cmd-lang = { path = "../nu-cmd-lang", version = "0.94.3" } diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index a23f9c4ef4..0901d56588 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -127,7 +127,7 @@ pub fn add_shell_command_context(mut engine_state: EngineState) -> EngineState { SysTemp, SysUsers, UName, - + Which, }; // Help @@ -172,9 +172,6 @@ pub fn add_shell_command_context(mut engine_state: EngineState) -> EngineState { ))] bind_command! { Ps }; - #[cfg(feature = "which-support")] - bind_command! { Which }; - // Strings bind_command! { Char, diff --git a/crates/nu-command/src/system/which_.rs b/crates/nu-command/src/system/which_.rs index 81b6057864..f0cf4ece39 100644 --- a/crates/nu-command/src/system/which_.rs +++ b/crates/nu-command/src/system/which_.rs @@ -92,7 +92,6 @@ fn get_entries_in_nu( all_entries } -#[cfg(feature = "which-support")] fn get_first_entry_in_path( item: &str, span: Span, @@ -104,17 +103,6 @@ fn get_first_entry_in_path( .ok() } -#[cfg(not(feature = "which-support"))] -fn get_first_entry_in_path( - _item: &str, - _span: Span, - _cwd: impl AsRef, - _paths: impl AsRef, -) -> Option { - None -} - -#[cfg(feature = "which-support")] fn get_all_entries_in_path( item: &str, span: Span, @@ -129,16 +117,6 @@ fn get_all_entries_in_path( .unwrap_or_default() } -#[cfg(not(feature = "which-support"))] -fn get_all_entries_in_path( - _item: &str, - _span: Span, - _cwd: impl AsRef, - _paths: impl AsRef, -) -> Vec { - vec![] -} - #[derive(Debug)] struct WhichArgs { applications: Vec>, diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 922e804405..a1cb82a4b4 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -125,7 +125,6 @@ mod upsert; mod url; mod use_; mod where_; -#[cfg(feature = "which-support")] mod which; mod while_; mod with_env; diff --git a/tests/shell/environment/nu_env.rs b/tests/shell/environment/nu_env.rs index c19b724ceb..d249d90678 100644 --- a/tests/shell/environment/nu_env.rs +++ b/tests/shell/environment/nu_env.rs @@ -36,7 +36,6 @@ fn picks_up_env_keys_when_entering_trusted_directory() { }) } -#[cfg(feature = "which-support")] #[test] #[serial] fn picks_up_and_lets_go_env_keys_when_entering_trusted_directory_with_implied_cd() { @@ -433,7 +432,6 @@ fn given_a_hierarchy_of_trusted_directories_going_back_restores_overwritten_vari }) } -#[cfg(feature = "which-support")] #[test] #[serial] fn local_config_env_var_present_and_removed_correctly() { @@ -487,7 +485,6 @@ fn local_config_env_var_present_and_removed_correctly() { }); } -#[cfg(feature = "which-support")] #[test] #[serial] fn local_config_env_var_gets_overwritten() { @@ -553,7 +550,6 @@ fn local_config_env_var_gets_overwritten() { }); } -#[cfg(feature = "which-support")] #[test] #[serial] fn autoenv_test_entry_scripts() { @@ -601,7 +597,6 @@ fn autoenv_test_entry_scripts() { }); } -#[cfg(feature = "which-support")] #[test] #[serial] fn autoenv_test_exit_scripts() { diff --git a/tests/shell/mod.rs b/tests/shell/mod.rs index ba7465f5e7..62f79a59c1 100644 --- a/tests/shell/mod.rs +++ b/tests/shell/mod.rs @@ -3,9 +3,7 @@ use nu_test_support::playground::Playground; use nu_test_support::{nu, nu_repl_code, pipeline}; use pretty_assertions::assert_eq; -#[cfg(feature = "which-support")] mod environment; - mod pipeline; mod repl; diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index a73cbc878d..d138b65766 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -3,7 +3,6 @@ use nu_test_support::nu; use nu_test_support::playground::Playground; use pretty_assertions::assert_eq; -#[cfg(feature = "which-support")] #[test] fn shows_error_for_command_not_found() { let actual = nu!("ferris_is_not_here.exe"); @@ -11,7 +10,6 @@ fn shows_error_for_command_not_found() { assert!(!actual.err.is_empty()); } -#[cfg(feature = "which-support")] #[test] fn shows_error_for_command_not_found_in_pipeline() { let actual = nu!("ferris_is_not_here.exe | echo done"); @@ -20,7 +18,6 @@ fn shows_error_for_command_not_found_in_pipeline() { } #[ignore] // jt: we can't test this using the -c workaround currently -#[cfg(feature = "which-support")] #[test] fn automatically_change_directory() { use nu_test_support::playground::Playground; diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs index 98f3fb88c9..84f0bf146f 100644 --- a/tests/shell/pipeline/commands/internal.rs +++ b/tests/shell/pipeline/commands/internal.rs @@ -550,7 +550,6 @@ fn dynamic_closure_rest_args() { assert_eq!(actual.out, "1, 2, 3"); } -#[cfg(feature = "which-support")] #[test] fn argument_subexpression_reports_errors() { let actual = nu!("echo (ferris_is_not_here.exe)"); From 323d5457f923c55d1b504ddca5614d283cd421af Mon Sep 17 00:00:00 2001 From: Maxim Zhiburt Date: Thu, 13 Jun 2024 04:35:04 +0300 Subject: [PATCH 25/38] Improve performance of `explore` - 1 (#13116) Could be improved further I guess; but not here; You can test the speed differences using data from #13088 ```nu open data.db | get profiles | explore ``` address: #13062 ________ 1. Noticed that search does not work anymore (even on `main` branch). 2. Not sure about resolved merged conflicts, seems fine, but maybe something was lost. --------- Co-authored-by: Reilly Wood --- crates/nu-explore/src/commands/nu.rs | 8 +- crates/nu-explore/src/commands/table.rs | 2 +- crates/nu-explore/src/commands/try.rs | 2 +- crates/nu-explore/src/views/record/mod.rs | 178 ++++++++++-------- .../src/views/record/table_widget.rs | 88 ++++----- crates/nu-explore/src/views/try.rs | 20 +- crates/nu-table/src/table.rs | 9 +- 7 files changed, 159 insertions(+), 148 deletions(-) diff --git a/crates/nu-explore/src/commands/nu.rs b/crates/nu-explore/src/commands/nu.rs index 1310dfbd1f..f8aaf44c24 100644 --- a/crates/nu-explore/src/commands/nu.rs +++ b/crates/nu-explore/src/commands/nu.rs @@ -27,7 +27,7 @@ impl NuCmd { } impl ViewCommand for NuCmd { - type View = NuView<'static>; + type View = NuView; fn name(&self) -> &'static str { Self::NAME @@ -72,12 +72,12 @@ impl ViewCommand for NuCmd { } } -pub enum NuView<'a> { - Records(RecordView<'a>), +pub enum NuView { + Records(RecordView), Preview(Preview), } -impl View for NuView<'_> { +impl View for NuView { fn draw(&mut self, f: &mut Frame, area: Rect, cfg: ViewConfig<'_>, layout: &mut Layout) { match self { NuView::Records(v) => v.draw(f, area, cfg, layout), diff --git a/crates/nu-explore/src/commands/table.rs b/crates/nu-explore/src/commands/table.rs index 6bc6268067..e5a3ae5599 100644 --- a/crates/nu-explore/src/commands/table.rs +++ b/crates/nu-explore/src/commands/table.rs @@ -30,7 +30,7 @@ impl TableCmd { } impl ViewCommand for TableCmd { - type View = RecordView<'static>; + type View = RecordView; fn name(&self) -> &'static str { Self::NAME diff --git a/crates/nu-explore/src/commands/try.rs b/crates/nu-explore/src/commands/try.rs index a81ce13e21..e70b4237d9 100644 --- a/crates/nu-explore/src/commands/try.rs +++ b/crates/nu-explore/src/commands/try.rs @@ -22,7 +22,7 @@ impl TryCmd { } impl ViewCommand for TryCmd { - type View = TryView<'static>; + type View = TryView; fn name(&self) -> &'static str { Self::NAME diff --git a/crates/nu-explore/src/views/record/mod.rs b/crates/nu-explore/src/views/record/mod.rs index 557995c40b..9e67e08120 100644 --- a/crates/nu-explore/src/views/record/mod.rs +++ b/crates/nu-explore/src/views/record/mod.rs @@ -23,23 +23,20 @@ use nu_protocol::{ Config, Record, Span, Value, }; use ratatui::{layout::Rect, widgets::Block}; -use std::{borrow::Cow, collections::HashMap}; +use std::collections::HashMap; pub use self::table_widget::Orientation; #[derive(Debug, Clone)] -pub struct RecordView<'a> { - layer_stack: Vec>, +pub struct RecordView { + layer_stack: Vec, mode: UIMode, orientation: Orientation, cfg: ExploreConfig, } -impl<'a> RecordView<'a> { - pub fn new( - columns: impl Into>, - records: impl Into]>>, - ) -> Self { +impl RecordView { + pub fn new(columns: Vec, records: Vec>) -> Self { Self { layer_stack: vec![RecordLayer::new(columns, records)], mode: UIMode::View, @@ -64,13 +61,13 @@ impl<'a> RecordView<'a> { } // todo: rename to get_layer - pub fn get_layer_last(&self) -> &RecordLayer<'a> { + pub fn get_layer_last(&self) -> &RecordLayer { self.layer_stack .last() .expect("we guarantee that 1 entry is always in a list") } - pub fn get_layer_last_mut(&mut self) -> &mut RecordLayer<'a> { + pub fn get_layer_last_mut(&mut self) -> &mut RecordLayer { self.layer_stack .last_mut() .expect("we guarantee that 1 entry is always in a list") @@ -134,32 +131,39 @@ impl<'a> RecordView<'a> { Orientation::Left => (column, row), }; - if layer.records.len() > row && layer.records[row].len() > column { - layer.records[row][column].clone() - } else { - Value::nothing(Span::unknown()) + if row >= layer.count_rows() || column >= layer.count_columns() { + // actually must never happen; unless cursor works incorrectly + // if being sure about cursor it can be deleted; + return Value::nothing(Span::unknown()); } + + layer.record_values[row][column].clone() } - /// Create a table widget. - /// WARNING: this is currently really slow on large data sets. - /// It creates a string representation of every cell in the table and looks at every row for lscolorize. - fn create_table_widget(&'a self, cfg: ViewConfig<'a>) -> TableWidget<'a> { - let layer = self.get_layer_last(); - let mut data = convert_records_to_string(&layer.records, cfg.nu_config, cfg.style_computer); - lscolorize(&layer.columns, &mut data, cfg.lscolors); - - let headers = layer.columns.as_ref(); + fn create_table_widget<'a>(&'a mut self, cfg: ViewConfig<'a>) -> TableWidget<'a> { + let style = self.cfg.table; let style_computer = cfg.style_computer; let Position { row, column } = self.get_window_origin(); + let layer = self.get_layer_last_mut(); + if layer.record_text.is_none() { + let mut data = + convert_records_to_string(&layer.record_values, cfg.nu_config, cfg.style_computer); + lscolorize(&layer.column_names, &mut data, cfg.lscolors); + + layer.record_text = Some(data); + } + + let headers = &layer.column_names; + let data = layer.record_text.as_ref().expect("always ok"); + TableWidget::new( headers, data, style_computer, row, column, - self.cfg.table, + style, layer.orientation, ) } @@ -197,7 +201,7 @@ impl<'a> RecordView<'a> { } } -impl View for RecordView<'_> { +impl View for RecordView { fn draw(&mut self, f: &mut Frame, area: Rect, cfg: ViewConfig<'_>, layout: &mut Layout) { let mut table_layout = TableWidgetState::default(); // TODO: creating the table widget is O(N) where N is the number of cells in the grid. @@ -265,7 +269,7 @@ impl View for RecordView<'_> { let style_computer = StyleComputer::new(&dummy_engine_state, &dummy_stack, HashMap::new()); let data = convert_records_to_string( - &self.get_layer_last().records, + &self.get_layer_last().record_values, &nu_protocol::Config::default(), &style_computer, ); @@ -274,7 +278,7 @@ impl View for RecordView<'_> { } fn show_data(&mut self, pos: usize) -> bool { - let data = &self.get_layer_last().records; + let data = &self.get_layer_last().record_values; let mut i = 0; for (row, cells) in data.iter().enumerate() { @@ -332,30 +336,35 @@ enum UIMode { } #[derive(Debug, Clone)] -pub struct RecordLayer<'a> { - columns: Cow<'a, [String]>, - records: Cow<'a, [Vec]>, +pub struct RecordLayer { + column_names: Vec, + // These are the raw records in the current layer. The sole reason we keep this around is so we can return the original value + // if it's being peeked. Otherwise we could accept an iterator over it. + // or if it could be Clonable we could do that anyway; + // cause it would keep memory footprint lower while keep everything working + // (yee would make return O(n); we would need to traverse iterator once again; but maybe worth it) + record_values: Vec>, + // This is the text representation of the record values (the actual text that will be displayed to users). + // It's an Option because we need configuration to set it and we (currently) don't have access to configuration when things are created. + record_text: Option>>, orientation: Orientation, name: Option, was_transposed: bool, cursor: WindowCursor2D, } -impl<'a> RecordLayer<'a> { - fn new( - columns: impl Into>, - records: impl Into]>>, - ) -> Self { - let columns = columns.into(); - let records = records.into(); - +impl RecordLayer { + fn new(columns: Vec, records: Vec>) -> Self { // TODO: refactor so this is fallible and returns a Result instead of panicking let cursor = WindowCursor2D::new(records.len(), columns.len()).expect("Failed to create cursor"); + let column_names = columns.iter().map(|s| strip_string(s)).collect(); + Self { - columns, - records, + column_names, + record_values: records, + record_text: None, cursor, orientation: Orientation::Top, name: None, @@ -369,21 +378,21 @@ impl<'a> RecordLayer<'a> { fn count_rows(&self) -> usize { match self.orientation { - Orientation::Top => self.records.len(), - Orientation::Left => self.columns.len(), + Orientation::Top => self.record_values.len(), + Orientation::Left => self.column_names.len(), } } fn count_columns(&self) -> usize { match self.orientation { - Orientation::Top => self.columns.len(), - Orientation::Left => self.records.len(), + Orientation::Top => self.column_names.len(), + Orientation::Left => self.record_values.len(), } } fn get_column_header(&self) -> Option { let col = self.cursor.column(); - self.columns.get(col).map(|header| header.to_string()) + self.column_names.get(col).map(|header| header.to_string()) } fn reset_cursor(&mut self) { @@ -570,13 +579,13 @@ fn handle_key_event_cursor_mode( } } -fn create_layer(value: Value) -> Result> { +fn create_layer(value: Value) -> Result { let (columns, values) = collect_input(value)?; Ok(RecordLayer::new(columns, values)) } -fn push_layer(view: &mut RecordView<'_>, mut next_layer: RecordLayer<'static>) { +fn push_layer(view: &mut RecordView, mut next_layer: RecordLayer) { let layer = view.get_layer_last(); let header = layer.get_column_header(); @@ -599,9 +608,9 @@ fn estimate_page_size(area: Rect, show_head: bool) -> u16 { } /// scroll to the end of the data -fn tail_data(state: &mut RecordView<'_>, page_size: usize) { +fn tail_data(state: &mut RecordView, page_size: usize) { let layer = state.get_layer_last_mut(); - let count_rows = layer.records.len(); + let count_rows = layer.count_rows(); if count_rows > page_size { layer .cursor @@ -620,6 +629,7 @@ fn convert_records_to_string( row.iter() .map(|value| { let text = value.clone().to_abbreviated_string(cfg); + let text = strip_string(&text); let float_precision = cfg.float_precision as usize; make_styled_string(style_computer, text, Some(value), float_precision) @@ -649,12 +659,16 @@ fn build_last_value(v: &RecordView) -> Value { fn build_table_as_list(v: &RecordView) -> Value { let layer = v.get_layer_last(); - let cols = &layer.columns; let vals = layer - .records + .record_values .iter() .map(|vals| { - let record = cols.iter().cloned().zip(vals.iter().cloned()).collect(); + let record = layer + .column_names + .iter() + .cloned() + .zip(vals.iter().cloned()) + .collect(); Value::record(record, NuSpan::unknown()) }) .collect(); @@ -665,16 +679,15 @@ fn build_table_as_list(v: &RecordView) -> Value { fn build_table_as_record(v: &RecordView) -> Value { let layer = v.get_layer_last(); - let record = if let Some(row) = layer.records.first() { - layer - .columns + let mut record = Record::new(); + if let Some(row) = layer.record_values.first() { + record = layer + .column_names .iter() .cloned() .zip(row.iter().cloned()) - .collect() - } else { - Record::new() - }; + .collect(); + } Value::record(record, NuSpan::unknown()) } @@ -708,17 +721,12 @@ fn get_percentage(value: usize, max: usize) -> usize { ((value as f32 / max as f32) * 100.0).floor() as usize } -fn transpose_table(layer: &mut RecordLayer<'_>) { - let count_rows = layer.records.len(); - let count_columns = layer.columns.len(); +fn transpose_table(layer: &mut RecordLayer) { + let count_rows = layer.record_values.len(); + let count_columns = layer.column_names.len(); if layer.was_transposed { - let data = match &mut layer.records { - Cow::Owned(data) => data, - Cow::Borrowed(_) => unreachable!("must never happen"), - }; - - let headers = pop_first_column(data); + let headers = pop_first_column(&mut layer.record_values); let headers = headers .into_iter() .map(|value| match value { @@ -727,23 +735,25 @@ fn transpose_table(layer: &mut RecordLayer<'_>) { }) .collect(); - let data = _transpose_table(data, count_rows, count_columns - 1); + let data = _transpose_table(&layer.record_values, count_rows, count_columns - 1); - layer.records = Cow::Owned(data); - layer.columns = Cow::Owned(headers); - } else { - let mut data = _transpose_table(&layer.records, count_rows, count_columns); + layer.record_values = data; + layer.column_names = headers; - for (column, column_name) in layer.columns.iter().enumerate() { - let value = Value::string(column_name, NuSpan::unknown()); - - data[column].insert(0, value); - } - - layer.records = Cow::Owned(data); - layer.columns = (1..count_rows + 1 + 1).map(|i| i.to_string()).collect(); + return; } + let mut data = _transpose_table(&layer.record_values, count_rows, count_columns); + + for (column, column_name) in layer.column_names.iter().enumerate() { + let value = Value::string(column_name, NuSpan::unknown()); + + data[column].insert(0, value); + } + + layer.record_values = data; + layer.column_names = (1..count_rows + 1 + 1).map(|i| i.to_string()).collect(); + layer.was_transposed = !layer.was_transposed; } @@ -770,3 +780,9 @@ fn _transpose_table( data } + +fn strip_string(text: &str) -> String { + String::from_utf8(strip_ansi_escapes::strip(text)) + .map_err(|_| ()) + .unwrap_or_else(|_| text.to_owned()) +} diff --git a/crates/nu-explore/src/views/record/table_widget.rs b/crates/nu-explore/src/views/record/table_widget.rs index 2410d1bd11..7149a7ce60 100644 --- a/crates/nu-explore/src/views/record/table_widget.rs +++ b/crates/nu-explore/src/views/record/table_widget.rs @@ -13,15 +13,12 @@ use ratatui::{ text::Span, widgets::{Block, Borders, Paragraph, StatefulWidget, Widget}, }; -use std::{ - borrow::Cow, - cmp::{max, Ordering}, -}; +use std::cmp::{max, Ordering}; #[derive(Debug, Clone)] pub struct TableWidget<'a> { - columns: Cow<'a, [String]>, - data: Cow<'a, [Vec]>, + columns: &'a [String], + data: &'a [Vec], index_row: usize, index_column: usize, config: TableConfig, @@ -39,8 +36,8 @@ pub enum Orientation { impl<'a> TableWidget<'a> { #[allow(clippy::too_many_arguments)] pub fn new( - columns: impl Into>, - data: impl Into]>>, + columns: &'a [String], + data: &'a [Vec], style_computer: &'a StyleComputer<'a>, index_row: usize, index_column: usize, @@ -48,8 +45,8 @@ impl<'a> TableWidget<'a> { header_position: Orientation, ) -> Self { Self { - columns: columns.into(), - data: data.into(), + columns, + data, style_computer, index_row, index_column, @@ -197,22 +194,25 @@ impl<'a> TableWidget<'a> { } if show_head { - let mut header = [head_row_text(&head, self.style_computer)]; + let head_style = head_style(&head, self.style_computer); if head_width > use_space as usize { - truncate_str(&mut header[0].0, use_space as usize) + truncate_str(&mut head, use_space as usize) } + let head_iter = [(&head, head_style)].into_iter(); let mut w = width; w += render_space(buf, w, head_y, 1, padding_l); - w += render_column(buf, w, head_y, use_space, &header); + w += render_column(buf, w, head_y, use_space, head_iter); w += render_space(buf, w, head_y, 1, padding_r); let x = w - padding_r - use_space; - state.layout.push(&header[0].0, x, head_y, use_space, 1); + state.layout.push(&head, x, head_y, use_space, 1); } + let head_rows = column.iter().map(|(t, s)| (t, *s)); + width += render_space(buf, width, data_y, data_height, padding_l); - width += render_column(buf, width, data_y, use_space, &column); + width += render_column(buf, width, data_y, use_space, head_rows); width += render_space(buf, width, data_y, data_height, padding_r); for (row, (text, _)) in column.iter().enumerate() { @@ -305,10 +305,9 @@ impl<'a> TableWidget<'a> { return; } - let columns = columns + let columns_iter = columns .iter() - .map(|s| head_row_text(s, self.style_computer)) - .collect::>(); + .map(|s| (s.clone(), head_style(s, self.style_computer))); if !show_index { let x = area.x + left_w; @@ -326,12 +325,12 @@ impl<'a> TableWidget<'a> { let x = area.x + left_w; left_w += render_space(buf, x, area.y, 1, padding_l); let x = area.x + left_w; - left_w += render_column(buf, x, area.y, columns_width as u16, &columns); + left_w += render_column(buf, x, area.y, columns_width as u16, columns_iter); let x = area.x + left_w; left_w += render_space(buf, x, area.y, 1, padding_r); let layout_x = left_w - padding_r - columns_width as u16; - for (i, (text, _)) in columns.iter().enumerate() { + for (i, text) in columns.iter().enumerate() { state .layout .push(text, layout_x, area.y + i as u16, columns_width as u16, 1); @@ -377,10 +376,12 @@ impl<'a> TableWidget<'a> { break; } + let head_rows = column.iter().map(|(t, s)| (t, *s)); + let x = area.x + left_w; left_w += render_space(buf, x, area.y, area.height, padding_l); let x = area.x + left_w; - left_w += render_column(buf, x, area.y, column_width, &column); + left_w += render_column(buf, x, area.y, column_width, head_rows); let x = area.x + left_w; left_w += render_space(buf, x, area.y, area.height, padding_r); @@ -619,14 +620,14 @@ fn repeat_vertical( c: char, style: TextStyle, ) { - let text = std::iter::repeat(c) - .take(width as usize) - .collect::(); + let text = String::from(c); let style = text_style_to_tui_style(style); - let span = Span::styled(text, style); + let span = Span::styled(&text, style); for row in 0..height { - buf.set_span(x, y + row, &span, width); + for col in 0..width { + buf.set_span(x + col, y + row, &span, 1); + } } } @@ -676,35 +677,28 @@ fn calculate_column_width(column: &[NuText]) -> usize { .unwrap_or(0) } -fn render_column( +fn render_column( buf: &mut ratatui::buffer::Buffer, x: u16, y: u16, available_width: u16, - rows: &[NuText], -) -> u16 { - for (row, (text, style)) in rows.iter().enumerate() { - let style = text_style_to_tui_style(*style); - let text = strip_string(text); - let span = Span::styled(text, style); + rows: impl Iterator, +) -> u16 +where + T: AsRef, + S: Into, +{ + for (row, (text, style)) in rows.enumerate() { + let style = text_style_to_tui_style(style.into()); + let span = Span::styled(text.as_ref(), style); buf.set_span(x, y + row as u16, &span, available_width); } available_width } -fn strip_string(text: &str) -> String { - String::from_utf8(strip_ansi_escapes::strip(text)) - .map_err(|_| ()) - .unwrap_or_else(|_| text.to_owned()) -} - -fn head_row_text(head: &str, style_computer: &StyleComputer) -> NuText { - ( - String::from(head), - TextStyle::with_style( - Alignment::Center, - style_computer.compute("header", &Value::string(head, nu_protocol::Span::unknown())), - ), - ) +fn head_style(head: &str, style_computer: &StyleComputer) -> TextStyle { + let style = + style_computer.compute("header", &Value::string(head, nu_protocol::Span::unknown())); + TextStyle::with_style(Alignment::Center, style) } diff --git a/crates/nu-explore/src/views/try.rs b/crates/nu-explore/src/views/try.rs index 9acbb84208..7ad6d38aef 100644 --- a/crates/nu-explore/src/views/try.rs +++ b/crates/nu-explore/src/views/try.rs @@ -16,21 +16,21 @@ use ratatui::{ }; use std::cmp::min; -pub struct TryView<'a> { +pub struct TryView { input: Value, command: String, - reactive: bool, - table: Option>, + immediate: bool, + table: Option, view_mode: bool, border_color: Style, } -impl<'a> TryView<'a> { +impl TryView { pub fn new(input: Value) -> Self { Self { input, table: None, - reactive: false, + immediate: false, border_color: Style::default(), view_mode: false, command: String::new(), @@ -48,7 +48,7 @@ impl<'a> TryView<'a> { } } -impl View for TryView<'_> { +impl View for TryView { fn draw(&mut self, f: &mut Frame, area: Rect, cfg: ViewConfig<'_>, layout: &mut Layout) { let border_color = self.border_color; @@ -178,7 +178,7 @@ impl View for TryView<'_> { if !self.command.is_empty() { self.command.pop(); - if self.reactive { + if self.immediate { match self.try_run(engine_state, stack) { Ok(_) => info.report = Some(Report::default()), Err(err) => info.report = Some(Report::error(format!("Error: {err}"))), @@ -191,7 +191,7 @@ impl View for TryView<'_> { KeyCode::Char(c) => { self.command.push(*c); - if self.reactive { + if self.immediate { match self.try_run(engine_state, stack) { Ok(_) => info.report = Some(Report::default()), Err(err) => info.report = Some(Report::error(format!("Error: {err}"))), @@ -235,7 +235,7 @@ impl View for TryView<'_> { fn setup(&mut self, config: ViewConfig<'_>) { self.border_color = nu_style_to_tui(config.explore_config.table.separator_style); - self.reactive = config.explore_config.try_reactive; + self.immediate = config.explore_config.try_reactive; let mut r = RecordView::new(vec![], vec![]); r.setup(config); @@ -253,7 +253,7 @@ fn run_command( input: &Value, engine_state: &EngineState, stack: &mut Stack, -) -> Result> { +) -> Result { let pipeline = run_command_with_value(command, input, engine_state, stack)?; let is_record = matches!(pipeline, PipelineData::Value(Value::Record { .. }, ..)); diff --git a/crates/nu-table/src/table.rs b/crates/nu-table/src/table.rs index faf47d84cd..bdaf7fb6c3 100644 --- a/crates/nu-table/src/table.rs +++ b/crates/nu-table/src/table.rs @@ -259,9 +259,7 @@ fn draw_table( table.with(width_ctrl); } - let tablewidth = table.total_width(); - - table_to_string(table, tablewidth, termwidth) + table_to_string(table, termwidth) } fn set_indent(table: &mut Table, left: usize, right: usize) { @@ -289,7 +287,9 @@ fn set_border_head(table: &mut Table, with_footer: bool, wctrl: TableWidthCtrl) } } -fn table_to_string(table: Table, total_width: usize, termwidth: usize) -> Option { +fn table_to_string(table: Table, termwidth: usize) -> Option { + let total_width = table.total_width(); + if total_width > termwidth { None } else { @@ -318,6 +318,7 @@ impl TableOption, ColoredConfig> for dim: &mut CompleteDimensionVecRecords<'_>, ) { let total_width = get_total_width2(&self.width, cfg); + if total_width > self.max { TableTrim { max: self.max, From af6e1cb5a67640fc7b46703c98bdd9aa4c7d43b6 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Thu, 13 Jun 2024 20:55:17 -0400 Subject: [PATCH 26/38] Added search terms to `if` (#13145) # Description In this PR, I continue my tradition of trivial but hopefully helpful `help` tweaks. As mentioned in #13143, I noticed that `help -f else` oddly didn't return the `if` statement itself. Perhaps not so oddly, since who the heck is going to go looking for *"else"* in the help? Well, I did ... Added *"else"* and *"conditional"* to the search terms for `if`. I'll work on the meat of #13143 next - That's more substantiative. # User-Facing Changes Help only # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` - # After Submitting --- crates/nu-cmd-lang/src/core_commands/if_.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/nu-cmd-lang/src/core_commands/if_.rs b/crates/nu-cmd-lang/src/core_commands/if_.rs index 83808c8e06..a2071530ac 100644 --- a/crates/nu-cmd-lang/src/core_commands/if_.rs +++ b/crates/nu-cmd-lang/src/core_commands/if_.rs @@ -122,6 +122,10 @@ impl Command for If { } } + fn search_terms(&self) -> Vec<&str> { + vec!["else", "conditional"] + } + fn examples(&self) -> Vec { vec![ Example { From b1cf0e258dc103f28b23190e495a2695bb0f9c97 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Thu, 13 Jun 2024 20:56:11 -0400 Subject: [PATCH 27/38] Expand tables in help examples in std (#13146) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Some command help has example results with nested `table` data which is displayed as the "non-expanded" form. E.g.: ```nu ╭───┬────────────────╮ │ 0 │ [list 2 items] │ │ 1 │ [list 2 items] │ ╰───┴────────────────╯ ``` For a good example, see `help zip`. While we could simply remove the offending Example `result`'s from the command itself, `std help` is capable of expanding the table properly. It already formats the output of each example result using `table`, so simply making it a `table -e` fixes the output. While I wish we had a way of expanding the tables in the builtin `help`, that seems to be the same type of problem as in formatting the `cal` output (see #11954). I personally think it's better to add this feature to `std help` than to remove the offending example results, but as long as `std help` is optional, only a small percentage of users are going to see the "expected" results. # User-Facing Changes Excerpt from `std help zip` before change: ```nu Zip two lists > [1 2] | zip [3 4] ╭───┬────────────────╮ │ 0 │ [list 2 items] │ │ 1 │ [list 2 items] │ ╰───┴────────────────╯ ``` After: ```nu Zip two lists > [1 2] | zip [3 4] ╭───┬───────────╮ │ 0 │ ╭───┬───╮ │ │ │ │ 0 │ 1 │ │ │ │ │ 1 │ 3 │ │ │ │ ╰───┴───╯ │ │ 1 │ ╭───┬───╮ │ │ │ │ 0 │ 2 │ │ │ │ │ 1 │ 4 │ │ │ │ ╰───┴───╯ │ ╰───┴───────────╯ ``` # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` - # After Submitting --- crates/nu-std/std/help.nu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-std/std/help.nu b/crates/nu-std/std/help.nu index df6c4c6ca6..614ec402d1 100644 --- a/crates/nu-std/std/help.nu +++ b/crates/nu-std/std/help.nu @@ -658,7 +658,7 @@ def build-command-page [command: record] { $" > ($example.example | nu-highlight)" (if not ($example.result | is-empty) { $example.result - | table + | table -e | to text | if ($example.result | describe) == "binary" { str join } else { lines } | each {|line| From 3a6d8aac0bce6a70351fcfe860e0aa3694054a99 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Sat, 15 Jun 2024 13:27:55 -0400 Subject: [PATCH 28/38] Return an empty list when no `std help --find` results are found (#13160) # Description Fixes #13143 by returning an empty list when there are no results found by `std help --find/-f` # User-Facing Changes In addition, prints a message to stderr. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting --- crates/nu-std/std/help.nu | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/nu-std/std/help.nu b/crates/nu-std/std/help.nu index 614ec402d1..16a20504cb 100644 --- a/crates/nu-std/std/help.nu +++ b/crates/nu-std/std/help.nu @@ -771,6 +771,11 @@ You can also learn more at (ansi default_italic)(ansi light_cyan_underline)https let modules = (try { modules $target_item --find $find }) if not ($modules | is-empty) { return $modules } + + if ($find | is-not-empty) { + print -e $"No help results found mentioning: ($find)" + return [] + } let span = (metadata $item | get span) error make { From b79a2255d2ab2bb93571df55371290814ca718e5 Mon Sep 17 00:00:00 2001 From: Piepmatz Date: Tue, 18 Jun 2024 01:05:11 +0200 Subject: [PATCH 29/38] Add derive macros for `FromValue` and `IntoValue` to ease the use of `Value`s in Rust code (#13031) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description After discussing with @sholderbach the cumbersome usage of `nu_protocol::Value` in Rust, I created a derive macro to simplify it. I’ve added a new crate called `nu-derive-value`, which includes two macros, `IntoValue` and `FromValue`. These are re-exported in `nu-protocol` and should be encouraged to be used via that re-export. The macros ensure that all types can easily convert from and into `Value`. For example, as a plugin author, you can define your plugin configuration using a Rust struct and easily convert it using `FromValue`. This makes plugin configuration less of a hassle. I introduced the `IntoValue` trait for a standardized approach to converting values into `Value` (and a fallible variant `TryIntoValue`). This trait could potentially replace existing `into_value` methods. Along with this, I've implemented `FromValue` for several standard types and refined other implementations to use blanket implementations where applicable. I made these design choices with input from @devyn. There are more improvements possible, but this is a solid start and the PR is already quite substantial. # User-Facing Changes For `nu-protocol` users, these changes simplify the handling of `Value`s. There are no changes for end-users of nushell itself. # Tests + Formatting Documenting the macros itself is not really possible, as they cannot really reference any other types since they are the root of the dependency graph. The standard library has the same problem ([std::Debug](https://doc.rust-lang.org/stable/std/fmt/derive.Debug.html)). However I documented the `FromValue` and `IntoValue` traits completely. For testing, I made of use `proc-macro2` in the derive macro code. This would allow testing the generated source code. Instead I just tested that the derived functionality is correct. This is done in `nu_protocol::value::test_derive`, as a consumer of `nu-derive-value` needs to do the testing of the macro usage. I think that these tests should provide a stable baseline so that users can be sure that the impl works. # After Submitting With these macros available, we can probably use them in some examples for plugins to showcase the use of them. --- Cargo.lock | 22 + Cargo.toml | 6 + crates/nu-cmd-base/src/hook.rs | 2 +- crates/nu-command/src/charting/histogram.rs | 2 +- .../src/conversions/into/cell_path.rs | 2 +- crates/nu-command/src/filters/split_by.rs | 2 +- crates/nu-command/src/filters/uniq_by.rs | 2 +- crates/nu-command/src/sort_utils.rs | 2 +- crates/nu-derive-value/Cargo.toml | 21 + crates/nu-derive-value/LICENSE | 21 + crates/nu-derive-value/src/attributes.rs | 116 +++ crates/nu-derive-value/src/error.rs | 82 ++ crates/nu-derive-value/src/from.rs | 539 ++++++++++++ crates/nu-derive-value/src/into.rs | 266 ++++++ crates/nu-derive-value/src/lib.rs | 69 ++ crates/nu-derive-value/src/tests.rs | 157 ++++ crates/nu-protocol/Cargo.toml | 2 + crates/nu-protocol/src/errors/shell_error.rs | 4 +- crates/nu-protocol/src/lib.rs | 2 + crates/nu-protocol/src/value/from_value.rs | 822 ++++++++++-------- crates/nu-protocol/src/value/into_value.rs | 196 +++++ crates/nu-protocol/src/value/mod.rs | 38 +- crates/nu-protocol/src/value/test_derive.rs | 386 ++++++++ .../src/cool_custom_value.rs | 2 +- 24 files changed, 2378 insertions(+), 385 deletions(-) create mode 100644 crates/nu-derive-value/Cargo.toml create mode 100644 crates/nu-derive-value/LICENSE create mode 100644 crates/nu-derive-value/src/attributes.rs create mode 100644 crates/nu-derive-value/src/error.rs create mode 100644 crates/nu-derive-value/src/from.rs create mode 100644 crates/nu-derive-value/src/into.rs create mode 100644 crates/nu-derive-value/src/lib.rs create mode 100644 crates/nu-derive-value/src/tests.rs create mode 100644 crates/nu-protocol/src/value/into_value.rs create mode 100644 crates/nu-protocol/src/value/test_derive.rs diff --git a/Cargo.lock b/Cargo.lock index e0c8b048ab..b88af73c25 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -920,6 +920,15 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "convert_case" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec182b0ca2f35d8fc196cf3404988fd8b8c739a4d270ff118a398feb0cbec1ca" +dependencies = [ + "unicode-segmentation", +] + [[package]] name = "core-foundation" version = "0.9.4" @@ -3020,6 +3029,17 @@ dependencies = [ "winreg", ] +[[package]] +name = "nu-derive-value" +version = "0.94.3" +dependencies = [ + "convert_case", + "proc-macro-error", + "proc-macro2", + "quote", + "syn 2.0.60", +] + [[package]] name = "nu-engine" version = "0.94.3" @@ -3209,11 +3229,13 @@ dependencies = [ "byte-unit", "chrono", "chrono-humanize", + "convert_case", "fancy-regex", "indexmap", "lru", "miette", "nix", + "nu-derive-value", "nu-path", "nu-system", "nu-test-support", diff --git a/Cargo.toml b/Cargo.toml index d59d7cdbff..18f45987e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ members = [ "crates/nu-lsp", "crates/nu-pretty-hex", "crates/nu-protocol", + "crates/nu-derive-value", "crates/nu-plugin", "crates/nu-plugin-core", "crates/nu-plugin-engine", @@ -74,6 +75,7 @@ chardetng = "0.1.17" chrono = { default-features = false, version = "0.4.34" } chrono-humanize = "0.2.3" chrono-tz = "0.8" +convert_case = "0.6" crossbeam-channel = "0.5.8" crossterm = "0.27" csv = "1.3" @@ -123,11 +125,14 @@ pathdiff = "0.2" percent-encoding = "2" pretty_assertions = "1.4" print-positions = "0.6" +proc-macro-error = { version = "1.0", default-features = false } +proc-macro2 = "1.0" procfs = "0.16.0" pwd = "1.3" quick-xml = "0.31.0" quickcheck = "1.0" quickcheck_macros = "1.0" +quote = "1.0" rand = "0.8" ratatui = "0.26" rayon = "1.10" @@ -147,6 +152,7 @@ serde_urlencoded = "0.7.1" serde_yaml = "0.9" sha2 = "0.10" strip-ansi-escapes = "0.2.0" +syn = "2.0" sysinfo = "0.30" tabled = { version = "0.14.0", default-features = false } tempfile = "3.10" diff --git a/crates/nu-cmd-base/src/hook.rs b/crates/nu-cmd-base/src/hook.rs index 76c13bd5c3..cef5348618 100644 --- a/crates/nu-cmd-base/src/hook.rs +++ b/crates/nu-cmd-base/src/hook.rs @@ -194,7 +194,7 @@ pub fn eval_hook( let Some(follow) = val.get("code") else { return Err(ShellError::CantFindColumn { col_name: "code".into(), - span, + span: Some(span), src_span: span, }); }; diff --git a/crates/nu-command/src/charting/histogram.rs b/crates/nu-command/src/charting/histogram.rs index 52964b087d..29515c96c5 100755 --- a/crates/nu-command/src/charting/histogram.rs +++ b/crates/nu-command/src/charting/histogram.rs @@ -194,7 +194,7 @@ fn run_histogram( if inputs.is_empty() { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: head_span, + span: Some(head_span), src_span: list_span, }); } diff --git a/crates/nu-command/src/conversions/into/cell_path.rs b/crates/nu-command/src/conversions/into/cell_path.rs index c05dad57ba..1342fb14c0 100644 --- a/crates/nu-command/src/conversions/into/cell_path.rs +++ b/crates/nu-command/src/conversions/into/cell_path.rs @@ -154,7 +154,7 @@ fn record_to_path_member( let Some(value) = record.get("value") else { return Err(ShellError::CantFindColumn { col_name: "value".into(), - span: val_span, + span: Some(val_span), src_span: span, }); }; diff --git a/crates/nu-command/src/filters/split_by.rs b/crates/nu-command/src/filters/split_by.rs index 0d3bf1cd30..419119fe0c 100644 --- a/crates/nu-command/src/filters/split_by.rs +++ b/crates/nu-command/src/filters/split_by.rs @@ -130,7 +130,7 @@ pub fn split( Some(group_key) => Ok(group_key.coerce_string()?), None => Err(ShellError::CantFindColumn { col_name: column_name.item.to_string(), - span: column_name.span, + span: Some(column_name.span), src_span: row.span(), }), } diff --git a/crates/nu-command/src/filters/uniq_by.rs b/crates/nu-command/src/filters/uniq_by.rs index 7bbeb0afe2..e1a502b06b 100644 --- a/crates/nu-command/src/filters/uniq_by.rs +++ b/crates/nu-command/src/filters/uniq_by.rs @@ -123,7 +123,7 @@ fn validate(vec: &[Value], columns: &[String], span: Span) -> Result<(), ShellEr if let Some(nonexistent) = nonexistent_column(columns, record.columns()) { return Err(ShellError::CantFindColumn { col_name: nonexistent, - span, + span: Some(span), src_span: val_span, }); } diff --git a/crates/nu-command/src/sort_utils.rs b/crates/nu-command/src/sort_utils.rs index 50d8256353..01bb604eac 100644 --- a/crates/nu-command/src/sort_utils.rs +++ b/crates/nu-command/src/sort_utils.rs @@ -81,7 +81,7 @@ pub fn sort( if let Some(nonexistent) = nonexistent_column(&sort_columns, record.columns()) { return Err(ShellError::CantFindColumn { col_name: nonexistent, - span, + span: Some(span), src_span: val_span, }); } diff --git a/crates/nu-derive-value/Cargo.toml b/crates/nu-derive-value/Cargo.toml new file mode 100644 index 0000000000..45395b2ddb --- /dev/null +++ b/crates/nu-derive-value/Cargo.toml @@ -0,0 +1,21 @@ +[package] +authors = ["The Nushell Project Developers"] +description = "Macros implementation of #[derive(FromValue, IntoValue)]" +edition = "2021" +license = "MIT" +name = "nu-derive-value" +repository = "https://github.com/nushell/nushell/tree/main/crates/nu-derive-value" +version = "0.94.3" + +[lib] +proc-macro = true +# we can only use exposed macros in doctests really, +# so we cannot test anything useful in a doctest +doctest = false + +[dependencies] +proc-macro2 = { workspace = true } +syn = { workspace = true } +quote = { workspace = true } +proc-macro-error = { workspace = true } +convert_case = { workspace = true } diff --git a/crates/nu-derive-value/LICENSE b/crates/nu-derive-value/LICENSE new file mode 100644 index 0000000000..ae174e8595 --- /dev/null +++ b/crates/nu-derive-value/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2019 - 2023 The Nushell Project Developers + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/crates/nu-derive-value/src/attributes.rs b/crates/nu-derive-value/src/attributes.rs new file mode 100644 index 0000000000..6969b64287 --- /dev/null +++ b/crates/nu-derive-value/src/attributes.rs @@ -0,0 +1,116 @@ +use convert_case::Case; +use syn::{spanned::Spanned, Attribute, Fields, LitStr}; + +use crate::{error::DeriveError, HELPER_ATTRIBUTE}; + +#[derive(Debug)] +pub struct ContainerAttributes { + pub rename_all: Case, +} + +impl Default for ContainerAttributes { + fn default() -> Self { + Self { + rename_all: Case::Snake, + } + } +} + +impl ContainerAttributes { + pub fn parse_attrs<'a, M>( + iter: impl Iterator, + ) -> Result> { + let mut container_attrs = ContainerAttributes::default(); + for attr in filter(iter) { + // This is a container to allow returning derive errors inside the parse_nested_meta fn. + let mut err = Ok(()); + + attr.parse_nested_meta(|meta| { + let ident = meta.path.require_ident()?; + match ident.to_string().as_str() { + "rename_all" => { + // The matched case are all useful variants from `convert_case` with aliases + // that `serde` uses. + let case: LitStr = meta.value()?.parse()?; + let case = match case.value().as_str() { + "UPPER CASE" | "UPPER WITH SPACES CASE" => Case::Upper, + "lower case" | "lower with spaces case" => Case::Lower, + "Title Case" => Case::Title, + "camelCase" => Case::Camel, + "PascalCase" | "UpperCamelCase" => Case::Pascal, + "snake_case" => Case::Snake, + "UPPER_SNAKE_CASE" | "SCREAMING_SNAKE_CASE" => Case::UpperSnake, + "kebab-case" => Case::Kebab, + "COBOL-CASE" | "UPPER-KEBAB-CASE" | "SCREAMING-KEBAB-CASE" => { + Case::Cobol + } + "Train-Case" => Case::Train, + "flatcase" | "lowercase" => Case::Flat, + "UPPERFLATCASE" | "UPPERCASE" => Case::UpperFlat, + // Although very funny, we don't support `Case::{Toggle, Alternating}`, + // as we see no real benefit. + c => { + err = Err(DeriveError::InvalidAttributeValue { + value_span: case.span(), + value: Box::new(c.to_string()), + }); + return Ok(()); // We stored the err in `err`. + } + }; + container_attrs.rename_all = case; + } + ident => { + err = Err(DeriveError::UnexpectedAttribute { + meta_span: ident.span(), + }); + } + } + + Ok(()) + }) + .map_err(DeriveError::Syn)?; + + err?; // Shortcircuit here if `err` is holding some error. + } + + Ok(container_attrs) + } +} + +pub fn filter<'a>( + iter: impl Iterator, +) -> impl Iterator { + iter.filter(|attr| attr.path().is_ident(HELPER_ATTRIBUTE)) +} + +// The deny functions are built to easily deny the use of the helper attribute if used incorrectly. +// As the usage of it gets more complex, these functions might be discarded or replaced. + +/// Deny any attribute that uses the helper attribute. +pub fn deny(attrs: &[Attribute]) -> Result<(), DeriveError> { + match filter(attrs.iter()).next() { + Some(attr) => Err(DeriveError::InvalidAttributePosition { + attribute_span: attr.span(), + }), + None => Ok(()), + } +} + +/// Deny any attributes that uses the helper attribute on any field. +pub fn deny_fields(fields: &Fields) -> Result<(), DeriveError> { + match fields { + Fields::Named(fields) => { + for field in fields.named.iter() { + deny(&field.attrs)?; + } + } + Fields::Unnamed(fields) => { + for field in fields.unnamed.iter() { + deny(&field.attrs)?; + } + } + Fields::Unit => (), + } + + Ok(()) +} diff --git a/crates/nu-derive-value/src/error.rs b/crates/nu-derive-value/src/error.rs new file mode 100644 index 0000000000..b7fd3e2f91 --- /dev/null +++ b/crates/nu-derive-value/src/error.rs @@ -0,0 +1,82 @@ +use std::{any, fmt::Debug, marker::PhantomData}; + +use proc_macro2::Span; +use proc_macro_error::{Diagnostic, Level}; + +#[derive(Debug)] +pub enum DeriveError { + /// Marker variant, makes the `M` generic parameter valid. + _Marker(PhantomData), + + /// Parsing errors thrown by `syn`. + Syn(syn::parse::Error), + + /// `syn::DeriveInput` was a union, currently not supported + UnsupportedUnions, + + /// Only plain enums are supported right now. + UnsupportedEnums { fields_span: Span }, + + /// Found a `#[nu_value(x)]` attribute where `x` is unexpected. + UnexpectedAttribute { meta_span: Span }, + + /// Found a `#[nu_value(x)]` attribute at a invalid position. + InvalidAttributePosition { attribute_span: Span }, + + /// Found a valid `#[nu_value(x)]` attribute but the passed values is invalid. + InvalidAttributeValue { + value_span: Span, + value: Box, + }, +} + +impl From> for Diagnostic { + fn from(value: DeriveError) -> Self { + let derive_name = any::type_name::().split("::").last().expect("not empty"); + match value { + DeriveError::_Marker(_) => panic!("used marker variant"), + + DeriveError::Syn(e) => Diagnostic::spanned(e.span(), Level::Error, e.to_string()), + + DeriveError::UnsupportedUnions => Diagnostic::new( + Level::Error, + format!("`{derive_name}` cannot be derived from unions"), + ) + .help("consider refactoring to a struct".to_string()) + .note("if you really need a union, consider opening an issue on Github".to_string()), + + DeriveError::UnsupportedEnums { fields_span } => Diagnostic::spanned( + fields_span, + Level::Error, + format!("`{derive_name}` can only be derived from plain enums"), + ) + .help( + "consider refactoring your data type to a struct with a plain enum as a field" + .to_string(), + ) + .note("more complex enums could be implemented in the future".to_string()), + + DeriveError::InvalidAttributePosition { attribute_span } => Diagnostic::spanned( + attribute_span, + Level::Error, + "invalid attribute position".to_string(), + ) + .help(format!( + "check documentation for `{derive_name}` for valid placements" + )), + + DeriveError::UnexpectedAttribute { meta_span } => { + Diagnostic::spanned(meta_span, Level::Error, "unknown attribute".to_string()).help( + format!("check documentation for `{derive_name}` for valid attributes"), + ) + } + + DeriveError::InvalidAttributeValue { value_span, value } => { + Diagnostic::spanned(value_span, Level::Error, format!("invalid value {value:?}")) + .help(format!( + "check documentation for `{derive_name}` for valid attribute values" + )) + } + } + } +} diff --git a/crates/nu-derive-value/src/from.rs b/crates/nu-derive-value/src/from.rs new file mode 100644 index 0000000000..033026c149 --- /dev/null +++ b/crates/nu-derive-value/src/from.rs @@ -0,0 +1,539 @@ +use convert_case::Casing; +use proc_macro2::TokenStream as TokenStream2; +use quote::{quote, ToTokens}; +use syn::{ + spanned::Spanned, Attribute, Data, DataEnum, DataStruct, DeriveInput, Fields, Generics, Ident, +}; + +use crate::attributes::{self, ContainerAttributes}; + +#[derive(Debug)] +pub struct FromValue; +type DeriveError = super::error::DeriveError; + +/// Inner implementation of the `#[derive(FromValue)]` macro for structs and enums. +/// +/// Uses `proc_macro2::TokenStream` for better testing support, unlike `proc_macro::TokenStream`. +/// +/// This function directs the `FromValue` trait derivation to the correct implementation based on +/// the input type: +/// - For structs: [`derive_struct_from_value`] +/// - For enums: [`derive_enum_from_value`] +/// - Unions are not supported and will return an error. +pub fn derive_from_value(input: TokenStream2) -> Result { + let input: DeriveInput = syn::parse2(input).map_err(DeriveError::Syn)?; + match input.data { + Data::Struct(data_struct) => Ok(derive_struct_from_value( + input.ident, + data_struct, + input.generics, + input.attrs, + )?), + Data::Enum(data_enum) => Ok(derive_enum_from_value( + input.ident, + data_enum, + input.generics, + input.attrs, + )?), + Data::Union(_) => Err(DeriveError::UnsupportedUnions), + } +} + +/// Implements the `#[derive(FromValue)]` macro for structs. +/// +/// This function ensures that the helper attribute is not used anywhere, as it is not supported for +/// structs. +/// Other than this, this function provides the impl signature for `FromValue`. +/// The implementation for `FromValue::from_value` is handled by [`struct_from_value`] and the +/// `FromValue::expected_type` is handled by [`struct_expected_type`]. +fn derive_struct_from_value( + ident: Ident, + data: DataStruct, + generics: Generics, + attrs: Vec, +) -> Result { + attributes::deny(&attrs)?; + attributes::deny_fields(&data.fields)?; + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let from_value_impl = struct_from_value(&data); + let expected_type_impl = struct_expected_type(&data.fields); + Ok(quote! { + #[automatically_derived] + impl #impl_generics nu_protocol::FromValue for #ident #ty_generics #where_clause { + #from_value_impl + #expected_type_impl + } + }) +} + +/// Implements `FromValue::from_value` for structs. +/// +/// This function constructs the `from_value` function for structs. +/// The implementation is straightforward as most of the heavy lifting is handled by +/// `parse_value_via_fields`, and this function only needs to construct the signature around it. +/// +/// For structs with named fields, this constructs a large return type where each field +/// contains the implementation for that specific field. +/// In structs with unnamed fields, a [`VecDeque`](std::collections::VecDeque) is used to load each +/// field one after another, and the result is used to construct the tuple. +/// For unit structs, this only checks if the input value is `Value::Nothing`. +/// +/// # Examples +/// +/// These examples show what the macro would generate. +/// +/// Struct with named fields: +/// ```rust +/// #[derive(IntoValue)] +/// struct Pet { +/// name: String, +/// age: u8, +/// favorite_toy: Option, +/// } +/// +/// impl nu_protocol::FromValue for Pet { +/// fn from_value( +/// v: nu_protocol::Value +/// ) -> std::result::Result { +/// let span = v.span(); +/// let mut record = v.into_record()?; +/// std::result::Result::Ok(Pet { +/// name: ::from_value( +/// record +/// .remove("name") +/// .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { +/// col_name: std::string::ToString::to_string("name"), +/// span: std::option::Option::None, +/// src_span: span +/// })?, +/// )?, +/// age: ::from_value( +/// record +/// .remove("age") +/// .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { +/// col_name: std::string::ToString::to_string("age"), +/// span: std::option::Option::None, +/// src_span: span +/// })?, +/// )?, +/// favorite_toy: as nu_protocol::FromValue>::from_value( +/// record +/// .remove("favorite_toy") +/// .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { +/// col_name: std::string::ToString::to_string("favorite_toy"), +/// span: std::option::Option::None, +/// src_span: span +/// })?, +/// )?, +/// }) +/// } +/// } +/// ``` +/// +/// Struct with unnamed fields: +/// ```rust +/// #[derive(IntoValue)] +/// struct Color(u8, u8, u8); +/// +/// impl nu_protocol::FromValue for Color { +/// fn from_value( +/// v: nu_protocol::Value +/// ) -> std::result::Result { +/// let span = v.span(); +/// let list = v.into_list()?; +/// let mut deque: std::collections::VecDeque<_> = std::convert::From::from(list); +/// std::result::Result::Ok(Self( +/// { +/// ::from_value( +/// deque +/// .pop_front() +/// .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { +/// col_name: std::string::ToString::to_string(&0), +/// span: std::option::Option::None, +/// src_span: span +/// })?, +/// )? +/// }, +/// { +/// ::from_value( +/// deque +/// .pop_front() +/// .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { +/// col_name: std::string::ToString::to_string(&1), +/// span: std::option::Option::None, +/// src_span: span +/// })?, +/// )? +/// }, +/// { +/// ::from_value( +/// deque +/// .pop_front() +/// .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { +/// col_name: std::string::ToString::to_string(&2), +/// span: std::option::Option::None, +/// src_span: span +/// })?, +/// )? +/// } +/// )) +/// } +/// } +/// ``` +/// +/// Unit struct: +/// ```rust +/// #[derive(IntoValue)] +/// struct Unicorn; +/// +/// impl nu_protocol::FromValue for Unicorn { +/// fn from_value( +/// v: nu_protocol::Value +/// ) -> std::result::Result { +/// match v { +/// nu_protocol::Value::Nothing {..} => Ok(Self), +/// v => std::result::Result::Err(nu_protocol::ShellError::CantConvert { +/// to_type: std::string::ToString::to_string(&::expected_type()), +/// from_type: std::string::ToString::to_string(&v.get_type()), +/// span: v.span(), +/// help: std::option::Option::None +/// }) +/// } +/// } +/// } +/// ``` +fn struct_from_value(data: &DataStruct) -> TokenStream2 { + let body = parse_value_via_fields(&data.fields, quote!(Self)); + quote! { + fn from_value( + v: nu_protocol::Value + ) -> std::result::Result { + #body + } + } +} + +/// Implements `FromValue::expected_type` for structs. +/// +/// This function constructs the `expected_type` function for structs. +/// The type depends on the `fields`: named fields construct a record type with every key and type +/// laid out. +/// Unnamed fields construct a custom type with the name in the format like +/// `list[type0, type1, type2]`. +/// No fields expect the `Type::Nothing`. +/// +/// # Examples +/// +/// These examples show what the macro would generate. +/// +/// Struct with named fields: +/// ```rust +/// #[derive(IntoValue)] +/// struct Pet { +/// name: String, +/// age: u8, +/// favorite_toy: Option, +/// } +/// +/// impl nu_protocol::FromValue for Pet { +/// fn expected_type() -> nu_protocol::Type { +/// nu_protocol::Type::Record( +/// std::vec![ +/// ( +/// std::string::ToString::to_string("name"), +/// ::expected_type(), +/// ), +/// ( +/// std::string::ToString::to_string("age"), +/// ::expected_type(), +/// ), +/// ( +/// std::string::ToString::to_string("favorite_toy"), +/// as nu_protocol::FromValue>::expected_type(), +/// ) +/// ].into_boxed_slice() +/// ) +/// } +/// } +/// ``` +/// +/// Struct with unnamed fields: +/// ```rust +/// #[derive(IntoValue)] +/// struct Color(u8, u8, u8); +/// +/// impl nu_protocol::FromValue for Color { +/// fn expected_type() -> nu_protocol::Type { +/// nu_protocol::Type::Custom( +/// std::format!( +/// "[{}, {}, {}]", +/// ::expected_type(), +/// ::expected_type(), +/// ::expected_type() +/// ) +/// .into_boxed_str() +/// ) +/// } +/// } +/// ``` +/// +/// Unit struct: +/// ```rust +/// #[derive(IntoValue)] +/// struct Unicorn; +/// +/// impl nu_protocol::FromValue for Color { +/// fn expected_type() -> nu_protocol::Type { +/// nu_protocol::Type::Nothing +/// } +/// } +/// ``` +fn struct_expected_type(fields: &Fields) -> TokenStream2 { + let ty = match fields { + Fields::Named(fields) => { + let fields = fields.named.iter().map(|field| { + let ident = field.ident.as_ref().expect("named has idents"); + let ident_s = ident.to_string(); + let ty = &field.ty; + quote! {( + std::string::ToString::to_string(#ident_s), + <#ty as nu_protocol::FromValue>::expected_type(), + )} + }); + quote!(nu_protocol::Type::Record( + std::vec![#(#fields),*].into_boxed_slice() + )) + } + Fields::Unnamed(fields) => { + let mut iter = fields.unnamed.iter(); + let fields = fields.unnamed.iter().map(|field| { + let ty = &field.ty; + quote!(<#ty as nu_protocol::FromValue>::expected_type()) + }); + let mut template = String::new(); + template.push('['); + if iter.next().is_some() { + template.push_str("{}") + } + iter.for_each(|_| template.push_str(", {}")); + template.push(']'); + quote! { + nu_protocol::Type::Custom( + std::format!( + #template, + #(#fields),* + ) + .into_boxed_str() + ) + } + } + Fields::Unit => quote!(nu_protocol::Type::Nothing), + }; + + quote! { + fn expected_type() -> nu_protocol::Type { + #ty + } + } +} + +/// Implements the `#[derive(FromValue)]` macro for enums. +/// +/// This function constructs the implementation of the `FromValue` trait for enums. +/// It is designed to be on the same level as [`derive_struct_from_value`], even though this +/// function only provides the impl signature for `FromValue`. +/// The actual implementation for `FromValue::from_value` is handled by [`enum_from_value`]. +/// +/// Since variants are difficult to type with the current type system, this function uses the +/// default implementation for `expected_type`. +fn derive_enum_from_value( + ident: Ident, + data: DataEnum, + generics: Generics, + attrs: Vec, +) -> Result { + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let from_value_impl = enum_from_value(&data, &attrs)?; + Ok(quote! { + #[automatically_derived] + impl #impl_generics nu_protocol::FromValue for #ident #ty_generics #where_clause { + #from_value_impl + } + }) +} + +/// Implements `FromValue::from_value` for enums. +/// +/// This function constructs the `from_value` implementation for enums. +/// It only accepts enums with unit variants, as it is currently unclear how other types of enums +/// should be represented via a `Value`. +/// This function checks that every field is a unit variant and constructs a match statement over +/// all possible variants. +/// The input value is expected to be a `Value::String` containing the name of the variant formatted +/// as defined by the `#[nu_value(rename_all = "...")]` attribute. +/// If no attribute is given, [`convert_case::Case::Snake`] is expected. +/// +/// If no matching variant is found, `ShellError::CantConvert` is returned. +/// +/// This is how such a derived implementation looks: +/// ```rust +/// #[derive(IntoValue)] +/// enum Weather { +/// Sunny, +/// Cloudy, +/// Raining +/// } +/// +/// impl nu_protocol::IntoValue for Weather { +/// fn into_value(self, span: nu_protocol::Span) -> nu_protocol::Value { +/// let span = v.span(); +/// let ty = v.get_type(); +/// +/// let s = v.into_string()?; +/// match s.as_str() { +/// "sunny" => std::result::Ok(Self::Sunny), +/// "cloudy" => std::result::Ok(Self::Cloudy), +/// "raining" => std::result::Ok(Self::Raining), +/// _ => std::result::Result::Err(nu_protocol::ShellError::CantConvert { +/// to_type: std::string::ToString::to_string( +/// &::expected_type() +/// ), +/// from_type: std::string::ToString::to_string(&ty), +/// span: span,help: std::option::Option::None, +/// }), +/// } +/// } +/// } +/// ``` +fn enum_from_value(data: &DataEnum, attrs: &[Attribute]) -> Result { + let container_attrs = ContainerAttributes::parse_attrs(attrs.iter())?; + let arms: Vec = data + .variants + .iter() + .map(|variant| { + attributes::deny(&variant.attrs)?; + let ident = &variant.ident; + let ident_s = format!("{ident}") + .as_str() + .to_case(container_attrs.rename_all); + match &variant.fields { + Fields::Named(fields) => Err(DeriveError::UnsupportedEnums { + fields_span: fields.span(), + }), + Fields::Unnamed(fields) => Err(DeriveError::UnsupportedEnums { + fields_span: fields.span(), + }), + Fields::Unit => Ok(quote!(#ident_s => std::result::Result::Ok(Self::#ident))), + } + }) + .collect::>()?; + + Ok(quote! { + fn from_value( + v: nu_protocol::Value + ) -> std::result::Result { + let span = v.span(); + let ty = v.get_type(); + + let s = v.into_string()?; + match s.as_str() { + #(#arms,)* + _ => std::result::Result::Err(nu_protocol::ShellError::CantConvert { + to_type: std::string::ToString::to_string( + &::expected_type() + ), + from_type: std::string::ToString::to_string(&ty), + span: span, + help: std::option::Option::None, + }), + } + } + }) +} + +/// Parses a `Value` into self. +/// +/// This function handles the actual parsing of a `Value` into self. +/// It takes two parameters: `fields` and `self_ident`. +/// The `fields` parameter determines the expected type of `Value`: named fields expect a +/// `Value::Record`, unnamed fields expect a `Value::List`, and a unit expects `Value::Nothing`. +/// +/// For named fields, the `fields` parameter indicates which field in the record corresponds to +/// which struct field. +/// For both named and unnamed fields, it also helps cast the type into a `FromValue` type. +/// This approach maintains +/// [hygiene](https://doc.rust-lang.org/reference/macros-by-example.html#hygiene). +/// +/// The `self_ident` parameter is used to describe the identifier of the returned value. +/// For structs, `Self` is usually sufficient, but for enums, `Self::Variant` may be needed in the +/// future. +/// +/// This function is more complex than the equivalent for `IntoValue` due to error handling +/// requirements. +/// For missing fields, `ShellError::CantFindColumn` is used, and for unit structs, +/// `ShellError::CantConvert` is used. +/// The implementation avoids local variables for fields to prevent accidental shadowing, ensuring +/// that poorly named fields don't cause issues. +/// While this style is not typically recommended in handwritten Rust, it is acceptable for code +/// generation. +fn parse_value_via_fields(fields: &Fields, self_ident: impl ToTokens) -> TokenStream2 { + match fields { + Fields::Named(fields) => { + let fields = fields.named.iter().map(|field| { + // TODO: handle missing fields for Options as None + let ident = field.ident.as_ref().expect("named has idents"); + let ident_s = ident.to_string(); + let ty = &field.ty; + quote! { + #ident: <#ty as nu_protocol::FromValue>::from_value( + record + .remove(#ident_s) + .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { + col_name: std::string::ToString::to_string(#ident_s), + span: std::option::Option::None, + src_span: span + })?, + )? + } + }); + quote! { + let span = v.span(); + let mut record = v.into_record()?; + std::result::Result::Ok(#self_ident {#(#fields),*}) + } + } + Fields::Unnamed(fields) => { + let fields = fields.unnamed.iter().enumerate().map(|(i, field)| { + let ty = &field.ty; + quote! {{ + <#ty as nu_protocol::FromValue>::from_value( + deque + .pop_front() + .ok_or_else(|| nu_protocol::ShellError::CantFindColumn { + col_name: std::string::ToString::to_string(&#i), + span: std::option::Option::None, + src_span: span + })?, + )? + }} + }); + quote! { + let span = v.span(); + let list = v.into_list()?; + let mut deque: std::collections::VecDeque<_> = std::convert::From::from(list); + std::result::Result::Ok(#self_ident(#(#fields),*)) + } + } + Fields::Unit => quote! { + match v { + nu_protocol::Value::Nothing {..} => Ok(#self_ident), + v => std::result::Result::Err(nu_protocol::ShellError::CantConvert { + to_type: std::string::ToString::to_string(&::expected_type()), + from_type: std::string::ToString::to_string(&v.get_type()), + span: v.span(), + help: std::option::Option::None + }) + } + }, + } +} diff --git a/crates/nu-derive-value/src/into.rs b/crates/nu-derive-value/src/into.rs new file mode 100644 index 0000000000..a7cec06378 --- /dev/null +++ b/crates/nu-derive-value/src/into.rs @@ -0,0 +1,266 @@ +use convert_case::Casing; +use proc_macro2::TokenStream as TokenStream2; +use quote::{quote, ToTokens}; +use syn::{ + spanned::Spanned, Attribute, Data, DataEnum, DataStruct, DeriveInput, Fields, Generics, Ident, + Index, +}; + +use crate::attributes::{self, ContainerAttributes}; + +#[derive(Debug)] +pub struct IntoValue; +type DeriveError = super::error::DeriveError; + +/// Inner implementation of the `#[derive(IntoValue)]` macro for structs and enums. +/// +/// Uses `proc_macro2::TokenStream` for better testing support, unlike `proc_macro::TokenStream`. +/// +/// This function directs the `IntoValue` trait derivation to the correct implementation based on +/// the input type: +/// - For structs: [`struct_into_value`] +/// - For enums: [`enum_into_value`] +/// - Unions are not supported and will return an error. +pub fn derive_into_value(input: TokenStream2) -> Result { + let input: DeriveInput = syn::parse2(input).map_err(DeriveError::Syn)?; + match input.data { + Data::Struct(data_struct) => Ok(struct_into_value( + input.ident, + data_struct, + input.generics, + input.attrs, + )?), + Data::Enum(data_enum) => Ok(enum_into_value( + input.ident, + data_enum, + input.generics, + input.attrs, + )?), + Data::Union(_) => Err(DeriveError::UnsupportedUnions), + } +} + +/// Implements the `#[derive(IntoValue)]` macro for structs. +/// +/// Automatically derives the `IntoValue` trait for any struct where each field implements +/// `IntoValue`. +/// For structs with named fields, the derived implementation creates a `Value::Record` using the +/// struct fields as keys. +/// Each field value is converted using the `IntoValue::into_value` method. +/// For structs with unnamed fields, this generates a `Value::List` with each field in the list. +/// For unit structs, this generates `Value::Nothing`, because there is no data. +/// +/// Note: The helper attribute `#[nu_value(...)]` is currently not allowed on structs. +/// +/// # Examples +/// +/// These examples show what the macro would generate. +/// +/// Struct with named fields: +/// ```rust +/// #[derive(IntoValue)] +/// struct Pet { +/// name: String, +/// age: u8, +/// favorite_toy: Option, +/// } +/// +/// impl nu_protocol::IntoValue for Pet { +/// fn into_value(self, span: nu_protocol::Span) -> nu_protocol::Value { +/// nu_protocol::Value::record(nu_protocol::record! { +/// "name" => nu_protocol::IntoValue::into_value(self.name, span), +/// "age" => nu_protocol::IntoValue::into_value(self.age, span), +/// "favorite_toy" => nu_protocol::IntoValue::into_value(self.favorite_toy, span), +/// }, span) +/// } +/// } +/// ``` +/// +/// Struct with unnamed fields: +/// ```rust +/// #[derive(IntoValue)] +/// struct Color(u8, u8, u8); +/// +/// impl nu_protocol::IntoValue for Color { +/// fn into_value(self, span: nu_protocol::Span) -> nu_protocol::Value { +/// nu_protocol::Value::list(vec![ +/// nu_protocol::IntoValue::into_value(self.0, span), +/// nu_protocol::IntoValue::into_value(self.1, span), +/// nu_protocol::IntoValue::into_value(self.2, span), +/// ], span) +/// } +/// } +/// ``` +/// +/// Unit struct: +/// ```rust +/// #[derive(IntoValue)] +/// struct Unicorn; +/// +/// impl nu_protocol::IntoValue for Unicorn { +/// fn into_value(self, span: nu_protocol::Span) -> nu_protocol::Value { +/// nu_protocol::Value::nothing(span) +/// } +/// } +/// ``` +fn struct_into_value( + ident: Ident, + data: DataStruct, + generics: Generics, + attrs: Vec, +) -> Result { + attributes::deny(&attrs)?; + attributes::deny_fields(&data.fields)?; + let record = match &data.fields { + Fields::Named(fields) => { + let accessor = fields + .named + .iter() + .map(|field| field.ident.as_ref().expect("named has idents")) + .map(|ident| quote!(self.#ident)); + fields_return_value(&data.fields, accessor) + } + Fields::Unnamed(fields) => { + let accessor = fields + .unnamed + .iter() + .enumerate() + .map(|(n, _)| Index::from(n)) + .map(|index| quote!(self.#index)); + fields_return_value(&data.fields, accessor) + } + Fields::Unit => quote!(nu_protocol::Value::nothing(span)), + }; + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + Ok(quote! { + #[automatically_derived] + impl #impl_generics nu_protocol::IntoValue for #ident #ty_generics #where_clause { + fn into_value(self, span: nu_protocol::Span) -> nu_protocol::Value { + #record + } + } + }) +} + +/// Implements the `#[derive(IntoValue)]` macro for enums. +/// +/// This function implements the derive macro `IntoValue` for enums. +/// Currently, only unit enum variants are supported as it is not clear how other types of enums +/// should be represented in a `Value`. +/// For simple enums, we represent the enum as a `Value::String`. For other types of variants, we return an error. +/// The variant name will be case-converted as described by the `#[nu_value(rename_all = "...")]` helper attribute. +/// If no attribute is used, the default is `case_convert::Case::Snake`. +/// The implementation matches over all variants, uses the appropriate variant name, and constructs a `Value::String`. +/// +/// This is how such a derived implementation looks: +/// ```rust +/// #[derive(IntoValue)] +/// enum Weather { +/// Sunny, +/// Cloudy, +/// Raining +/// } +/// +/// impl nu_protocol::IntoValue for Weather { +/// fn into_value(self, span: nu_protocol::Span) -> nu_protocol::Value { +/// match self { +/// Self::Sunny => nu_protocol::Value::string("sunny", span), +/// Self::Cloudy => nu_protocol::Value::string("cloudy", span), +/// Self::Raining => nu_protocol::Value::string("raining", span), +/// } +/// } +/// } +/// ``` +fn enum_into_value( + ident: Ident, + data: DataEnum, + generics: Generics, + attrs: Vec, +) -> Result { + let container_attrs = ContainerAttributes::parse_attrs(attrs.iter())?; + let arms: Vec = data + .variants + .into_iter() + .map(|variant| { + attributes::deny(&variant.attrs)?; + let ident = variant.ident; + let ident_s = format!("{ident}") + .as_str() + .to_case(container_attrs.rename_all); + match &variant.fields { + // In the future we can implement more complexe enums here. + Fields::Named(fields) => Err(DeriveError::UnsupportedEnums { + fields_span: fields.span(), + }), + Fields::Unnamed(fields) => Err(DeriveError::UnsupportedEnums { + fields_span: fields.span(), + }), + Fields::Unit => { + Ok(quote!(Self::#ident => nu_protocol::Value::string(#ident_s, span))) + } + } + }) + .collect::>()?; + + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + Ok(quote! { + impl #impl_generics nu_protocol::IntoValue for #ident #ty_generics #where_clause { + fn into_value(self, span: nu_protocol::Span) -> nu_protocol::Value { + match self { + #(#arms,)* + } + } + } + }) +} + +/// Constructs the final `Value` that the macro generates. +/// +/// This function handles the construction of the final `Value` that the macro generates. +/// It is currently only used for structs but may be used for enums in the future. +/// The function takes two parameters: the `fields`, which allow iterating over each field of a data +/// type, and the `accessor`. +/// The fields determine whether we need to generate a `Value::Record`, `Value::List`, or +/// `Value::Nothing`. +/// For named fields, they are also directly used to generate the record key. +/// +/// The `accessor` parameter generalizes how the data is accessed. +/// For named fields, this is usually the name of the fields preceded by `self` in a struct, and +/// maybe something else for enums. +/// For unnamed fields, this should be an iterator similar to the one with named fields, but +/// accessing tuple fields, so we get `self.n`. +/// For unit structs, this parameter is ignored. +/// By using the accessor like this, we can have the same code for structs and enums with data +/// variants in the future. +fn fields_return_value( + fields: &Fields, + accessor: impl Iterator, +) -> TokenStream2 { + match fields { + Fields::Named(fields) => { + let items: Vec = fields + .named + .iter() + .zip(accessor) + .map(|(field, accessor)| { + let ident = field.ident.as_ref().expect("named has idents"); + let field = ident.to_string(); + quote!(#field => nu_protocol::IntoValue::into_value(#accessor, span)) + }) + .collect(); + quote! { + nu_protocol::Value::record(nu_protocol::record! { + #(#items),* + }, span) + } + } + Fields::Unnamed(fields) => { + let items = + fields.unnamed.iter().zip(accessor).map( + |(_, accessor)| quote!(nu_protocol::IntoValue::into_value(#accessor, span)), + ); + quote!(nu_protocol::Value::list(std::vec![#(#items),*], span)) + } + Fields::Unit => quote!(nu_protocol::Value::nothing(span)), + } +} diff --git a/crates/nu-derive-value/src/lib.rs b/crates/nu-derive-value/src/lib.rs new file mode 100644 index 0000000000..269566fc88 --- /dev/null +++ b/crates/nu-derive-value/src/lib.rs @@ -0,0 +1,69 @@ +//! Macro implementations of `#[derive(FromValue, IntoValue)]`. +//! +//! As this crate is a [`proc_macro`] crate, it is only allowed to export +//! [procedural macros](https://doc.rust-lang.org/reference/procedural-macros.html). +//! Therefore, it only exports [`IntoValue`] and [`FromValue`]. +//! +//! To get documentation for other functions and types used in this crate, run +//! `cargo doc -p nu-derive-value --document-private-items`. +//! +//! This crate uses a lot of +//! [`proc_macro2::TokenStream`](https://docs.rs/proc-macro2/1.0.24/proc_macro2/struct.TokenStream.html) +//! as `TokenStream2` to allow testing the behavior of the macros directly, including the output +//! token stream or if the macro errors as expected. +//! The tests for functionality can be found in `nu_protocol::value::test_derive`. +//! +//! This documentation is often less reference-heavy than typical Rust documentation. +//! This is because this crate is a dependency for `nu_protocol`, and linking to it would create a +//! cyclic dependency. +//! Also all examples in the documentation aren't tested as this crate cannot be compiled as a +//! normal library very easily. +//! This might change in the future if cargo allows building a proc-macro crate differently for +//! `cfg(doctest)` as they are already doing for `cfg(test)`. +//! +//! The generated code from the derive macros tries to be as +//! [hygienic](https://doc.rust-lang.org/reference/macros-by-example.html#hygiene) as possible. +//! This ensures that the macro can be called anywhere without requiring specific imports. +//! This results in obtuse code, which isn't recommended for manual, handwritten Rust +//! but ensures that no other code may influence this generated code or vice versa. + +use proc_macro::TokenStream; +use proc_macro2::TokenStream as TokenStream2; +use proc_macro_error::{proc_macro_error, Diagnostic}; + +mod attributes; +mod error; +mod from; +mod into; +#[cfg(test)] +mod tests; + +const HELPER_ATTRIBUTE: &str = "nu_value"; + +/// Derive macro generating an impl of the trait `IntoValue`. +/// +/// For further information, see the docs on the trait itself. +#[proc_macro_derive(IntoValue, attributes(nu_value))] +#[proc_macro_error] +pub fn derive_into_value(input: TokenStream) -> TokenStream { + let input = TokenStream2::from(input); + let output = match into::derive_into_value(input) { + Ok(output) => output, + Err(e) => Diagnostic::from(e).abort(), + }; + TokenStream::from(output) +} + +/// Derive macro generating an impl of the trait `FromValue`. +/// +/// For further information, see the docs on the trait itself. +#[proc_macro_derive(FromValue, attributes(nu_value))] +#[proc_macro_error] +pub fn derive_from_value(input: TokenStream) -> TokenStream { + let input = TokenStream2::from(input); + let output = match from::derive_from_value(input) { + Ok(output) => output, + Err(e) => Diagnostic::from(e).abort(), + }; + TokenStream::from(output) +} diff --git a/crates/nu-derive-value/src/tests.rs b/crates/nu-derive-value/src/tests.rs new file mode 100644 index 0000000000..b94d12a732 --- /dev/null +++ b/crates/nu-derive-value/src/tests.rs @@ -0,0 +1,157 @@ +// These tests only check that the derive macros throw the relevant errors. +// Functionality of the derived types is tested in nu_protocol::value::test_derive. + +use crate::error::DeriveError; +use crate::from::derive_from_value; +use crate::into::derive_into_value; +use quote::quote; + +#[test] +fn unsupported_unions() { + let input = quote! { + #[nu_value] + union SomeUnion { + f1: u32, + f2: f32, + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::UnsupportedUnions)), + "expected `DeriveError::UnsupportedUnions`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::UnsupportedUnions)), + "expected `DeriveError::UnsupportedUnions`, got {:?}", + into_res + ); +} + +#[test] +fn unsupported_enums() { + let input = quote! { + #[nu_value(rename_all = "SCREAMING_SNAKE_CASE")] + enum ComplexEnum { + Unit, + Unnamed(u32, f32), + Named { + u: u32, + f: f32, + } + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::UnsupportedEnums { .. })), + "expected `DeriveError::UnsupportedEnums`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::UnsupportedEnums { .. })), + "expected `DeriveError::UnsupportedEnums`, got {:?}", + into_res + ); +} + +#[test] +fn unexpected_attribute() { + let input = quote! { + #[nu_value(what)] + enum SimpleEnum { + A, + B, + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::UnexpectedAttribute { .. })), + "expected `DeriveError::UnexpectedAttribute`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::UnexpectedAttribute { .. })), + "expected `DeriveError::UnexpectedAttribute`, got {:?}", + into_res + ); +} + +#[test] +fn deny_attribute_on_structs() { + let input = quote! { + #[nu_value] + struct SomeStruct; + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::InvalidAttributePosition { .. })), + "expected `DeriveError::InvalidAttributePosition`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::InvalidAttributePosition { .. })), + "expected `DeriveError::InvalidAttributePosition`, got {:?}", + into_res + ); +} + +#[test] +fn deny_attribute_on_fields() { + let input = quote! { + struct SomeStruct { + #[nu_value] + field: () + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::InvalidAttributePosition { .. })), + "expected `DeriveError::InvalidAttributePosition`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::InvalidAttributePosition { .. })), + "expected `DeriveError::InvalidAttributePosition`, got {:?}", + into_res + ); +} + +#[test] +fn invalid_attribute_value() { + let input = quote! { + #[nu_value(rename_all = "CrazY-CasE")] + enum SimpleEnum { + A, + B + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::InvalidAttributeValue { .. })), + "expected `DeriveError::InvalidAttributeValue`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::InvalidAttributeValue { .. })), + "expected `DeriveError::InvalidAttributeValue`, got {:?}", + into_res + ); +} diff --git a/crates/nu-protocol/Cargo.toml b/crates/nu-protocol/Cargo.toml index de73ba1a2c..849a968eb8 100644 --- a/crates/nu-protocol/Cargo.toml +++ b/crates/nu-protocol/Cargo.toml @@ -16,11 +16,13 @@ bench = false nu-utils = { path = "../nu-utils", version = "0.94.3" } nu-path = { path = "../nu-path", version = "0.94.3" } nu-system = { path = "../nu-system", version = "0.94.3" } +nu-derive-value = { path = "../nu-derive-value", version = "0.94.3" } brotli = { workspace = true, optional = true } byte-unit = { version = "5.1", features = [ "serde" ] } chrono = { workspace = true, features = [ "serde", "std", "unstable-locales" ], default-features = false } chrono-humanize = { workspace = true } +convert_case = { workspace = true } fancy-regex = { workspace = true } indexmap = { workspace = true } lru = { workspace = true } diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index e201915fcf..30752d5c9e 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -557,12 +557,12 @@ pub enum ShellError { /// ## Resolution /// /// Check the spelling of your column name. Did you forget to rename a column somewhere? - #[error("Cannot find column")] + #[error("Cannot find column '{col_name}'")] #[diagnostic(code(nu::shell::column_not_found))] CantFindColumn { col_name: String, #[label = "cannot find column '{col_name}'"] - span: Span, + span: Option, #[label = "value originates here"] src_span: Span, }, diff --git a/crates/nu-protocol/src/lib.rs b/crates/nu-protocol/src/lib.rs index d09186cf46..78e7d40680 100644 --- a/crates/nu-protocol/src/lib.rs +++ b/crates/nu-protocol/src/lib.rs @@ -39,3 +39,5 @@ pub use span::*; pub use syntax_shape::*; pub use ty::*; pub use value::*; + +pub use nu_derive_value::*; diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index 97fb95d482..1033eb03b2 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -1,37 +1,188 @@ use crate::{ ast::{CellPath, PathMember}, engine::Closure, - NuGlob, Range, Record, ShellError, Spanned, Value, + NuGlob, Range, Record, ShellError, Spanned, Type, Value, }; use chrono::{DateTime, FixedOffset}; -use std::path::PathBuf; +use std::{ + any, + cmp::Ordering, + collections::{HashMap, VecDeque}, + path::PathBuf, + str::FromStr, +}; +/// A trait for loading a value from a [`Value`]. +/// +/// # Derivable +/// This trait can be used with `#[derive]`. +/// When derived on structs with named fields, it expects a [`Value::Record`] where each field of +/// the struct maps to a corresponding field in the record. +/// For structs with unnamed fields, it expects a [`Value::List`], and the fields are populated in +/// the order they appear in the list. +/// Unit structs expect a [`Value::Nothing`], as they contain no data. +/// Attempting to convert from a non-matching `Value` type will result in an error. +/// +/// Only enums with no fields may derive this trait. +/// The expected value representation will be the name of the variant as a [`Value::String`]. +/// By default, variant names will be expected in ["snake_case"](convert_case::Case::Snake). +/// You can customize the case conversion using `#[nu_value(rename_all = "kebab-case")]` on the enum. +/// All deterministic and useful case conversions provided by [`convert_case::Case`] are supported +/// by specifying the case name followed by "case". +/// Also all values for +/// [`#[serde(rename_all = "...")]`](https://serde.rs/container-attrs.html#rename_all) are valid +/// here. +/// +/// ``` +/// # use nu_protocol::{FromValue, Value, ShellError}; +/// #[derive(FromValue, Debug, PartialEq)] +/// #[nu_value(rename_all = "COBOL-CASE")] +/// enum Bird { +/// MountainEagle, +/// ForestOwl, +/// RiverDuck, +/// } +/// +/// assert_eq!( +/// Bird::from_value(Value::test_string("RIVER-DUCK")).unwrap(), +/// Bird::RiverDuck +/// ); +/// ``` pub trait FromValue: Sized { + // TODO: instead of ShellError, maybe we could have a FromValueError that implements Into + /// Loads a value from a [`Value`]. + /// + /// This method retrieves a value similarly to how strings are parsed using [`FromStr`]. + /// The operation might fail if the `Value` contains unexpected types or structures. fn from_value(v: Value) -> Result; -} -impl FromValue for Value { - fn from_value(v: Value) -> Result { - Ok(v) + /// Expected `Value` type. + /// + /// This is used to print out errors of what type of value is expected for conversion. + /// Even if not used in [`from_value`](FromValue::from_value) this should still be implemented + /// so that other implementations like `Option` or `Vec` can make use of it. + /// It is advised to call this method in `from_value` to ensure that expected type in the error + /// is consistent. + /// + /// Unlike the default implementation, derived implementations explicitly reveal the concrete + /// type, such as [`Type::Record`] or [`Type::List`], instead of an opaque type. + fn expected_type() -> Type { + Type::Custom( + any::type_name::() + .split(':') + .last() + .expect("str::split returns an iterator with at least one element") + .to_string() + .into_boxed_str(), + ) } } -impl FromValue for Spanned { +// Primitive Types + +impl FromValue for [T; N] +where + T: FromValue, +{ fn from_value(v: Value) -> Result { let span = v.span(); - match v { - Value::Int { val, .. } => Ok(Spanned { item: val, span }), - Value::Filesize { val, .. } => Ok(Spanned { item: val, span }), - Value::Duration { val, .. } => Ok(Spanned { item: val, span }), + let v_ty = v.get_type(); + let vec = Vec::::from_value(v)?; + vec.try_into() + .map_err(|err_vec: Vec| ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v_ty.to_string(), + span, + help: Some(match err_vec.len().cmp(&N) { + Ordering::Less => format!( + "input list too short ({}), expected length of {N}, add missing values", + err_vec.len() + ), + Ordering::Equal => { + unreachable!("conversion would have worked if the length would be the same") + } + Ordering::Greater => format!( + "input list too long ({}), expected length of {N}, remove trailing values", + err_vec.len() + ), + }), + }) + } + fn expected_type() -> Type { + Type::Custom(format!("list<{};{N}>", T::expected_type()).into_boxed_str()) + } +} + +impl FromValue for bool { + fn from_value(v: Value) -> Result { + match v { + Value::Bool { val, .. } => Ok(val), v => Err(ShellError::CantConvert { - to_type: "int".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } + + fn expected_type() -> Type { + Type::Bool + } +} + +impl FromValue for char { + fn from_value(v: Value) -> Result { + let span = v.span(); + let v_ty = v.get_type(); + match v { + Value::String { ref val, .. } => match char::from_str(val) { + Ok(c) => Ok(c), + Err(_) => Err(ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v_ty.to_string(), + span, + help: Some("make the string only one char long".to_string()), + }), + }, + _ => Err(ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v_ty.to_string(), + span, + help: None, + }), + } + } + + fn expected_type() -> Type { + Type::String + } +} + +impl FromValue for f32 { + fn from_value(v: Value) -> Result { + f64::from_value(v).map(|float| float as f32) + } +} + +impl FromValue for f64 { + fn from_value(v: Value) -> Result { + match v { + Value::Float { val, .. } => Ok(val), + Value::Int { val, .. } => Ok(val as f64), + v => Err(ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v.get_type().to_string(), + span: v.span(), + help: None, + }), + } + } + + fn expected_type() -> Type { + Type::Float + } } impl FromValue for i64 { @@ -42,128 +193,207 @@ impl FromValue for i64 { Value::Duration { val, .. } => Ok(val), v => Err(ShellError::CantConvert { - to_type: "int".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } + + fn expected_type() -> Type { + Type::Int + } } -impl FromValue for Spanned { - fn from_value(v: Value) -> Result { - let span = v.span(); - match v { - Value::Int { val, .. } => Ok(Spanned { - item: val as f64, - span, - }), - Value::Float { val, .. } => Ok(Spanned { item: val, span }), +macro_rules! impl_from_value_for_int { + ($type:ty) => { + impl FromValue for $type { + fn from_value(v: Value) -> Result { + let span = v.span(); + let int = i64::from_value(v)?; + const MIN: i64 = <$type>::MIN as i64; + const MAX: i64 = <$type>::MAX as i64; + #[allow(overlapping_range_endpoints)] // calculating MIN-1 is not possible for i64::MIN + #[allow(unreachable_patterns)] // isize might max out i64 number range + <$type>::try_from(int).map_err(|_| match int { + MIN..=MAX => unreachable!( + "int should be within the valid range for {}", + stringify!($type) + ), + i64::MIN..=MIN => ShellError::GenericError { + error: "Integer too small".to_string(), + msg: format!("{int} is smaller than {}", <$type>::MIN), + span: Some(span), + help: None, + inner: vec![], + }, + MAX..=i64::MAX => ShellError::GenericError { + error: "Integer too large".to_string(), + msg: format!("{int} is larger than {}", <$type>::MAX), + span: Some(span), + help: None, + inner: vec![], + }, + }) + } + fn expected_type() -> Type { + i64::expected_type() + } + } + }; +} + +impl_from_value_for_int!(i8); +impl_from_value_for_int!(i16); +impl_from_value_for_int!(i32); +impl_from_value_for_int!(isize); + +macro_rules! impl_from_value_for_uint { + ($type:ty, $max:expr) => { + impl FromValue for $type { + fn from_value(v: Value) -> Result { + let span = v.span(); + const MAX: i64 = $max; + match v { + Value::Int { val, .. } + | Value::Filesize { val, .. } + | Value::Duration { val, .. } => { + match val { + i64::MIN..=-1 => Err(ShellError::NeedsPositiveValue { span }), + 0..=MAX => Ok(val as $type), + #[allow(unreachable_patterns)] // u64 will max out the i64 number range + n => Err(ShellError::GenericError { + error: "Integer too large".to_string(), + msg: format!("{n} is larger than {MAX}"), + span: Some(span), + help: None, + inner: vec![], + }), + } + } + v => Err(ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v.get_type().to_string(), + span: v.span(), + help: None, + }), + } + } + + fn expected_type() -> Type { + Type::Custom("non-negative int".to_string().into_boxed_str()) + } + } + }; +} + +// Sadly we cannot implement FromValue for u8 without losing the impl of Vec, +// Rust would find two possible implementations then, Vec and Vec, +// and wouldn't compile. +// The blanket implementation for Vec is probably more useful than +// implementing FromValue for u8. + +impl_from_value_for_uint!(u16, u16::MAX as i64); +impl_from_value_for_uint!(u32, u32::MAX as i64); +impl_from_value_for_uint!(u64, i64::MAX); // u64::Max would be -1 as i64 +#[cfg(target_pointer_width = "64")] +impl_from_value_for_uint!(usize, i64::MAX); +#[cfg(target_pointer_width = "32")] +impl_from_value_for_uint!(usize, usize::MAX); + +impl FromValue for () { + fn from_value(v: Value) -> Result { + match v { + Value::Nothing { .. } => Ok(()), v => Err(ShellError::CantConvert { - to_type: "float".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } + + fn expected_type() -> Type { + Type::Nothing + } } -impl FromValue for f64 { +macro_rules! tuple_from_value { + ($template:literal, $($t:ident:$n:tt),+) => { + impl<$($t),+> FromValue for ($($t,)+) where $($t: FromValue,)+ { + fn from_value(v: Value) -> Result { + let span = v.span(); + match v { + Value::List { vals, .. } => { + let mut deque = VecDeque::from(vals); + + Ok(($( + { + let v = deque.pop_front().ok_or_else(|| ShellError::CantFindColumn { + col_name: $n.to_string(), + span: None, + src_span: span + })?; + $t::from_value(v)? + }, + )*)) + }, + v => Err(ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v.get_type().to_string(), + span: v.span(), + help: None, + }), + } + } + + fn expected_type() -> Type { + Type::Custom( + format!( + $template, + $($t::expected_type()),* + ) + .into_boxed_str(), + ) + } + } + }; +} + +// Tuples in std are implemented for up to 12 elements, so we do it here too. +tuple_from_value!("[{}]", T0:0); +tuple_from_value!("[{}, {}]", T0:0, T1:1); +tuple_from_value!("[{}, {}, {}]", T0:0, T1:1, T2:2); +tuple_from_value!("[{}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3); +tuple_from_value!("[{}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4); +tuple_from_value!("[{}, {}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4, T5:5); +tuple_from_value!("[{}, {}, {}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6); +tuple_from_value!("[{}, {}, {}, {}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7); +tuple_from_value!("[{}, {}, {}, {}, {}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8); +tuple_from_value!("[{}, {}, {}, {}, {}, {}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8, T9:9); +tuple_from_value!("[{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8, T9:9, T10:10); +tuple_from_value!("[{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}]", T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8, T9:9, T10:10, T11:11); + +// Other std Types + +impl FromValue for PathBuf { fn from_value(v: Value) -> Result { match v { - Value::Float { val, .. } => Ok(val), - Value::Int { val, .. } => Ok(val as f64), + Value::String { val, .. } => Ok(val.into()), v => Err(ShellError::CantConvert { - to_type: "float".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } -} -impl FromValue for Spanned { - fn from_value(v: Value) -> Result { - let span = v.span(); - match v { - Value::Int { val, .. } => { - if val.is_negative() { - Err(ShellError::NeedsPositiveValue { span }) - } else { - Ok(Spanned { - item: val as usize, - span, - }) - } - } - Value::Filesize { val, .. } => { - if val.is_negative() { - Err(ShellError::NeedsPositiveValue { span }) - } else { - Ok(Spanned { - item: val as usize, - span, - }) - } - } - Value::Duration { val, .. } => { - if val.is_negative() { - Err(ShellError::NeedsPositiveValue { span }) - } else { - Ok(Spanned { - item: val as usize, - span, - }) - } - } - - v => Err(ShellError::CantConvert { - to_type: "non-negative int".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } - } -} - -impl FromValue for usize { - fn from_value(v: Value) -> Result { - let span = v.span(); - match v { - Value::Int { val, .. } => { - if val.is_negative() { - Err(ShellError::NeedsPositiveValue { span }) - } else { - Ok(val as usize) - } - } - Value::Filesize { val, .. } => { - if val.is_negative() { - Err(ShellError::NeedsPositiveValue { span }) - } else { - Ok(val as usize) - } - } - Value::Duration { val, .. } => { - if val.is_negative() { - Err(ShellError::NeedsPositiveValue { span }) - } else { - Ok(val as usize) - } - } - - v => Err(ShellError::CantConvert { - to_type: "non-negative int".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } + fn expected_type() -> Type { + Type::String } } @@ -174,176 +404,111 @@ impl FromValue for String { Value::CellPath { val, .. } => Ok(val.to_string()), Value::String { val, .. } => Ok(val), v => Err(ShellError::CantConvert { - to_type: "string".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } -} -impl FromValue for Spanned { - fn from_value(v: Value) -> Result { - let span = v.span(); - Ok(Spanned { - item: match v { - Value::CellPath { val, .. } => val.to_string(), - Value::String { val, .. } => val, - v => { - return Err(ShellError::CantConvert { - to_type: "string".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }) - } - }, - span, - }) + fn expected_type() -> Type { + Type::String } } -impl FromValue for NuGlob { +// This impl is different from Vec as it reads from Value::Binary and +// Value::String instead of Value::List. +// This also denies implementing FromValue for u8. +impl FromValue for Vec { fn from_value(v: Value) -> Result { - // FIXME: we may want to fail a little nicer here match v { - Value::CellPath { val, .. } => Ok(NuGlob::Expand(val.to_string())), - Value::String { val, .. } => Ok(NuGlob::DoNotExpand(val)), - Value::Glob { - val, - no_expand: quoted, - .. - } => { - if quoted { - Ok(NuGlob::DoNotExpand(val)) - } else { - Ok(NuGlob::Expand(val)) - } - } + Value::Binary { val, .. } => Ok(val), + Value::String { val, .. } => Ok(val.into_bytes()), v => Err(ShellError::CantConvert { - to_type: "string".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } -} -impl FromValue for Spanned { - fn from_value(v: Value) -> Result { - let span = v.span(); - Ok(Spanned { - item: match v { - Value::CellPath { val, .. } => NuGlob::Expand(val.to_string()), - Value::String { val, .. } => NuGlob::DoNotExpand(val), - Value::Glob { - val, - no_expand: quoted, - .. - } => { - if quoted { - NuGlob::DoNotExpand(val) - } else { - NuGlob::Expand(val) - } - } - v => { - return Err(ShellError::CantConvert { - to_type: "string".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }) - } - }, - span, - }) + fn expected_type() -> Type { + Type::Binary } } -impl FromValue for Vec { +// Blanket std Implementations + +impl FromValue for Option +where + T: FromValue, +{ fn from_value(v: Value) -> Result { - // FIXME: we may want to fail a little nicer here match v { - Value::List { vals, .. } => vals - .into_iter() - .map(|val| match val { - Value::String { val, .. } => Ok(val), - c => Err(ShellError::CantConvert { - to_type: "string".into(), - from_type: c.get_type().to_string(), - span: c.span(), - help: None, - }), - }) - .collect::, ShellError>>(), - v => Err(ShellError::CantConvert { - to_type: "string".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), + Value::Nothing { .. } => Ok(None), + v => T::from_value(v).map(Option::Some), } } + + fn expected_type() -> Type { + T::expected_type() + } } -impl FromValue for Vec> { +impl FromValue for HashMap +where + V: FromValue, +{ fn from_value(v: Value) -> Result { - // FIXME: we may want to fail a little nicer here - match v { - Value::List { vals, .. } => vals - .into_iter() - .map(|val| { - let val_span = val.span(); - match val { - Value::String { val, .. } => Ok(Spanned { - item: val, - span: val_span, - }), - c => Err(ShellError::CantConvert { - to_type: "string".into(), - from_type: c.get_type().to_string(), - span: c.span(), - help: None, - }), - } - }) - .collect::>, ShellError>>(), - v => Err(ShellError::CantConvert { - to_type: "string".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } + let record = v.into_record()?; + let items: Result, ShellError> = record + .into_iter() + .map(|(k, v)| Ok((k, V::from_value(v)?))) + .collect(); + Ok(HashMap::from_iter(items?)) + } + + fn expected_type() -> Type { + Type::Record(vec![].into_boxed_slice()) } } -impl FromValue for Vec { +impl FromValue for Vec +where + T: FromValue, +{ fn from_value(v: Value) -> Result { match v { Value::List { vals, .. } => vals .into_iter() - .map(|val| match val { - Value::Bool { val, .. } => Ok(val), - c => Err(ShellError::CantConvert { - to_type: "bool".into(), - from_type: c.get_type().to_string(), - span: c.span(), - help: None, - }), - }) - .collect::, ShellError>>(), + .map(|v| T::from_value(v)) + .collect::, ShellError>>(), v => Err(ShellError::CantConvert { - to_type: "bool".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } + + fn expected_type() -> Type { + Type::List(Box::new(T::expected_type())) + } +} + +// Nu Types + +impl FromValue for Value { + fn from_value(v: Value) -> Result { + Ok(v) + } + + fn expected_type() -> Type { + Type::Any + } } impl FromValue for CellPath { @@ -372,36 +537,25 @@ impl FromValue for CellPath { } } x => Err(ShellError::CantConvert { - to_type: "cell path".into(), + to_type: Self::expected_type().to_string(), from_type: x.get_type().to_string(), span, help: None, }), } } -} -impl FromValue for bool { - fn from_value(v: Value) -> Result { - match v { - Value::Bool { val, .. } => Ok(val), - v => Err(ShellError::CantConvert { - to_type: "bool".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } + fn expected_type() -> Type { + Type::CellPath } } -impl FromValue for Spanned { +impl FromValue for Closure { fn from_value(v: Value) -> Result { - let span = v.span(); match v { - Value::Bool { val, .. } => Ok(Spanned { item: val, span }), + Value::Closure { val, .. } => Ok(*val), v => Err(ShellError::CantConvert { - to_type: "bool".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, @@ -415,28 +569,48 @@ impl FromValue for DateTime { match v { Value::Date { val, .. } => Ok(val), v => Err(ShellError::CantConvert { - to_type: "date".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } + + fn expected_type() -> Type { + Type::Date + } } -impl FromValue for Spanned> { +impl FromValue for NuGlob { fn from_value(v: Value) -> Result { - let span = v.span(); + // FIXME: we may want to fail a little nicer here match v { - Value::Date { val, .. } => Ok(Spanned { item: val, span }), + Value::CellPath { val, .. } => Ok(NuGlob::Expand(val.to_string())), + Value::String { val, .. } => Ok(NuGlob::DoNotExpand(val)), + Value::Glob { + val, + no_expand: quoted, + .. + } => { + if quoted { + Ok(NuGlob::DoNotExpand(val)) + } else { + Ok(NuGlob::Expand(val)) + } + } v => Err(ShellError::CantConvert { - to_type: "date".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } + + fn expected_type() -> Type { + Type::String + } } impl FromValue for Range { @@ -444,94 +618,16 @@ impl FromValue for Range { match v { Value::Range { val, .. } => Ok(*val), v => Err(ShellError::CantConvert { - to_type: "range".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, }), } } -} -impl FromValue for Spanned { - fn from_value(v: Value) -> Result { - let span = v.span(); - match v { - Value::Range { val, .. } => Ok(Spanned { item: *val, span }), - v => Err(ShellError::CantConvert { - to_type: "range".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } - } -} - -impl FromValue for Vec { - fn from_value(v: Value) -> Result { - match v { - Value::Binary { val, .. } => Ok(val), - Value::String { val, .. } => Ok(val.into_bytes()), - v => Err(ShellError::CantConvert { - to_type: "binary data".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } - } -} - -impl FromValue for Spanned> { - fn from_value(v: Value) -> Result { - let span = v.span(); - match v { - Value::Binary { val, .. } => Ok(Spanned { item: val, span }), - Value::String { val, .. } => Ok(Spanned { - item: val.into_bytes(), - span, - }), - v => Err(ShellError::CantConvert { - to_type: "binary data".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } - } -} - -impl FromValue for Spanned { - fn from_value(v: Value) -> Result { - let span = v.span(); - match v { - Value::String { val, .. } => Ok(Spanned { - item: val.into(), - span, - }), - v => Err(ShellError::CantConvert { - to_type: "range".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } - } -} - -impl FromValue for Vec { - fn from_value(v: Value) -> Result { - // FIXME: we may want to fail a little nicer here - match v { - Value::List { vals, .. } => Ok(vals), - v => Err(ShellError::CantConvert { - to_type: "Vector of values".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } + fn expected_type() -> Type { + Type::Range } } @@ -540,7 +636,7 @@ impl FromValue for Record { match v { Value::Record { val, .. } => Ok(val.into_owned()), v => Err(ShellError::CantConvert { - to_type: "Record".into(), + to_type: Self::expected_type().to_string(), from_type: v.get_type().to_string(), span: v.span(), help: None, @@ -549,31 +645,39 @@ impl FromValue for Record { } } -impl FromValue for Closure { - fn from_value(v: Value) -> Result { - match v { - Value::Closure { val, .. } => Ok(*val), - v => Err(ShellError::CantConvert { - to_type: "Closure".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } - } -} +// Blanket Nu Implementations -impl FromValue for Spanned { +impl FromValue for Spanned +where + T: FromValue, +{ fn from_value(v: Value) -> Result { let span = v.span(); - match v { - Value::Closure { val, .. } => Ok(Spanned { item: *val, span }), - v => Err(ShellError::CantConvert { - to_type: "Closure".into(), - from_type: v.get_type().to_string(), - span: v.span(), - help: None, - }), - } + Ok(Spanned { + item: T::from_value(v)?, + span, + }) + } + + fn expected_type() -> Type { + T::expected_type() + } +} + +#[cfg(test)] +mod tests { + use crate::{engine::Closure, FromValue, Record, Type}; + + #[test] + fn expected_type_default_impl() { + assert_eq!( + Record::expected_type(), + Type::Custom("Record".to_string().into_boxed_str()) + ); + + assert_eq!( + Closure::expected_type(), + Type::Custom("Closure".to_string().into_boxed_str()) + ); } } diff --git a/crates/nu-protocol/src/value/into_value.rs b/crates/nu-protocol/src/value/into_value.rs new file mode 100644 index 0000000000..c27b19b651 --- /dev/null +++ b/crates/nu-protocol/src/value/into_value.rs @@ -0,0 +1,196 @@ +use std::collections::HashMap; + +use crate::{Record, ShellError, Span, Value}; + +/// A trait for converting a value into a [`Value`]. +/// +/// This conversion is infallible, for fallible conversions use [`TryIntoValue`]. +/// +/// # Derivable +/// This trait can be used with `#[derive]`. +/// When derived on structs with named fields, the resulting value representation will use +/// [`Value::Record`], where each field of the record corresponds to a field of the struct. +/// For structs with unnamed fields, the value representation will be [`Value::List`], with all +/// fields inserted into a list. +/// Unit structs will be represented as [`Value::Nothing`] since they contain no data. +/// +/// Only enums with no fields may derive this trait. +/// The resulting value representation will be the name of the variant as a [`Value::String`]. +/// By default, variant names will be converted to ["snake_case"](convert_case::Case::Snake). +/// You can customize the case conversion using `#[nu_value(rename_all = "kebab-case")]` on the enum. +/// All deterministic and useful case conversions provided by [`convert_case::Case`] are supported +/// by specifying the case name followed by "case". +/// Also all values for +/// [`#[serde(rename_all = "...")]`](https://serde.rs/container-attrs.html#rename_all) are valid +/// here. +/// +/// ``` +/// # use nu_protocol::{IntoValue, Value, Span}; +/// #[derive(IntoValue)] +/// #[nu_value(rename_all = "COBOL-CASE")] +/// enum Bird { +/// MountainEagle, +/// ForestOwl, +/// RiverDuck, +/// } +/// +/// assert_eq!( +/// Bird::RiverDuck.into_value(Span::unknown()), +/// Value::test_string("RIVER-DUCK") +/// ); +/// ``` +pub trait IntoValue: Sized { + /// Converts the given value to a [`Value`]. + fn into_value(self, span: Span) -> Value; +} + +// Primitive Types + +impl IntoValue for [T; N] +where + T: IntoValue, +{ + fn into_value(self, span: Span) -> Value { + Vec::from(self).into_value(span) + } +} + +macro_rules! primitive_into_value { + ($type:ty, $method:ident) => { + primitive_into_value!($type => $type, $method); + }; + + ($type:ty => $as_type:ty, $method:ident) => { + impl IntoValue for $type { + fn into_value(self, span: Span) -> Value { + Value::$method(<$as_type>::from(self), span) + } + } + }; +} + +primitive_into_value!(bool, bool); +primitive_into_value!(char, string); +primitive_into_value!(f32 => f64, float); +primitive_into_value!(f64, float); +primitive_into_value!(i8 => i64, int); +primitive_into_value!(i16 => i64, int); +primitive_into_value!(i32 => i64, int); +primitive_into_value!(i64, int); +primitive_into_value!(u8 => i64, int); +primitive_into_value!(u16 => i64, int); +primitive_into_value!(u32 => i64, int); +// u64 and usize may be truncated as Value only supports i64. + +impl IntoValue for isize { + fn into_value(self, span: Span) -> Value { + Value::int(self as i64, span) + } +} + +impl IntoValue for () { + fn into_value(self, span: Span) -> Value { + Value::nothing(span) + } +} + +macro_rules! tuple_into_value { + ($($t:ident:$n:tt),+) => { + impl<$($t),+> IntoValue for ($($t,)+) where $($t: IntoValue,)+ { + fn into_value(self, span: Span) -> Value { + let vals = vec![$(self.$n.into_value(span)),+]; + Value::list(vals, span) + } + } + } +} + +// Tuples in std are implemented for up to 12 elements, so we do it here too. +tuple_into_value!(T0:0); +tuple_into_value!(T0:0, T1:1); +tuple_into_value!(T0:0, T1:1, T2:2); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4, T5:5); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8, T9:9); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8, T9:9, T10:10); +tuple_into_value!(T0:0, T1:1, T2:2, T3:3, T4:4, T5:5, T6:6, T7:7, T8:8, T9:9, T10:10, T11:11); + +// Other std Types + +impl IntoValue for String { + fn into_value(self, span: Span) -> Value { + Value::string(self, span) + } +} + +impl IntoValue for Vec +where + T: IntoValue, +{ + fn into_value(self, span: Span) -> Value { + Value::list(self.into_iter().map(|v| v.into_value(span)).collect(), span) + } +} + +impl IntoValue for Option +where + T: IntoValue, +{ + fn into_value(self, span: Span) -> Value { + match self { + Some(v) => v.into_value(span), + None => Value::nothing(span), + } + } +} + +impl IntoValue for HashMap +where + V: IntoValue, +{ + fn into_value(self, span: Span) -> Value { + let mut record = Record::new(); + for (k, v) in self.into_iter() { + // Using `push` is fine as a hashmaps have unique keys. + // To ensure this uniqueness, we only allow hashmaps with strings as + // keys and not keys which implement `Into` or `ToString`. + record.push(k, v.into_value(span)); + } + Value::record(record, span) + } +} + +// Nu Types + +impl IntoValue for Value { + fn into_value(self, span: Span) -> Value { + self.with_span(span) + } +} + +// TODO: use this type for all the `into_value` methods that types implement but return a Result +/// A trait for trying to convert a value into a `Value`. +/// +/// Types like streams may fail while collecting the `Value`, +/// for these types it is useful to implement a fallible variant. +/// +/// This conversion is fallible, for infallible conversions use [`IntoValue`]. +/// All types that implement `IntoValue` will automatically implement this trait. +pub trait TryIntoValue: Sized { + // TODO: instead of ShellError, maybe we could have a IntoValueError that implements Into + /// Tries to convert the given value into a `Value`. + fn try_into_value(self, span: Span) -> Result; +} + +impl TryIntoValue for T +where + T: IntoValue, +{ + fn try_into_value(self, span: Span) -> Result { + Ok(self.into_value(span)) + } +} diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index f924218b79..df030ad3e9 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -4,7 +4,10 @@ mod filesize; mod from; mod from_value; mod glob; +mod into_value; mod range; +#[cfg(test)] +mod test_derive; pub mod record; pub use custom_value::CustomValue; @@ -12,6 +15,7 @@ pub use duration::*; pub use filesize::*; pub use from_value::FromValue; pub use glob::*; +pub use into_value::{IntoValue, TryIntoValue}; pub use range::{FloatRange, IntRange, Range}; pub use record::Record; @@ -1089,7 +1093,7 @@ impl Value { } else { return Err(ShellError::CantFindColumn { col_name: column_name.clone(), - span: *origin_span, + span: Some(*origin_span), src_span: span, }); } @@ -1126,7 +1130,7 @@ impl Value { } else { Err(ShellError::CantFindColumn { col_name: column_name.clone(), - span: *origin_span, + span: Some(*origin_span), src_span: val_span, }) } @@ -1136,7 +1140,7 @@ impl Value { } _ => Err(ShellError::CantFindColumn { col_name: column_name.clone(), - span: *origin_span, + span: Some(*origin_span), src_span: val_span, }), } @@ -1237,7 +1241,7 @@ impl Value { v => { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }); } @@ -1262,7 +1266,7 @@ impl Value { v => { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }); } @@ -1342,7 +1346,7 @@ impl Value { } else { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v_span, }); } @@ -1351,7 +1355,7 @@ impl Value { v => { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }); } @@ -1364,7 +1368,7 @@ impl Value { } else { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v_span, }); } @@ -1373,7 +1377,7 @@ impl Value { v => { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }); } @@ -1427,7 +1431,7 @@ impl Value { if record.to_mut().remove(col_name).is_none() && !optional { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v_span, }); } @@ -1435,7 +1439,7 @@ impl Value { v => { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }); } @@ -1447,7 +1451,7 @@ impl Value { if record.to_mut().remove(col_name).is_none() && !optional { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v_span, }); } @@ -1455,7 +1459,7 @@ impl Value { } v => Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }), }, @@ -1504,7 +1508,7 @@ impl Value { } else if !optional { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v_span, }); } @@ -1512,7 +1516,7 @@ impl Value { v => { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }); } @@ -1526,7 +1530,7 @@ impl Value { } else if !optional { return Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v_span, }); } @@ -1534,7 +1538,7 @@ impl Value { } v => Err(ShellError::CantFindColumn { col_name: col_name.clone(), - span: *span, + span: Some(*span), src_span: v.span(), }), }, diff --git a/crates/nu-protocol/src/value/test_derive.rs b/crates/nu-protocol/src/value/test_derive.rs new file mode 100644 index 0000000000..1865a05418 --- /dev/null +++ b/crates/nu-protocol/src/value/test_derive.rs @@ -0,0 +1,386 @@ +use crate::{record, FromValue, IntoValue, Record, Span, Value}; +use std::collections::HashMap; + +// Make nu_protocol available in this namespace, consumers of this crate will +// have this without such an export. +// The derive macro fully qualifies paths to "nu_protocol". +use crate as nu_protocol; + +trait IntoTestValue { + fn into_test_value(self) -> Value; +} + +impl IntoTestValue for T +where + T: IntoValue, +{ + fn into_test_value(self) -> Value { + self.into_value(Span::test_data()) + } +} + +#[derive(IntoValue, FromValue, Debug, PartialEq)] +struct NamedFieldsStruct +where + T: IntoValue + FromValue, +{ + array: [u16; 4], + bool: bool, + char: char, + f32: f32, + f64: f64, + i8: i8, + i16: i16, + i32: i32, + i64: i64, + isize: isize, + u16: u16, + u32: u32, + unit: (), + tuple: (u32, bool), + some: Option, + none: Option, + vec: Vec, + string: String, + hashmap: HashMap, + nested: Nestee, +} + +#[derive(IntoValue, FromValue, Debug, PartialEq)] +struct Nestee { + u32: u32, + some: Option, + none: Option, +} + +impl NamedFieldsStruct { + fn make() -> Self { + Self { + array: [1, 2, 3, 4], + bool: true, + char: 'a', + f32: std::f32::consts::PI, + f64: std::f64::consts::E, + i8: 127, + i16: -32768, + i32: 2147483647, + i64: -9223372036854775808, + isize: 2, + u16: 65535, + u32: 4294967295, + unit: (), + tuple: (1, true), + some: Some(123), + none: None, + vec: vec![10, 20, 30], + string: "string".to_string(), + hashmap: HashMap::from_iter([("a".to_string(), 10), ("b".to_string(), 20)]), + nested: Nestee { + u32: 3, + some: Some(42), + none: None, + }, + } + } + + fn value() -> Value { + Value::test_record(record! { + "array" => Value::test_list(vec![ + Value::test_int(1), + Value::test_int(2), + Value::test_int(3), + Value::test_int(4) + ]), + "bool" => Value::test_bool(true), + "char" => Value::test_string('a'), + "f32" => Value::test_float(std::f32::consts::PI.into()), + "f64" => Value::test_float(std::f64::consts::E), + "i8" => Value::test_int(127), + "i16" => Value::test_int(-32768), + "i32" => Value::test_int(2147483647), + "i64" => Value::test_int(-9223372036854775808), + "isize" => Value::test_int(2), + "u16" => Value::test_int(65535), + "u32" => Value::test_int(4294967295), + "unit" => Value::test_nothing(), + "tuple" => Value::test_list(vec![ + Value::test_int(1), + Value::test_bool(true) + ]), + "some" => Value::test_int(123), + "none" => Value::test_nothing(), + "vec" => Value::test_list(vec![ + Value::test_int(10), + Value::test_int(20), + Value::test_int(30) + ]), + "string" => Value::test_string("string"), + "hashmap" => Value::test_record(record! { + "a" => Value::test_int(10), + "b" => Value::test_int(20) + }), + "nested" => Value::test_record(record! { + "u32" => Value::test_int(3), + "some" => Value::test_int(42), + "none" => Value::test_nothing(), + }) + }) + } +} + +#[test] +fn named_fields_struct_into_value() { + let expected = NamedFieldsStruct::value(); + let actual = NamedFieldsStruct::make().into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn named_fields_struct_from_value() { + let expected = NamedFieldsStruct::make(); + let actual = NamedFieldsStruct::from_value(NamedFieldsStruct::value()).unwrap(); + assert_eq!(expected, actual); +} + +#[test] +fn named_fields_struct_roundtrip() { + let expected = NamedFieldsStruct::make(); + let actual = + NamedFieldsStruct::from_value(NamedFieldsStruct::make().into_test_value()).unwrap(); + assert_eq!(expected, actual); + + let expected = NamedFieldsStruct::value(); + let actual = NamedFieldsStruct::::from_value(NamedFieldsStruct::value()) + .unwrap() + .into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn named_fields_struct_missing_value() { + let value = Value::test_record(Record::new()); + let res: Result, _> = NamedFieldsStruct::from_value(value); + assert!(res.is_err()); +} + +#[test] +fn named_fields_struct_incorrect_type() { + // Should work for every type that is not a record. + let value = Value::test_nothing(); + let res: Result, _> = NamedFieldsStruct::from_value(value); + assert!(res.is_err()); +} + +#[derive(IntoValue, FromValue, Debug, PartialEq)] +struct UnnamedFieldsStruct(u32, String, T) +where + T: IntoValue + FromValue; + +impl UnnamedFieldsStruct { + fn make() -> Self { + UnnamedFieldsStruct(420, "Hello, tuple!".to_string(), 33.33) + } + + fn value() -> Value { + Value::test_list(vec![ + Value::test_int(420), + Value::test_string("Hello, tuple!"), + Value::test_float(33.33), + ]) + } +} + +#[test] +fn unnamed_fields_struct_into_value() { + let expected = UnnamedFieldsStruct::value(); + let actual = UnnamedFieldsStruct::make().into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn unnamed_fields_struct_from_value() { + let expected = UnnamedFieldsStruct::make(); + let value = UnnamedFieldsStruct::value(); + let actual = UnnamedFieldsStruct::from_value(value).unwrap(); + assert_eq!(expected, actual); +} + +#[test] +fn unnamed_fields_struct_roundtrip() { + let expected = UnnamedFieldsStruct::make(); + let actual = + UnnamedFieldsStruct::from_value(UnnamedFieldsStruct::make().into_test_value()).unwrap(); + assert_eq!(expected, actual); + + let expected = UnnamedFieldsStruct::value(); + let actual = UnnamedFieldsStruct::::from_value(UnnamedFieldsStruct::value()) + .unwrap() + .into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn unnamed_fields_struct_missing_value() { + let value = Value::test_list(vec![]); + let res: Result, _> = UnnamedFieldsStruct::from_value(value); + assert!(res.is_err()); +} + +#[test] +fn unnamed_fields_struct_incorrect_type() { + // Should work for every type that is not a record. + let value = Value::test_nothing(); + let res: Result, _> = UnnamedFieldsStruct::from_value(value); + assert!(res.is_err()); +} + +#[derive(IntoValue, FromValue, Debug, PartialEq)] +struct UnitStruct; + +#[test] +fn unit_struct_into_value() { + let expected = Value::test_nothing(); + let actual = UnitStruct.into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn unit_struct_from_value() { + let expected = UnitStruct; + let actual = UnitStruct::from_value(Value::test_nothing()).unwrap(); + assert_eq!(expected, actual); +} + +#[test] +fn unit_struct_roundtrip() { + let expected = UnitStruct; + let actual = UnitStruct::from_value(UnitStruct.into_test_value()).unwrap(); + assert_eq!(expected, actual); + + let expected = Value::test_nothing(); + let actual = UnitStruct::from_value(Value::test_nothing()) + .unwrap() + .into_test_value(); + assert_eq!(expected, actual); +} + +#[derive(IntoValue, FromValue, Debug, PartialEq)] +enum Enum { + AlphaOne, + BetaTwo, + CharlieThree, +} + +impl Enum { + fn make() -> [Self; 3] { + [Enum::AlphaOne, Enum::BetaTwo, Enum::CharlieThree] + } + + fn value() -> Value { + Value::test_list(vec![ + Value::test_string("alpha_one"), + Value::test_string("beta_two"), + Value::test_string("charlie_three"), + ]) + } +} + +#[test] +fn enum_into_value() { + let expected = Enum::value(); + let actual = Enum::make().into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn enum_from_value() { + let expected = Enum::make(); + let actual = <[Enum; 3]>::from_value(Enum::value()).unwrap(); + assert_eq!(expected, actual); +} + +#[test] +fn enum_roundtrip() { + let expected = Enum::make(); + let actual = <[Enum; 3]>::from_value(Enum::make().into_test_value()).unwrap(); + assert_eq!(expected, actual); + + let expected = Enum::value(); + let actual = <[Enum; 3]>::from_value(Enum::value()) + .unwrap() + .into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn enum_unknown_variant() { + let value = Value::test_string("delta_four"); + let res = Enum::from_value(value); + assert!(res.is_err()); +} + +#[test] +fn enum_incorrect_type() { + // Should work for every type that is not a record. + let value = Value::test_nothing(); + let res = Enum::from_value(value); + assert!(res.is_err()); +} + +// Generate the `Enum` from before but with all possible `rename_all` variants. +macro_rules! enum_rename_all { + ($($ident:ident: $case:literal => [$a1:literal, $b2:literal, $c3:literal]),*) => { + $( + #[derive(Debug, PartialEq, IntoValue, FromValue)] + #[nu_value(rename_all = $case)] + enum $ident { + AlphaOne, + BetaTwo, + CharlieThree + } + + impl $ident { + fn make() -> [Self; 3] { + [Self::AlphaOne, Self::BetaTwo, Self::CharlieThree] + } + + fn value() -> Value { + Value::test_list(vec![ + Value::test_string($a1), + Value::test_string($b2), + Value::test_string($c3), + ]) + } + } + )* + + #[test] + fn enum_rename_all_into_value() {$({ + let expected = $ident::value(); + let actual = $ident::make().into_test_value(); + assert_eq!(expected, actual); + })*} + + #[test] + fn enum_rename_all_from_value() {$({ + let expected = $ident::make(); + let actual = <[$ident; 3]>::from_value($ident::value()).unwrap(); + assert_eq!(expected, actual); + })*} + } +} + +enum_rename_all! { + Upper: "UPPER CASE" => ["ALPHA ONE", "BETA TWO", "CHARLIE THREE"], + Lower: "lower case" => ["alpha one", "beta two", "charlie three"], + Title: "Title Case" => ["Alpha One", "Beta Two", "Charlie Three"], + Camel: "camelCase" => ["alphaOne", "betaTwo", "charlieThree"], + Pascal: "PascalCase" => ["AlphaOne", "BetaTwo", "CharlieThree"], + Snake: "snake_case" => ["alpha_one", "beta_two", "charlie_three"], + UpperSnake: "UPPER_SNAKE_CASE" => ["ALPHA_ONE", "BETA_TWO", "CHARLIE_THREE"], + Kebab: "kebab-case" => ["alpha-one", "beta-two", "charlie-three"], + Cobol: "COBOL-CASE" => ["ALPHA-ONE", "BETA-TWO", "CHARLIE-THREE"], + Train: "Train-Case" => ["Alpha-One", "Beta-Two", "Charlie-Three"], + Flat: "flatcase" => ["alphaone", "betatwo", "charliethree"], + UpperFlat: "UPPERFLATCASE" => ["ALPHAONE", "BETATWO", "CHARLIETHREE"] +} diff --git a/crates/nu_plugin_custom_values/src/cool_custom_value.rs b/crates/nu_plugin_custom_values/src/cool_custom_value.rs index d838aa3f9e..ea0cf8ec92 100644 --- a/crates/nu_plugin_custom_values/src/cool_custom_value.rs +++ b/crates/nu_plugin_custom_values/src/cool_custom_value.rs @@ -87,7 +87,7 @@ impl CustomValue for CoolCustomValue { } else { Err(ShellError::CantFindColumn { col_name: column_name, - span: path_span, + span: Some(path_span), src_span: self_span, }) } From 97876fb0b4fd423a67401e395809635f1a571375 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 18 Jun 2024 05:16:08 -0700 Subject: [PATCH 30/38] Fix compilation for `nu_protocol::value::from_value` on 32-bit targets (#13169) # Description Needed to cast `usize::MAX` to `i64` for it to compile properly cc @cptpiepmatz --- crates/nu-protocol/src/value/from_value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index 1033eb03b2..1c3bc7dfd4 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -301,7 +301,7 @@ impl_from_value_for_uint!(u64, i64::MAX); // u64::Max would be -1 as i64 #[cfg(target_pointer_width = "64")] impl_from_value_for_uint!(usize, i64::MAX); #[cfg(target_pointer_width = "32")] -impl_from_value_for_uint!(usize, usize::MAX); +impl_from_value_for_uint!(usize, usize::MAX as i64); impl FromValue for () { fn from_value(v: Value) -> Result { From ae6489f04b1cc274ff2566cfa5cfdfecf2003d89 Mon Sep 17 00:00:00 2001 From: "Edwin.JH.Lee" Date: Tue, 18 Jun 2024 20:16:42 +0800 Subject: [PATCH 31/38] Update README.md (#13157) Add x-cmd in the support list. # Description We like nushell, so we manage to add x-cmd support in the nushell. Here is our demo. https://www.x-cmd.com/mod/nu Thank you. --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 0f1114c992..9b88a92528 100644 --- a/README.md +++ b/README.md @@ -222,6 +222,7 @@ Please submit an issue or PR to be added to this list. - [clap](https://github.com/clap-rs/clap/tree/master/clap_complete_nushell) - [Dorothy](http://github.com/bevry/dorothy) - [Direnv](https://github.com/direnv/direnv/blob/master/docs/hook.md#nushell) +- [x-cmd](https://x-cmd.com/mod/nu) ## Contributing From 28ed0fe700167e09633ebf49c495d168cd9a7561 Mon Sep 17 00:00:00 2001 From: Wind Date: Tue, 18 Jun 2024 20:19:13 +0800 Subject: [PATCH 32/38] Improves commands that support range input (#13113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Fixes: #13105 Fixes: #13077 This pr makes `str substring`, `bytes at` work better with negative index. And it also fixes the false range semantic on `detect columns -c` in some cases. # User-Facing Changes For `str substring`, `bytes at`, it will no-longer return an error if start index is larger than end index. It makes sense to return an empty string of empty bytes directly. ### Before ```nushell # str substring ❯ ("aaa" | str substring 2..-3) == "" Error: nu::shell::type_mismatch × Type mismatch. ╭─[entry #23:1:10] 1 │ ("aaa" | str substring 2..-3) == "" · ──────┬────── · ╰── End must be greater than or equal to Start 2 │ true ╰──── # bytes at ❯ ("aaa" | encode utf-8 | bytes at 2..-3) == ("" | encode utf-8) Error: nu::shell::type_mismatch × Type mismatch. ╭─[entry #27:1:25] 1 │ ("aaa" | encode utf-8 | bytes at 2..-3) == ("" | encode utf-8) · ────┬─── · ╰── End must be greater than or equal to Start ╰──── ``` ### After ```nushell # str substring ❯ ("aaa" | str substring 2..-3) == "" true # bytes at ❯ ("aaa" | encode utf-8 | bytes at 2..-3) == ("" | encode utf-8) true ``` # Tests + Formatting Added some tests, adjust existing tests --- crates/nu-cmd-base/src/util.rs | 5 +- crates/nu-command/src/bytes/at.rs | 48 +++------- .../nu-command/src/strings/str_/substring.rs | 95 +++++++++---------- .../tests/commands/detect_columns.rs | 12 +-- crates/nu-command/tests/commands/str_/mod.rs | 23 ++++- 5 files changed, 85 insertions(+), 98 deletions(-) diff --git a/crates/nu-cmd-base/src/util.rs b/crates/nu-cmd-base/src/util.rs index 559161329b..905c990a9e 100644 --- a/crates/nu-cmd-base/src/util.rs +++ b/crates/nu-cmd-base/src/util.rs @@ -20,13 +20,14 @@ pub fn get_guaranteed_cwd(engine_state: &EngineState, stack: &Stack) -> PathBuf type MakeRangeError = fn(&str, Span) -> ShellError; +/// Returns a inclusive pair of boundary in given `range`. pub fn process_range(range: &Range) -> Result<(isize, isize), MakeRangeError> { match range { Range::IntRange(range) => { let start = range.start().try_into().unwrap_or(0); let end = match range.end() { - Bound::Included(v) => (v + 1) as isize, - Bound::Excluded(v) => v as isize, + Bound::Included(v) => v as isize, + Bound::Excluded(v) => (v - 1) as isize, Bound::Unbounded => isize::MAX, }; Ok((start, end)) diff --git a/crates/nu-command/src/bytes/at.rs b/crates/nu-command/src/bytes/at.rs index e90312603f..c5164c3550 100644 --- a/crates/nu-command/src/bytes/at.rs +++ b/crates/nu-command/src/bytes/at.rs @@ -128,48 +128,24 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { let range = &args.indexes; match input { Value::Binary { val, .. } => { - use std::cmp::{self, Ordering}; let len = val.len() as isize; - let start = if range.0 < 0 { range.0 + len } else { range.0 }; + let end = if range.1 < 0 { range.1 + len } else { range.1 }; - let end = if range.1 < 0 { - cmp::max(range.1 + len, 0) - } else { - range.1 - }; - - if start < len && end >= 0 { - match start.cmp(&end) { - Ordering::Equal => Value::binary(vec![], head), - Ordering::Greater => Value::error( - ShellError::TypeMismatch { - err_message: "End must be greater than or equal to Start".to_string(), - span: head, - }, - head, - ), - Ordering::Less => Value::binary( - if end == isize::MAX { - val.iter() - .skip(start as usize) - .copied() - .collect::>() - } else { - val.iter() - .skip(start as usize) - .take((end - start) as usize) - .copied() - .collect() - }, - head, - ), - } - } else { + if start > end { Value::binary(vec![], head) + } else { + let val_iter = val.iter().skip(start as usize); + Value::binary( + if end == isize::MAX { + val_iter.copied().collect::>() + } else { + val_iter.take((end - start + 1) as usize).copied().collect() + }, + head, + ) } } - Value::Error { .. } => input.clone(), other => Value::error( diff --git a/crates/nu-command/src/strings/str_/substring.rs b/crates/nu-command/src/strings/str_/substring.rs index 99ba8ede1e..5ad7a967da 100644 --- a/crates/nu-command/src/strings/str_/substring.rs +++ b/crates/nu-command/src/strings/str_/substring.rs @@ -5,7 +5,6 @@ use nu_cmd_base::{ }; use nu_engine::command_prelude::*; use nu_protocol::{engine::StateWorkingSet, Range}; -use std::cmp::Ordering; use unicode_segmentation::UnicodeSegmentation; #[derive(Clone)] @@ -151,6 +150,11 @@ impl Command for SubCommand { example: " '🇯🇵ほげ ふが ぴよ' | str substring --grapheme-clusters 4..5", result: Some(Value::test_string("ふが")), }, + Example { + description: "sub string by negative index", + example: " 'good nushell' | str substring 5..-2", + result: Some(Value::test_string("nushel")), + }, ] } } @@ -167,56 +171,46 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value { options.0 }; let end: isize = if options.1 < 0 { - std::cmp::max(len + options.1, 0) + options.1 + len } else { options.1 }; - if start < len && end >= 0 { - match start.cmp(&end) { - Ordering::Equal => Value::string("", head), - Ordering::Greater => Value::error( - ShellError::TypeMismatch { - err_message: "End must be greater than or equal to Start".to_string(), - span: head, - }, - head, - ), - Ordering::Less => Value::string( - { - if end == isize::MAX { - if args.graphemes { - s.graphemes(true) - .skip(start as usize) - .collect::>() - .join("") - } else { - String::from_utf8_lossy( - &s.bytes().skip(start as usize).collect::>(), - ) - .to_string() - } - } else if args.graphemes { + if start > end { + Value::string("", head) + } else { + Value::string( + { + if end == isize::MAX { + if args.graphemes { s.graphemes(true) .skip(start as usize) - .take((end - start) as usize) .collect::>() .join("") } else { String::from_utf8_lossy( - &s.bytes() - .skip(start as usize) - .take((end - start) as usize) - .collect::>(), + &s.bytes().skip(start as usize).collect::>(), ) .to_string() } - }, - head, - ), - } - } else { - Value::string("", head) + } else if args.graphemes { + s.graphemes(true) + .skip(start as usize) + .take((end - start + 1) as usize) + .collect::>() + .join("") + } else { + String::from_utf8_lossy( + &s.bytes() + .skip(start as usize) + .take((end - start + 1) as usize) + .collect::>(), + ) + .to_string() + } + }, + head, + ) } } // Propagate errors by explicitly matching them before the final case. @@ -243,6 +237,7 @@ mod tests { test_examples(SubCommand {}) } + #[derive(Debug)] struct Expectation<'a> { options: (isize, isize), expected: &'a str, @@ -266,18 +261,19 @@ mod tests { let word = Value::test_string("andres"); let cases = vec![ - expectation("a", (0, 1)), - expectation("an", (0, 2)), - expectation("and", (0, 3)), - expectation("andr", (0, 4)), - expectation("andre", (0, 5)), + expectation("a", (0, 0)), + expectation("an", (0, 1)), + expectation("and", (0, 2)), + expectation("andr", (0, 3)), + expectation("andre", (0, 4)), + expectation("andres", (0, 5)), expectation("andres", (0, 6)), - expectation("", (0, -6)), - expectation("a", (0, -5)), - expectation("an", (0, -4)), - expectation("and", (0, -3)), - expectation("andr", (0, -2)), - expectation("andre", (0, -1)), + expectation("a", (0, -6)), + expectation("an", (0, -5)), + expectation("and", (0, -4)), + expectation("andr", (0, -3)), + expectation("andre", (0, -2)), + expectation("andres", (0, -1)), // str substring [ -4 , _ ] // str substring -4 , expectation("dres", (-4, isize::MAX)), @@ -292,6 +288,7 @@ mod tests { ]; for expectation in &cases { + println!("{:?}", expectation); let expected = expectation.expected; let actual = action( &word, diff --git a/crates/nu-command/tests/commands/detect_columns.rs b/crates/nu-command/tests/commands/detect_columns.rs index 5bc057fa18..296d039b02 100644 --- a/crates/nu-command/tests/commands/detect_columns.rs +++ b/crates/nu-command/tests/commands/detect_columns.rs @@ -31,12 +31,12 @@ fn detect_columns_with_legacy_and_flag_c() { ( "$\"c1 c2 c3 c4 c5(char nl)a b c d e\"", "[[c1,c3,c4,c5]; ['a b',c,d,e]]", - "0..0", + "0..1", ), ( "$\"c1 c2 c3 c4 c5(char nl)a b c d e\"", "[[c1,c2,c3,c4]; [a,b,c,'d e']]", - "(-2)..(-2)", + "(-2)..(-1)", ), ( "$\"c1 c2 c3 c4 c5(char nl)a b c d e\"", @@ -72,10 +72,10 @@ drwxr-xr-x 2 root root 4.0K Mar 20 08:28 =(char nl) drwxr-xr-x 4 root root 4.0K Mar 20 08:18 ~(char nl) -rw-r--r-- 1 root root 3.0K Mar 20 07:23 ~asdf(char nl)\""; let expected = "[ -['column0', 'column1', 'column2', 'column3', 'column4', 'column5', 'column8']; -['drwxr-xr-x', '2', 'root', 'root', '4.0K', 'Mar 20 08:28', '='], -['drwxr-xr-x', '4', 'root', 'root', '4.0K', 'Mar 20 08:18', '~'], -['-rw-r--r--', '1', 'root', 'root', '3.0K', 'Mar 20 07:23', '~asdf'] +['column0', 'column1', 'column2', 'column3', 'column4', 'column5', 'column7', 'column8']; +['drwxr-xr-x', '2', 'root', 'root', '4.0K', 'Mar 20', '08:28', '='], +['drwxr-xr-x', '4', 'root', 'root', '4.0K', 'Mar 20', '08:18', '~'], +['-rw-r--r--', '1', 'root', 'root', '3.0K', 'Mar 20', '07:23', '~asdf'] ]"; let range = "5..6"; let cmd = format!( diff --git a/crates/nu-command/tests/commands/str_/mod.rs b/crates/nu-command/tests/commands/str_/mod.rs index 43f59b3e10..2aeabfd04a 100644 --- a/crates/nu-command/tests/commands/str_/mod.rs +++ b/crates/nu-command/tests/commands/str_/mod.rs @@ -255,7 +255,7 @@ fn substrings_the_input() { } #[test] -fn substring_errors_if_start_index_is_greater_than_end_index() { +fn substring_empty_if_start_index_is_greater_than_end_index() { Playground::setup("str_test_9", |dirs, sandbox| { sandbox.with_files(&[FileWithContent( "sample.toml", @@ -270,12 +270,10 @@ fn substring_errors_if_start_index_is_greater_than_end_index() { r#" open sample.toml | str substring 6..4 fortune.teller.phone + | get fortune.teller.phone "# )); - - assert!(actual - .err - .contains("End must be greater than or equal to Start")) + assert_eq!(actual.out, "") }) } @@ -375,6 +373,21 @@ fn substrings_the_input_and_treats_end_index_as_length_if_blank_end_index_given( }) } +#[test] +fn substring_by_negative_index() { + Playground::setup("str_test_13", |dirs, _| { + let actual = nu!( + cwd: dirs.test(), "'apples' | str substring 0..-1", + ); + assert_eq!(actual.out, "apples"); + + let actual = nu!( + cwd: dirs.test(), "'apples' | str substring 0..<-1", + ); + assert_eq!(actual.out, "apple"); + }) +} + #[test] fn str_reverse() { let actual = nu!(r#" From c171a2b8b7253bd0fb9c427a1076c3c548cd8bce Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Tue, 18 Jun 2024 17:34:18 +0200 Subject: [PATCH 33/38] Update `sys users` signature (#13172) Fixes #13171 # Description Corrects the `sys users` signature to match the returned type. Before this change `sys users | where name == root` would result in a type error. # User-Facing Changes # Tests + Formatting # After Submitting --- crates/nu-command/src/system/sys/users.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-command/src/system/sys/users.rs b/crates/nu-command/src/system/sys/users.rs index 7d4a43cae5..04d9b9db91 100644 --- a/crates/nu-command/src/system/sys/users.rs +++ b/crates/nu-command/src/system/sys/users.rs @@ -13,7 +13,7 @@ impl Command for SysUsers { fn signature(&self) -> Signature { Signature::build("sys users") .category(Category::System) - .input_output_types(vec![(Type::Nothing, Type::record())]) + .input_output_types(vec![(Type::Nothing, Type::table())]) } fn usage(&self) -> &str { From a894e9c2469d998f4d190516cc0510a1f302703b Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Tue, 18 Jun 2024 13:48:06 -0500 Subject: [PATCH 34/38] update try command's help (#13173) # Description This PR updates the `try` command to show that `catch` is a closure and can be used as such. ### Before ![image](https://github.com/nushell/nushell/assets/343840/dc330b10-cd68-4d70-9ff8-aa1e7cbda5f3) ### After ![image](https://github.com/nushell/nushell/assets/343840/146a7514-6026-4b53-bdf0-603c77c8a259) # User-Facing Changes # Tests + Formatting # After Submitting --- crates/nu-cmd-lang/src/core_commands/try_.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/try_.rs b/crates/nu-cmd-lang/src/core_commands/try_.rs index 0b399e368a..1f16f97908 100644 --- a/crates/nu-cmd-lang/src/core_commands/try_.rs +++ b/crates/nu-cmd-lang/src/core_commands/try_.rs @@ -10,7 +10,7 @@ impl Command for Try { } fn usage(&self) -> &str { - "Try to run a block, if it fails optionally run a catch block." + "Try to run a block, if it fails optionally run a catch closure." } fn signature(&self) -> nu_protocol::Signature { @@ -18,7 +18,7 @@ impl Command for Try { .input_output_types(vec![(Type::Any, Type::Any)]) .required("try_block", SyntaxShape::Block, "Block to run.") .optional( - "catch_block", + "catch_closure", SyntaxShape::Keyword( b"catch".to_vec(), Box::new(SyntaxShape::OneOf(vec![ @@ -26,7 +26,7 @@ impl Command for Try { SyntaxShape::Closure(Some(vec![SyntaxShape::Any])), ])), ), - "Block to run if try block fails.", + "Closure to run if try block fails.", ) .category(Category::Core) } @@ -85,9 +85,14 @@ impl Command for Try { }, Example { description: "Try to run a missing command", - example: "try { asdfasdf } catch { 'missing' } ", + example: "try { asdfasdf } catch { 'missing' }", result: Some(Value::test_string("missing")), }, + Example { + description: "Try to run a missing command and report the message", + example: "try { asdfasdf } catch { |err| $err.msg }", + result: None, + }, ] } } From 532c0023c24ef44fbad303cf8e0e9a0e32d0e29e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 19 Jun 2024 00:59:03 +0000 Subject: [PATCH 35/38] Bump crate-ci/typos from 1.22.4 to 1.22.7 (#13176) --- .github/workflows/typos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/typos.yml b/.github/workflows/typos.yml index f81f5c89a5..d33750e337 100644 --- a/.github/workflows/typos.yml +++ b/.github/workflows/typos.yml @@ -10,4 +10,4 @@ jobs: uses: actions/checkout@v4.1.6 - name: Check spelling - uses: crate-ci/typos@v1.22.4 + uses: crate-ci/typos@v1.22.7 From 103f59be52eadcb8f478264487e1f4834ce027bf Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 19 Jun 2024 01:09:25 +0000 Subject: [PATCH 36/38] Bump git2 from 0.18.3 to 0.19.0 (#13179) --- Cargo.lock | 8 ++++---- crates/nu_plugin_gstat/Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b88af73c25..0db346b785 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1687,9 +1687,9 @@ checksum = "4271d37baee1b8c7e4b708028c57d816cf9d2434acb33a549475f78c181f6253" [[package]] name = "git2" -version = "0.18.3" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "232e6a7bfe35766bf715e55a88b39a700596c0ccfd88cd3680b4cdb40d66ef70" +checksum = "b903b73e45dc0c6c596f2d37eccece7c1c8bb6e4407b001096387c63d0d93724" dependencies = [ "bitflags 2.5.0", "libc", @@ -2288,9 +2288,9 @@ dependencies = [ [[package]] name = "libgit2-sys" -version = "0.16.2+1.7.2" +version = "0.17.0+1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee4126d8b4ee5c9d9ea891dd875cfdc1e9d0950437179104b183d7d8a74d24e8" +checksum = "10472326a8a6477c3c20a64547b0059e4b0d086869eee31e6d7da728a8eb7224" dependencies = [ "cc", "libc", diff --git a/crates/nu_plugin_gstat/Cargo.toml b/crates/nu_plugin_gstat/Cargo.toml index 01e17fdb54..c356f3d7d3 100644 --- a/crates/nu_plugin_gstat/Cargo.toml +++ b/crates/nu_plugin_gstat/Cargo.toml @@ -19,4 +19,4 @@ bench = false nu-plugin = { path = "../nu-plugin", version = "0.94.3" } nu-protocol = { path = "../nu-protocol", version = "0.94.3" } -git2 = "0.18" +git2 = "0.19" From 12991cd36feedaac0ccb86be61e78a8ca4ffd261 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 18 Jun 2024 21:37:24 -0700 Subject: [PATCH 37/38] Change the error style during tests to `plain` (#13061) # Description This fixes issues with trying to run the tests with a terminal that is small enough to cause errors to wrap around, or in cases where the test environment might produce strings that are reasonably expected to wrap around anyway. "Fancy" errors are too fancy for tests to work predictably :wink: cc @abusch # User-Facing Changes - Added `--error-style` option for use with `--commands` (like `--table-mode`) # Tests + Formatting Surprisingly, all of the tests pass, including in small windows! I only had to make one change to a test for `error make` which was looking for the box drawing characters miette uses to determine whether the span label was showing up - but the plain error style output is even better and easier to match on, so this test is actually more specific now. --- benches/benchmarks.rs | 6 ++-- crates/nu-cli/src/eval_cmds.rs | 34 +++++++++++++++++-- crates/nu-cli/src/lib.rs | 2 +- .../nu-command/tests/commands/error_make.rs | 5 +-- crates/nu-test-support/src/macros.rs | 4 +++ src/command.rs | 16 +++++++-- src/run.rs | 9 +++-- 7 files changed, 62 insertions(+), 14 deletions(-) diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index 84552daef9..7a23715c8d 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -47,8 +47,7 @@ fn setup_stack_and_engine_from_command(command: &str) -> (Stack, EngineState) { &mut engine, &mut stack, PipelineData::empty(), - None, - false, + Default::default(), ) .unwrap(); @@ -90,8 +89,7 @@ fn bench_command( &mut engine, &mut stack, PipelineData::empty(), - None, - false, + Default::default(), ) .unwrap(), ); diff --git a/crates/nu-cli/src/eval_cmds.rs b/crates/nu-cli/src/eval_cmds.rs index 8fa3bf30e5..13141f6174 100644 --- a/crates/nu-cli/src/eval_cmds.rs +++ b/crates/nu-cli/src/eval_cmds.rs @@ -8,15 +8,45 @@ use nu_protocol::{ }; use std::sync::Arc; +#[derive(Default)] +pub struct EvaluateCommandsOpts { + pub table_mode: Option, + pub error_style: Option, + pub no_newline: bool, +} + /// Run a command (or commands) given to us by the user pub fn evaluate_commands( commands: &Spanned, engine_state: &mut EngineState, stack: &mut Stack, input: PipelineData, - table_mode: Option, - no_newline: bool, + opts: EvaluateCommandsOpts, ) -> Result<(), ShellError> { + let EvaluateCommandsOpts { + table_mode, + error_style, + no_newline, + } = opts; + + // Handle the configured error style early + if let Some(e_style) = error_style { + match e_style.coerce_str()?.parse() { + Ok(e_style) => { + Arc::make_mut(&mut engine_state.config).error_style = e_style; + } + Err(err) => { + return Err(ShellError::GenericError { + error: "Invalid value for `--error-style`".into(), + msg: err.into(), + span: Some(e_style.span()), + help: None, + inner: vec![], + }); + } + } + } + // Translate environment variables from Strings to Values convert_env_values(engine_state, stack)?; diff --git a/crates/nu-cli/src/lib.rs b/crates/nu-cli/src/lib.rs index c4342dc3a0..6f151adad1 100644 --- a/crates/nu-cli/src/lib.rs +++ b/crates/nu-cli/src/lib.rs @@ -17,7 +17,7 @@ mod validation; pub use commands::add_cli_context; pub use completions::{FileCompletion, NuCompleter, SemanticSuggestion, SuggestionKind}; pub use config_files::eval_config_contents; -pub use eval_cmds::evaluate_commands; +pub use eval_cmds::{evaluate_commands, EvaluateCommandsOpts}; pub use eval_file::evaluate_file; pub use menus::NuHelpCompleter; pub use nu_cmd_base::util::get_init_cwd; diff --git a/crates/nu-command/tests/commands/error_make.rs b/crates/nu-command/tests/commands/error_make.rs index 0c6908d2aa..f5714c88ac 100644 --- a/crates/nu-command/tests/commands/error_make.rs +++ b/crates/nu-command/tests/commands/error_make.rs @@ -4,8 +4,9 @@ use nu_test_support::nu; fn error_label_works() { let actual = nu!("error make {msg:foo label:{text:unseen}}"); - assert!(actual.err.contains("unseen")); - assert!(actual.err.contains("╰──")); + assert!(actual + .err + .contains("label at line 1, columns 1 to 10: unseen")); } #[test] diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index c18dd5ac81..51d743e1a4 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -283,6 +283,8 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef, with_std: bool) -> O if !with_std { command.arg("--no-std-lib"); } + // Use plain errors to help make error text matching more consistent + command.args(["--error-style", "plain"]); command .arg(format!("-c {}", escape_quote_string(&commands))) .stdout(Stdio::piped()) @@ -369,6 +371,8 @@ where .envs(envs) .arg("--commands") .arg(command) + // Use plain errors to help make error text matching more consistent + .args(["--error-style", "plain"]) .arg("--config") .arg(temp_config_file) .arg("--env-config") diff --git a/src/command.rs b/src/command.rs index 15da0e3e47..5ba8a60996 100644 --- a/src/command.rs +++ b/src/command.rs @@ -29,8 +29,10 @@ pub(crate) fn gather_commandline_args() -> (Vec, String, Vec) { } let flag_value = match arg.as_ref() { - "--commands" | "-c" | "--table-mode" | "-m" | "-e" | "--execute" | "--config" - | "--env-config" | "-I" | "ide-ast" => args.next().map(|a| escape_quote_string(&a)), + "--commands" | "-c" | "--table-mode" | "-m" | "--error-style" | "-e" | "--execute" + | "--config" | "--env-config" | "-I" | "ide-ast" => { + args.next().map(|a| escape_quote_string(&a)) + } #[cfg(feature = "plugin")] "--plugin-config" => args.next().map(|a| escape_quote_string(&a)), "--log-level" | "--log-target" | "--log-include" | "--log-exclude" | "--testbin" @@ -102,6 +104,8 @@ pub(crate) fn parse_commandline_args( let execute = call.get_flag_expr("execute"); let table_mode: Option = call.get_flag(engine_state, &mut stack, "table-mode")?; + let error_style: Option = + call.get_flag(engine_state, &mut stack, "error-style")?; let no_newline = call.get_named_arg("no-newline"); // ide flags @@ -245,6 +249,7 @@ pub(crate) fn parse_commandline_args( ide_check, ide_ast, table_mode, + error_style, no_newline, }); } @@ -278,6 +283,7 @@ pub(crate) struct NushellCliArgs { pub(crate) log_exclude: Option>>, pub(crate) execute: Option>, pub(crate) table_mode: Option, + pub(crate) error_style: Option, pub(crate) no_newline: Option>, pub(crate) include_path: Option>, pub(crate) lsp: bool, @@ -325,6 +331,12 @@ impl Command for Nu { "the table mode to use. rounded is default.", Some('m'), ) + .named( + "error-style", + SyntaxShape::String, + "the error style to use (fancy or plain). default: fancy", + None, + ) .switch("no-newline", "print the result for --commands(-c) without a newline", None) .switch( "no-config-file", diff --git a/src/run.rs b/src/run.rs index 2996bc76f8..db779b0058 100644 --- a/src/run.rs +++ b/src/run.rs @@ -7,7 +7,7 @@ use crate::{ use log::trace; #[cfg(feature = "plugin")] use nu_cli::read_plugin_file; -use nu_cli::{evaluate_commands, evaluate_file, evaluate_repl}; +use nu_cli::{evaluate_commands, evaluate_file, evaluate_repl, EvaluateCommandsOpts}; use nu_protocol::{ engine::{EngineState, Stack}, report_error_new, PipelineData, Spanned, @@ -114,8 +114,11 @@ pub(crate) fn run_commands( engine_state, &mut stack, input, - parsed_nu_cli_args.table_mode, - parsed_nu_cli_args.no_newline.is_some(), + EvaluateCommandsOpts { + table_mode: parsed_nu_cli_args.table_mode, + error_style: parsed_nu_cli_args.error_style, + no_newline: parsed_nu_cli_args.no_newline.is_some(), + }, ) { report_error_new(engine_state, &err); std::process::exit(1); From 44aa0a2de41295851a035fde3c3d5b7a2e3e7030 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Wed, 19 Jun 2024 21:12:25 -0400 Subject: [PATCH 38/38] Table help rendering (#13182) # Description Mostly fixes #13149 with much of the credit to @fdncred. This PR runs `table --expand` against `help` example results. This is essentially the same fix that #13146 was for `std help`. It also changes the shape of the result for the `table --expand` example, as it was hardcoded wrong. ~Still needed is a fix for the `table --collapse` example.~ Note that this is also still a bug in `std help` that I didn't noticed before. # User-Facing Changes Certain tables are now rendered correctly in the help examples for: * `table` * `zip` * `flatten` * And almost certainly others # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting --------- Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com> --- crates/nu-command/src/viewers/table.rs | 10 ++++++++-- crates/nu-engine/src/documentation.rs | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 2fe9319821..075a4e31c4 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -170,7 +170,10 @@ impl Command for Table { }), Value::test_record(record! { "a" => Value::test_int(3), - "b" => Value::test_int(4), + "b" => Value::test_list(vec![ + Value::test_int(4), + Value::test_int(4), + ]) }), ])), }, @@ -184,7 +187,10 @@ impl Command for Table { }), Value::test_record(record! { "a" => Value::test_int(3), - "b" => Value::test_int(4), + "b" => Value::test_list(vec![ + Value::test_int(4), + Value::test_int(4), + ]) }), ])), }, diff --git a/crates/nu-engine/src/documentation.rs b/crates/nu-engine/src/documentation.rs index 94cef3298e..a7d4950036 100644 --- a/crates/nu-engine/src/documentation.rs +++ b/crates/nu-engine/src/documentation.rs @@ -3,7 +3,7 @@ use nu_protocol::{ ast::{Argument, Call, Expr, Expression, RecordItem}, debugger::WithoutDebug, engine::{Command, EngineState, Stack, UNKNOWN_SPAN_ID}, - record, Category, Example, IntoPipelineData, PipelineData, Signature, Span, SpanId, + record, Category, Example, IntoPipelineData, PipelineData, Signature, Span, SpanId, Spanned, SyntaxShape, Type, Value, }; use std::{collections::HashMap, fmt::Write}; @@ -296,6 +296,28 @@ fn get_documentation( } if let Some(result) = &example.result { + let mut table_call = Call::new(Span::unknown()); + if example.example.ends_with("--collapse") { + // collapse the result + table_call.add_named(( + Spanned { + item: "collapse".to_string(), + span: Span::unknown(), + }, + None, + None, + )) + } else { + // expand the result + table_call.add_named(( + Spanned { + item: "expand".to_string(), + span: Span::unknown(), + }, + None, + None, + )) + } let table = engine_state .find_decl("table".as_bytes(), &[]) .and_then(|decl_id| { @@ -304,7 +326,7 @@ fn get_documentation( .run( engine_state, stack, - &Call::new(Span::new(0, 0)), + &table_call, PipelineData::Value(result.clone(), None), ) .ok()