mirror of
https://github.com/nushell/nushell.git
synced 2025-06-08 19:17:16 +02:00
630 Commits
Author | SHA1 | Message | Date | |
---|---|---|---|---|
|
13d5a15f75
|
Run-time pipeline input typechecking tweaks (#14922)
<!-- 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. --> This PR makes two changes related to [run-time pipeline input type checking](https://github.com/nushell/nushell/pull/14741): 1. The check which bypasses type checking for commands with only `Type::Nothing` input types has been expanded to work with commands with multiple `Type::Nothing` inputs for different outputs. For example, `ast` has three input/output type pairs, but all of the inputs are `Type::Nothing`: ``` ╭───┬─────────┬────────╮ │ # │ input │ output │ ├───┼─────────┼────────┤ │ 0 │ nothing │ table │ │ 1 │ nothing │ record │ │ 2 │ nothing │ string │ ╰───┴─────────┴────────╯ ``` Before this PR, passing a value (which would otherwise be ignored) to `ast` caused a run-time type error: ``` Error: nu:🐚:only_supports_this_input_type × Input type not supported. ╭─[entry #1:1:6] 1 │ echo 123 | ast -j -f "hi" · ─┬─ ─┬─ · │ ╰── only nothing, nothing, and nothing input data is supported · ╰── input type: int ╰──── ``` After this PR, no error is raised. This doesn't really matter for `ast` (the only other built-in command with a similar input/output type signature is `cal`), but it's more logically consistent. 2. Bypasses input type-checking (parse-time ***and*** run-time) for some (not all, see below) commands which have both a `Type::Nothing` input and some other non-nothing `Type` input. This is accomplished by adding a `Type::Any` input with the same output as the corresponding `Type::Nothing` input/output pair. This is necessary because some commands are intended to operate on an argument with empty pipeline input, or operate on an empty pipeline input with no argument. This causes issues when a value is implicitly passed to one of these commands. I [discovered this issue](https://discord.com/channels/601130461678272522/615962413203718156/1329945784346611712) when working with an example where the `open` command is used in `sort-by` closure: ```nushell ls | sort-by { open -r $in.name | lines | length } ``` Before this PR (but after the run-time input type checking PR), this error is raised: ``` Error: nu:🐚:only_supports_this_input_type × Input type not supported. ╭─[entry #1:1:1] 1 │ ls | sort-by { open -r $in.name | lines | length } · ─┬ ──┬─ · │ ╰── only nothing and string input data is supported · ╰── input type: record<name: string, type: string, size: filesize, modified: date> ╰──── ``` While this error is technically correct, we don't actually want to return an error here since `open` ignores its pipeline input when an argument is passed. This would be a parse-time error as well if the parser was able to infer that the closure input type was a record, but our type inference isn't that robust currently, so this technically incorrect form snuck by type checking until #14741. However, there are some commands with the same kind of type signature where this behavior is actually desirable. This means we can't just bypass type-checking for any command with a `Type::Nothing` input. These commands operate on true `null` values, rather than ignoring their input. For example, `length` returns `0` when passed a `null` value. It's correct, and even desirable, to throw a run-time error when `length` is passed an unexpected type. For example, a string, which should instead be measured with `str length`: ```nushell ["hello" "world"] | sort-by { length } # => Error: nu:🐚:only_supports_this_input_type # => # => × Input type not supported. # => ╭─[entry #32:1:10] # => 1 │ ["hello" "world"] | sort-by { length } # => · ───┬─── ───┬── # => · │ ╰── only list<any>, binary, and nothing input data is supported # => · ╰── input type: string # => ╰──── ``` We need a more robust way for commands to express how they handle the `Type::Nothing` input case. I think a possible solution here is to allow commands to express that they operate on `PipelineData::Empty`, rather than `Value::Nothing`. Then, a command like `open` could have an empty pipeline input type rather than a `Type::Nothing`, and the parse-time and run-time pipeline input type checks know that `open` will safely ignore an incorrectly typed input. That being said, we have a release coming up and the above solution might take a while to implement, so while unfortunate, bypassing input type-checking for these problematic commands serves as a workaround to avoid breaking changes in the release until a more robust solution is implemented. This PR bypasses input type-checking for the following commands: * `load-env`: can take record of envvars as input or argument * `nu-check`: checks input string or filename argument * `open`: can take filename as input or argument * `polars when`: can be used with input, or can be chained with another `polars when` * `stor insert`: data record can be passed as input or argument * `stor update`: data record can be passed as input or argument * `format date`: `--list` ignores input value * `into datetime`: `--list` ignores input value (also added a `Type::Nothing` input which was missing from this command) These commands have a similar input/output signature to the above commands, but are working as intended: * `cd`: The input/output signature was actually incorrect, `cd` always ignores its input. I fixed this in this PR. * `generate` * `get` * `history import` * `interleave` * `into bool` * `length` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> As a temporary workaround, pipeline input type-checking for the following commands has been bypassed to avoid undesirable run-time input type checking errors which were previously not caught at parse-time: * `open` * `load-env` * `format date` * `into datetime` * `nu-check` * `stor insert` * `stor update` * `polars when` # 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> CI became green in the time it took me to type the description 😄 # 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. --> N/A |
||
|
339c5b7c83
|
fix: clippy warning of rust 1.8.4 (#14984)
# Description I'm on rust toolchain 1.8.4, and I can see clippy warnings that can't be caught by the ci workflow, primarily related to lifetime params. I think it doesn't hurt to fix those in advance. # User-Facing Changes # Tests + Formatting # After Submitting |
||
|
948965d42f
|
Immediately return error if detected as pipeline input or positional argument (#14874)
<!-- 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. --> This PR returns error values while checking pipeline input types and positional argument types. This should help return non-nested errors earlier and prevent confusing errors. The positional argument change is directly related to an example given on Discord. Before this PR, this is the error shown: ``` Error: nu:🐚:cant_convert × Can't convert to record. ╭─[/home/rose/tmp/script.nu:23:5] 22 │ let entry = $in 23 │ ╭─▶ { 24 │ │ name: $entry, 25 │ │ details: { 26 │ │ context: $context 27 │ │ } 28 │ ├─▶ } · ╰──── can't convert error to record 29 │ } ╰──── ``` After this PR, this is the error shown: ``` Error: nu:🐚:eval_block_with_input × Eval block failed with pipeline input ╭─[/home/rose/tmp/script.nu:23:5] 22 │ let entry = $in 23 │ ╭─▶ { 24 │ │ name: $entry, 25 │ │ details: { 26 │ │ context: $context 27 │ │ } 28 │ ├─▶ } · ╰──── source value 29 │ } ╰──── Error: nu:🐚:type_mismatch × Type mismatch. ╭─[/home/rose/tmp/much.nu:3:38] 2 │ $in | each { |elem| 3 │ print $elem.details.context.yaml.0 · ┬ · ╰── Can't access record values with a row index. Try specifying a column name instead 4 │ } | each { |elem| ╰──── ``` I'm not certain if the pipeline input error check actually can ever be triggered, but it seems to be a good defensive error handling strategy regardless. My addition of the `Value::Error` case in the first place would suggest it can be, but after looking at it more closely the error that caused me to add the case in the first place was actually unrelated to input typechecking. Additionally, this PR does not affect the handling of nested errors, so something like: ```nushell try { ... } catch {|e| $e | reject raw | to nuon } ``` works the same before and after this PR. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> Errors values detected as arguments to commands or as pipeline input to commands are immediately thrown, rather than passed to the command. # 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # 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. --> N/A |
||
|
6be42d94d9
|
Replaced IoError::new calls that still had Span::unknown() (#14968)
<!-- 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. --> It seems in my PR #14927 I missed a few calls to `IoError::new` that had `Span::unknown` inside them, which shouldn't be used but rather `IoError::new_internal`. I replaced these calls. Thanks to @132ikl to finding out that I forgot some. 😄 # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> Pretty much none really. # 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # 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. --> |
||
|
aa62de78e6
|
Fix: Directories using a tilde to represent HOME will now be converted to an absolute-path before running an external (#14959)
Fixes #14780 - Directories using a tilde to represent HOME will now be converted to an absolute-path before running an external. |
||
|
66bc0542e0
|
Refactor I/O Errors (#14927)
<!--
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.
-->
As mentioned in #10698, we have too many `ShellError` variants, with
some even overlapping in meaning. This PR simplifies and improves I/O
error handling by restructuring `ShellError` related to I/O issues.
Previously, `ShellError::IOError` only contained a message string,
making it convenient but overly generic. It was widely used without
providing spans (#4323).
This PR introduces a new `ShellError::Io` variant that consolidates
multiple I/O-related errors (except for `ShellError::NetworkFailure`,
which remains distinct for now). The new `ShellError::Io` variant
replaces the following:
- `FileNotFound`
- `FileNotFoundCustom`
- `IOInterrupted`
- `IOError`
- `IOErrorSpanned`
- `NotADirectory`
- `DirectoryNotFound`
- `MoveNotPossible`
- `CreateNotPossible`
- `ChangeAccessTimeNotPossible`
- `ChangeModifiedTimeNotPossible`
- `RemoveNotPossible`
- `ReadingFile`
## The `IoError`
`IoError` includes the following fields:
1. **`kind`**: Extends `std::io::ErrorKind` to specify the type of I/O
error without needing new `ShellError` variants. This aligns with the
approach used in `std::io::Error`. This adds a second dimension to error
reporting by combining the `kind` field with `ShellError` variants,
making it easier to describe errors in more detail. As proposed by
@kubouch in [#design-discussion on
Discord](https://discord.com/channels/601130461678272522/615329862395101194/1323699197165178930),
this helps reduce the number of `ShellError` variants. In the error
report, the `kind` field is displayed as the "source" of the error,
e.g., "I/O error," followed by the specific kind of I/O error.
2. **`span`**: A non-optional field to encourage providing spans for
better error reporting (#4323).
3. **`path`**: Optional `PathBuf` to give context about the file or
directory involved in the error (#7695). If provided, it’s shown as a
help entry in error reports.
4. **`additional_context`**: Allows adding custom messages when the
span, kind, and path are insufficient. This is rendered in the error
report at the labeled span.
5. **`location`**: Sometimes, I/O errors occur in the engine itself and
are not caused directly by user input. In such cases, if we don’t have a
span and must set it to `Span::unknown()`, we need another way to
reference the error. For this, the `location` field uses the new
`Location` struct, which records the Rust file and line number where the
error occurred. This ensures that we at least know the Rust code
location that failed, helping with debugging. To make this work, a new
`location!` macro was added, which retrieves `file!`, `line!`, and
`column!` values accurately. If `Location::new` is used directly, it
issues a warning to remind developers to use the macro instead, ensuring
consistent and correct usage.
### Constructor Behavior
`IoError` provides five constructor methods:
- `new` and `new_with_additional_context`: Used for errors caused by
user input and require a valid (non-unknown) span to ensure precise
error reporting.
- `new_internal` and `new_internal_with_path`: Used for internal errors
where a span is not available. These methods require additional context
and the `Location` struct to pinpoint the source of the error in the
engine code.
- `factory`: Returns a closure that maps an `std::io::Error` to an
`IoError`. This is useful for handling multiple I/O errors that share
the same span and path, streamlining error handling in such cases.
## New Report Look
This is simulation how the I/O errors look like (the `open crates` is
simulated to show how internal errors are referenced now):

## `Span::test_data()`
To enable better testing, `Span::test_data()` now returns a value
distinct from `Span::unknown()`. Both `Span::test_data()` and
`Span::unknown()` refer to invalid source code, but having a separate
value for test data helps identify issues during testing while keeping
spans unique.
## Cursed Sneaky Error Transfers
I removed the conversions between `std::io::Error` and `ShellError` as
they often removed important information and were used too broadly to
handle I/O errors. This also removed the problematic implementation
found here:
|
||
|
089c5221cc
|
Add new operators has and not-has (#14841)
# Description This PR add 2 new operators, `has` and `not-has`. They are basically `in` and `not-in` with the order of operands swapped. Motivation for this was the awkward way of searching for rows that contain an item using `where` ```nushell [[name, children]; [foo, [a, b, c]], [bar [d, e, f]]] | where ("e" in $it.children) ``` vs ```nushell [[name, children]; [foo, [a, b, c]], [bar [d, e, f]]] | where children has "e" ``` # User-Facing Changes Added `has` and `not-has` operators, mirroring `in` and `not-in`. # Tests + Formatting - 🟢 toolkit fmt - 🟢 toolkit clippy - 🟢 toolkit test - 🟢 toolkit test stdlib # After Submitting |
||
|
737ea3940e
|
finish removing terminal_size dep (#14819)
# Description This PR is a follow on to https://github.com/nushell/nushell/pull/14423 and finishes removing the `terminal_size` crate in favor of `crossterm`'s `size()`. # 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 toolkit.nu; toolkit test stdlib"` 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. --> |
||
|
72d50cf8b7
|
Convert Path to list in main and preserve case (#14764)
# Description Fixes multiple issues related to `ENV_CONVERSION` and path-conversion-to-list. * #14681 removed some calls to `convert_env_values()`, but we found that this caused `nu -n` to no longer convert the path properly. * `ENV_CONVERSIONS` have apparently never preserved case, meaning a conversion with a key of `foo` would not update `$env.FOO` but rather create a new environment variable with a different case. * There was a partial code-path that attempted to solve this for `PATH`, but it only worked for `PATH` and `Path`. * `convert_env_values()`, which handled `ENV_CONVERSIONS` was called in multiple places in the startup depending on flags. This PR: * Refactors the startup to handle the conversion in `main()` rather than in each potential startup path * Updates `get_env_var_insensitive()` functions added in #14390 to return the name of the environment variable with its original case. This allows code that updates environment variables to preserve the case. * Makes use of the updated function in `ENV_CONVERSIONS` to preserve the case of any updated environment variables. The `ENV_CONVERSION` key itself is still case **insensitive**. * Makes use of the updated function to preserve the case of the `PATH` environment variable (normally handled separately, regardless of whether or not there was an `ENV_CONVERSION` for it). ## Before `env_convert_values` was run: * Before the user `env.nu` ran, which included `nu -c <commandstring>` and `nu <script.nu>` * Before the REPL loaded, which included `nu -n` ## After `env_convert_values` always runs once in `main()` before any config file is processed or the REPL is started # User-Facing Changes Bug fixes # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` Added additional tests to prevent future regression. # After Submitting There is additional cleanup that should probably be done in `convert_env_values()`. This function previously handled `ENV_CONVERSIONS`, but there is no longer any need for this since `convert_env_vars()` runs whenever `$env.ENV_CONVERSIONS` changes now. This means that the only relevant task in the old `convert_env_values()` is to convert the `PATH` to a list, and ensure that it is a list of strings. It's still calling the `from_string` conversion on every variable (just once) even though there are no `ENV_CONVERSIONS` at this point. Leaving that to another PR though, while we get the core issue fixed with this one. |
||
|
214714e0ab
|
Add run-time type checking for command pipeline input (#14741)
<!-- 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. --> This PR adds type checking of all command input types at run-time. Generally, these errors should be caught by the parser, but sometimes we can't know the type of a value at parse-time. The simplest example is using the `echo` command, which has an output type of `any`, so prefixing a literal with `echo` will bypass parse-time type checking. Before this PR, each command has to individually check its input types. This can result in scenarios where the input/output types don't match the actual command behavior. This can cause valid usage with an non-`any` type to become a parse-time error if a command is missing that type in its pipeline input/output (`drop nth` and `history import` do this before this PR). Alternatively, a command may not list a type in its input/output types, but doesn't actually reject that type in its code, which can have unintended side effects (`get` does this on an empty pipeline input, and `sort` used to before #13154). After this PR, the type of the pipeline input is checked to ensure it matches one of the input types listed in the proceeding command's input/output types. While each of the issues in the "before this PR" section could be addressed with each command individually, this PR solves this issue for _all_ commands. **This will likely cause some breakage**, as some commands have incorrect input/output types, and should be adjusted. Also, some scripts may have erroneous usage of commands. In writing this PR, I discovered that `toolkit.nu` was passing `null` values to `str join`, which doesn't accept nothing types (if folks think it should, we can adjust it in this PR or in a different PR). I found some issues in the standard library and its tests. I also found that carapace's vendor script had an incorrect chaining of `get -i`: ```nushell let expanded_alias = (scope aliases | where name == $spans.0 | get -i 0 | get -i expansion) ``` Before this PR, if the `get -i 0` ever actually did evaluate to `null`, the second `get` invocation would error since `get` doesn't operate on `null` values. After this PR, this is immediately a run-time error, alerting the user to the problematic code. As a side note, we'll need to PR this fix (`get -i 0 | get -i expansion` -> `get -i 0.expansion`) to carapace. A notable exception to the type checking is commands with input type of `nothing -> <type>`. In this case, any input type is allowed. This allows piping values into the command without an error being thrown. For example, `123 | echo $in` would be an error without this exception. Additionally, custom types bypass type checking (I believe this also happens during parsing, but not certain) I added a `is_subtype` method to `Value` and `PipelineData`. It functions slightly differently than `get_type().is_subtype()`, as noted in the doccomments. Notably, it respects structural typing of lists and tables. For example, the type of a value `[{a: 123} {a: 456, b: 789}]` is a subtype of `table<a: int>`, whereas the type returned by `Value::get_type` is a `list<any>`. Similarly, `PipelineData` has some special handling for `ListStream`s and `ByteStream`s. The latter was needed for this PR to work properly with external commands. Here's some examples. Before: ```nu 1..2 | drop nth 1 Error: nu::parser::input_type_mismatch × Command does not support range input. ╭─[entry #9:1:8] 1 │ 1..2 | drop nth 1 · ────┬─── · ╰── command doesn't support range input ╰──── echo 1..2 | drop nth 1 # => ╭───┬───╮ # => │ 0 │ 1 │ # => ╰───┴───╯ ``` After this PR, I've adjusted `drop nth`'s input/output types to accept range input. Before this PR, zip accepted any value despite not being listed in its input/output types. This caused different behavior depending on if you triggered a parse error or not: ```nushell 1 | zip [2] # => Error: nu::parser::input_type_mismatch # => # => × Command does not support int input. # => ╭─[entry #3:1:5] # => 1 │ 1 | zip [2] # => · ─┬─ # => · ╰── command doesn't support int input # => ╰──── echo 1 | zip [2] # => ╭───┬───────────╮ # => │ 0 │ ╭───┬───╮ │ # => │ │ │ 0 │ 1 │ │ # => │ │ │ 1 │ 2 │ │ # => │ │ ╰───┴───╯ │ # => ╰───┴───────────╯ ``` After this PR, it works the same in both cases. For cases like this, if we do decide we want `zip` or other commands to accept any input value, then we should explicitly add that to the input types. ```nushell 1 | zip [2] # => Error: nu::parser::input_type_mismatch # => # => × Command does not support int input. # => ╭─[entry #3:1:5] # => 1 │ 1 | zip [2] # => · ─┬─ # => · ╰── command doesn't support int input # => ╰──── echo 1 | zip [2] # => Error: nu:🐚:only_supports_this_input_type # => # => × Input type not supported. # => ╭─[entry #14:2:6] # => 2 │ echo 1 | zip [2] # => · ┬ ─┬─ # => · │ ╰── only list<any> and range input data is supported # => · ╰── input type: int # => ╰──── ``` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> **Breaking change**: The type of a command's input is now checked against the input/output types of that command at run-time. While these errors should mostly be caught at parse-time, in cases where they can't be detected at parse-time they will be caught at run-time instead. This applies to both internal commands and custom commands. Example function and corresponding parse-time error (same before and after PR): ```nushell def foo []: int -> nothing { print $"my cool int is ($in)" } 1 | foo # => my cool int is 1 "evil string" | foo # => Error: nu::parser::input_type_mismatch # => # => × Command does not support string input. # => ╭─[entry #16:1:17] # => 1 │ "evil string" | foo # => · ─┬─ # => · ╰── command doesn't support string input # => ╰──── # => ``` Before: ```nu echo "evil string" | foo # => my cool int is evil string ``` After: ```nu echo "evil string" | foo # => Error: nu:🐚:only_supports_this_input_type # => # => × Input type not supported. # => ╭─[entry #17:1:6] # => 1 │ echo "evil string" | foo # => · ──────┬────── ─┬─ # => · │ ╰── only int input data is supported # => · ╰── input type: string # => ╰──── ``` Known affected internal commands which erroneously accepted any type: * `str join` * `zip` * `reduce` # 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # 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. --> * Play whack-a-mole with the commands and scripts this will inevitably break |
||
|
dc52a6fec5
|
Handle permission denied error at nu_engine::glob_from (#14679)
<!-- 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. --> #14528 mentioned that trying to `open` a file in a directory where you don't have read access results in a "file not found" error. I investigated the error and could find the root issue in the `nu_engine::glob_from` function. It uses `std::path::Path::canonicalize` some layers down and that may return an `std::io::Error`. All these errors were handled as "directory not found" which will be translated to "file not found" in the `open` command. To fix this, I handled the `PermssionDenied` error kind of the io error and passed that down. Now trying to `open` a file from a directory with no permissions returns a "permission denied" error. Before/After:  # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> That error is fixed, so correct error message. # 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # 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. --> fixes #14528 |
||
|
0d3f76ddef
|
Remove no-longer-needed convert_env_values calls (#14681)
# Description Takes advantage of #14591 to remove the now-necessary calls to `convert_env_values()` that I added in #14249. The function is now just called once to convert `PATH`. Also removed the Windows-build-time checks for `ensure_path`, since previous case-insensitivity fixes make this unnecessary as well. # User-Facing Changes None - #14591 now handles conversion 'on-demand'. # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting N/A |
||
|
62bd6fe08b
|
Create nu_glob::is_glob function (#14717)
# Description Adds an `is_glob` function to the nu-glob crate that takes a string pattern and returns whether or not it's a glob that would be expanded by nu-glob. Right now, this just means checking if it contains `*`, `?`, or `[`. Previously, this same code was duplicated in the following places: - `ls`: Determining whether to read a folder's contents or expand a glob - `run_external.rs` in nu-command: Arguments to externals only have n-dots and tilde expansion applied if they weren't globs - `glob_from` in nu-engine: - `glob_from` can get the prefix in a simpler way for non-globs - If the canonical path for a non-glob path contains glob metacharacters, it needs to be escaped - `completion_common.rs` in nu-cli: File/folder completions containing glob metacharacters need to be wrapped in quotes All of these locations can use `nu_glob::is_glob` now instead of rolling their own checks. This does mean that nu-cli now has a dependency on nu-glob. # User-Facing Changes Users of nu-glob will now be able to check if a given pattern is a glob expanded by nu-glob. For users of Nushell, completion suggestions for files containing `]` will no longer be wrapped in quotes if they contain no other glob metacharacters. This is because unmatched `]`s are ignored by nu-glob, but we used to consider such file completions contaminated anyway. # Tests + Formatting This is a very basic function, so I just added some doctests. # After Submitting This is meant to be used in https://github.com/nushell/nushell/pull/14674. |
||
|
5f3c8d45d8
|
Add auto option for config.use_ansi_coloring (#14647)
<!--
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.
-->
In this PR I continued the idea of #11494, it added an `auto` option to
the ansi coloring config option, I did this too but in a more simple
approach.
So I added a new enum `UseAnsiColoring` with the three values `True`,
`False` and `Auto`. When that value is set to `auto`, the default value,
it will use `std::io::stdout().is_terminal()` to decided whether to use
ansi coloring. This allows to dynamically decide whether to print ansi
color codes or not, [cargo does it the same
way](
|
||
|
1b01598840
|
Run ENV_CONVERSIONS whenever it's modified (#14591)
- this PR should close #14514 # Description Makes updates to `$env.ENV_CONVERSIONS` take effect immediately. # User-Facing Changes No breaking change, `$env.ENV_CONVERSIONS` can be set and its effect used in the same file. # Tests + Formatting - 🟢 toolkit fmt - 🟢 toolkit clippy - 🟢 toolkit test - 🟢 toolkit test stdlib # After Submitting N/A |
||
|
35d2750757
|
Change how and and or operations are compiled to IR to support custom values (#14653)
# Description Because `and` and `or` are short-circuiting operations in Nushell, they must be compiled to a sequence that avoids evaluating the RHS if the LHS is already sufficient to determine the output - i.e., `false` for `and` and `true` for `or`. I initially implemented this with `branch-if` instructions, simply returning the RHS if it needed to be evaluated, and returning the short-circuited boolean value if it did not. Example for `$a and $b`: ``` 0: load-variable %0, var 999 "$a" 1: branch-if %0, 3 2: jump 5 3: load-variable %0, var 1000 "$b" # label(0), from(1:) 4: jump 6 5: load-literal %0, bool(false) # label(1), from(2:) 6: span %0 # label(2), from(4:) 7: return %0 ``` Unfortunately, this broke polars, because using `and`/`or` on custom values is perfectly valid and they're allowed to define that behavior differently, and the polars plugin uses this for boolean masks. But without using the `binary-op` instruction, that custom behavior is never invoked. Additionally, `branch-if` requires a boolean, and custom values are not booleans. This changes the IR to the following, using the `match` instruction to check for the specific short-circuit value instead, and still invoking `binary-op` otherwise: ``` 0: load-variable %0, var 125 "$a" 1: match (false), %0, 4 2: load-variable %1, var 124 "$b" 3: binary-op %0, Boolean(And), %1 4: span %0 # label(0), from(1:) 5: return %0 ``` I've also renamed `Pattern::Value` to `Pattern::Expression` and added a proper `Pattern::Value` variant that actually contains a `Value` instead. I'm still hoping to remove `Pattern::Expression` eventually, because it's kind of a hack - we don't actually evaluate the expression, we just match it against a few cases specifically for pattern matching, and it's one of the cases where AST leaks into IR and I want to remove all of those cases, because AST should not leak into IR. Fixes #14518 # User-Facing Changes - `and` and `or` now support custom values again. - the IR is actually a little bit cleaner, though it may be a bit slower; `match` is more complex. # Tests + Formatting The existing tests pass, but I didn't add anything new. Unfortunately I don't think there's anything built-in to trigger this, but maybe some testcases could be added to polars to test it. |
||
|
e2c4ff8180
|
Revert "Feature: PWD-per-drive to facilitate working on multiple drives at Windows" (#14598)
Reverts nushell/nushell#14411 |
||
|
cc0616b753
|
return const values from scope variables (#14577)
Fixes #14542 # User-Facing Changes Constant values are no longer missing from `scope variables` output when the IR evaluator is enabled: ```diff const foo = 1 scope variables | where name == "$foo" | get value.0 | to nuon -null +int ``` |
||
|
8f9aa1a250
|
Change help commands to use name from scope instead of the name from the declaration (#14490)
# Description Before this PR, `help commands` uses the name from a command's declaration rather than the name in the scope. This is problematic when trying to view the help page for the `main` command of a module. For example, `std bench`: ```nushell use std/bench help bench # => Error: nu::parser::not_found # => # => × Not found. # => ╭─[entry #10:1:6] # => 1 │ help bench # => · ──┬── # => · ╰── did not find anything under this name # => ╰──── ``` This can also cause confusion when importing specific commands from modules. Furthermore, if there are multiple commands with the same name from different modules, the help text for _both_ will appear when querying their help text (this is especially problematic for `main` commands, see #14033): ```nushell use std/iter help iter find # => Error: nu::parser::not_found # => # => × Not found. # => ╭─[entry #3:1:6] # => 1│ help iter find # => · ────┬──── # => · ╰── did not find anything under this name # => ╰──── help find # => Searches terms in the input. # => # => Search terms: filter, regex, search, condition # => # => Usage: # => > find {flags} ...(rest) # [...] # => Returns the first element of the list that matches the # => closure predicate, `null` otherwise # [...] # (full text omitted for brevity) ``` This PR changes `help commands` to use the name as it is in scope, so prefixing any command in scope with `help` will show the correct help text. ```nushell use std/bench help bench # [help text for std bench] use std/iter help iter find # [help text for std iter find] use std help std bench # [help text for std bench] help std iter find # [help text for std iter find] ``` Additionally, the IR code generation for commands called with the `--help` text has been updated to reflect this change. This does have one side effect: when a module has a `main` command defined, running `help <name>` (which checks `help aliases`, then `help commands`, then `help modules`) will show the help text for the `main` command rather than the module. The help text for the module is still accessible with `help modules <name>`. Fixes #10499, #10311, #11609, #13470, #14033, and #14402. Partially fixes #10707. Does **not** fix #11447. # User-Facing Changes * Help text for commands can be obtained by running `help <command name>`, where the command name is the same thing you would type in order to execute the command. Previously, it was the name of the function as written in the source file. * For example, for the following module `spam` with command `meow`: ```nushell module spam { # help text export def meow [] {} } ``` * Before this PR: * Regardless of how `meow` is `use`d, the help text is viewable by running `help meow`. * After this PR: * When imported with `use spam`: The `meow` command is executed by running `spam meow` and the `help` text is viewable by running `help spam meow`. * When imported with `use spam foo`: The `meow` command is executed by running `meow` and the `help` text is viewable by running `meow`. * When a module has a `main` command defined, `help <module name>` will return help for the main command, rather than the module. To access the help for the module, use `help modules <module name>`. # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting N/A |
||
|
7d2e8875e0
|
Make timeit take only closures as an argument (#14483)
# Description Fixes #14401 where expressions passed to `timeit` will execute twice. This PR removes the expression support for `timeit`, as this behavior is almost exclusive to `timeit` and can hinder migration to the IR evaluator in the future. Additionally, `timeit` used to be able to take a `block` as an argument. Blocks should probably only be allowed for parser keywords, so this PR changes `timeit` to instead only take closures as an argument. This also fixes an issue where environment updates inside the `timeit` block would affect the parent scope and all commands later in the pipeline. ```nu > timeit { $env.FOO = 'bar' }; print $env.FOO bar ``` # User-Facing Changes `timeit` now only takes a closure as the first argument. # After Submitting Update examples in the book/docs if necessary. |
||
|
fc29d82614
|
Only run from_string conversion on strings (#14509)
# Description #14249 loaded `convert_env_values()` several times to force more updates to `ENV_CONVERSION`. This allows the user to treat variables as structured data inside `config.nu` (and others). Unfortunately, `convert_env_values()` did not originally anticipate being called more than once, so it would attempt to re-convert values that had already been converted. This usually leads to an error in the conversion closure. With this PR, values are only converted with `from_string` if they are still strings; otherwise they are skipped and their existing value is used. # User-Facing Changes No user-facing change when compared to 0.100, since closures written for 0.100's `ENV_CONVERSION` now work again without errors. # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` - # After Submitting Will remove the "workaround" from the Config doc preview. |
||
|
4c9078cccc
|
add file column to scope modules output (#14524)
# Description This PR adds a `file` column to the `scope modules` output table.  # 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 toolkit.nu; toolkit test stdlib"` 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. --> |
||
|
a332712275
|
add function to make env vars case-insensitive (#14390)
# Description This PR adds a new function that allows one to get an env var case-insensitively. I did this so we can hopefully stop having problems when Windows has HKLM as path and HKCU as Path. Instead of just changing every function that used the original one, I chose the ones that I thought were specific to getting the path. I didn't want to go all in and make every env get case insensitive, but maybe we should? 🤷🏻♂️ closes #12676 # 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 toolkit.nu; toolkit test stdlib"` 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. --> |
||
|
c8b5909ee8
|
Feature: PWD-per-drive to facilitate working on multiple drives at Windows (#14411)
This PR implements PWD-per-drive as described in discussion #14355 # Description On Windows, CMD or PowerShell assigns each drive its own current directory. For example, if you are in 'C:\Windows', switch to 'D:', and navigate to 'D:\Game', you can return to 'C:\Windows' by simply typing 'C:'. This PR enables Nushell on Windows to have the same capability, allowing each drive to maintain its own PWD (Present Working Directory). # User-Facing Changes Currently, 'cd' or 'ls' only accept absolute paths if the path starts with 'C:' or another drive letter. With PWD-per-drive, users can use 'cd' (or auto cd) and 'ls' in the same way as 'cd' and 'dir' in PowerShell, or similarly to 'cd' and 'dir' in CMD (noting that cd in CMD has slightly different behavior, 'cd' for another drive only changes current directory of that drive, but does not switch there). Interaction example on switching between drives: ```Nushell ~>D: D:\>cd Test D:\Test\>C: ~>D: D:\Test\>C: ~>cd D:.. D:\>C:x/../y/../z/.. ~>cd D:Test\Test D:\Test\Test>C: ~>D:... D:\> ``` Interaction example on auto-completion at cmd line: ```Nushell ~>cd D:\test[Enter] D:\test>~[Enter] ~>D:[TAB] ~>D:\test[Enter] D:\test>c:.c[TAB] c:\users\nushell\.cargo\ c:\users\nushell\.config\ ``` Interaction example on pass PWD-per-drive to child process: (Note CMD will use it, but PowerShell will ignore it though it still prepares such info for child process) ```Nushell ~>cd D:\Test D:\Test>cd E:\Test E:\Test\>~ ~>CMD Microsoft Windows [Version 10.0.22631.4460] (c) Microsoft Corporation. All rights reserved. C:\Users\Nushell>d: D:\Test>e: E:\Test> ``` # Brief Change Description 1.Added 'crates/nu-path/src/pwd_per_drive.rs' to implement a 26-slot array mapping drive letters to PWDs. Test cases are included in the same file, along with a doctest for the usage of PWD-per-drive. 2. Modified 'crates/nu-path/src/lib.rs' to declare module of pwd_per_drive and export struct for PWD-per-drive. 3. Modified 'crates/nu-protocol/src/engine/stack.rs' to sync PWD when set_cwd() is called. Add PWD-per-drive map as member. Clone between parent and child. Stub/proxy for nu_path::expand_path_with() to facilitate filesystem commands using PWD-per-drive. 4. Modified 'crates/nu-cli/src/repl.rs' auto_cd uses PWD-per-drive to expand path. 5. Modified 'crates/nu-cli/src/completions/completion_common.rs' to expand relative path when press [TAB] at command line. 6. Modified 'crates/nu-engine/src/env.rs' to collect PWD-per-drive info as env vars for child process as CMD or PowerShell do, this can let child process inherit PWD-per-drive info. 7. Modified 'crates/nu-engine/src/eval.rs', caller clone callee's PWD-per-drive info, supporting 'def --env' 8. Modified 'crates/nu-engine/src/eval_ir.rs', 'def --env' support. Remove duplicated fn redirect_env() 9. Modified 'src/run.rs', to init PWD-per-drive when startup. filesystem commands that modified: 1. Modified 'crates/nu-command/src/filesystem/cd.rs', 1 line change to use stackscoped PWD-per-drive. Other commands, commit pending.... Local test def --env OK: ```nushell E:\study\nushell> def --env env_cd_demo [] { ::: cd ~ ::: cd D:\Project ::: cd E:Crates ::: } E:\study\nushell> E:\study\nushell> def cd_no_demo [] { ::: cd ~ ::: cd D:\Project ::: cd E:Crates ::: } E:\study\nushell> cd_no_demo E:\study\nushell> C: C:\>D: D:\>E: E:\study\nushell>env_cd_demo E:\study\nushell\crates> C: ~>D: D:\Project>E: E:\study\nushell\crates> ``` # Tests + Formatting - `cargo fmt --all -- --check` passed. - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` passed. - `cargo test --workspace` passed on Windows developer mode and Ubuntu. - `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` passed. - nushell: ``` > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` passed --------- Co-authored-by: pegasus.cadence@gmail.com <pegasus.cadence@gmail.com> |
||
|
bcd85b6f3e
|
Remove duplicate implementations of CallExt::rest (#14484)
# Description Removes unnecessary usages of `Call::rest_iter_flattened` and `get_rest_for_glob_pattern` and replaces them with `CallExt::rest`. # User-Facing Changes None |
||
|
c560bac13f
|
Add --long flag for sys cpu (#14485)
# Description Fixes #14470 where the `sys cpu` command is slow. This was done by removing the `cpu_usage` column from the default output, since it takes 400ms to calculate. Instead a `--long` flag was added that, when provided, adds back the `cpu_usage` column. ```nu # Before > bench { sys cpu | length } | get mean 401ms 591µs 896ns # After > bench { sys cpu | length } | get mean 500µs 13ns # around 1-2ms in practice ``` # User-Facing Changes - `sys cpu` no longer has a `cpu_usage` column by default. - Added a `--long` flag for `sys cpu` to add back the removed column. |
||
|
3d5f853b03
|
Start to Add WASM Support Again (#14418)
<!-- 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. --> The [nushell/demo](https://github.com/nushell/demo) project successfully demonstrated running Nushell in the browser using WASM. However, the current version of Nushell cannot be easily built for the `wasm32-unknown-unknown` target, the default for `wasm-bindgen`. This PR introduces initial support for the `wasm32-unknown-unknown` target by disabling OS-dependent features such as filesystem access, IO, and platform/system-specific functionality. This separation is achieved using a new `os` feature in the following crates: - `nu-cmd-lang` - `nu-command` - `nu-engine` - `nu-protocol` The `os` feature includes all functionality that interacts with an operating system. It is enabled by default, but can be disabled using `--no-default-features`. All crates that depend on these core crates now use `--no-default-features` to allow compilation for WASM. To demonstrate compatibility, the following script builds all crates expected to work with WASM. Direct user interaction, running external commands, working with plugins, and features requiring `openssl` are out of scope for now due to their complexity or reliance on C libraries, which are difficult to compile and link in a WASM environment. ```nushell [ # compatible crates "nu-cmd-base", "nu-cmd-extra", "nu-cmd-lang", "nu-color-config", "nu-command", "nu-derive-value", "nu-engine", "nu-glob", "nu-json", "nu-parser", "nu-path", "nu-pretty-hex", "nu-protocol", "nu-std", "nu-system", "nu-table", "nu-term-grid", "nu-utils", "nuon" ] | each {cargo build -p $in --target wasm32-unknown-unknown --no-default-features} ``` ## Caveats This PR has a few caveats: 1. **`miette` and `terminal-size` Dependency Issue** `miette` depends on `terminal-size`, which uses `rustix` when the target is not Windows. However, `rustix` requires `std::os::unix`, which is unavailable in WASM. To address this, I opened a [PR](https://github.com/eminence/terminal-size/pull/68) for `terminal-size` to conditionally compile `rustix` only when the target is Unix. For now, the `Cargo.toml` includes patches to: - Use my forked version of `terminal-size`. - ~~Use an unreleased version of `miette` that depends on `terminal-size@0.4`.~~ These patches are temporary and can be removed once the upstream changes are merged and released. 2. **Test Output Adjustments** Due to the slight bump in the `miette` version, one test required adjustments to accommodate minor formatting changes in the error output, such as shifted newlines. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> This shouldn't break anything but allows using some crates for targeting `wasm32-unknown-unknown` to revive the demo page eventually. # 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` I did not add any extra tests, I just checked that compiling works, also when using the host target but unselecting the `os` feature. # 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. --> ~~Breaking the wasm support can be easily done by adding some `use`s or by adding a new dependency, we should definitely add some CI that also at least builds against wasm to make sure that building for it keep working.~~ I added a job to build wasm. --------- Co-authored-by: Ian Manske <ian.manske@pm.me> |
||
|
e1f74a6d57
|
Add label rendering to try/catch rendered errors (#14477)
<!-- 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. --> Before this PR, you can access rendered error values that are raised in a `try/catch` block by accessing the `rendered` element of the catch error value: ``` $ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } my cool error: nu:🐚:directory_not_found × Directory not found help: /home/rose/nonexist.txt does not exist ``` However, the rendered errors don't include the labels present in the real rendered error, which would look like this: ``` $ ls nonexist.txt Error: nu:🐚:directory_not_found × Directory not found ╭─[entry #46:1:4] 1 │ ls nonexist.txt · ──────┬───── · ╰── directory not found ╰──── help: /home/rose/nonexist.txt does not exist ``` After this PR, the rendered error includes the labels: ``` $ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } my cool error: Error: nu:🐚:directory_not_found × Directory not found ╭─[entry #4:1:10] 1 │ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } · ──────┬───── · ╰── directory not found ╰──── help: /home/rose/nonexist.txt does not exist ``` This change is accomplished by using the standard error formatting code to render an error. This respects the error theme as before without any extra scaffolding, but it means that e.g., the terminal size is also respected. I think this is fine because the way the error is rendered already changed based on config, and I think that a "rendered" error should give back _exactly_ what would be shown to the user anyway. @fdncred, let me know if you have any concerns with the way this is handled since you were the one who implemented this feature in the first place. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> The `rendered` element of the `try`/`catch` error record now includes labels in the error output. # 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # 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. --> N/A |
||
|
4d3283e235
|
Change append operator to concatenation operator (#14344)
# Description The "append" operator currently serves as both the append operator and the concatenation operator. This dual role creates ambiguity when operating on nested lists. ```nu [1 2] ++ 3 # appends a value to a list [1 2 3] [1 2] ++ [3 4] # concatenates two lists [1 2 3 4] [[1 2] [3 4]] ++ [5 6] # does this give [[1 2] [3 4] [5 6]] # or [[1 2] [3 4] 5 6] ``` Another problem is that `++=` can change the type of a variable: ```nu mut str = 'hello ' $str ++= ['world'] ($str | describe) == list<string> ``` Note that appending is only relevant for lists, but concatenation is relevant for lists, strings, and binary values. Additionally, appending can be expressed in terms of concatenation (see example below). So, this PR changes the `++` operator to only perform concatenation. # User-Facing Changes Using the `++` operator with a list and a non-list value will now be a compile time or runtime error. ```nu mut list = [] $list ++= 1 # error ``` Instead, concatenate a list with one element: ```nu $list ++= [1] ``` Or use `append`: ```nu $list = $list | append 1 ``` # After Submitting Update book and docs. --------- Co-authored-by: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> |
||
|
215ca6c5ca
|
Remove the NU_DISABLE_IR option (#14293)
# Description Removes the `NU_DISABLE_IR` option and some code related to evaluating blocks with the AST evaluator. Does not entirely remove the AST evaluator yet. We still have some dependencies on expression evaluation in a few minor places which will take a little bit of effort to fix. Also changes `debug profile` to always include instructions, because the output is a little confusing otherwise, and removes the different options for instructions/exprs. # User-Facing Changes - `NU_DISABLE_IR` no longer has any effect, and is removed. There is no way to use the AST evaluator. - `debug profile` no longer has `--exprs`, `--instructions` options. - `debug profile` lists `pc` and `instruction` columns by default now. # Tests + Formatting Eval tests fixed to only use IR. # After Submitting - [ ] release notes - [ ] finish removing AST evaluator, come up with solutions for the expression evaluation. |
||
|
b7af715f6b
|
IR: Don't generate instructions for def and export def . (#14114)
# Description Fixes: #14110 Fixes: #14087 I think it's ok to not generating instruction to `def` and `export def` call. Because they just return `PipelineData::Empty` without doing anything. If nushell generates instructions for `def` and `export def`, nushell will try to capture variables for these block. It's not the time to do this. # User-Facing Changes ``` nu -c " def bar [] { let x = 1 ($x | foo) } def foo [] { foo } " ``` Will no longer raise error. # Tests + Formatting Added 4 tests |
||
|
69d81cc065
|
add command_type to help (#14165)
# Description This PR adds an indicator when listing subcommands. That indicator tells whether the command is a plugin, alias, or custom_command.  I changed some of the API to make this work a little easier, namely `get_signatures()` is now `get_signatures_and_declids()`. It was used in only one other place (run-external), so I thought it was fine to change it. There is a long-standing issue with aliases where they reference the command name instead of the alias name. This PR doesn't fix that bug. Example. ```nushell ❯ alias "str fill" = str wrap ``` ```nushell ❯ str ... other stuff Subcommands: str wrap (alias) - Alias for `str wrap` str wrap (plugin) - Wrap text passed into pipeline. ``` # User-Facing Changes Slightly different output of subcommands. |
||
|
8d4426f2f8
|
add is_const to help commands and scope commands (#14125)
# Description This PR adds `is_const` to `help commands` and `scope commands` so we can see which commands are const commands.  |
||
|
8c8f795e9e |
add rendered and json error messages in try/catch (#14082)
# Description This PR adds a couple more options for dealing with try/catch errors. It adds a `json` version of the error and a `rendered` version of the error. It also respects the error_style configuration point.  # 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 toolkit.nu; toolkit test stdlib"` 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. --> |
||
|
28b6db115a
|
Revert PRs for 0.99.1 patch (#14119)
# Description Temporarily reverts PRs merged after the 0.99.1 bump. |
||
|
e735bd475f
|
add rendered and json error messages in try/catch (#14082)
# Description This PR adds a couple more options for dealing with try/catch errors. It adds a `json` version of the error and a `rendered` version of the error. It also respects the error_style configuration point.  # 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 toolkit.nu; toolkit test stdlib"` 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. --> |
||
|
de08b68ba8
|
Fix try printing when it is not the last pipeline element (#13992)
# Description Fixes #13991. This was done by more clearly separating the case when a pipeline is drained vs when it is being written (to a file). I also added an `OutDest::Print` case which might not be strictly necessary, but is a helpful addition. # User-Facing Changes Bug fix. # Tests + Formatting Added a test. # After Submitting There are still a few redirection bugs that I found, but they require larger code changes, so I'll leave them until after the release. |
||
|
68377c176d
|
Remove superfluous separator when there's no flag description/comment (#14007)
# Description Fixes a small side-issue in #10977 - If a command flag didn't have a comment/description, it would still show an unnecessary separator at the end of the line. This fixes that, plus uses the `: ` (colon) to separate the flag from the description. This aligns with the way that named parameters are handled. # User-Facing Changes Help/doc only # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting N/A |
||
|
157494e803
|
Make get_env_var return a reference to a Value (#13987)
# Description Title says it all, changes `EngineState::get_env_var` to return a `Option<&'a Value>` instead of an owned `Option<Value>`. This avoids some unnecessary clones. I also made a similar change to the `PluginExecutionContext` trait. |
||
|
b2d0d9cf13
|
Make SpanId and RegId also use new ID struct (#13963)
# Description In the PR #13832 I used some newtypes for the old IDs. `SpanId` and `RegId` already used newtypes, to streamline the code, I made them into the same style as the other marker-based IDs. Since `RegId` should be a bit smaller (it uses a `u32` instead of `usize`) according to @devyn, I made the `Id` type generic with `usize` as the default inner value. The question still stands how `Display` should be implemented if even. # User-Facing Changes Users of the internal values of `RegId` or `SpanId` have breaking changes but who outside nushell itself even uses these? # After Submitting The IDs will be streamlined and all type-safe. |
||
|
f0c83a4459
|
Replace raw usize IDs with new types (#13832)
# Description In this PR I replaced most of the raw usize IDs with [newtypes](https://doc.rust-lang.org/rust-by-example/generics/new_types.html). Some other IDs already started using new types and in this PR I did not want to touch them. To make the implementation less repetitive, I made use of a generic `Id<T>` with marker structs. If this lands I would try to move make other IDs also in this pattern. Also at some places I needed to use `cast`, I'm not sure if the type was incorrect and therefore casting not needed or if actually different ID types intermingle sometimes. # User-Facing Changes Probably few, if you got a `DeclId` via a function and placed it later again it will still work. |
||
|
03ee54a4df
|
Fix try not working with let , etc. (#13885)
# Description Partialy addresses #13868. `try` does not catch non-zero exit code errors from the last command in a pipeline if the result is assigned to a variable using `let` (or `mut`). This was fixed by adding a new `OutDest::Value` case. This is used when the pipeline is in a "value" position. I.e., it will be collected into a value. This ended up replacing most of the usages of `OutDest::Capture`. So, this PR also renames `OutDest::Capture` to `OutDest::PipeSeparate` to better fit the few remaining use cases for it. # User-Facing Changes Bug fix. # Tests + Formatting Added two tests. |
||
|
a59477205d
|
Fix try : Add set_last_error() to prepare_error_handler() for IR eval (#13838)
# Description Fixes a bug with `set_last_error()` introduced by @IanManske not being called during the jump to an error handler in IR eval. Without this, `$env.LAST_EXIT_CODE` wasn't getting set in the `catch` block for an external. # Tests + Formatting Added a `tests/eval` test to cover this in both IR and non-IR eval |
||
|
6b5906613c
|
Fix remaining mismatch for env handling in IR (#13796)
# Description This fixes a couple of remaining differences between the IR evaluator's handling of env vars and the AST evaluator's handling of env vars. Blocker for #13718 (this is why those tests failed) # User-Facing Changes 1. Handles checking overlays for hidden env vars properly, when getting an env var from IR instruction 2. Updates config properly when doing `redirect_env()` (these probably shouldn't be separate functions anyway, though, they're basically the same. I did this because I intended to remove one, but now it's just like that) # Tests + Formatting The `nu_repl` testbin now handles `NU_USE_IR` properly, so these tests now work as expected. # After Submitting - [ ] check in on #13718 again |
||
|
493850b1bf
|
Fix IR for try (#13811)
# Description Fixes a bug in the IR for `try` to match that of the regular evaluator (continuing from #13515): ```nushell # without IR: try { ^false } catch { 'caught' } # == 'caught' # with IR: try { ^false } catch { 'caught' } # error, non-zero exit code ``` In this PR, both now evaluate to `caught`. For the implementation, I had to add another instruction, and feel free to suggest better alternatives. In the future, it might be possible to get rid of this extra instruction. # User-Facing Changes Bug fix, `try { ^false } catch { 'caught' }` now works in IR. |
||
|
3d008e2c4e
|
Error on non-zero exit statuses (#13515)
# Description This PR makes it so that non-zero exit codes and termination by signal are treated as a normal `ShellError`. Currently, these are silent errors. That is, if an external command fails, then it's code block is aborted, but the parent block can sometimes continue execution. E.g., see #8569 and this example: ```nushell [1 2] | each { ^false } ``` Before this would give: ``` ╭───┬──╮ │ 0 │ │ │ 1 │ │ ╰───┴──╯ ``` Now, this shows an error: ``` Error: nu:🐚:eval_block_with_input × Eval block failed with pipeline input ╭─[entry #1:1:2] 1 │ [1 2] | each { ^false } · ┬ · ╰── source value ╰──── Error: nu:🐚:non_zero_exit_code × External command had a non-zero exit code ╭─[entry #1:1:17] 1 │ [1 2] | each { ^false } · ──┬── · ╰── exited with code 1 ╰──── ``` This PR fixes #12874, fixes #5960, fixes #10856, and fixes #5347. This PR also partially addresses #10633 and #10624 (only the last command of a pipeline is currently checked). It looks like #8569 is already fixed, but this PR will make sure it is definitely fixed (fixes #8569). # User-Facing Changes - Non-zero exit codes and termination by signal now cause an error to be thrown. - The error record value passed to a `catch` block may now have an `exit_code` column containing the integer exit code if the error was due to an external command. - Adds new config values, `display_errors.exit_code` and `display_errors.termination_signal`, which determine whether an error message should be printed in the respective error cases. For non-interactive sessions, these are set to `true`, and for interactive sessions `display_errors.exit_code` is false (via the default config). # Tests Added a few tests. # After Submitting - Update docs and book. - Future work: - Error if other external commands besides the last in a pipeline exit with a non-zero exit code. Then, deprecate `do -c` since this will be the default behavior everywhere. - Add a better mechanism for exit codes and deprecate `$env.LAST_EXIT_CODE` (it's buggy). |
||
|
7003b007d5
|
doc: fix broken doc links (#13644)
Some broken doc links I saw when compiling with `cargo +stable doc --no-deps --document-private-items --workspace --open` |
||
|
dfdb2b5d31
|
Improve help output for scripts (#13445)
# 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. --> Currently the parser and the documentation generation use the signature of the command, which means that it doesn't pick up on the changed name of the `main` block, and therefore shows the name of the command as "main" and doesn't find the subcommands. This PR changes the aforementioned places to use the block signature to fix these issues. This closes #13397. Incidentally it also causes input/output types to be shown in the help, which is kinda pointless for scripts since they don't operate on structured data but maybe not worth the effort to remove. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> ``` # example.nu export def main [] { help main } export def 'main sub' [] { print 'sub' } ``` Before:   After:   # Tests <!-- 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 toolkit.nu; toolkit test stdlib"` 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 > ``` --> Tests are still missing for the subcommands and the input/output types --------- Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com> |
||
|
95b78eee25
|
Change the usage misnomer to "description" (#13598)
# Description The meaning of the word usage is specific to describing how a command function is *used* and not a synonym for general description. Usage can be used to describe the SYNOPSIS or EXAMPLES sections of a man page where the permitted argument combinations are shown or example *uses* are given. Let's not confuse people and call it what it is a description. Our `help` command already creates its own *Usage* section based on the available arguments and doesn't refer to the description with usage. # User-Facing Changes `help commands` and `scope commands` will now use `description` or `extra_description` `usage`-> `description` `extra_usage` -> `extra_description` Breaking change in the plugin protocol: In the signature record communicated with the engine. `usage`-> `description` `extra_usage` -> `extra_description` The same rename also takes place for the methods on `SimplePluginCommand` and `PluginCommand` # Tests + Formatting - Updated plugin protocol specific changes # After Submitting - [ ] update plugin protocol doc |
||
|
9172b22985
|
Rework help generation internals (#13531)
Reworking some of the sprawling code we use to generate the `help cmd` or `cmd --help` output. This touches mainly the rendering and not the gathering of the necessary data (see open bugs under [label:help-system](https://github.com/nushell/nushell/issues?q=sort%3Aupdated-desc+is%3Aopen+label%3Ahelp-system)) Fixes #9076 Fixes the syntax shape output on flags to be consistent. ## Example ```nushell def test [ positional: int, documented: float, # this has documentation default = 50, optional?, --a = "bla", --bla (-b) = "bla", # named with default ] {} ``` ### before  ### after  |