From c4dca5fe03c0a33de0184564a62f37093b1c55d3 Mon Sep 17 00:00:00 2001 From: francesco-gaglione <94604837+francesco-gaglione@users.noreply.github.com> Date: Mon, 13 May 2024 15:37:53 +0200 Subject: [PATCH 01/19] Merged tests to produce a single binary (#12826) This PR should close #7147 # Description Merged src/tests into /tests to produce a single binary. ![image](https://github.com/nushell/nushell/assets/94604837/84726469-d447-4619-b6d1-2d1415d0f42e) # User-Facing Changes No user facing changes # Tests + Formatting Moved tests. Tollkit check pr pass. # After Submitting --------- Co-authored-by: Ian Manske --- CONTRIBUTING.md | 1 - src/main.rs | 2 -- tests/main.rs | 1 + tests/repl/mod.rs | 27 +++++++++++++++++++ {src/tests => tests/repl}/test_bits.rs | 2 +- {src/tests => tests/repl}/test_cell_path.rs | 2 +- {src/tests => tests/repl}/test_commandline.rs | 2 +- .../tests => tests/repl}/test_conditionals.rs | 2 +- {src/tests => tests/repl}/test_config.rs | 3 +-- {src/tests => tests/repl}/test_config_path.rs | 0 {src/tests => tests/repl}/test_converters.rs | 2 +- .../repl}/test_custom_commands.rs | 2 +- {src/tests => tests/repl}/test_engine.rs | 2 +- {src/tests => tests/repl}/test_env.rs | 2 +- {src/tests => tests/repl}/test_help.rs | 2 +- {src/tests => tests/repl}/test_hiding.rs | 2 +- {src/tests => tests/repl}/test_ide.rs | 2 +- {src/tests => tests/repl}/test_iteration.rs | 2 +- .../repl}/test_known_external.rs | 3 +-- {src/tests => tests/repl}/test_math.rs | 2 +- {src/tests => tests/repl}/test_modules.rs | 2 +- {src/tests => tests/repl}/test_parser.rs | 4 +-- {src/tests => tests/repl}/test_ranges.rs | 2 +- {src/tests => tests/repl}/test_regex.rs | 2 +- {src/tests => tests/repl}/test_signatures.rs | 2 +- {src/tests => tests/repl}/test_spread.rs | 2 +- {src/tests => tests/repl}/test_stdlib.rs | 2 +- {src/tests => tests/repl}/test_strings.rs | 2 +- .../repl}/test_table_operations.rs | 2 +- {src/tests => tests/repl}/test_type_check.rs | 2 +- {src => tests/repl}/tests.rs | 27 ------------------- 31 files changed, 53 insertions(+), 59 deletions(-) create mode 100644 tests/repl/mod.rs rename {src/tests => tests/repl}/test_bits.rs (97%) rename {src/tests => tests/repl}/test_cell_path.rs (98%) rename {src/tests => tests/repl}/test_commandline.rs (98%) rename {src/tests => tests/repl}/test_conditionals.rs (96%) rename {src/tests => tests/repl}/test_config.rs (98%) rename {src/tests => tests/repl}/test_config_path.rs (100%) rename {src/tests => tests/repl}/test_converters.rs (96%) rename {src/tests => tests/repl}/test_custom_commands.rs (98%) rename {src/tests => tests/repl}/test_engine.rs (99%) rename {src/tests => tests/repl}/test_env.rs (91%) rename {src/tests => tests/repl}/test_help.rs (94%) rename {src/tests => tests/repl}/test_hiding.rs (99%) rename {src/tests => tests/repl}/test_ide.rs (76%) rename {src/tests => tests/repl}/test_iteration.rs (94%) rename {src/tests => tests/repl}/test_known_external.rs (97%) rename {src/tests => tests/repl}/test_math.rs (98%) rename {src/tests => tests/repl}/test_modules.rs (98%) rename {src/tests => tests/repl}/test_parser.rs (99%) rename {src/tests => tests/repl}/test_ranges.rs (92%) rename {src/tests => tests/repl}/test_regex.rs (96%) rename {src/tests => tests/repl}/test_signatures.rs (99%) rename {src/tests => tests/repl}/test_spread.rs (98%) rename {src/tests => tests/repl}/test_stdlib.rs (86%) rename {src/tests => tests/repl}/test_strings.rs (98%) rename {src/tests => tests/repl}/test_table_operations.rs (99%) rename {src/tests => tests/repl}/test_type_check.rs (98%) rename {src => tests/repl}/tests.rs (89%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1ecda698c7..d5cf758b56 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,7 +55,6 @@ It is good practice to cover your changes with a test. Also, try to think about Tests can be found in different places: * `/tests` -* `src/tests` * command examples * crate-specific tests diff --git a/src/main.rs b/src/main.rs index 714b932df8..db0c80d4f2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,8 +7,6 @@ mod signals; #[cfg(unix)] mod terminal; mod test_bins; -#[cfg(test)] -mod tests; #[cfg(feature = "mimalloc")] #[global_allocator] diff --git a/tests/main.rs b/tests/main.rs index db44c4e019..6824f26f13 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -11,5 +11,6 @@ mod path; mod plugin_persistence; #[cfg(feature = "plugin")] mod plugins; +mod repl; mod scope; mod shell; diff --git a/tests/repl/mod.rs b/tests/repl/mod.rs new file mode 100644 index 0000000000..5ed7f29ba5 --- /dev/null +++ b/tests/repl/mod.rs @@ -0,0 +1,27 @@ +mod test_bits; +mod test_cell_path; +mod test_commandline; +mod test_conditionals; +mod test_config; +mod test_config_path; +mod test_converters; +mod test_custom_commands; +mod test_engine; +mod test_env; +mod test_help; +mod test_hiding; +mod test_ide; +mod test_iteration; +mod test_known_external; +mod test_math; +mod test_modules; +mod test_parser; +mod test_ranges; +mod test_regex; +mod test_signatures; +mod test_spread; +mod test_stdlib; +mod test_strings; +mod test_table_operations; +mod test_type_check; +mod tests; diff --git a/src/tests/test_bits.rs b/tests/repl/test_bits.rs similarity index 97% rename from src/tests/test_bits.rs rename to tests/repl/test_bits.rs index fdb672d023..410f48d83b 100644 --- a/src/tests/test_bits.rs +++ b/tests/repl/test_bits.rs @@ -1,4 +1,4 @@ -use crate::tests::{run_test, TestResult}; +use crate::repl::tests::{run_test, TestResult}; #[test] fn bits_and() -> TestResult { diff --git a/src/tests/test_cell_path.rs b/tests/repl/test_cell_path.rs similarity index 98% rename from src/tests/test_cell_path.rs rename to tests/repl/test_cell_path.rs index 56b6751638..c89c50259b 100644 --- a/src/tests/test_cell_path.rs +++ b/tests/repl/test_cell_path.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; // Tests for null / null / Value::Nothing #[test] diff --git a/src/tests/test_commandline.rs b/tests/repl/test_commandline.rs similarity index 98% rename from src/tests/test_commandline.rs rename to tests/repl/test_commandline.rs index 130faf933a..0443ee4d6c 100644 --- a/src/tests/test_commandline.rs +++ b/tests/repl/test_commandline.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn commandline_test_get_empty() -> TestResult { diff --git a/src/tests/test_conditionals.rs b/tests/repl/test_conditionals.rs similarity index 96% rename from src/tests/test_conditionals.rs rename to tests/repl/test_conditionals.rs index 20307ffe65..dd1e16c5a8 100644 --- a/src/tests/test_conditionals.rs +++ b/tests/repl/test_conditionals.rs @@ -1,4 +1,4 @@ -use crate::tests::{run_test, TestResult}; +use crate::repl::tests::{run_test, TestResult}; #[test] fn if_test1() -> TestResult { diff --git a/src/tests/test_config.rs b/tests/repl/test_config.rs similarity index 98% rename from src/tests/test_config.rs rename to tests/repl/test_config.rs index 96bd7f714d..005ae63171 100644 --- a/src/tests/test_config.rs +++ b/tests/repl/test_config.rs @@ -1,5 +1,4 @@ -use super::{fail_test, run_test, run_test_std}; -use crate::tests::TestResult; +use crate::repl::tests::{fail_test, run_test, run_test_std, TestResult}; #[test] fn mutate_nu_config() -> TestResult { diff --git a/src/tests/test_config_path.rs b/tests/repl/test_config_path.rs similarity index 100% rename from src/tests/test_config_path.rs rename to tests/repl/test_config_path.rs diff --git a/src/tests/test_converters.rs b/tests/repl/test_converters.rs similarity index 96% rename from src/tests/test_converters.rs rename to tests/repl/test_converters.rs index c41868539e..cfa165e95d 100644 --- a/src/tests/test_converters.rs +++ b/tests/repl/test_converters.rs @@ -1,4 +1,4 @@ -use crate::tests::{run_test, TestResult}; +use crate::repl::tests::{run_test, TestResult}; #[test] fn from_json_1() -> TestResult { diff --git a/src/tests/test_custom_commands.rs b/tests/repl/test_custom_commands.rs similarity index 98% rename from src/tests/test_custom_commands.rs rename to tests/repl/test_custom_commands.rs index 4cc81878e4..43a24fa250 100644 --- a/src/tests/test_custom_commands.rs +++ b/tests/repl/test_custom_commands.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, run_test_contains, TestResult}; +use crate::repl::tests::{fail_test, run_test, run_test_contains, TestResult}; use nu_test_support::nu; use pretty_assertions::assert_eq; diff --git a/src/tests/test_engine.rs b/tests/repl/test_engine.rs similarity index 99% rename from src/tests/test_engine.rs rename to tests/repl/test_engine.rs index 9f5be84763..c1eb919afa 100644 --- a/src/tests/test_engine.rs +++ b/tests/repl/test_engine.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; use rstest::rstest; #[test] diff --git a/src/tests/test_env.rs b/tests/repl/test_env.rs similarity index 91% rename from src/tests/test_env.rs rename to tests/repl/test_env.rs index 159b3d0a20..7963f7580b 100644 --- a/src/tests/test_env.rs +++ b/tests/repl/test_env.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; use nu_test_support::nu; #[test] diff --git a/src/tests/test_help.rs b/tests/repl/test_help.rs similarity index 94% rename from src/tests/test_help.rs rename to tests/repl/test_help.rs index b529f7cd19..16d168587a 100644 --- a/src/tests/test_help.rs +++ b/tests/repl/test_help.rs @@ -1,4 +1,4 @@ -use crate::tests::{run_test, TestResult}; +use crate::repl::tests::{run_test, TestResult}; use rstest::rstest; #[rstest] diff --git a/src/tests/test_hiding.rs b/tests/repl/test_hiding.rs similarity index 99% rename from src/tests/test_hiding.rs rename to tests/repl/test_hiding.rs index 81484cc51e..ee2016c54d 100644 --- a/src/tests/test_hiding.rs +++ b/tests/repl/test_hiding.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; // TODO: Test the use/hide tests also as separate lines in REPL (i.e., with merging the delta in between) #[test] diff --git a/src/tests/test_ide.rs b/tests/repl/test_ide.rs similarity index 76% rename from src/tests/test_ide.rs rename to tests/repl/test_ide.rs index b5e40cfe8d..45cc87aa4d 100644 --- a/src/tests/test_ide.rs +++ b/tests/repl/test_ide.rs @@ -1,4 +1,4 @@ -use crate::tests::{test_ide_contains, TestResult}; +use crate::repl::tests::{test_ide_contains, TestResult}; #[test] fn parser_recovers() -> TestResult { diff --git a/src/tests/test_iteration.rs b/tests/repl/test_iteration.rs similarity index 94% rename from src/tests/test_iteration.rs rename to tests/repl/test_iteration.rs index da3eef9ce2..bcc21b9ef9 100644 --- a/src/tests/test_iteration.rs +++ b/tests/repl/test_iteration.rs @@ -1,4 +1,4 @@ -use crate::tests::{run_test, TestResult}; +use crate::repl::tests::{run_test, TestResult}; #[test] fn better_block_types() -> TestResult { diff --git a/src/tests/test_known_external.rs b/tests/repl/test_known_external.rs similarity index 97% rename from src/tests/test_known_external.rs rename to tests/repl/test_known_external.rs index 586ee40cd3..c140f4ee90 100644 --- a/src/tests/test_known_external.rs +++ b/tests/repl/test_known_external.rs @@ -1,7 +1,6 @@ +use crate::repl::tests::{fail_test, run_test, run_test_contains, TestResult}; use std::process::Command; -use crate::tests::{fail_test, run_test, run_test_contains, TestResult}; - // cargo version prints a string of the form: // cargo 1.60.0 (d1fd9fe2c 2022-03-01) diff --git a/src/tests/test_math.rs b/tests/repl/test_math.rs similarity index 98% rename from src/tests/test_math.rs rename to tests/repl/test_math.rs index ed372b198a..af44f8a857 100644 --- a/src/tests/test_math.rs +++ b/tests/repl/test_math.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn add_simple() -> TestResult { diff --git a/src/tests/test_modules.rs b/tests/repl/test_modules.rs similarity index 98% rename from src/tests/test_modules.rs rename to tests/repl/test_modules.rs index 26ca62bf49..dd62c74b77 100644 --- a/src/tests/test_modules.rs +++ b/tests/repl/test_modules.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn module_def_imports_1() -> TestResult { diff --git a/src/tests/test_parser.rs b/tests/repl/test_parser.rs similarity index 99% rename from src/tests/test_parser.rs rename to tests/repl/test_parser.rs index 3d19f05a9c..c0e7279b7b 100644 --- a/src/tests/test_parser.rs +++ b/tests/repl/test_parser.rs @@ -1,9 +1,7 @@ -use crate::tests::{fail_test, run_test, run_test_with_env, TestResult}; +use crate::repl::tests::{fail_test, run_test, run_test_contains, run_test_with_env, TestResult}; use nu_test_support::{nu, nu_repl_code}; use std::collections::HashMap; -use super::run_test_contains; - #[test] fn env_shorthand() -> TestResult { run_test("FOO=BAR if false { 3 } else { 4 }", "4") diff --git a/src/tests/test_ranges.rs b/tests/repl/test_ranges.rs similarity index 92% rename from src/tests/test_ranges.rs rename to tests/repl/test_ranges.rs index 3a007196e6..96f59eea84 100644 --- a/src/tests/test_ranges.rs +++ b/tests/repl/test_ranges.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn int_in_inc_range() -> TestResult { diff --git a/src/tests/test_regex.rs b/tests/repl/test_regex.rs similarity index 96% rename from src/tests/test_regex.rs rename to tests/repl/test_regex.rs index adb59cc4a7..6f9063c81a 100644 --- a/src/tests/test_regex.rs +++ b/tests/repl/test_regex.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn contains() -> TestResult { diff --git a/src/tests/test_signatures.rs b/tests/repl/test_signatures.rs similarity index 99% rename from src/tests/test_signatures.rs rename to tests/repl/test_signatures.rs index 1ca395c90f..5e265974d5 100644 --- a/src/tests/test_signatures.rs +++ b/tests/repl/test_signatures.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn list_annotations() -> TestResult { diff --git a/src/tests/test_spread.rs b/tests/repl/test_spread.rs similarity index 98% rename from src/tests/test_spread.rs rename to tests/repl/test_spread.rs index 3dceea7089..419188be33 100644 --- a/src/tests/test_spread.rs +++ b/tests/repl/test_spread.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; use nu_test_support::nu; #[test] diff --git a/src/tests/test_stdlib.rs b/tests/repl/test_stdlib.rs similarity index 86% rename from src/tests/test_stdlib.rs rename to tests/repl/test_stdlib.rs index 3be252ef77..401fd3c0aa 100644 --- a/src/tests/test_stdlib.rs +++ b/tests/repl/test_stdlib.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test_std, TestResult}; +use crate::repl::tests::{fail_test, run_test_std, TestResult}; #[test] fn library_loaded() -> TestResult { diff --git a/src/tests/test_strings.rs b/tests/repl/test_strings.rs similarity index 98% rename from src/tests/test_strings.rs rename to tests/repl/test_strings.rs index 7d2ae1de84..cafd6cc681 100644 --- a/src/tests/test_strings.rs +++ b/tests/repl/test_strings.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn cjk_in_substrings() -> TestResult { diff --git a/src/tests/test_table_operations.rs b/tests/repl/test_table_operations.rs similarity index 99% rename from src/tests/test_table_operations.rs rename to tests/repl/test_table_operations.rs index 9971261190..5d61e0d902 100644 --- a/src/tests/test_table_operations.rs +++ b/tests/repl/test_table_operations.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn illegal_column_duplication() -> TestResult { diff --git a/src/tests/test_type_check.rs b/tests/repl/test_type_check.rs similarity index 98% rename from src/tests/test_type_check.rs rename to tests/repl/test_type_check.rs index ed912f8fff..87216e6672 100644 --- a/src/tests/test_type_check.rs +++ b/tests/repl/test_type_check.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::repl::tests::{fail_test, run_test, TestResult}; #[test] fn chained_operator_typecheck() -> TestResult { diff --git a/src/tests.rs b/tests/repl/tests.rs similarity index 89% rename from src/tests.rs rename to tests/repl/tests.rs index 487173b9b1..e3a5e89471 100644 --- a/src/tests.rs +++ b/tests/repl/tests.rs @@ -1,30 +1,3 @@ -mod test_bits; -mod test_cell_path; -mod test_commandline; -mod test_conditionals; -mod test_config; -mod test_config_path; -mod test_converters; -mod test_custom_commands; -mod test_engine; -mod test_env; -mod test_help; -mod test_hiding; -mod test_ide; -mod test_iteration; -mod test_known_external; -mod test_math; -mod test_modules; -mod test_parser; -mod test_ranges; -mod test_regex; -mod test_signatures; -mod test_spread; -mod test_stdlib; -mod test_strings; -mod test_table_operations; -mod test_type_check; - use assert_cmd::prelude::*; use pretty_assertions::assert_eq; use std::collections::HashMap; From 905ec88091a118ba953548ae12fbdbdf4831fd93 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Mon, 13 May 2024 13:45:44 +0000 Subject: [PATCH 02/19] Update PR template (#12838) # Description Updates the command listed in the PR template to test the standard library, following from #11151. --- .github/pull_request_template.md | 2 +- crates/nu-std/testing.nu | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 40cda47bbc..23d6a5ff95 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -26,7 +26,7 @@ Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) -- `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library +- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows diff --git a/crates/nu-std/testing.nu b/crates/nu-std/testing.nu index ddae1d371e..489430e462 100644 --- a/crates/nu-std/testing.nu +++ b/crates/nu-std/testing.nu @@ -287,7 +287,7 @@ export def run-tests [ --list, # list the selected tests without running them. --threads: int@"nu-complete threads", # Amount of threads to use for parallel execution. Default: All threads are utilized ] { - let available_threads = (sys | get cpu | length) + let available_threads = (sys cpu | length) # Can't use pattern matching here due to https://github.com/nushell/nushell/issues/9198 let threads = (if $threads == null { From aaf973bbba9c1478be78fd192ae01dabc701a9ee Mon Sep 17 00:00:00 2001 From: Piepmatz Date: Mon, 13 May 2024 20:48:38 +0200 Subject: [PATCH 03/19] Add Stack::stdout_file and Stack::stderr_file to capture stdout/-err of external commands (#12857) # Description In this PR I added two new methods to `Stack`, `stdout_file` and `stderr_file`. These two modify the inner `StackOutDest` and set a `File` into the `stdout` and `stderr` respectively. Different to the `push_redirection` methods, these do not require to hold a guard up all the time but require ownership of the stack. This is primarly useful for applications that use `nu` as a language but not the `nushell`. This PR replaces my first attempt #12851 to add a way to capture stdout/-err of external commands. Capturing the stdout without having to write into a file is possible with crates like [`os_pipe`](https://docs.rs/os_pipe), an example for this is given in the doc comment of the `stdout_file` command and can be executed as a doctest (although it doesn't validate that you actually got any data). This implementation takes `File` as input to make it easier to implement on different operating systems without having to worry about `OwnedHandle` or `OwnedFd`. Also this doesn't expose any use `os_pipe` to not leak its types into this API, making it depend on it. As in my previous attempt, @IanManske guided me here. # User-Facing Changes This change has no effect on `nushell` and therefore no user-facing changes. # Tests + Formatting This only exposes a new way of using already existing code and has therefore no further testing. The doctest succeeds on my machine at least (x86 Windows, 64 Bit). # After Submitting All the required documentation is already part of this PR. --- Cargo.lock | 1 + Cargo.toml | 2 +- crates/nu-protocol/Cargo.toml | 1 + crates/nu-protocol/src/engine/stack.rs | 52 ++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index f55ed5ceb4..4b1e1fadaf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3259,6 +3259,7 @@ dependencies = [ "nu-test-support", "nu-utils", "num-format", + "os_pipe", "pretty_assertions", "rmp-serde", "rstest", diff --git a/Cargo.toml b/Cargo.toml index beabd26e20..78db3877ea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -119,7 +119,7 @@ num-traits = "0.2" omnipath = "0.1" once_cell = "1.18" open = "5.1" -os_pipe = "1.1" +os_pipe = { version = "1.1", features = ["io_safety"] } pathdiff = "0.2" percent-encoding = "2" pretty_assertions = "1.4" diff --git a/crates/nu-protocol/Cargo.toml b/crates/nu-protocol/Cargo.toml index e2420a6e83..be45738447 100644 --- a/crates/nu-protocol/Cargo.toml +++ b/crates/nu-protocol/Cargo.toml @@ -47,6 +47,7 @@ nu-test-support = { path = "../nu-test-support", version = "0.93.1" } pretty_assertions = { workspace = true } rstest = { workspace = true } tempfile = { workspace = true } +os_pipe = { workspace = true } [package.metadata.docs.rs] all-features = true diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index b577d55270..65f2981fbf 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -7,6 +7,7 @@ use crate::{ }; use std::{ collections::{HashMap, HashSet}, + fs::File, sync::Arc, }; @@ -593,6 +594,57 @@ impl Stack { self } + /// Replaces the default stdout of the stack with a given file. + /// + /// This method configures the default stdout to redirect to a specified file. + /// It is primarily useful for applications using `nu` as a language, where the stdout of + /// external commands that are not explicitly piped can be redirected to a file. + /// + /// # Using Pipes + /// + /// For use in third-party applications pipes might be very useful as they allow using the + /// stdout of external commands for different uses. + /// For example the [`os_pipe`](https://docs.rs/os_pipe) crate provides a elegant way to to + /// access the stdout. + /// + /// ``` + /// # use std::{fs::File, io::{self, Read}, thread, error}; + /// # use nu_protocol::engine::Stack; + /// # + /// let (mut reader, writer) = os_pipe::pipe().unwrap(); + /// // Use a thread to avoid blocking the execution of the called command. + /// let reader = thread::spawn(move || { + /// let mut buf: Vec = Vec::new(); + /// reader.read_to_end(&mut buf)?; + /// Ok::<_, io::Error>(buf) + /// }); + /// + /// #[cfg(windows)] + /// let file = std::os::windows::io::OwnedHandle::from(writer).into(); + /// #[cfg(unix)] + /// let file = std::os::unix::io::OwnedFd::from(writer).into(); + /// + /// let stack = Stack::new().stdout_file(file); + /// + /// // Execute some nu code. + /// + /// drop(stack); // drop the stack so that the writer will be dropped too + /// let buf = reader.join().unwrap().unwrap(); + /// // Do with your buffer whatever you want. + /// ``` + pub fn stdout_file(mut self, file: File) -> Self { + self.out_dest.stdout = OutDest::File(Arc::new(file)); + self + } + + /// Replaces the default stderr of the stack with a given file. + /// + /// For more info, see [`stdout_file`](Self::stdout_file). + pub fn stderr_file(mut self, file: File) -> Self { + self.out_dest.stderr = OutDest::File(Arc::new(file)); + self + } + /// Set the PWD environment variable to `path`. /// /// This method accepts `path` with trailing slashes, but they're removed From 98369985b1252fe18cd50c0f87d38c09d7fb474a Mon Sep 17 00:00:00 2001 From: Jack Wright <56345+ayax79@users.noreply.github.com> Date: Mon, 13 May 2024 16:17:31 -0700 Subject: [PATCH 04/19] Allow custom value operations to work on eager and lazy dataframes interchangeably. (#12819) Fixes Bug #12809 The example that @maxim-uvarov posted now works as expected: Screenshot 2024-05-09 at 16 21 01 --- .../values/nu_dataframe/operations.rs | 2 +- .../values/nu_lazyframe/custom_value.rs | 48 +++++++++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/operations.rs b/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/operations.rs index 42d803b0e4..db5409aff2 100644 --- a/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/operations.rs +++ b/crates/nu_plugin_polars/src/dataframe/values/nu_dataframe/operations.rs @@ -27,7 +27,7 @@ impl NuDataFrame { let rhs_span = right.span(); match right { Value::Custom { .. } => { - let rhs = NuDataFrame::try_from_value(plugin, right)?; + let rhs = NuDataFrame::try_from_value_coerce(plugin, right, rhs_span)?; match (self.is_series(), rhs.is_series()) { (true, true) => { diff --git a/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/custom_value.rs b/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/custom_value.rs index 731d210dd8..b3cd9500dd 100644 --- a/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/custom_value.rs +++ b/crates/nu_plugin_polars/src/dataframe/values/nu_lazyframe/custom_value.rs @@ -7,7 +7,7 @@ use uuid::Uuid; use crate::{ values::{CustomValueSupport, NuDataFrame, PolarsPluginCustomValue}, - PolarsPlugin, + Cacheable, PolarsPlugin, }; use super::NuLazyFrame; @@ -76,11 +76,49 @@ impl PolarsPluginCustomValue for NuLazyFrameCustomValue { _engine: &EngineInterface, other_value: Value, ) -> Result, ShellError> { - // to compare, we need to convert to NuDataframe - let df = NuLazyFrame::try_from_custom_value(plugin, self)?; - let df = df.collect(other_value.span())?; + let eager = NuLazyFrame::try_from_custom_value(plugin, self)?.collect(Span::unknown())?; let other = NuDataFrame::try_from_value_coerce(plugin, &other_value, other_value.span())?; - let res = df.is_equal(&other); + let res = eager.is_equal(&other); Ok(res) } + + fn custom_value_operation( + &self, + plugin: &PolarsPlugin, + engine: &EngineInterface, + lhs_span: Span, + operator: nu_protocol::Spanned, + right: Value, + ) -> Result { + let eager = NuLazyFrame::try_from_custom_value(plugin, self)?.collect(Span::unknown())?; + Ok(eager + .compute_with_value(plugin, lhs_span, operator.item, operator.span, &right)? + .cache(plugin, engine, lhs_span)? + .into_value(lhs_span)) + } + + fn custom_value_follow_path_int( + &self, + plugin: &PolarsPlugin, + _engine: &EngineInterface, + _self_span: Span, + index: nu_protocol::Spanned, + ) -> Result { + let eager = NuLazyFrame::try_from_custom_value(plugin, self)?.collect(Span::unknown())?; + eager.get_value(index.item, index.span) + } + + fn custom_value_follow_path_string( + &self, + plugin: &PolarsPlugin, + engine: &EngineInterface, + self_span: Span, + column_name: nu_protocol::Spanned, + ) -> Result { + let eager = NuLazyFrame::try_from_custom_value(plugin, self)?.collect(Span::unknown())?; + let column = eager.column(&column_name.item, self_span)?; + Ok(column + .cache(plugin, engine, self_span)? + .into_value(self_span)) + } } From cd381b74e035a6b3d403e670e6d782cdea49309f Mon Sep 17 00:00:00 2001 From: Maxime Jacob Date: Mon, 13 May 2024 21:22:39 -0400 Subject: [PATCH 05/19] Fix improperly escaped strings in stor insert (#12820) - fixes #12764 Replaced the custom logic with values_to_sql method that is already used in crate::database. This will ensure that handling of parameters is the same between sqlite and stor. --- crates/nu-command/src/database/mod.rs | 2 +- crates/nu-command/src/database/values/mod.rs | 2 +- crates/nu-command/src/stor/insert.rs | 300 ++++++++++++++----- 3 files changed, 228 insertions(+), 76 deletions(-) diff --git a/crates/nu-command/src/database/mod.rs b/crates/nu-command/src/database/mod.rs index 4fddf6638e..22b92a81d3 100644 --- a/crates/nu-command/src/database/mod.rs +++ b/crates/nu-command/src/database/mod.rs @@ -5,7 +5,7 @@ use commands::add_commands_decls; pub use values::{ convert_sqlite_row_to_nu_value, convert_sqlite_value_to_nu_value, open_connection_in_memory, - open_connection_in_memory_custom, SQLiteDatabase, MEMORY_DB, + open_connection_in_memory_custom, values_to_sql, SQLiteDatabase, MEMORY_DB, }; use nu_protocol::engine::StateWorkingSet; diff --git a/crates/nu-command/src/database/values/mod.rs b/crates/nu-command/src/database/values/mod.rs index 4442ec0783..b9bd3d5d4c 100644 --- a/crates/nu-command/src/database/values/mod.rs +++ b/crates/nu-command/src/database/values/mod.rs @@ -3,5 +3,5 @@ pub mod sqlite; pub use sqlite::{ convert_sqlite_row_to_nu_value, convert_sqlite_value_to_nu_value, open_connection_in_memory, - open_connection_in_memory_custom, SQLiteDatabase, MEMORY_DB, + open_connection_in_memory_custom, values_to_sql, SQLiteDatabase, MEMORY_DB, }; diff --git a/crates/nu-command/src/stor/insert.rs b/crates/nu-command/src/stor/insert.rs index 2aeb076d44..e0c0ad4d28 100644 --- a/crates/nu-command/src/stor/insert.rs +++ b/crates/nu-command/src/stor/insert.rs @@ -1,5 +1,6 @@ -use crate::database::{SQLiteDatabase, MEMORY_DB}; +use crate::database::{values_to_sql, SQLiteDatabase, MEMORY_DB}; use nu_engine::command_prelude::*; +use rusqlite::params_from_iter; #[derive(Clone)] pub struct StorInsert; @@ -57,86 +58,81 @@ impl Command for StorInsert { // let config = engine_state.get_config(); let db = Box::new(SQLiteDatabase::new(std::path::Path::new(MEMORY_DB), None)); - if table_name.is_none() { - return Err(ShellError::MissingParameter { - param_name: "requires at table name".into(), - span, - }); - } - let new_table_name = table_name.unwrap_or("table".into()); - if let Ok(conn) = db.open_connection() { - match columns { - Some(record) => { - let mut create_stmt = format!("INSERT INTO {} ( ", new_table_name); - let cols = record.columns(); - cols.for_each(|col| { - create_stmt.push_str(&format!("{}, ", col)); - }); - if create_stmt.ends_with(", ") { - create_stmt.pop(); - create_stmt.pop(); - } + process(table_name, span, &db, columns)?; - create_stmt.push_str(") VALUES ( "); - let vals = record.values(); - vals.for_each(|val| match val { - Value::Int { val, .. } => { - create_stmt.push_str(&format!("{}, ", val)); - } - Value::Float { val, .. } => { - create_stmt.push_str(&format!("{}, ", val)); - } - Value::String { val, .. } => { - create_stmt.push_str(&format!("'{}', ", val)); - } - Value::Date { val, .. } => { - create_stmt.push_str(&format!("'{}', ", val)); - } - Value::Bool { val, .. } => { - create_stmt.push_str(&format!("{}, ", val)); - } - _ => { - // return Err(ShellError::UnsupportedInput { - // msg: format!("{} is not a valid datepart, expected one of year, month, day, hour, minute, second, millisecond, microsecond, nanosecond", part.item), - // input: "value originates from here".to_string(), - // msg_span: span, - // input_span: val.span(), - // }); - } - }); - if create_stmt.ends_with(", ") { - create_stmt.pop(); - create_stmt.pop(); - } - - create_stmt.push(')'); - - // dbg!(&create_stmt); - - conn.execute(&create_stmt, []) - .map_err(|err| ShellError::GenericError { - error: "Failed to open SQLite connection in memory from insert".into(), - msg: err.to_string(), - span: Some(Span::test_data()), - help: None, - inner: vec![], - })?; - } - None => { - return Err(ShellError::MissingParameter { - param_name: "requires at least one column".into(), - span: call.head, - }); - } - }; - } - // dbg!(db.clone()); Ok(Value::custom(db, span).into_pipeline_data()) } } +fn process( + table_name: Option, + span: Span, + db: &SQLiteDatabase, + columns: Option, +) -> Result<(), ShellError> { + if table_name.is_none() { + return Err(ShellError::MissingParameter { + param_name: "requires at table name".into(), + span, + }); + } + let new_table_name = table_name.unwrap_or("table".into()); + if let Ok(conn) = db.open_connection() { + match columns { + Some(record) => { + let mut create_stmt = format!("INSERT INTO {} ( ", new_table_name); + let cols = record.columns(); + cols.for_each(|col| { + create_stmt.push_str(&format!("{}, ", col)); + }); + if create_stmt.ends_with(", ") { + create_stmt.pop(); + create_stmt.pop(); + } + + // Values are set as placeholders. + create_stmt.push_str(") VALUES ( "); + for (index, _) in record.columns().enumerate() { + create_stmt.push_str(&format!("?{}, ", index + 1)); + } + + if create_stmt.ends_with(", ") { + create_stmt.pop(); + create_stmt.pop(); + } + + create_stmt.push(')'); + + // dbg!(&create_stmt); + + // Get the params from the passed values + let params = values_to_sql(record.values().cloned())?; + + conn.execute(&create_stmt, params_from_iter(params)) + .map_err(|err| ShellError::GenericError { + error: "Failed to open SQLite connection in memory from insert".into(), + msg: err.to_string(), + span: Some(Span::test_data()), + help: None, + inner: vec![], + })?; + } + None => { + return Err(ShellError::MissingParameter { + param_name: "requires at least one column".into(), + span, + }); + } + }; + } + // dbg!(db.clone()); + Ok(()) +} + #[cfg(test)] mod test { + use chrono::DateTime; + use super::*; #[test] @@ -145,4 +141,160 @@ mod test { test_examples(StorInsert {}) } + + #[test] + fn test_process_with_simple_parameters() { + let db = Box::new(SQLiteDatabase::new(std::path::Path::new(MEMORY_DB), None)); + let create_stmt = "CREATE TABLE test_process_with_simple_parameters ( + int_column INTEGER, + real_column REAL, + str_column VARCHAR(255), + bool_column BOOLEAN, + date_column DATETIME DEFAULT(STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')) + )"; + + let conn = db + .open_connection() + .expect("Test was unable to open connection."); + conn.execute(create_stmt, []) + .expect("Failed to create table as part of test."); + let table_name = Some("test_process_with_simple_parameters".to_string()); + let span = Span::unknown(); + let mut columns = Record::new(); + columns.insert("int_column".to_string(), Value::test_int(42)); + columns.insert("real_column".to_string(), Value::test_float(3.1)); + columns.insert( + "str_column".to_string(), + Value::test_string("SimpleString".to_string()), + ); + columns.insert("bool_column".to_string(), Value::test_bool(true)); + columns.insert( + "date_column".to_string(), + Value::test_date( + DateTime::parse_from_str("2021-12-30 00:00:00 +0000", "%Y-%m-%d %H:%M:%S %z") + .expect("Date string should parse."), + ), + ); + + let result = process(table_name, span, &db, Some(columns)); + + assert!(result.is_ok()); + } + + #[test] + fn test_process_string_with_space() { + let db = Box::new(SQLiteDatabase::new(std::path::Path::new(MEMORY_DB), None)); + let create_stmt = "CREATE TABLE test_process_string_with_space ( + str_column VARCHAR(255) + )"; + + let conn = db + .open_connection() + .expect("Test was unable to open connection."); + conn.execute(create_stmt, []) + .expect("Failed to create table as part of test."); + let table_name = Some("test_process_string_with_space".to_string()); + let span = Span::unknown(); + let mut columns = Record::new(); + columns.insert( + "str_column".to_string(), + Value::test_string("String With Spaces".to_string()), + ); + + let result = process(table_name, span, &db, Some(columns)); + + assert!(result.is_ok()); + } + + #[test] + fn test_no_errors_when_string_too_long() { + let db = Box::new(SQLiteDatabase::new(std::path::Path::new(MEMORY_DB), None)); + let create_stmt = "CREATE TABLE test_errors_when_string_too_long ( + str_column VARCHAR(8) + )"; + + let conn = db + .open_connection() + .expect("Test was unable to open connection."); + conn.execute(create_stmt, []) + .expect("Failed to create table as part of test."); + let table_name = Some("test_errors_when_string_too_long".to_string()); + let span = Span::unknown(); + let mut columns = Record::new(); + columns.insert( + "str_column".to_string(), + Value::test_string("ThisIsALongString".to_string()), + ); + + let result = process(table_name, span, &db, Some(columns)); + // SQLite uses dynamic typing, making any length acceptable for a varchar column + assert!(result.is_ok()); + } + + #[test] + fn test_no_errors_when_param_is_wrong_type() { + let db = Box::new(SQLiteDatabase::new(std::path::Path::new(MEMORY_DB), None)); + let create_stmt = "CREATE TABLE test_errors_when_param_is_wrong_type ( + int_column INT + )"; + + let conn = db + .open_connection() + .expect("Test was unable to open connection."); + conn.execute(create_stmt, []) + .expect("Failed to create table as part of test."); + let table_name = Some("test_errors_when_param_is_wrong_type".to_string()); + let span = Span::unknown(); + let mut columns = Record::new(); + columns.insert( + "int_column".to_string(), + Value::test_string("ThisIsTheWrongType".to_string()), + ); + + let result = process(table_name, span, &db, Some(columns)); + // SQLite uses dynamic typing, making any type acceptable for a column + assert!(result.is_ok()); + } + + #[test] + fn test_errors_when_column_doesnt_exist() { + let db = Box::new(SQLiteDatabase::new(std::path::Path::new(MEMORY_DB), None)); + let create_stmt = "CREATE TABLE test_errors_when_column_doesnt_exist ( + int_column INT + )"; + + let conn = db + .open_connection() + .expect("Test was unable to open connection."); + conn.execute(create_stmt, []) + .expect("Failed to create table as part of test."); + let table_name = Some("test_errors_when_column_doesnt_exist".to_string()); + let span = Span::unknown(); + let mut columns = Record::new(); + columns.insert( + "not_a_column".to_string(), + Value::test_string("ThisIsALongString".to_string()), + ); + + let result = process(table_name, span, &db, Some(columns)); + + assert!(result.is_err()); + } + + #[test] + fn test_errors_when_table_doesnt_exist() { + let db = Box::new(SQLiteDatabase::new(std::path::Path::new(MEMORY_DB), None)); + + let table_name = Some("test_errors_when_table_doesnt_exist".to_string()); + let span = Span::unknown(); + let mut columns = Record::new(); + columns.insert( + "str_column".to_string(), + Value::test_string("ThisIsALongString".to_string()), + ); + + let result = process(table_name, span, &db, Some(columns)); + + assert!(result.is_err()); + } } From 2ed77aef1dc9241c169c9e441aeb53f5a01932a0 Mon Sep 17 00:00:00 2001 From: Maxime Jacob Date: Tue, 14 May 2024 10:13:49 -0400 Subject: [PATCH 06/19] Fix panic when exploring empty dictionary (#12860) - fixes #12841 # Description Add boundary checks to ensure that the row and column chosen in RecordView are not over the length of the possible row and columns. If we are out of bounds, we default to Value::nothing. # Tests + Formatting Tests ran and formatting done --- crates/nu-explore/src/views/record/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/nu-explore/src/views/record/mod.rs b/crates/nu-explore/src/views/record/mod.rs index 6534ba3589..0ed8e3e761 100644 --- a/crates/nu-explore/src/views/record/mod.rs +++ b/crates/nu-explore/src/views/record/mod.rs @@ -20,7 +20,7 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use nu_color_config::{get_color_map, StyleComputer}; use nu_protocol::{ engine::{EngineState, Stack}, - Record, Value, + Record, Span, Value, }; use ratatui::{layout::Rect, widgets::Block}; use std::{borrow::Cow, collections::HashMap}; @@ -180,7 +180,11 @@ impl<'a> RecordView<'a> { Orientation::Left => (column, row), }; - layer.records[row][column].clone() + if layer.records.len() > row && layer.records[row].len() > column { + layer.records[row][column].clone() + } else { + Value::nothing(Span::unknown()) + } } fn create_tablew(&'a self, cfg: ViewConfig<'a>) -> TableW<'a> { From aa46bc97b354d28c14c5ad2c51c9eb7f94276bec Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Tue, 14 May 2024 10:21:50 -0400 Subject: [PATCH 07/19] Search terms for compact command (#12864) # Description There was a question in Discord today about how to remove empty rows from a table. The user found the `compact` command on their own, but I realized that there were no search terms on the command. I've added 'empty' and 'remove', although I subsequently figured out that 'empty' is found in the "usage" anyway. That said, I don't think it hurts to have good search terms behind it regardless. # User-Facing Changes Just the help # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting --- crates/nu-command/src/filters/compact.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/nu-command/src/filters/compact.rs b/crates/nu-command/src/filters/compact.rs index d8872448be..9a4e968e9b 100644 --- a/crates/nu-command/src/filters/compact.rs +++ b/crates/nu-command/src/filters/compact.rs @@ -31,6 +31,10 @@ impl Command for Compact { "Creates a table with non-empty rows." } + fn search_terms(&self) -> Vec<&str> { + vec!["empty", "remove"] + } + fn run( &self, engine_state: &EngineState, From c3da44cbb727348411d3520bf2b186860496f8da Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Tue, 14 May 2024 21:10:06 +0000 Subject: [PATCH 08/19] Fix `char` panic (#12867) # Description The `char` command can panic due to a failed `expect`: `char --integer ...[77 78 79]` This PR fixes the panic for the `--integer` flag and also the `--unicode` flag. # After Submitting Check other commands and places where similar bugs can occur due to usages of `Call::positional_nth` and related methods. --- crates/nu-command/src/strings/char_.rs | 90 ++++++++++++-------------- 1 file changed, 41 insertions(+), 49 deletions(-) diff --git a/crates/nu-command/src/strings/char_.rs b/crates/nu-command/src/strings/char_.rs index e227f2667f..7d8d152b56 100644 --- a/crates/nu-command/src/strings/char_.rs +++ b/crates/nu-command/src/strings/char_.rs @@ -235,18 +235,18 @@ impl Command for Char { // handle -i flag if integer { - let int_args: Vec = call.rest_const(working_set, 0)?; - handle_integer_flag(int_args, call, call_span) + let int_args = call.rest_const(working_set, 0)?; + handle_integer_flag(int_args, call_span) } // handle -u flag else if unicode { - let string_args: Vec = call.rest_const(working_set, 0)?; - handle_unicode_flag(string_args, call, call_span) + let string_args = call.rest_const(working_set, 0)?; + handle_unicode_flag(string_args, call_span) } // handle the rest else { - let string_args: Vec = call.rest_const(working_set, 0)?; - handle_the_rest(string_args, call, call_span) + let string_args = call.rest_const(working_set, 0)?; + handle_the_rest(string_args, call_span) } } @@ -270,18 +270,18 @@ impl Command for Char { // handle -i flag if integer { - let int_args: Vec = call.rest(engine_state, stack, 0)?; - handle_integer_flag(int_args, call, call_span) + let int_args = call.rest(engine_state, stack, 0)?; + handle_integer_flag(int_args, call_span) } // handle -u flag else if unicode { - let string_args: Vec = call.rest(engine_state, stack, 0)?; - handle_unicode_flag(string_args, call, call_span) + let string_args = call.rest(engine_state, stack, 0)?; + handle_unicode_flag(string_args, call_span) } // handle the rest else { - let string_args: Vec = call.rest(engine_state, stack, 0)?; - handle_the_rest(string_args, call, call_span) + let string_args = call.rest(engine_state, stack, 0)?; + handle_the_rest(string_args, call_span) } } } @@ -309,8 +309,7 @@ fn generate_character_list(ctrlc: Option>, call_span: Span) -> P } fn handle_integer_flag( - int_args: Vec, - call: &Call, + int_args: Vec>, call_span: Span, ) -> Result { if int_args.is_empty() { @@ -319,20 +318,17 @@ fn handle_integer_flag( span: call_span, }); } - let mut multi_byte = String::new(); - for (i, &arg) in int_args.iter().enumerate() { - let span = call - .positional_nth(i) - .expect("Unexpected missing argument") - .span; - multi_byte.push(integer_to_unicode_char(arg, span)?) - } - Ok(Value::string(multi_byte, call_span).into_pipeline_data()) + + let str = int_args + .into_iter() + .map(integer_to_unicode_char) + .collect::>()?; + + Ok(Value::string(str, call_span).into_pipeline_data()) } fn handle_unicode_flag( - string_args: Vec, - call: &Call, + string_args: Vec>, call_span: Span, ) -> Result { if string_args.is_empty() { @@ -341,57 +337,53 @@ fn handle_unicode_flag( span: call_span, }); } - let mut multi_byte = String::new(); - for (i, arg) in string_args.iter().enumerate() { - let span = call - .positional_nth(i) - .expect("Unexpected missing argument") - .span; - multi_byte.push(string_to_unicode_char(arg, span)?) - } - Ok(Value::string(multi_byte, call_span).into_pipeline_data()) + + let str = string_args + .into_iter() + .map(string_to_unicode_char) + .collect::>()?; + + Ok(Value::string(str, call_span).into_pipeline_data()) } fn handle_the_rest( - string_args: Vec, - call: &Call, + string_args: Vec>, call_span: Span, ) -> Result { - if string_args.is_empty() { + let Some(s) = string_args.first() else { return Err(ShellError::MissingParameter { param_name: "missing name of the character".into(), span: call_span, }); - } - let special_character = str_to_character(&string_args[0]); + }; + + let special_character = str_to_character(&s.item); + if let Some(output) = special_character { Ok(Value::string(output, call_span).into_pipeline_data()) } else { Err(ShellError::TypeMismatch { err_message: "error finding named character".into(), - span: call - .positional_nth(0) - .expect("Unexpected missing argument") - .span, + span: s.span, }) } } -fn integer_to_unicode_char(value: i64, t: Span) -> Result { - let decoded_char = value.try_into().ok().and_then(std::char::from_u32); +fn integer_to_unicode_char(value: Spanned) -> Result { + let decoded_char = value.item.try_into().ok().and_then(std::char::from_u32); if let Some(ch) = decoded_char { Ok(ch) } else { Err(ShellError::TypeMismatch { err_message: "not a valid Unicode codepoint".into(), - span: t, + span: value.span, }) } } -fn string_to_unicode_char(s: &str, t: Span) -> Result { - let decoded_char = u32::from_str_radix(s, 16) +fn string_to_unicode_char(s: Spanned) -> Result { + let decoded_char = u32::from_str_radix(&s.item, 16) .ok() .and_then(std::char::from_u32); @@ -400,7 +392,7 @@ fn string_to_unicode_char(s: &str, t: Span) -> Result { } else { Err(ShellError::TypeMismatch { err_message: "error decoding Unicode character".into(), - span: t, + span: s.span, }) } } From cb64c78a3b7a7b56a70a884096472531ccdf4c02 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 15 May 2024 09:05:55 +0800 Subject: [PATCH 09/19] Bump interprocess from 2.0.1 to 2.1.0 (#12869) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [interprocess](https://github.com/kotauskas/interprocess) from 2.0.1 to 2.1.0.
Release notes

Sourced from interprocess's releases.

2.1.0 – listeners are now iterators

  • Fixes #49
  • Adds Iterator impl on local socket listeners (closes #64)
  • Miscellaneous documentation fixes
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=interprocess&package-manager=cargo&previous-version=2.0.1&new-version=2.1.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> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4b1e1fadaf..e3d29fef42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2034,9 +2034,9 @@ dependencies = [ [[package]] name = "interprocess" -version = "2.0.1" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c7fb8583fab9503654385e2bafda123376445a77027a1b106dd7e44cf51122f" +checksum = "7b4d0250d41da118226e55b3d50ca3f0d9e0a0f6829b92f543ac0054aeea1572" dependencies = [ "libc", "recvmsg", diff --git a/Cargo.toml b/Cargo.toml index 78db3877ea..6b035ee0f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,7 +94,7 @@ heck = "0.5.0" human-date-parser = "0.1.1" indexmap = "2.2" indicatif = "0.17" -interprocess = "2.0.1" +interprocess = "2.1.0" is_executable = "1.0" itertools = "0.12" libc = "0.2" From 9bf4d3ece6484d6aeb23422447ec6b11b6fb7a7e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 15 May 2024 09:06:09 +0800 Subject: [PATCH 10/19] Bump rust-embed from 8.3.0 to 8.4.0 (#12870) Bumps [rust-embed](https://github.com/pyros2097/rust-embed) from 8.3.0 to 8.4.0.
Changelog

Sourced from rust-embed's changelog.

[8.4.0] - 2024-05-11

  • Re-export RustEmbed as Embed #245. Thanks to pyrossh
  • Do not build glob matchers repeatedly when include-exclude feature is enabled #244. Thanks to osiewicz
  • Add metadata_only attribute #241. Thanks to ddfisher
  • Replace expect with a safer alternative that returns None instead #240. Thanks to costinsin
  • Eliminate unnecessary to_path call #239. Thanks to smoelius
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=rust-embed&package-manager=cargo&previous-version=8.3.0&new-version=8.4.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> --- Cargo.lock | 12 ++++++------ Cargo.toml | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e3d29fef42..656f859a63 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5057,9 +5057,9 @@ dependencies = [ [[package]] name = "rust-embed" -version = "8.3.0" +version = "8.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb78f46d0066053d16d4ca7b898e9343bc3530f71c61d5ad84cd404ada068745" +checksum = "19549741604902eb99a7ed0ee177a0663ee1eda51a29f71401f166e47e77806a" dependencies = [ "rust-embed-impl", "rust-embed-utils", @@ -5068,9 +5068,9 @@ dependencies = [ [[package]] name = "rust-embed-impl" -version = "8.3.0" +version = "8.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b91ac2a3c6c0520a3fb3dd89321177c3c692937c4eb21893378219da10c44fc8" +checksum = "cb9f96e283ec64401f30d3df8ee2aaeb2561f34c824381efa24a35f79bf40ee4" dependencies = [ "proc-macro2", "quote", @@ -5081,9 +5081,9 @@ dependencies = [ [[package]] name = "rust-embed-utils" -version = "8.3.0" +version = "8.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "86f69089032567ffff4eada41c573fc43ff466c7db7c5688b2e7969584345581" +checksum = "38c74a686185620830701348de757fd36bef4aa9680fd23c49fc539ddcc1af32" dependencies = [ "sha2", "walkdir", diff --git a/Cargo.toml b/Cargo.toml index 6b035ee0f8..1366ecbdd8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -140,7 +140,7 @@ ropey = "1.6.1" roxmltree = "0.19" rstest = { version = "0.18", default-features = false } rusqlite = "0.31" -rust-embed = "8.3.0" +rust-embed = "8.4.0" same-file = "1.0" serde = { version = "1.0", default-features = false } serde_json = "1.0" From 155934f783e15c6ffb3a42119105799a625f7bbc Mon Sep 17 00:00:00 2001 From: Wind Date: Wed, 15 May 2024 09:14:11 +0800 Subject: [PATCH 11/19] make better messages for incomplete string (#12868) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Fixes: #12795 The issue is caused by an empty position of `ParseError::UnexpectedEof`. So no detailed message is displayed. To fix the issue, I adjust the start of span to `span.end - 1`. In this way, we can make sure that it never points to an empty position. After lexing item, I also reorder the unclosed character checking . Now it will be checking unclosed opening delimiters first. # User-Facing Changes After this pr, it outputs detailed error message for incomplete string when running scripts. ## Before ``` ❯ nu -c "'ab" Error: nu::parser::unexpected_eof × Unexpected end of code. ╭─[source:1:4] 1 │ 'ab ╰──── > ./target/debug/nu -c "r#'ab" Error: nu::parser::unexpected_eof × Unexpected end of code. ╭─[source:1:6] 1 │ r#'ab ╰──── ``` ## After ``` > nu -c "'ab" Error: nu::parser::unexpected_eof × Unexpected end of code. ╭─[source:1:3] 1 │ 'ab · ┬ · ╰── expected closing ' ╰──── > ./target/debug/nu -c "r#'ab" Error: nu::parser::unexpected_eof × Unexpected end of code. ╭─[source:1:5] 1 │ r#'ab · ┬ · ╰── expected closing '# ╰──── ``` # Tests + Formatting Added some tests for incomplete string. --------- Co-authored-by: Ian Manske --- crates/nu-parser/src/lex.rs | 42 ++++++++++++++++++++----------------- tests/repl/test_strings.rs | 16 ++++++++++++-- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index f9e75d13f0..4639cae25d 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -129,7 +129,7 @@ pub fn lex_item( }, Some(ParseError::UnexpectedEof( (start as char).to_string(), - Span::new(span.end, span.end), + Span::new(span.end - 1, span.end), )), ); } @@ -247,21 +247,6 @@ pub fn lex_item( let span = Span::new(span_offset + token_start, span_offset + *curr_offset); - // If there is still unclosed opening delimiters, remember they were missing - if let Some(block) = block_level.last() { - let delim = block.closing(); - let cause = - ParseError::UnexpectedEof((delim as char).to_string(), Span::new(span.end, span.end)); - - return ( - Token { - contents: TokenContents::Item, - span, - }, - Some(cause), - ); - } - if let Some(delim) = quote_start { // The non-lite parse trims quotes on both sides, so we add the expected quote so that // anyone wanting to consume this partial parse (e.g., completions) will be able to get @@ -273,11 +258,28 @@ pub fn lex_item( }, Some(ParseError::UnexpectedEof( (delim as char).to_string(), - Span::new(span.end, span.end), + Span::new(span.end - 1, span.end), )), ); } + // If there is still unclosed opening delimiters, remember they were missing + if let Some(block) = block_level.last() { + let delim = block.closing(); + let cause = ParseError::UnexpectedEof( + (delim as char).to_string(), + Span::new(span.end - 1, span.end), + ); + + return ( + Token { + contents: TokenContents::Item, + span, + }, + Some(cause), + ); + } + // If we didn't accumulate any characters, it's an unexpected error. if *curr_offset - token_start == 0 { return ( @@ -395,9 +397,11 @@ fn lex_raw_string( *curr_offset += 1 } if !matches { + let mut expected = '\''.to_string(); + expected.push_str(&"#".repeat(prefix_sharp_cnt)); return Err(ParseError::UnexpectedEof( - "#".to_string(), - Span::new(span_offset + *curr_offset, span_offset + *curr_offset), + expected, + Span::new(span_offset + *curr_offset - 1, span_offset + *curr_offset), )); } Ok(()) diff --git a/tests/repl/test_strings.rs b/tests/repl/test_strings.rs index cafd6cc681..fa6c419e23 100644 --- a/tests/repl/test_strings.rs +++ b/tests/repl/test_strings.rs @@ -154,6 +154,18 @@ fn raw_string_inside_closure() -> TestResult { } #[test] -fn incomplete_raw_string() -> TestResult { - fail_test("r#abc", "expected '") +fn incomplete_string() -> TestResult { + fail_test("r#abc", "expected '")?; + fail_test("r#'bc", "expected closing '#")?; + fail_test("'ab\"", "expected closing '")?; + fail_test("\"ab'", "expected closing \"")?; + fail_test( + r#"def func [] { + { + "A": ""B" # <- the quote is bad + } +} +"#, + "expected closing \"", + ) } From a7807735b15ad377f281091a85afb3791d9df608 Mon Sep 17 00:00:00 2001 From: Andy Gayton Date: Tue, 14 May 2024 21:48:27 -0400 Subject: [PATCH 12/19] Add a passing test for interactivity on slow pipelines (#12865) # Description This PR adds a single test to assert interactivity on slow pipelines Currently the timeout is set to 6 seconds, as the test can sometimes take ~3secs to run on my local m1 mac air, which I don't think is an indication of a slow pipeline, but rather slow test start up time... --- tests/plugins/stream.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/plugins/stream.rs b/tests/plugins/stream.rs index ee62703017..8530e5bc32 100644 --- a/tests/plugins/stream.rs +++ b/tests/plugins/stream.rs @@ -1,3 +1,5 @@ +use rstest::rstest; + use nu_test_support::nu_with_plugins; use pretty_assertions::assert_eq; @@ -190,3 +192,18 @@ fn generate_sequence() { assert_eq!(actual.out, "[0,2,4,6,8,10]"); } + +#[rstest] +#[timeout(std::time::Duration::from_secs(6))] +fn echo_interactivity_on_slow_pipelines() { + // This test works by putting 0 on the upstream immediately, followed by 1 after 10 seconds. + // If values aren't streamed to the plugin as they become available, `example echo` won't emit + // anything until both 0 and 1 are available. The desired behavior is that `example echo` gets + // the 0 immediately, which is consumed by `first`, allowing the pipeline to terminate early. + let actual = nu_with_plugins!( + cwd: "tests/fixtures/formats", + plugin: ("nu_plugin_example"), + r#"[1] | each { |n| sleep 10sec; $n } | prepend 0 | example echo | first"# + ); + assert_eq!(actual.out, "0"); +} From 0cfbdc909e7e04138881eee714eaeea762e4a5c5 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Wed, 15 May 2024 07:40:04 +0000 Subject: [PATCH 13/19] Fix `sys` panic (#12846) # Description This should fix #10155 where the `sys` command can panic due to date math in certain cases / on certain systems. # User-Facing Changes The `boot_time` column now has a date value instead of a formatted date string. This is technically a breaking change. --- crates/nu-command/src/system/sys/mod.rs | 34 +++++++++++++++---------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/crates/nu-command/src/system/sys/mod.rs b/crates/nu-command/src/system/sys/mod.rs index 5e0467c251..fcb40fd37d 100644 --- a/crates/nu-command/src/system/sys/mod.rs +++ b/crates/nu-command/src/system/sys/mod.rs @@ -16,9 +16,8 @@ pub use sys_::Sys; pub use temp::SysTemp; pub use users::SysUsers; -use chrono::{DateTime, Local}; +use chrono::{DateTime, FixedOffset, Local}; use nu_protocol::{record, Record, Span, Value}; -use std::time::{Duration, UNIX_EPOCH}; use sysinfo::{ Components, CpuRefreshKind, Disks, Networks, System, Users, MINIMUM_CPU_UPDATE_INTERVAL, }; @@ -171,22 +170,31 @@ pub fn host(span: Span) -> Record { record.push("hostname", Value::string(trim_cstyle_null(hostname), span)); } - record.push( - "uptime", - Value::duration(1000000000 * System::uptime() as i64, span), - ); + let uptime = System::uptime() + .saturating_mul(1_000_000_000) + .try_into() + .unwrap_or(i64::MAX); - // Creates a new SystemTime from the specified number of whole seconds - let d = UNIX_EPOCH + Duration::from_secs(System::boot_time()); - // Create DateTime from SystemTime - let datetime = DateTime::::from(d); - // Convert to local time and then rfc3339 - let timestamp_str = datetime.with_timezone(datetime.offset()).to_rfc3339(); - record.push("boot_time", Value::string(timestamp_str, span)); + 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() From defed3001da941e76f3df059544f689fe85c6cc0 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Wed, 15 May 2024 09:44:09 -0500 Subject: [PATCH 14/19] make it clearer what is being loaded with --log-level info (#12875) # Description A common question we get is what config files are loaded when and with what parameters. It's for this reason that I wrote [this gist](https://gist.github.com/fdncred/b87b784f04984dc31a150baed9ad2447). Another way to figure this out is to use `nu --log-level info`. This will show some performance timings but will also show what is being loaded when. For the most part the `[INFO]` lines show the performance timings and the `[WARN]` lines show the files. This PR tries to make things a little bit clearer when using the `--log-level info` parameter. # User-Facing Changes # Tests + Formatting # After Submitting --- src/config_files.rs | 51 ++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/src/config_files.rs b/src/config_files.rs index ba8a3436a0..ec4511860f 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -1,4 +1,4 @@ -use log::{info, trace}; +use log::warn; #[cfg(feature = "plugin")] use nu_cli::read_plugin_file; use nu_cli::{eval_config_contents, eval_source}; @@ -27,7 +27,10 @@ pub(crate) fn read_config_file( config_file: Option>, is_env_config: bool, ) { - trace!("read_config_file {:?}", &config_file); + warn!( + "read_config_file() config_file_specified: {:?}, is_env_config: {is_env_config}", + &config_file + ); // Load config startup file if let Some(file) = config_file { let working_set = StateWorkingSet::new(engine_state); @@ -122,21 +125,27 @@ pub(crate) fn read_config_file( } pub(crate) fn read_loginshell_file(engine_state: &mut EngineState, stack: &mut Stack) { + warn!( + "read_loginshell_file() {}:{}:{}", + file!(), + line!(), + column!() + ); + // read and execute loginshell file if exists if let Some(mut config_path) = nu_path::config_dir() { config_path.push(NUSHELL_FOLDER); config_path.push(LOGINSHELL_FILE); + warn!("loginshell_file: {}", config_path.display()); + if config_path.exists() { eval_config_contents(config_path, engine_state, stack); } } - - info!("read_loginshell_file {}:{}:{}", file!(), line!(), column!()); } pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut Stack) { - trace!("read_default_env_file"); let config_file = get_default_env(); eval_source( engine_state, @@ -147,7 +156,13 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut false, ); - info!("read_config_file {}:{}:{}", file!(), line!(), column!()); + warn!( + "read_default_env_file() env_file_contents: {config_file} {}:{}:{}", + file!(), + line!(), + column!() + ); + // Merge the environment in case env vars changed in the config match engine_state.cwd(Some(stack)) { Ok(cwd) => { @@ -167,10 +182,9 @@ fn eval_default_config( config_file: &str, is_env_config: bool, ) { - trace!( - "eval_default_config: config_file: {:?}, is_env_config: {}", - &config_file, - is_env_config + warn!( + "eval_default_config() config_file_specified: {:?}, is_env_config: {}", + &config_file, is_env_config ); println!("Continuing without config file"); // Just use the contents of "default_config.nu" or "default_env.nu" @@ -208,11 +222,9 @@ pub(crate) fn setup_config( env_file: Option>, is_login_shell: bool, ) { - trace!( - "setup_config: config: {:?}, env: {:?}, login: {}", - &config_file, - &env_file, - is_login_shell + warn!( + "setup_config() config_file_specified: {:?}, env_file_specified: {:?}, login: {}", + &config_file, &env_file, is_login_shell ); let result = catch_unwind(AssertUnwindSafe(|| { #[cfg(feature = "plugin")] @@ -240,12 +252,9 @@ pub(crate) fn set_config_path( key: &str, config_file: Option<&Spanned>, ) { - trace!( - "set_config_path: cwd: {:?}, default_config: {}, key: {}, config_file: {:?}", - &cwd, - &default_config_name, - &key, - &config_file + warn!( + "set_config_path() cwd: {:?}, default_config: {}, key: {}, config_file_specified: {:?}", + &cwd, &default_config_name, &key, &config_file ); let config_path = match config_file { Some(s) => canonicalize_with(&s.item, cwd).ok(), From 72b880662b75d5a418369dec98288351769a3990 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Wed, 15 May 2024 12:16:59 -0400 Subject: [PATCH 15/19] Fixed a nitpick usage-help error - closure v. block (#12876) # Description So minor, but had to be fixed sometime. `help each while` used the term "block" in the "usage", but the argument type is a closure. # User-Facing Changes help-only --- crates/nu-cmd-extra/src/extra/filters/each_while.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-cmd-extra/src/extra/filters/each_while.rs b/crates/nu-cmd-extra/src/extra/filters/each_while.rs index 2dda815d43..939f194f43 100644 --- a/crates/nu-cmd-extra/src/extra/filters/each_while.rs +++ b/crates/nu-cmd-extra/src/extra/filters/each_while.rs @@ -10,7 +10,7 @@ impl Command for EachWhile { } fn usage(&self) -> &str { - "Run a block on each row of the input list until a null is found, then create a new list with the results." + "Run a closure on each row of the input list until a null is found, then create a new list with the results." } fn search_terms(&self) -> Vec<&str> { From b08135d877d94279d4250d0ae5d0ee2887530c53 Mon Sep 17 00:00:00 2001 From: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com> Date: Wed, 15 May 2024 13:49:08 -0400 Subject: [PATCH 16/19] Fixed small error in the help-examples for the get command (#12877) # Description Another small error in Help, this time for the `get` command example. # User-Facing Changes Help only --- crates/nu-command/src/filters/get.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-command/src/filters/get.rs b/crates/nu-command/src/filters/get.rs index c30ad1f3a4..a481372db1 100644 --- a/crates/nu-command/src/filters/get.rs +++ b/crates/nu-command/src/filters/get.rs @@ -115,7 +115,7 @@ If multiple cell paths are given, this will produce a list of values."# }, Example { description: - "Extract the name of the 3rd record in a list (same as `ls | $in.name`)", + "Extract the name of the 3rd record in a list (same as `ls | $in.name.2`)", example: "ls | get name.2", result: None, }, From 06fe7d1e16c405a72ad08b2dc68b83fd947ddeba Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Wed, 15 May 2024 17:59:42 +0000 Subject: [PATCH 17/19] Remove usages of `Call::positional_nth` (#12871) # Description Following from #12867, this PR replaces usages of `Call::positional_nth` with existing spans. This removes several `expect`s from the code. Also remove unused `positional_nth_mut` and `positional_iter_mut` --- crates/nu-command/src/env/with_env.rs | 10 ++-------- crates/nu-command/src/filesystem/touch.rs | 17 ++++------------- crates/nu-command/src/platform/ansi/ansi_.rs | 5 +---- crates/nu-protocol/src/ast/call.rs | 20 -------------------- 4 files changed, 7 insertions(+), 45 deletions(-) diff --git a/crates/nu-command/src/env/with_env.rs b/crates/nu-command/src/env/with_env.rs index 7eaf64cb54..ff66d21f49 100644 --- a/crates/nu-command/src/env/with_env.rs +++ b/crates/nu-command/src/env/with_env.rs @@ -90,10 +90,7 @@ fn with_env( return Err(ShellError::CantConvert { to_type: "record".into(), from_type: x.get_type().to_string(), - span: call - .positional_nth(1) - .expect("already checked through .req") - .span, + span: x.span(), help: None, }); } @@ -124,10 +121,7 @@ fn with_env( return Err(ShellError::CantConvert { to_type: "record".into(), from_type: x.get_type().to_string(), - span: call - .positional_nth(1) - .expect("already checked through .req") - .span, + span: x.span(), help: None, }); } diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index ce8ed0dad3..08843b11f0 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -117,7 +117,7 @@ impl Command for Touch { #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; - for (index, glob) in files.into_iter().enumerate() { + for glob in files { let path = expand_path_with(glob.item.as_ref(), &cwd, glob.item.is_expand()); // If --no-create is passed and the file/dir does not exist there's nothing to do @@ -135,10 +135,7 @@ impl Command for Touch { { return Err(ShellError::CreateNotPossible { msg: format!("Failed to create file: {err}"), - span: call - .positional_nth(index) - .expect("already checked positional") - .span, + span: glob.span, }); }; } @@ -148,10 +145,7 @@ impl Command for Touch { { return Err(ShellError::ChangeModifiedTimeNotPossible { msg: format!("Failed to change the modified time: {err}"), - span: call - .positional_nth(index) - .expect("already checked positional") - .span, + span: glob.span, }); }; } @@ -161,10 +155,7 @@ impl Command for Touch { { return Err(ShellError::ChangeAccessTimeNotPossible { msg: format!("Failed to change the access time: {err}"), - span: call - .positional_nth(index) - .expect("already checked positional") - .span, + span: glob.span, }); }; } diff --git a/crates/nu-command/src/platform/ansi/ansi_.rs b/crates/nu-command/src/platform/ansi/ansi_.rs index 427c7c3a73..9e0b0bba66 100644 --- a/crates/nu-command/src/platform/ansi/ansi_.rs +++ b/crates/nu-command/src/platform/ansi/ansi_.rs @@ -784,10 +784,7 @@ fn heavy_lifting(code: Value, escape: bool, osc: bool, call: &Call) -> Result { return Err(ShellError::TypeMismatch { err_message: String::from("Unknown ansi code"), - span: call - .positional_nth(0) - .expect("Unexpected missing argument") - .span, + span: code.span(), }) } } diff --git a/crates/nu-protocol/src/ast/call.rs b/crates/nu-protocol/src/ast/call.rs index 6689bc3a55..9aaf17d5db 100644 --- a/crates/nu-protocol/src/ast/call.rs +++ b/crates/nu-protocol/src/ast/call.rs @@ -154,30 +154,10 @@ impl Call { }) } - pub fn positional_iter_mut(&mut self) -> impl Iterator { - self.arguments - .iter_mut() - .take_while(|arg| match arg { - Argument::Spread(_) => false, // Don't include positional arguments given to rest parameter - _ => true, - }) - .filter_map(|arg| match arg { - Argument::Named(_) => None, - Argument::Positional(positional) => Some(positional), - Argument::Unknown(unknown) => Some(unknown), - Argument::Spread(_) => None, - }) - } - pub fn positional_nth(&self, i: usize) -> Option<&Expression> { self.positional_iter().nth(i) } - // TODO this method is never used. Delete? - pub fn positional_nth_mut(&mut self, i: usize) -> Option<&mut Expression> { - self.positional_iter_mut().nth(i) - } - pub fn positional_len(&self) -> usize { self.positional_iter().count() } From 6f3dbc97bb77296b68e6da8a9c09d1a5ce5e9003 Mon Sep 17 00:00:00 2001 From: Jack Wright <56345+ayax79@users.noreply.github.com> Date: Wed, 15 May 2024 14:55:07 -0700 Subject: [PATCH 18/19] fixed syntax shape requirements for --quantiles option for polars summary (#12878) Fix for #12730 All of the code expected a list of floats, but the syntax shape expected a table. Resolved by changing the syntax shape to list of floats. cc: @maxim-uvarov --- crates/nu_plugin_polars/src/dataframe/eager/summary.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu_plugin_polars/src/dataframe/eager/summary.rs b/crates/nu_plugin_polars/src/dataframe/eager/summary.rs index 8e5151837d..2b22d73bfb 100644 --- a/crates/nu_plugin_polars/src/dataframe/eager/summary.rs +++ b/crates/nu_plugin_polars/src/dataframe/eager/summary.rs @@ -38,7 +38,7 @@ impl PluginCommand for Summary { ) .named( "quantiles", - SyntaxShape::Table(vec![]), + SyntaxShape::List(Box::new(SyntaxShape::Float)), "provide optional quantiles", Some('q'), ) From e20113a0eb8622e27f2ed203f2b628c3d2f7e5f5 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Wed, 15 May 2024 22:59:10 +0000 Subject: [PATCH 19/19] Remove stack debug assert (#12861) # Description In order for `Stack::unwrap_unique` to work as intended, we currently manually track all references to the parent stack and ensure that they are cleared before calling `Stack::unwrap_unique` in the REPL. We also only call `Stack::unwrap_unique` after all code from the current REPL entry has finished executing. Since `Value`s cannot store `Stack` references, then this should have worked in theory. However, we forgot to account for threads. `run-external` (and maybe the plugin writers) can spawn threads that clone the `Stack`, holding on to references of the parent stack. These threads are not waited/joined upon, and so may finish after the eval has already returned. This PR removes the `Stack::unwrap_unique` function and associated debug assert that was [causing panics](https://gist.github.com/cablehead/f3d2608a1629e607c2d75290829354f7) like @cablehead found. # After Submitting Make values cheaper to clone as a more robust solution to the performance issues with cloning the stack. --------- Co-authored-by: Wind --- crates/nu-cli/src/repl.rs | 4 +++- crates/nu-protocol/src/engine/stack.rs | 32 ++++++-------------------- 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 2482a7920e..29c2f62734 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -542,7 +542,9 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { let shell_integration_osc633 = config.shell_integration_osc633; let shell_integration_reset_application_mode = config.shell_integration_reset_application_mode; - let mut stack = Stack::unwrap_unique(stack_arc); + // TODO: we may clone the stack, this can lead to major performance issues + // so we should avoid it or making stack cheaper to clone. + let mut stack = Arc::unwrap_or_clone(stack_arc); perf( "line_editor setup", diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 65f2981fbf..19726db9c0 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -75,26 +75,10 @@ impl Stack { } } - /// Unwrap a uniquely-owned stack. - /// - /// In debug mode, this panics if there are multiple references. - /// In production this will instead clone the underlying stack. - pub fn unwrap_unique(stack_arc: Arc) -> Stack { - // If you hit an error here, it's likely that you created an extra - // Arc pointing to the stack somewhere. Make sure that it gets dropped before - // getting here! - Arc::try_unwrap(stack_arc).unwrap_or_else(|arc| { - // in release mode, we clone the stack, but this can lead to - // major performance issues, so we should avoid it - debug_assert!(false, "More than one stack reference remaining!"); - (*arc).clone() - }) - } - /// Create a new child stack from a parent. /// /// Changes from this child can be merged back into the parent with - /// Stack::with_changes_from_child + /// [`Stack::with_changes_from_child`] pub fn with_parent(parent: Arc) -> Stack { Stack { // here we are still cloning environment variable-related information @@ -109,19 +93,17 @@ impl Stack { } } - /// Take an Arc of a parent (assumed to be unique), and a child, and apply - /// all the changes from a child back to the parent. + /// Take an [`Arc`] parent, and a child, and apply all the changes from a child back to the parent. /// - /// Here it is assumed that child was created with a call to Stack::with_parent - /// with parent + /// Here it is assumed that `child` was created by a call to [`Stack::with_parent`] with `parent`. + /// + /// For this to be performant and not clone `parent`, `child` should be the only other + /// referencer of `parent`. pub fn with_changes_from_child(parent: Arc, child: Stack) -> Stack { // we're going to drop the link to the parent stack on our new stack // so that we can unwrap the Arc as a unique reference - // - // This makes the new_stack be in a bit of a weird state, so we shouldn't call - // any structs drop(child.parent_stack); - let mut unique_stack = Stack::unwrap_unique(parent); + let mut unique_stack = Arc::unwrap_or_clone(parent); unique_stack .vars