Dev #1
Merged
Dev #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
or any directory that user has access via group
…r_groups` Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
hardfau1t
pushed a commit
that referenced
this pull request
Feb 16, 2024
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> This fixes an issue brought up by nihilander in [Discord](https://discord.com/channels/601130461678272522/614593951969574961/1201594105986285649). # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> Nushell panics when the spread operator is used like this (the `...$rest` shouldn't actually be parsed as a spread operator at all): ```nu $ def foo [...rest: string] {...$rest} $ foo bar baz thread 'main' panicked at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nu-protocol-0.89.0/src/signature.rs:650:9: Internal error: can't run a predeclaration without a body stack backtrace: 0: rust_begin_unwind 1: core::panicking::panic_fmt 2: <nu_protocol::signature::Predeclaration as nu_protocol::engine::command::Command>::run 3: nu_engine::eval::eval_call 4: nu_engine::eval::eval_expression_with_input 5: nu_engine::eval::eval_element_with_input 6: nu_engine::eval::eval_block 7: nu_cli::util::eval_source 8: nu_cli::repl::evaluate_repl 9: nu::run::run_repl 10: nu::main note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. ``` The problem was that whenever the parser saw something like `{...$`, `{...(`, or `{...[`, it would treat that as a record with a spread expression, ignoring the syntax shape of the block it was parsing. This should now be fixed, and the snippet above instead gives the following error: ```nu Error: nu::shell::external_command × External command failed ╭─[entry #1:1:1] 1 │ def foo [...rest] {...$rest} · ────┬─── · ╰── executable was not found ╰──── help: No such file or directory (os error 2) ``` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> Stuff like `do { ...$rest }` will now try to run a command `...$rest` rather than complaining that variable `$rest` doesn't exist. # Tests + Formatting <!-- Don't forget to add tests that cover your changes. 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 > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> Sorry about the issue, I am not touching the parser again for a long time :)
hardfau1t
pushed a commit
that referenced
this pull request
Feb 16, 2024
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> Fixes nushell#11711 Previously, syntax `def a [] (echo 4)` was allowed to parse and then failed with panic duting eval. Current error: ``` Error: nu::parser::parse_mismatch × Parse mismatch during operation. ╭─[entry #1:1:1] 1 │ def a [] (echo 4) · ────┬─── · ╰── expected definition body closure { ... } ╰──── ``` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting <!-- Don't forget to add tests that cover your changes. 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 > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
hardfau1t
pushed a commit
that referenced
this pull request
Feb 16, 2024
…r ints (nushell#11724) # Description This PR changes `into int` and `into filesize` so that they allow thousands separators. ### Before ```nushell ❯ '1,000' | into filesize Error: nu::shell::cant_convert × Can't convert to int. ╭─[entry #1:1:1] 1 │ '1,000' | into filesize · ───┬─── · ╰── can't convert string to int ╰──── ❯ '1,000' | into int Error: nu:🐚:cant_convert × Can't convert to int. ╭─[entry nushell#2:1:1] 1 │ '1,000' | into int · ────┬─── · ╰── can't convert string to int ╰──── help: string "1,000" does not represent a valid integer ``` ### After ```nushell ❯ '1,000' | into filesize 1.0 KB ❯ '1,000' | into int 1000 ``` This works by getting the system locale and from that, determining what the thousands separator is. So, hopefully, this will work across locales. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting <!-- Don't forget to add tests that cover your changes. 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 > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
hardfau1t
pushed a commit
that referenced
this pull request
Feb 16, 2024
# Description Close: nushell#9673 Close: nushell#8277 Close: nushell#10944 This pr introduces the following syntax: 1. `e>|`, pipe stderr to next command. Example: `$env.FOO=bar nu --testbin echo_env_stderr FOO e>| str length` 2. `o+e>|` and `e+o>|`, pipe both stdout and stderr to next command, example: `$env.FOO=bar nu --testbin echo_env_mixed out-err FOO FOO e+o>| str length` Note: it only works for external commands. ~There is no different for internal commands, that is, the following three commands do the same things:~ Edit: it raises errors if we want to pipes for internal commands ``` ❯ ls e>| str length Error: × `e>|` only works with external streams ╭─[entry #1:1:1] 1 │ ls e>| str length · ─┬─ · ╰── `e>|` only works on external streams ╰──── ❯ ls e+o>| str length Error: × `o+e>|` only works with external streams ╭─[entry nushell#2:1:1] 1 │ ls e+o>| str length · ──┬── · ╰── `o+e>|` only works on external streams ╰──── ``` This can help us to avoid some strange issues like the following: `$env.FOO=bar (nu --testbin echo_env_stderr FOO) e>| str length` Which is hard to understand and hard to explain to users. # User-Facing Changes Nan # Tests + Formatting To be done # After Submitting Maybe update documentation about these syntax.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
User-Facing Changes
Tests + Formatting
After Submitting