<!--
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.
-->
# 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.
-->
The idea comes from @amtoine, I think it would be good to keey
`display_error.exit_code` same value, if user is using default config or
using no config file at all.
# 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.
# Description
@Yethal discovered that `FooterMode::Auto` in the config as
`$env.config.footer_mode = auto` did not work. This PR attempts to fix
that problem by implementing the auto algorithm that was already
supposed to work.
# 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.
-->
# Description
This PR standardizes updates to the config through a new
`UpdateFromValue` trait. For now, this trait is private in case we need
to make changes to it.
Note that this PR adds some additional `ShellError` cases to create
standard error messages for config errors. A follow-up PR will move
usages of the old error cases to these new ones. This PR also uses
`Type::custom` in lots of places (e.g., for string enums). Not sure if
this is something we want to encourage.
# User-Facing Changes
Should be none.
# Description
Currently there is a bit of chaos regarding construction of history file
paths. Various pieces of code across a number of crates reimplement the
same/similar logic:
- There is `get_history_path`, but it requires a directory parameter (it
really just joins it with a file name).
- Some places use a const for the directory parameter, others use a
string literal - in all cases the value seems to be `"nushell"`.
- Some places assume the `"nushell"` value, other plumb it down from
close to the top of the call stack.
- Some places use a constant for history file names while others assume
it.
This PR tries to make it so that the history/config path format is
defined in a single places and so dependencies on it are easier to
follow:
- It removes `get_history_path` and adds a `file_path` method to
`HistoryConfig` instead (an extra motivation being, this is a convenient
place that can be used from all creates that need a history file path)
- Adds a `nu_config_dir` function that returns the nushell configuration
directory.
- Updates existing code to rely on the above, effectively removing
duplicate uses of `"nushell"` and `NUSHELL_FOLDER` and assumptions about
file names associated with different history formats
# User-Facing Changes
None
# Description
Closes#12535
Implements sort-by functionality of #8322
Fixes sort-by part of #8667
This PR does two main things: add a new cell path and closure parameter
to `sort-by`, and attempt to make Nushell's sorting behavior
well-defined.
## `sort-by` features
The `columns` parameter is replaced with a `comparator` parameter, which
can be a cell path or a closure. Examples are from docs PR.
1. Cell paths
The basic interactive usage of `sort-by` is the same. For example, `ls |
sort-by modified` still works the same as before. It is not quite a
drop-in replacement, see [behavior changes](#behavior-changes).
Here's an example of how the cell path comparator might be useful:
```nu
> let cities = [
{name: 'New York', info: { established: 1624, population: 18_819_000 } }
{name: 'Kyoto', info: { established: 794, population: 37_468_000 } }
{name: 'São Paulo', info: { established: 1554, population: 21_650_000 }
}
]
> $cities | sort-by info.established
╭───┬───────────┬────────────────────────────╮
│ # │ name │ info │
├───┼───────────┼────────────────────────────┤
│ 0 │ Kyoto │ ╭─────────────┬──────────╮ │
│ │ │ │ established │ 794 │ │
│ │ │ │ population │ 37468000 │ │
│ │ │ ╰─────────────┴──────────╯ │
│ 1 │ São Paulo │ ╭─────────────┬──────────╮ │
│ │ │ │ established │ 1554 │ │
│ │ │ │ population │ 21650000 │ │
│ │ │ ╰─────────────┴──────────╯ │
│ 2 │ New York │ ╭─────────────┬──────────╮ │
│ │ │ │ established │ 1624 │ │
│ │ │ │ population │ 18819000 │ │
│ │ │ ╰─────────────┴──────────╯ │
╰───┴───────────┴────────────────────────────╯
```
2. Key closures
You can supply a closure which will transform each value into a sorting
key (without changing the underlying data). Here's an example of a key
closure, where we want to sort a list of assignments by their average
grade:
```nu
> let assignments = [
{name: 'Homework 1', grades: [97 89 86 92 89] }
{name: 'Homework 2', grades: [91 100 60 82 91] }
{name: 'Exam 1', grades: [78 88 78 53 90] }
{name: 'Project', grades: [92 81 82 84 83] }
]
> $assignments | sort-by { get grades | math avg }
╭───┬────────────┬───────────────────────╮
│ # │ name │ grades │
├───┼────────────┼───────────────────────┤
│ 0 │ Exam 1 │ [78, 88, 78, 53, 90] │
│ 1 │ Project │ [92, 81, 82, 84, 83] │
│ 2 │ Homework 2 │ [91, 100, 60, 82, 91] │
│ 3 │ Homework 1 │ [97, 89, 86, 92, 89] │
╰───┴────────────┴───────────────────────╯
```
3. Custom sort closure
The `--custom`, or `-c`, flag will tell `sort-by` to interpret closures
as custom sort closures. A custom sort closure has two parameters, and
returns a boolean. The closure should return `true` if the first
parameter comes _before_ the second parameter in the sort order.
For a simple example, we could rewrite a cell path sort as a custom sort
(see
[here](https://github.com/nushell/nushell.github.io/pull/1568/files#diff-a7a233e66a361d8665caf3887eb71d4288000001f401670c72b95cc23a948e86R231)
for a more complex example):
```nu
> ls | sort-by -c {|a, b| $a.size < $b.size }
╭───┬─────────────────────┬──────┬──────────┬────────────────╮
│ # │ name │ type │ size │ modified │
├───┼─────────────────────┼──────┼──────────┼────────────────┤
│ 0 │ my-secret-plans.txt │ file │ 100 B │ 10 minutes ago │
│ 1 │ shopping_list.txt │ file │ 100 B │ 2 months ago │
│ 2 │ myscript.nu │ file │ 1.1 KiB │ 2 weeks ago │
│ 3 │ bigfile.img │ file │ 10.0 MiB │ 3 weeks ago │
╰───┴─────────────────────┴──────┴──────────┴────────────────╯
```
## Making sort more consistent
I think it's important for something as essential as `sort` to have
well-defined semantics. This PR contains some changes to try to make the
behavior of `sort` and `sort-by` consistent. In addition, after working
with the internals of sorting code, I have a much deeper understanding
of all of the edge cases. Here is my attempt to try to better define
some of the semantics of sorting (if you are just interested in changes,
skip to "User-Facing changes")
- `sort`, `sort -v`, and `sort-by` now all work the same. Each
individual sort implementation has been refactored into two functions in
`sort_utils.rs`: `sort`, and `sort_by`. These can also be used in other
parts of Nushell where values need to be sorted.
- `sort` and `sort-by` used to handle `-i` and `-n` differently.
- `sort -n` would consider all values which can't be coerced into a
string to be equal
- `sort-by -i` and `sort-by -n` would only work if all values were
strings
- In this PR, insensitive sort only affects comparison between strings,
and natural sort only applies to numbers and strings (see below).
- (not a change) Before and after this PR, `sort` and `sort-by` support
sorting mixed types. There was a lot of discussion about potentially
making `sort` and `sort-by` only work on lists of homogeneous types, but
the general consensus was that `sort` should not error just because its
input contains incompatible types.
- In order to try to make working with data containing `null` values
easier, I changed the PartialOrd order to sort `Nothing` values to the
end of a list, regardless of what other types the list contains. Before,
`null` would be sorted before `Binary`, `CellPath`, and `Custom` values.
- (not a change) When sorted, lists of mixed types will contain sorted
values of each type in order, for the most part
- (not a change) For example, `[0x[1] (date now) "a" ("yesterday" | into
datetime) "b" 0x[0]]` will be sorted as `["a", "b", a day ago, now, [0],
[1]]`, where sorted strings appear first, then sorted datetimes, etc.
- (not a change) The exception to this is `Int`s and `Float`s, which
will intermix, `Strings` and `Glob`s, which will intermix, and `None` as
described above. Additionally, natural sort will intermix strings with
ints and floats (see below).
- Natural sort no longer coerce all inputs to strings.
- I did originally make natural only apply to strings, but @fdncred
pointed out that the previous behavior also allowed you to sort numeric
strings with numbers. This seems like a useful feature if we are trying
to support sorting with mixed types, so I settled on coercing only
numbers (int, float). This can be reverted if people don't like it.
- Here is an example of this behavior in action, which is the same
before and after this PR:
```nushell
$ [1 "4" 3 "2"] | sort --natural
╭───┬───╮
│ 0 │ 1 │
│ 1 │ 2 │
│ 2 │ 3 │
│ 3 │ 4 │
╰───┴───╯
```
# User-Facing Changes
## New features
- Replaces the `columns` string parameter of `sort-by` with a cell path
or a closure.
- The cell path parameter works exactly as you would expect
- By default, the `closure` parameter acts as a "key sort"; that is,
each element is transformed by the closure into a sorting key
- With the `--custom` (`-c`) parameter, you can define a comparison
function for completely custom sorting order.
## Behavior changes
<details>
<summary><code>sort -v</code> does not coerce record values to
strings</summary>
This was a bit of a surprising behavior, and is now unified with the
behavior of `sort` and `sort-by`. Here's an example where you can
observe the values being implicitly coerced into strings for sorting, as
they are sorted like strings rather than numbers:
Old behavior:
```nushell
$ {foo: 9 bar: 10} | sort -v
╭─────┬────╮
│ bar │ 10 │
│ foo │ 9 │
╰─────┴────╯
```
New behavior:
```nushell
$ {foo: 9 bar: 10} | sort -v
╭─────┬────╮
│ foo │ 9 │
│ bar │ 10 │
╰─────┴────╯
```
</details>
<details>
<summary>Changed <code>sort-by</code> parameters from
<code>string</code> to <code>cell-path</code> or <code>closure</code>.
Typical interactive usage is the same as before, but if passing a
variable to <code>sort-by</code> it must be a cell path (or closure),
not a string</summary>
Old behavior:
```nushell
$ let sort = "modified"
$ ls | sort-by $sort
╭───┬──────┬──────┬──────┬────────────────╮
│ # │ name │ type │ size │ modified │
├───┼──────┼──────┼──────┼────────────────┤
│ 0 │ foo │ file │ 0 B │ 10 hours ago │
│ 1 │ bar │ file │ 0 B │ 35 seconds ago │
╰───┴──────┴──────┴──────┴────────────────╯
```
New behavior:
```nushell
$ let sort = "modified"
$ ls | sort-by $sort
Error: nu:🐚:type_mismatch
× Type mismatch.
╭─[entry #10:1:14]
1 │ ls | sort-by $sort
· ──┬──
· ╰── Cannot sort using a value which is not a cell path or closure
╰────
$ let sort = $."modified"
$ ls | sort-by $sort
╭───┬──────┬──────┬──────┬───────────────╮
│ # │ name │ type │ size │ modified │
├───┼──────┼──────┼──────┼───────────────┤
│ 0 │ foo │ file │ 0 B │ 10 hours ago │
│ 1 │ bar │ file │ 0 B │ 2 minutes ago │
╰───┴──────┴──────┴──────┴───────────────╯
```
</details>
<details>
<summary>Insensitve and natural sorting behavior reworked</summary>
Previously, the `-i` and `-n` worked differently for `sort` and
`sort-by` (see "Making sort more consistent"). Here are examples of how
these options result in different sorts now:
1. `sort -n`
- Old behavior (types other than numbers, strings, dates, and binary
sorted incorrectly)
```nushell
$ [2sec 1sec] | sort -n
╭───┬──────╮
│ 0 │ 2sec │
│ 1 │ 1sec │
╰───┴──────╯
```
- New behavior
```nushell
$ [2sec 1sec] | sort -n
╭───┬──────╮
│ 0 │ 1sec │
│ 1 │ 2sec │
╰───┴──────╯
```
2. `sort-by -i`
- Old behavior (uppercase words appear before lowercase words as they
would in a typical sort, indicating this is not actually an insensitive
sort)
```nushell
$ ["BAR" "bar" "foo" 2 "FOO" 1] | wrap a | sort-by -i a
╭───┬─────╮
│ # │ a │
├───┼─────┤
│ 0 │ 1 │
│ 1 │ 2 │
│ 2 │ BAR │
│ 3 │ FOO │
│ 4 │ bar │
│ 5 │ foo │
╰───┴─────╯
```
- New behavior (strings are sorted stably, indicating this is an
insensitive sort)
```nushell
$ ["BAR" "bar" "foo" 2 "FOO" 1] | wrap a | sort-by -i a
╭───┬─────╮
│ # │ a │
├───┼─────┤
│ 0 │ 1 │
│ 1 │ 2 │
│ 2 │ BAR │
│ 3 │ bar │
│ 4 │ foo │
│ 5 │ FOO │
╰───┴─────╯
```
3. `sort-by -n`
- Old behavior (natural sort does not work when data contains non-string
values)
```nushell
$ ["10" 8 "9"] | wrap a | sort-by -n a
╭───┬────╮
│ # │ a │
├───┼────┤
│ 0 │ 8 │
│ 1 │ 10 │
│ 2 │ 9 │
╰───┴────╯
```
- New behavior
```nushell
$ ["10" 8 "9"] | wrap a | sort-by -n a
╭───┬────╮
│ # │ a │
├───┼────┤
│ 0 │ 8 │
│ 1 │ 9 │
│ 2 │ 10 │
╰───┴────╯
```
</details>
<details>
<summary>
Sorting a list of non-record values with a non-existent column/path now
errors instead of sorting the values directly (<code>sort</code> should
be used for this, not <code>sort-by</code>)
</summary>
Old behavior:
```nushell
$ [2 1] | sort-by foo
╭───┬───╮
│ 0 │ 1 │
│ 1 │ 2 │
╰───┴───╯
```
New behavior:
```nushell
$ [2 1] | sort-by foo
Error: nu:🐚:incompatible_path_access
× Data cannot be accessed with a cell path
╭─[entry #29:1:17]
1 │ [2 1] | sort-by foo
· ─┬─
· ╰── int doesn't support cell paths
╰────
```
</details>
<details>
<summary><code>sort</code> and <code>sort-by</code> output
<code>List</code> instead of <code>ListStream</code> </summary>
This isn't a meaningful change (unless I misunderstand the purpose of
ListStream), since `sort` and `sort-by` both need to collect in order to
do the sorting anyway, but is user observable.
Old behavior:
```nushell
$ ls | sort | describe -d
╭──────────┬───────────────────╮
│ type │ stream │
│ origin │ nushell │
│ subtype │ {record 3 fields} │
│ metadata │ {record 1 field} │
╰──────────┴───────────────────╯
```
```nushell
$ ls | sort-by name | describe -d
╭──────────┬───────────────────╮
│ type │ stream │
│ origin │ nushell │
│ subtype │ {record 3 fields} │
│ metadata │ {record 1 field} │
╰──────────┴───────────────────╯
```
New behavior:
```nushell
ls | sort | describe -d
╭────────┬─────────────────╮
│ type │ list │
│ length │ 22 │
│ values │ [table 22 rows] │
╰────────┴─────────────────╯
```
```nushell
$ ls | sort-by name | describe -d
╭────────┬─────────────────╮
│ type │ list │
│ length │ 22 │
│ values │ [table 22 rows] │
╰────────┴─────────────────╯
```
</details>
- `sort` now errors when nothing is piped in (`sort-by` already did
this)
# Tests + Formatting
I added lots of unit tests on the new sort implementation to enforce new
sort behaviors and prevent regressions.
# After Submitting
See [docs PR](https://github.com/nushell/nushell.github.io/pull/1568),
which is ~2/3 finished.
---------
Co-authored-by: NotTheDr01ds <32344964+NotTheDr01ds@users.noreply.github.com>
Co-authored-by: Ian Manske <ian.manske@pm.me>
# Description
Fixes#14000 by once again calling `set_current_dir()`, but doing so
with the `cwd` from the current state, rather than the (previously
removed) argument to `merge_env()`.
# User-Facing Changes
Bug fix
# Tests + Formatting
If this looks good, I'll look at adding a test case for it.
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
N/A
Updated summary for commit
[612e0e2](612e0e2160)
- While folks are welcome to read through the entire comments, the core
information is summarized here.
# Description
This PR drastically improves startup times of Nushell by only parsing a
single submodule of the Standard Library that provides the `banner` and
`pwd` commands. All other Standard Library commands and submodules are
parsed when imported by the user. This cuts startup times by more than
60%.
At the moment, we have stopped adding to `std-lib` because every
addition adds a small amount to the Nushell startup time.
With this change, we should once again be able to allow new
functionality to be added to the Standard Library without it impacting
`nu` startup times.
# User-Facing Changes
* Nushell now starts about 60% faster
* Breaking change: The `dirs` (Shells) aliases will return a warning
message that it will not be auto-loaded in the following release, along
with instructions on how to restore it (and disable the message)
* The `use std <submodule> *` syntax is available for convenience, but
should be avoided in scripts as it parses the entire `std` module and
all other submodules and places it in scope. The correct syntax to
*just* load a submodule is `use std/<submodule> *` (asterisk optional).
The slash is important. This will be documented.
* `use std *` can be used for convenience to load all of the library but
still incurs the full loading-time.
* `std/dirs`: Semi-breaking change. The `dirs` command replaces the
`show` command. This is more in line with the directory-stack
functionality found in other shells. Existing users will not be impacted
by this as the alias (`shells`) remains the same.
* Breaking-change: Technically a breaking change, but probably only
impacts maintainers of `std`. The virtual path for the standard library
has changed. It could previously be imported using its virtual path (and
technically, this would have been the correct way to do it):
```nu
use NU_STDLIB_VIRTUAL_DIR/std
```
The path is now simply `std/`:
```nu
use std
```
All submodules have moved accordingly.
# Timings
Comparisons below were made:
* In a temporary, clean config directory using `$env.XDG_CONFIG_HOME =
(mktemp -d)`.
* `nu` was run with a release build
* `nu` was run one time to generate the default `config.nu` (etc.) files
- Otherwise timings would include the user-prompt
* The shell was exited and then restarted several times to get timing
samples
(Note: Old timings based on 0.97 rather than 0.98, but in the range of
being accurate)
| Scenario | `$nu.startup-time` |
| --- | --- |
| 0.97.2
([aaaab8e](aaaab8e070))
Without this PR | 23ms - 24ms |
| This PR with deprecated commands | 9ms - <11ms |
| This PR after deprecated commands are removed in following release |
8ms - <10ms |
| Final PR (remove deprecated), using `--no-std-lib` | 6.1ms to 6.4ms |
| Final PR (remove deprecated), using `--no-config-file` | 3.1ms - 3.6ms
|
| Final PR (remove deprecated), using `--no-config-file --no-std-lib` |
1ms - 1.5ms |
*These last two timings point to the opportunity for further
optimization (see comment in thread below (will link once I write it).*
# Implementation details for future maintenance
* `use std banner` is a ridiculously deceptive call. That call parses
and imports *all* of `std` into scope. Simply replacing it with `use
std/core *` is essentially what saves ~14-15ms. This *only* imports the
submodule with the `banner` and `pwd` commands.
* From the code-comments, the reason that `NU_STDLIB_VIRTUAL_DIR` was
used as a prefix was so that there wouldn't be an issue if a user had a
`./std/mod.nu` in the current directory. This does **not** appear to be
an issue. After removing the prefix, I tested with both a relative
module as well as one in the `$env.NU_LIB_DIRS` path, and in all cases
the *internal* `std` still took precedence.
* By removing the prefix, users can now `use std` (and variants) without
requiring that it already be parsed and in scope.
* In the next release, we'll stop autoloading the `dirs` (shells)
functionality. While this only costs an additional 1-1.5ms, I think it's
better moved to the `config.nu` where the user can optionally remove it.
The main reason is its use of aliases (which have also caused issues) -
The `n`, `p`, and `g` short-commands are valuable real-estate, and users
may want to map these to something else.
For this release, there's an `deprecated_dirs` module that is still
autoloaded. As with the top-level commands, use of these will give a
deprecation warning with instructions on how to handle going forward.
To help with this, moved the aliases to their own submodule inside the
`dirs` module.
* Also sneaks in a small change where the top-level `dirs` command is
now the replacement for `dirs show`
* Fixed a double-import of `assert` in `dirs.nu`
* The `show_banner` step is replaced with simply `banner` rather than
re-importing it.
* A `virtual_path` may now be referenced with either a forward-slash or
a backward-slash on Windows. This allows `use std/<submodule>` to work
on all platforms.
# Performance side-notes:
* Future parsing and/or IR improvements should improve performance even
further.
* While the existing load time penalty of `std-lib` was not noticeable
on many systems, Nushell runs on a wide-variety of hardware and OS
platforms. Slower platforms will naturally see a bigger jump in
performance here. For users starting multiple Nushell sessions
frequently (e.g., `tmux`, Zellij, `screen`, et. al.) it is recommended
to keep total startup time (including user configuration) under ~250ms.
# Tests + Formatting
* All tests are green
* Updated tests:
- Removed the test that confirmed that `std` was loaded (since we
don't).
- Removed the `shells` test since it is not autoloaded. Main `dirs.nu`
functionality is tested through `stdlib-test`.
- Many tests assumed that the library was fully loaded, because it was
(even though we didn't intend for it to be). Fixed those tests.
- Tests now import only the necessary submodules (e.g., `use
std/assert`, rather than `use std assert`)
- Some tests *thought* they were loading `std/log`, but were doing so
improperly. This was masked by the now-fixed "load-everything-into-scope
bug". Local CI would pass due the `$env.NU_LOG_<...>` variables being
inherited from the calling process, but would fail in the "clean" GitHub
CI environment. These tests have also been fixed.
* Added additional tests for the changes
# After Submitting
Will update the Standard Library doc page
# 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.
# Description
Fixes#13949 where `$env.LAST_EXIT_CODE` was not being set for internal
errors.
# Tests + Formatting
Cannot test due to limitations with the `nu_repl` test bin.
# 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.
# 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.
# Description
Old code was comparing remaining positional arguments with total number
of arguments, where it should've compared remaining positional with
with remaining arguments of any kind. This means that if a function was
given too few arguments, `calculate_end_span` would believe that it
actually had too many arguments, since after parsing the first few
arguments, the number of remaining arguments needed were fewer than the
*total* number of arguments, of which we had used several.
Fixes#9072
Fixes: https://github.com/nushell/nushell/issues/13930
Fixes: https://github.com/nushell/nushell/issues/12069
Fixes: https://github.com/nushell/nushell/issues/8385
Extracted from #10381
## Bonus
It also improves the error handling on missing positional arguments
before keywords (no longer crashing since #9851). Instead of just giving
the keyword to the parser for the missing positional, we give an
explicit error about a missing positional argument. I would like better
descriptions than "missing var_name" though, but I'm not sure if that's
available without
Old error
```
Error: nu::parser::parse_mismatch
× Parse mismatch during operation.
╭─[entry #1:1:1]
1 │ let = if foo
· ┬
· ╰── expected valid variable name
╰────
```
New error
```
Error: nu::parser::missing_positional
× Missing required positional argument.
╭─[entry #18:1:1]
1 │ let = foo
· ┬
· ╰── missing var_name
╰────
help: Usage: let <var_name> = <initial_value>
```
# User-Facing Changes
The program `alias = = =` is no longer accepted by the parser
This PR sets the current working directory to the location of the
Nushell executable at startup, using `std::env::set_current_dir()`. This
is desirable because after PR
https://github.com/nushell/nushell/pull/12922, we no longer change our
current working directory even after `cd` is executed, and some OS might
lock the directory where Nushell started.
The location of the Nushell executable is chosen because it cannot be
removed while Nushell is running anyways, so we don't have to worry
about OS locking it.
This PR has the side effect that it breaks buggy command even harder.
I'll keep this PR as a draft until these commands are fixed, but it
might be helpful to pull this PR if you're working on fixing one of
those bugs.
---------
Co-authored-by: Devyn Cairns <devyn.cairns@gmail.com>
Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
# Description
This PR updates the shell_integration defaults so that they work as
described in default_config.nu even when there is no config.nu file.
closes#13924
# 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.
-->
# Description
Instead of handling a foreground process being stopped in any way, we
were simply ignoring SIGTSTP (which the terminal usually sends to the
foreground process group when Ctrl-Z is pressed), and propagating this
to our foreground children. This works for most processes, but it
generally fails for applications which put the terminal in raw mode[1]
and implement their own suspension mechanism (typically TUI apps like
helix[2], neovim[3], top[4] or amp[5]). In these cases, triggering
suspension within the app results in the terminal getting blocked, since
the application is waiting for a SIGCONT, while nushell is waiting for
it to exit.
Fix this by unblocking SIGTSTP for our foreground children (neovim,
helix and probably others send this to themselves while trying to
suspend), and displaying the following message whenever one of them gets
stopped:
nushell currently does not support background jobs
press any key to continue
Pressing any key will then send SIGCONT to the child's process group and
resume waiting.
This fix is probably going to be superseded by a proper background job
implementation (#247) at some point, but for now it's better than
completely blocking the terminal.
[1]
https://docs.rs/crossterm/latest/crossterm/terminal/index.html#raw-mode
[2] https://helix-editor.com/
[3] https://neovim.io/
[4] https://man7.org/linux/man-pages/man1/top.1.html
[5] https://amp.rs/
- fixes#1038
- fixes#7335
- fixes#10335
# User-Facing Changes
While any foreground process is running, Ctrl-Z is no longer ignored.
Instead, the message described above is displayed, and nushell waits for
a key to be pressed.
# 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.
-->
# 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.
# Description
Similar to #13870 (thanks @WindSoilder), this PR adds a boolean which
determines whether to ignore any errors from an external command. This
is in order to fix#13876. I.e., `do -p` does not wait for externals to
complete before continuing.
# User-Facing Changes
Bug fix.
# Tests + Formatting
Added a test.
# Description
Makes IR the default evaluator, in preparation to remove the non-IR
evaluator in a future release.
# User-Facing Changes
* Remove `NU_USE_IR` option
* Add `NU_DISABLE_IR` option
* IR is enabled unless `NU_DISABLE_IR` is set
# After Submitting
- [ ] release notes
# 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.
# 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).
# Description
After merging #13788, I get an error message which says that
`use_grid_icons` is invalid.
I think it's good to report the specific error, and guide user to delete
it.
# Description
After looking at #13751 I noticed that the config setting
`use_grid_icons` seems out of place. So, I removed it from the config
and made it a parameter to the `grid` command.
# Description
This PR fixes#13732. However, I don't think it's a proper fix.
1. It doesn't really show what the problem is.
2. It kind of side-steps the error entirely.
I do think the change in span.rs may be valid because I don't think
span.end should ever be 0. In the example in 13732 the span end was
always 0 and so that made contains_span() return true, which seems like
a false positive.
The `checked_sub()` in ide.rs kind of just stops it from failing
outloud.
I'll leave it to smarter folks than me to land this if they think it's
worthy.
Discovered by @cptpiepmatz that #13749 broke the standalone check for
`nu-protocol`
Explicit use of the feature as workspace root also disables all features
for `serde`. Alternatively we could reconsider this there.
# Description
Cleans up and refactors the config code using the `IntoValue` macro.
Shoutout to @cptpiepmatz for making the macro!
# User-Facing Changes
Should be none.
# After Submitting
Somehow refactor the reverse transformation.
Closes#13687Closes#13686
# Description
Light refactoring of `send_request `in `client.rs`. In the end there are
more lines but now the logic is more concise and facilitates adding new
conditions in the future. Unit tests ran fine and I tested a few cases
manually.
Cool project btw, I'll be using nushell from now on.
# Description
This PR allows the helper attribute `nu_value(rename = "...")` to be
used on struct fields and enum variants. This allows renaming keys and
variants just like [`#[serde(rename =
"name")]`](https://serde.rs/field-attrs.html#rename). This has no
singular variants for `IntoValue` or `FromValue`, both need to use the
same (but I think this shouldn't be an issue for now).
# User-Facing Changes
Users of the derive macros `IntoValue` and `FromValue` may now use
`#[nu_value(rename = "...")]` to rename single fields, but no already
existing code will break.
# Description
This changes the behavior of `tee` to be more transparent when given a
value that isn't a list or range. Previously, anything that wasn't a
byte stream would converted to a list stream using the iterator
implementation, which led to some surprising results. Instead, now, if
the value is a string or binary, it will be treated the same way a byte
stream is, and the output of `tee` is a byte stream instead of the
original value. This is done so that we can synchronize with the other
thread on collect, and potentially capture any error produced by the
closure.
For values that can't be converted to streams, the closure is just run
with a clone of the value instead on another thread. Because we can't
wait for the other thread, there is no way to send an error back to the
original thread, so instead it's just written to stderr using
`report_error_new()`.
There are a couple of follow up edge cases I see where byte streams
aren't necessarily treated exactly the same way strings are, but this
should mostly be a good experience.
Fixes#13489.
# User-Facing Changes
Breaking change.
- `tee` now outputs and sends string/binary stream for string/binary
input.
- `tee` now outputs and sends the original value for any other input
other than lists/ranges.
# Tests + Formatting
Added for new behavior.
# After Submitting
- [ ] release notes: breaking change, command change
removing the `std` feature as well would drop some dependencies tied to
`rust_decimal` from the `Cargo.lock` but unclear to me what the actual
impact on compile times is.
We may want to consider dropping the `byte-unit` dependency altogether
as we have a significant fraction of our own logic to support the byte
units with 1024 and 1000 prefixes. Not sure which fraction is covered by
us or the dependency.
# Description
Implements `IntoValue` for `&str` and `DateTime` as well as other
nushell types like `Record` and `Closure`. Also allows `HashMap`s with
keys besides `String` to implement `IntoValue`.
# Description
`cargo` somewhat recently gained the capability to store `lints`
settings for the crate and workspace, that can override the defaults
from `rustc` and `clippy` lints. This means we can enforce some lints
without having to actively pass them to clippy via `cargo clippy -- -W
...`. So users just forking the repo have an easier time to follow
similar requirements like our CI.
## Limitation
An exception that remains is that those lints apply to both the primary
code base and the tests. Thus we can't include e.g. `unwrap_used`
without generating noise in the tests. Here the setup in the CI remains
the most helpful.
## Included lints
- Add `clippy::unchecked_duration_subtraction` (added by #12549)
# User-Facing Changes
Running `cargo clippy --workspace` should be closer to the CI. This has
benefits for editor configured runs of clippy and saves you from having
to use `toolkit` to be close to CI in more cases.
<!--
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.
-->
@sholderbach mentioned that I introduced `convert_case` as a dependency
while we already had `heck` for case conversion. So in this PR replaced
the use `convert_case` with `heck`. Mostly I rebuilt the `convert_case`
API with `heck` to work with it as I like the API of `convert_case` more
than `heck`.
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
Nothing changed, the use of `convert_case` wasn't exposed anywhere and
all case conversions are still available.
# 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
> ```
-->
No new tests required but my tests in `test_derive` captured some errors
I made while developing this change, (hurray, tests work 🎉)
- 🟢 `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.
-->
# Description
Using derived `IntoValue` and `FromValue` implementations on structs
with named fields currently produce `Value::Record`s where each key is
the key of the Rust struct. For records like the `$nu` constant, that
won't work as this record uses `kebab-case` for it's keys. To accomodate
this, I upgraded the `#[nu_value(rename_all = "...")]` helper attribute
to also work on structs with named fields which will rename the keys via
the same case conversion as the enums already have.
# User-Facing Changes
Users of these macros may choose different key styles for their in
`Value` representation.
# Tests + Formatting
I added the same test suite as enums already have and updated the traits
documentation with more examples that also pass the doc test.
# After Submitting
I played around with the `$nu` constant but got stuck at the point that
these keys are kebab-cased, with this, I can play around more with it.
# 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>
<!--
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 expanded the helper attribute `#[nu_value]` on
`#[derive(FromValue)]`. It now allows the usage of `#[nu_value(type_name
= "...")]` to set a type name for the `FromValue::expected_type`
implementation. Currently it only uses the default implementation but
I'd like to change that without having to manually implement the entire
trait on my own.
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
Users that derive `FromValue` may now change the name of the expected
type.
# 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
> ```
-->
I added some tests that check if this feature work and updated the
documentation about the derive macro.
- 🟢 `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.
-->
<!--
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.
-->
I was working with byte collections like `Vec<u8>` and
[`bytes::Bytes`](https://docs.rs/bytes/1.7.1/bytes/struct.Bytes.html),
both are currently not possible to be used directly in a struct that
derives `IntoValue` and `FromValue` at the same time. The `Vec<u8>` will
convert itself into a `Value::List` but expects a `Value::String` or
`Value::Binary` to load from. I now also implemented that it can load
from `Value::List` just like the other `Vec<uX>` versions. For further
working with byte collections the type `bytes::Bytes` is wildly used,
therefore I added a implementation for it. `bytes` is already part of
the dependency graph as many crates (more than 5000 to crates.io) use
it.
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
User of `nu-protocol` as library, e.g. plugin developers, can now use
byte collections more easily in their data structures and derive
`IntoValue` and `FromValue` for it.
# 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
> ```
-->
I added a few tests that check that these byte collections are correctly
translated in and from `Value`. They live in `test_derive.rs` as part of
the `ByteContainer` and I also explicitely tested that `FromValue` for
`Vec<u8>` works as expected.
- 🟢 `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.
-->
Maybe it should be explored if `Value::Binary` should use `bytes::Bytes`
instead of `Vec<u8>`.
# 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
# Description
Fixes#11267
Shifting by a `shift >= num_bits` is undefined in the underlying
operation. Previously we also had an overflow on negative shifts for the
operators `bit-shl` and `bit-shr`
Furthermore I found a severe bug in the implementation of shifting of
`binary` data with the commands `bits shl` and `bits shr`, this
categorically produced incorrect results with shifts that were not
`shift % 4 == 0`. `bits shr` also was able to produce outputs with
different size to the input if the shift was exceeding the length of the
input data by more than a byte.
# User-Facing Changes
It is now an error trying to shift by more than the available bits with:
- `bit-shl` operator
- `bit-shr` operator
- command `bits shl`
- command `bits shr`
# Tests + Formatting
Added testing for all relevant cases
# Description
As part of fixing https://github.com/nushell/nushell/issues/13586, this
PR checks the types of the operands when creating a range. Stuff like
`0..(glob .)` will be rejected at parse time. Additionally, `0..$x` will
be treated as a range and rejected if `x` is not defined, rather than
being treated as a string. A separate PR will need to be made to do
reject streams at runtime, so that stuff like `0..(open /dev/random)`
doesn't hang.
Internally, this PR adds a `ParseError::UnsupportedOperationTernary`
variant, for when you have a range like `1..2..(glob .)`.
# User-Facing Changes
Users will now receive an error if any of the operands in the ranges
they construct have types that aren't compatible with `Type::Number`.
Additionally, if a piece of code looks like a range but some parse error
is encountered while parsing it, that piece of code will still be
treated as a range and the user will be shown the parse error. This
means that a piece of code like `0..$x` will be treated as a range no
matter what. Previously, if `x` weren't the expression would've been
treated as a string `"0..$x"`. I feel like it makes the language less
complicated if we make it less context-sensitive.
Here's an example of the error you get:
```
> 0..(glob .)
Error: nu::parser::unsupported_operation
× range is not supported between int and any.
╭─[entry #1:1:1]
1 │ 0..(glob .)
· ─────┬─────┬┬
· │ │╰── any
· │ ╰── int
· ╰── doesn't support these values
╰────
```
And as an image:
Note: I made the operands themselves (above, `(glob .)`) be garbage,
rather than the `..` operator itself. This doesn't match the behavior of
the math operators (if you do `1 + "foo"`, `+` gets highlighted red).
This is because with ranges, the range operators aren't `Expression`s
themselves, so they can't be turned into garbage. I felt like here, it
makes more sense to highlight the individual operand anyway.
