Commit Graph

578 Commits

Author SHA1 Message Date
Stefan Holderbach
813aac89bd
Clippy fixes for toolchain bump (#13497)
- **Suggested default impl for the new `*Stack`s**
- **Change a hashmap to make clippy happy**
- **Clone from fix**
- **Fix conditional unused in test**
- then **Bump rust toolchain**
2024-07-31 14:49:22 +02:00
Devyn Cairns
d618fd0527
Fix bad method links in docstrings (#13471)
# Description

Seems like I developed a bit of a bad habit of trying to link

```rust
/// [`.foo()`]
```

in docstrings, and this just doesn't work automatically; you have to do 

```rust
/// [`.foo()`](Self::foo)
```

if you want it to actually link. I think I found and replaced all of
these.

# User-Facing Changes

Just docs.
2024-07-27 19:39:29 -07:00
Devyn Cairns
5f7afafe51
IR: fix incorrect capturing of subexpressions (#13467)
# Description


[Discovered](https://discord.com/channels/601130461678272522/614593951969574961/1266503282554179604)
by `@warp` on Discord.

The IR compiler was not properly setting redirect modes for
subexpressions because `FullCellPath` was always being compiled with
capture-out redirection. This is the correct behavior if there is a tail
to the `FullCellPath`, as we need the value in order to try to extract
anything from it (although this is unlikely to work) - however, the
parser also generates `FullCellPath`s with an empty tail quite often,
including for bare subexpressions.

Because of this, the following did not behave as expected:

```nushell
(docker run -it --rm alpine)
```

Capturing the output meant that `docker` didn't have direct access to
the terminal as a TTY.

As this is a minor bug fix, it should be okay to include in the 0.96.1
patch release.

# User-Facing Changes

- Fixes the bug as described when running with IR evaluation enabled.

# Tests + Formatting

I added a test for this, though we're not currently running all tests
with IR on the CI, but it should ensure this behaviour is consistent.
The equivalent minimum repro I could find was:

```nushell
(nu --testbin cococo); null
```

as this should cause the `cococo` message to appear on stdout, and if
Nushell is capturing the output, it would be discarded instead.
2024-07-27 19:38:57 -07:00
Devyn Cairns
aa7d7d0cc3
Overhaul $in expressions (#13357)
# Description

This grew quite a bit beyond its original scope, but I've tried to make
`$in` a bit more consistent and easier to work with.

Instead of the parser generating calls to `collect` and creating
closures, this adds `Expr::Collect` which just evaluates in the same
scope and doesn't require any closure.

When `$in` is detected in an expression, it is replaced with a new
variable (also called `$in`) and wrapped in `Expr::Collect`. During
eval, this expression is evaluated directly, with the input and with
that new variable set to the collected value.

Other than being faster and less prone to gotchas, it also makes it
possible to typecheck the output of an expression containing `$in`,
which is nice. This is a breaking change though, because of the lack of
the closure and because now typechecking will actually happen. Also, I
haven't attempted to typecheck the input yet.

The IR generated now just looks like this:

```gas
collect        %in
clone          %tmp, %in
store-variable $in, %tmp
# %out <- ...expression... <- %in
drop-variable  $in
```

(where `$in` is the local variable created for this collection, and not
`IN_VARIABLE_ID`)

which is a lot better than having to create a closure and call `collect
--keep-env`, dealing with all of the capture gathering and allocation
that entails. Ideally we can also detect whether that input is actually
needed, so maybe we don't have to clone, but I haven't tried to do that
yet. Theoretically now that the variable is a unique one every time, it
should be possible to give it a type - I just don't know how to
determine that yet.

On top of that, I've also reworked how `$in` works in pipeline-initial
position. Previously, it was a little bit inconsistent. For example,
this worked:

```nushell
> 3 | do { let x = $in; let y = $in; print $x $y }
3
3
```

However, this causes a runtime variable not found error on the second
`$in`:

```nushell
> def foo [] { let x = $in; let y = $in; print $x $y }; 3 | foo
Error: nu:🐚:variable_not_found

  × Variable not found
   ╭─[entry #115:1:35]
 1 │ def foo [] { let x = $in; let y = $in; print $x $y }; 3 | foo
   ·                                   ─┬─
   ·                                    ╰── variable not found
   ╰────
```

I've fixed this by making the first element `$in` detection *always*
happen at the block level, so if you use `$in` in pipeline-initial
position anywhere in a block, it will collect with an implicit
subexpression around the whole thing, and you can then use that `$in`
more than once. In doing this I also rewrote `parse_pipeline()` and
hopefully it's a bit more straightforward and possibly more efficient
too now.

Finally, I've tried to make `let` and `mut` a lot more straightforward
with how they handle the rest of the pipeline, and using a redirection
with `let`/`mut` now does what you'd expect if you assume that they
consume the whole pipeline - the redirection is just processed as
normal. These both work now:

```nushell
let x = ^foo err> err.txt
let y = ^foo out+err>| str length
```

It was previously possible to accomplish this with a subexpression, but
it just seemed like a weird gotcha that you couldn't do it. Intuitively,
`let` and `mut` just seem to take the whole line.

- closes #13137

# User-Facing Changes
- `$in` will behave more consistently with blocks and closures, since
the entire block is now just wrapped to handle it if it appears in the
first pipeline element
- `$in` no longer creates a closure, so what can be done within an
expression containing `$in` is less restrictive
- `$in` containing expressions are now type checked, rather than just
resulting in `any`. However, `$in` itself is still `any`, so this isn't
quite perfect yet
- Redirections are now allowed in `let` and `mut` and behave pretty much
how you'd expect

# Tests + Formatting
Added tests to cover the new behaviour.

# After Submitting
- [ ] release notes (definitely breaking change)
2024-07-17 16:02:42 -05:00
Stefan Holderbach
c5aa15c7f6
Add top-level crate documentation/READMEs (#12907)
# Description
Add `README.md` files to each crate in our workspace (-plugins) and also
include it in the `lib.rs` documentation for <docs.rs> (if there is no
existing `lib.rs` crate documentation)

In all new README I added the defensive comment that the crates are not
considered stable for public consumption. If necessary we can adjust
this if we deem a crate useful for plugin authors.
2024-07-14 10:10:41 +02:00
Devyn Cairns
a2758e6c40
Add IR support to the debugger (#13345)
# Description

This adds tracing for each individual instruction to the `Debugger`
trait. Register contents can be inspected both when entering and leaving
an instruction, and if an instruction produced an error, a reference to
the error is also available. It's not the full `EvalContext` but it's
most of the important parts for getting an idea of what's going on.

Added support for all of this to the `Profiler` / `debug profile` as
well, and the output is quite incredible - super verbose, but you can
see every instruction that's executed and also what the result was if
it's an instruction that has a clearly defined output (many do).

# User-Facing Changes

- Added `--instructions` to `debug profile`, which adds the `pc` and
`instruction` columns to the output.
- `--expr` only works in AST mode, and `--instructions` only works in IR
mode. In the wrong mode, the output for those columns is just blank.

# Tests + Formatting

All passing.

# After Submitting

- [ ] release notes
2024-07-13 01:58:21 -07:00
Devyn Cairns
ccd0160c32
Make the store-env IR instruction also update config (#13351)
# Description

Follow up fix to #13332, so that changes to config when running under IR
actually happen as well. Since I merged them around the same time, I
forgot about this.
2024-07-11 10:49:46 -07:00
Devyn Cairns
f65bc97a54
Update config directly at assignment (#13332)
# Description

Allows `Stack` to have a modified local `Config`, which is updated
immediately when `$env.config` is assigned to. This means that even
within a script, commands that come after `$env.config` changes will
always see those changes in `Stack::get_config()`.

Also fixed a lot of cases where `engine_state.get_config()` was used
even when `Stack` was available.

Closes #13324.

# User-Facing Changes
- Config changes apply immediately after the assignment is executed,
rather than whenever config is read by a command that needs it.
- Potentially slower performance when executing a lot of lines that
change `$env.config` one after another. Recommended to get `$env.config`
into a `mut` variable first and do modifications, then assign it back.
- Much faster performance when executing a script that made
modifications to `$env.config`, as the changes are only parsed once.

# Tests + Formatting
All passing.

# After Submitting
- [ ] release notes
2024-07-11 06:09:33 -07:00
Darren Schroeder
ac561b1b0e
quick fix up for ir pr as_refs (#13340)
# Description

Was having an issue compiling main after the IR pr. Talked to devyn and
he led me to change a couple things real quick and we're compiling once
again.
2024-07-11 09:19:06 +08:00
Devyn Cairns
1a5bf2447a
Use Arc for environment variables on the stack (#13333)
# Description

This is another easy performance lift that just changes `env_vars` and
`env_hidden` on `Stack` to use `Arc`. I noticed that these were being
cloned on essentially every closure invocation during captures
gathering, so we're paying the cost for all of that even when we don't
change anything. On top of that, for `env_vars`, there's actually an
entirely fresh `HashMap` created for each child scope, so it's highly
unlikely that we'll modify the parent ones.

Uses `Arc::make_mut` instead to take care of things when we need to
mutate something, and most of the time nothing has to be cloned at all.

# Benchmarks

The benefits are greater the more calls there are to env-cloning
functions like `captures_to_stack()`. Calling custom commands in a loop
is basically best case for a performance improvement. Plain `each` with
a literal block isn't so badly affected because the stack is set up
once.

## random_bytes.nu

```nushell
use std bench
do {
  const SCRIPT = ../nu_scripts/benchmarks/random-bytes.nu
  let before_change = bench { nu $SCRIPT }
  let after_change = bench { target/release/nu $SCRIPT }
  {
    before: ($before_change | reject times),
    after: ($after_change | reject times)
  }
}
```

```
╭────────┬──────────────────────────────╮
│        │ ╭──────┬───────────────────╮ │
│ before │ │ mean │ 603ms 759µs 727ns │ │
│        │ │ min  │ 593ms 298µs 167ns │ │
│        │ │ max  │ 648ms 612µs 291ns │ │
│        │ │ std  │ 9ms 335µs 251ns   │ │
│        │ ╰──────┴───────────────────╯ │
│        │ ╭──────┬───────────────────╮ │
│ after  │ │ mean │ 518ms 400µs 557ns │ │
│        │ │ min  │ 507ms 762µs 583ns │ │
│        │ │ max  │ 566ms 695µs 166ns │ │
│        │ │ std  │ 9ms 554µs 767ns   │ │
│        │ ╰──────┴───────────────────╯ │
╰────────┴──────────────────────────────╯
```

## gradient_benchmark_no_check.nu

```nushell
use std bench
do {
  const SCRIPT = ../nu_scripts/benchmarks/gradient_benchmark_no_check.nu
  let before_change = bench { nu $SCRIPT }
  let after_change = bench { target/release/nu $SCRIPT }
  {
    before: ($before_change | reject times),
    after: ($after_change | reject times)
  }
}
```

```
╭────────┬──────────────────────────────╮
│        │ ╭──────┬───────────────────╮ │
│ before │ │ mean │ 146ms 543µs 380ns │ │
│        │ │ min  │ 142ms 416µs 166ns │ │
│        │ │ max  │ 189ms 595µs       │ │
│        │ │ std  │ 7ms 140µs 342ns   │ │
│        │ ╰──────┴───────────────────╯ │
│        │ ╭──────┬───────────────────╮ │
│ after  │ │ mean │ 134ms 211µs 678ns │ │
│        │ │ min  │ 132ms 433µs 125ns │ │
│        │ │ max  │ 135ms 722µs 583ns │ │
│        │ │ std  │ 793µs 134ns       │ │
│        │ ╰──────┴───────────────────╯ │
╰────────┴──────────────────────────────╯
```

# User-Facing Changes
Better performance, particularly for custom commands, especially if
there are a lot of environment variables. Nothing else.

# Tests + Formatting
All passing.
2024-07-10 17:34:50 -07:00
Devyn Cairns
d7392f1f3b
Internal representation (IR) compiler and evaluator (#13330)
# Description

This PR adds an internal representation language to Nushell, offering an
alternative evaluator based on simple instructions, stream-containing
registers, and indexed control flow. The number of registers required is
determined statically at compile-time, and the fixed size required is
allocated upon entering the block.

Each instruction is associated with a span, which makes going backwards
from IR instructions to source code very easy.

Motivations for IR:

1. **Performance.** By simplifying the evaluation path and making it
more cache-friendly and branch predictor-friendly, code that does a lot
of computation in Nushell itself can be sped up a decent bit. Because
the IR is fairly easy to reason about, we can also implement
optimization passes in the future to eliminate and simplify code.
2. **Correctness.** The instructions mostly have very simple and
easily-specified behavior, so hopefully engine changes are a little bit
easier to reason about, and they can be specified in a more formal way
at some point. I have made an effort to document each of the
instructions in the docs for the enum itself in a reasonably specific
way. Some of the errors that would have happened during evaluation
before are now moved to the compilation step instead, because they don't
make sense to check during evaluation.
3. **As an intermediate target.** This is a good step for us to bring
the [`new-nu-parser`](https://github.com/nushell/new-nu-parser) in at
some point, as code generated from new AST can be directly compared to
code generated from old AST. If the IR code is functionally equivalent,
it will behave the exact same way.
4. **Debugging.** With a little bit more work, we can probably give
control over advancing the virtual machine that `IrBlock`s run on to
some sort of external driver, making things like breakpoints and single
stepping possible. Tools like `view ir` and [`explore
ir`](https://github.com/devyn/nu_plugin_explore_ir) make it easier than
before to see what exactly is going on with your Nushell code.

The goal is to eventually replace the AST evaluator entirely, once we're
sure it's working just as well. You can help dogfood this by running
Nushell with `$env.NU_USE_IR` set to some value. The environment
variable is checked when Nushell starts, so config runs with IR, or it
can also be set on a line at the REPL to change it dynamically. It is
also checked when running `do` in case within a script you want to just
run a specific piece of code with or without IR.

# Example

```nushell
view ir { |data|
  mut sum = 0
  for n in $data {
    $sum += $n
  }
  $sum
}
```
  
```gas
# 3 registers, 19 instructions, 0 bytes of data
   0: load-literal           %0, int(0)
   1: store-variable         var 904, %0 # let
   2: drain                  %0
   3: drop                   %0
   4: load-variable          %1, var 903
   5: iterate                %0, %1, end 15 # for, label(1), from(14:)
   6: store-variable         var 905, %0
   7: load-variable          %0, var 904
   8: load-variable          %2, var 905
   9: binary-op              %0, Math(Plus), %2
  10: span                   %0
  11: store-variable         var 904, %0
  12: load-literal           %0, nothing
  13: drain                  %0
  14: jump                   5
  15: drop                   %0          # label(0), from(5:)
  16: drain                  %0
  17: load-variable          %0, var 904
  18: return                 %0
```

# Benchmarks

All benchmarks run on a base model Mac Mini M1.

## Iterative Fibonacci sequence

This is about as best case as possible, making use of the much faster
control flow. Most code will not experience a speed improvement nearly
this large.

```nushell
def fib [n: int] {
  mut a = 0
  mut b = 1
  for _ in 2..=$n {
    let c = $a + $b
    $a = $b
    $b = $c
  }
  $b
}
use std bench
bench { 0..50 | each { |n| fib $n } }
```

IR disabled:

```
╭───────┬─────────────────╮
│ mean  │ 1ms 924µs 665ns │
│ min   │ 1ms 700µs 83ns  │
│ max   │ 3ms 450µs 125ns │
│ std   │ 395µs 759ns     │
│ times │ [list 50 items] │
╰───────┴─────────────────╯
```

IR enabled:

```
╭───────┬─────────────────╮
│ mean  │ 452µs 820ns     │
│ min   │ 427µs 417ns     │
│ max   │ 540µs 167ns     │
│ std   │ 17µs 158ns      │
│ times │ [list 50 items] │
╰───────┴─────────────────╯
```

![explore ir
view](https://github.com/nushell/nushell/assets/10729/d7bccc03-5222-461c-9200-0dce71b83b83)

##
[gradient_benchmark_no_check.nu](https://github.com/nushell/nu_scripts/blob/main/benchmarks/gradient_benchmark_no_check.nu)

IR disabled:

```
╭───┬──────────────────╮
│ 0 │ 27ms 929µs 958ns │
│ 1 │ 21ms 153µs 459ns │
│ 2 │ 18ms 639µs 666ns │
│ 3 │ 19ms 554µs 583ns │
│ 4 │ 13ms 383µs 375ns │
│ 5 │ 11ms 328µs 208ns │
│ 6 │  5ms 659µs 542ns │
╰───┴──────────────────╯
```

IR enabled:

```
╭───┬──────────────────╮
│ 0 │       22ms 662µs │
│ 1 │ 17ms 221µs 792ns │
│ 2 │ 14ms 786µs 708ns │
│ 3 │ 13ms 876µs 834ns │
│ 4 │  13ms 52µs 875ns │
│ 5 │ 11ms 269µs 666ns │
│ 6 │  6ms 942µs 500ns │
╰───┴──────────────────╯
```

##
[random-bytes.nu](https://github.com/nushell/nu_scripts/blob/main/benchmarks/random-bytes.nu)

I got pretty random results out of this benchmark so I decided not to
include it. Not clear why.

# User-Facing Changes
- IR compilation errors may appear even if the user isn't evaluating
with IR.
- IR evaluation can be enabled by setting the `NU_USE_IR` environment
variable to any value.
- New command `view ir` pretty-prints the IR for a block, and `view ir
--json` can be piped into an external tool like [`explore
ir`](https://github.com/devyn/nu_plugin_explore_ir).

# Tests + Formatting
All tests are passing with `NU_USE_IR=1`, and I've added some more eval
tests to compare the results for some very core operations. I will
probably want to add some more so we don't have to always check
`NU_USE_IR=1 toolkit test --workspace` on a regular basis.

# After Submitting
- [ ] release notes
- [ ] further documentation of instructions?
- [ ] post-release: publish `nu_plugin_explore_ir`
2024-07-10 17:33:59 -07:00
Ian Manske
e98b2ceb8c
Path migration 1 (#13309)
# Description
Part 1 of replacing `std::path` types with `nu_path` types added in
#13115.
2024-07-09 17:25:23 +08:00
Ian Manske
399a7c8836
Add and use new Signals struct (#13314)
# Description
This PR introduces a new `Signals` struct to replace our adhoc passing
around of `ctrlc: Option<Arc<AtomicBool>>`. Doing so has a few benefits:
- We can better enforce when/where resetting or triggering an interrupt
is allowed.
- Consolidates `nu_utils::ctrl_c::was_pressed` and other ad-hoc
re-implementations into a single place: `Signals::check`.
- This allows us to add other types of signals later if we want. E.g.,
exiting or suspension.
- Similarly, we can more easily change the underlying implementation if
we need to in the future.
- Places that used to have a `ctrlc` of `None` now use
`Signals::empty()`, so we can double check these usages for correctness
in the future.
2024-07-07 22:29:01 +00:00
Jakub Žádník
3fae77209a
Revert "Span ID Refactor (Step 2): Make Call SpanId-friendly (#13268)" (#13292)
This reverts commit 0cfd5fbece.

The original PR messed up syntax higlighting of aliases and causes
panics of completion in the presence of alias.

<!--
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.
-->
2024-07-04 00:02:13 +03:00
Jakub Žádník
0cfd5fbece
Span ID Refactor (Step 2): Make Call SpanId-friendly (#13268)
<!--
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.
-->

Part of https://github.com/nushell/nushell/issues/12963, step 2.

This PR refactors Call and related argument structures to remove their
dependency on `Expression::span` which will be removed in the future.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Should be none. If you see some error messages that look broken, please
report.

# 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.
-->
2024-07-03 09:00:52 +03:00
NotTheDr01ds
44aa0a2de4
Table help rendering (#13182)
# Description

Mostly fixes #13149 with much of the credit to @fdncred.

This PR runs `table --expand` against `help` example results. This is
essentially the same fix that #13146 was for `std help`.

It also changes the shape of the result for the `table --expand`
example, as it was hardcoded wrong.

~Still needed is a fix for the `table --collapse` example.~ Note that
this is also still a bug in `std help` that I didn't noticed before.

# User-Facing Changes

Certain tables are now rendered correctly in the help examples for:

* `table`
* `zip`
* `flatten`
* And almost certainly others

# Tests + Formatting

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
2024-06-19 20:12:25 -05:00
Jakub Žádník
e52d7bc585
Span ID Refactor (Step 2): Use SpanId of expressions in some places (#13102)
<!--
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.
-->

Part of https://github.com/nushell/nushell/issues/12963, step 2.

This PR refactors changes the use of `expression.span` to
`expression.span_id` via a new helper `Expression::span()`. A new
`GetSpan` is added to abstract getting the span from both `EngineState`
and `StateWorkingSet`.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

`format pattern` loses the ability to use variables in the pattern,
e.g., `... | format pattern 'value of {$it.name} is {$it.value}'`. This
is because the command did a custom parse-eval cycle, creating spans
that are not merged into the main engine state. We could clone the
engine state, add Clone trait to StateDelta and merge the cloned delta
to the cloned state, but IMO there is not much value from having this
ability, since we have string interpolation nowadays: `... | $"value of
($in.name) is ($in.value)"`.

# Tests + Formatting
<!--
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.
-->
2024-06-09 12:15:53 +03:00
Embers-of-the-Fire
96493b26d9
Make string related commands parse-time evaluatable (#13032)
<!--
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!
-->

Related meta-issue: #10239.

# 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 will modify some `str`-related commands so that they can be
evaluated at parse time.

See the following list for those implemented by this pr.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Available now:
- `str` subcommands
  - `trim`
  - `contains`
  - `distance`
  - `ends-with`
  - `expand`
  - `index-of`
  - `join`
  - `replace`
  - `reverse`
  - `starts-with`
  - `stats`
  - `substring`
  - `capitalize`
  - `downcase`
  - `upcase`
- `split` subcommands
  - `chars`
  - `column`
  - `list`
  - `row`
  - `words`
- `format` subcommands
  - `date`
  - `duration`
  - `filesize`
- string related commands
  - `parse`
  - `detect columns`
  - `encode` & `decode`

# 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
> ```
-->

Unresolved questions:
- [ ] Is there any routine of testing const expressions? I haven't found
any yet.
- [ ] Is const expressions required to behave just like there non-const
version, like what rust promises?

# 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.
-->

Unresolved questions:
- [ ] Do const commands need special marks in the docs?
2024-06-05 22:21:52 +03:00
Jakub Žádník
e4104d0792
Span ID Refactor - Step 1 (#12960)
# Description
First part of SpanID refactoring series. This PR adds a `SpanId` type
and a corresponding `span_id` field to `Expression`. Parser creating
expressions will now add them to an array in `StateWorkingSet`,
generates a corresponding ID and saves the ID to the Expression. The IDs
are not used anywhere yet.

For the rough overall plan, see
https://github.com/nushell/nushell/issues/12963.

# User-Facing Changes
Hopefully none. This is only a refactor of Nushell's internals that
shouldn't have visible side effects.

# Tests + Formatting

# After Submitting
2024-06-05 09:57:14 +08:00
Devyn Cairns
c61075e20e
Add string/binary type color to ByteStream (#12897)
# Description

This PR allows byte streams to optionally be colored as being
specifically binary or string data, which guarantees that they'll be
converted to `Binary` or `String` appropriately on `into_value()`,
making them compatible with `Type` guarantees. This makes them
significantly more broadly usable for command input and output.

There is still an `Unknown` type for byte streams coming from external
commands, which uses the same behavior as we previously did where it's a
string if it's UTF-8.

A small number of commands were updated to take advantage of this, just
to prove the point. I will be adding more after this merges.

# User-Facing Changes
- New types in `describe`: `string (stream)`, `binary (stream)`
- These commands now return a stream if their input was a stream:
  - `into binary`
  - `into string`
  - `bytes collect`
  - `str join`
  - `first` (binary)
  - `last` (binary)
  - `take` (binary)
  - `skip` (binary)
- Streams that are explicitly binary colored will print as a streaming
hexdump
  - example:
    ```nushell
    1.. | each { into binary } | bytes collect
    ```

# Tests + Formatting
I've added some tests to cover it at a basic level, and it doesn't break
anything existing, but I do think more would be nice. Some of those will
come when I modify more commands to stream.

# After Submitting
There are a few things I'm not quite satisfied with:

- **String trimming behavior.** We automatically trim newlines from
streams from external commands, but I don't think we should do this with
internal commands. If I call a command that happens to turn my string
into a stream, I don't want the newline to suddenly disappear. I changed
this to specifically do it only on `Child` and `File`, but I don't know
if this is quite right, and maybe we should bring back the old flag for
`trim_end_newline`
- **Known binary always resulting in a hexdump.** It would be nice to
have a `print --raw`, so that we can put binary data on stdout
explicitly if we want to. This PR doesn't change how external commands
work though - they still dump straight to stdout.

Otherwise, here's the normal checklist:

- [ ] release notes
- [ ] docs update for plugin protocol changes (added `type` field)

---------

Co-authored-by: Ian Manske <ian.manske@pm.me>
2024-05-20 00:35:32 +00:00
Ian Manske
baeba19b22
Make get_full_help take &dyn Command (#12903)
# Description
Changes `get_full_help` to take a `&dyn Command` instead of multiple
arguments (`&Signature`, `&Examples` `is_parser_keyword`). All of these
arguments can be gathered from a `Command`, so there is no need to pass
the pieces to `get_full_help`.

This PR also fixes an issue where the search terms are not shown if
`--help` is used on a command.
2024-05-19 19:56:33 +02:00
Ian Manske
cc9f41e553
Use CommandType in more places (#12832)
# Description
Kind of a vague title, but this PR does two main things:
1. Rather than overriding functions like `Command::is_parser_keyword`,
this PR instead changes commands to override `Command::command_type`.
The `CommandType` returned by `Command::command_type` is then used to
automatically determine whether `Command::is_parser_keyword` and the
other `is_{type}` functions should return true. These changes allow us
to remove the `CommandType::Other` case and should also guarantee than
only one of the `is_{type}` functions on `Command` will return true.
2. Uses the new, reworked `Command::command_type` function in the `scope
commands` and `which` commands.


# User-Facing Changes
- Breaking change for `scope commands`: multiple columns (`is_builtin`,
`is_keyword`, `is_plugin`, etc.) have been merged into the `type`
column.
- Breaking change: the `which` command can now report `plugin` or
`keyword` instead of `built-in` in the `type` column. It may also now
report `external` instead of `custom` in the `type` column for known
`extern`s.
2024-05-18 23:37:31 +00:00
Ian Manske
6fd854ed9f
Replace ExternalStream with new ByteStream type (#12774)
# Description
This PR introduces a `ByteStream` type which is a `Read`-able stream of
bytes. Internally, it has an enum over three different byte stream
sources:
```rust
pub enum ByteStreamSource {
    Read(Box<dyn Read + Send + 'static>),
    File(File),
    Child(ChildProcess),
}
```

This is in comparison to the current `RawStream` type, which is an
`Iterator<Item = Vec<u8>>` and has to allocate for each read chunk.

Currently, `PipelineData::ExternalStream` serves a weird dual role where
it is either external command output or a wrapper around `RawStream`.
`ByteStream` makes this distinction more clear (via `ByteStreamSource`)
and replaces `PipelineData::ExternalStream` in this PR:
```rust
pub enum PipelineData {
    Empty,
    Value(Value, Option<PipelineMetadata>),
    ListStream(ListStream, Option<PipelineMetadata>),
    ByteStream(ByteStream, Option<PipelineMetadata>),
}
```

The PR is relatively large, but a decent amount of it is just repetitive
changes.

This PR fixes #7017, fixes #10763, and fixes #12369.

This PR also improves performance when piping external commands. Nushell
should, in most cases, have competitive pipeline throughput compared to,
e.g., bash.
| Command | Before (MB/s) | After (MB/s) | Bash (MB/s) |
| -------------------------------------------------- | -------------:|
------------:| -----------:|
| `throughput \| rg 'x'` | 3059 | 3744 | 3739 |
| `throughput \| nu --testbin relay o> /dev/null` | 3508 | 8087 | 8136 |

# User-Facing Changes
- This is a breaking change for the plugin communication protocol,
because the `ExternalStreamInfo` was replaced with `ByteStreamInfo`.
Plugins now only have to deal with a single input stream, as opposed to
the previous three streams: stdout, stderr, and exit code.
- The output of `describe` has been changed for external/byte streams.
- Temporary breaking change: `bytes starts-with` no longer works with
byte streams. This is to keep the PR smaller, and `bytes ends-with`
already does not work on byte streams.
- If a process core dumped, then instead of having a `Value::Error` in
the `exit_code` column of the output returned from `complete`, it now is
a `Value::Int` with the negation of the signal number.

# After Submitting
- Update docs and book as necessary
- Release notes (e.g., plugin protocol changes)
- Adapt/convert commands to work with byte streams (high priority is
`str length`, `bytes starts-with`, and maybe `bytes ends-with`).
- Refactor the `tee` code, Devyn has already done some work on this.

---------

Co-authored-by: Devyn Cairns <devyn.cairns@gmail.com>
2024-05-16 07:11:18 -07:00
Ian Manske
cab86f49c0
Fix pipe redirection into complete (#12818)
# Description
Fixes #12796 where a combined out and err pipe redirection (`o+e>|`)
into `complete` still provides separate `stdout` and `stderr` columns in
the record. Now, the combined output will be in the `stdout` column.
This PR also fixes a similar error with the `e>|` pipe redirection.

# Tests + Formatting
Added two tests.
2024-05-11 15:32:00 +00:00
YizhePKU
b9a7faad5a
Implement PWD recovery (#12779)
This PR has two parts. The first part is the addition of the
`Stack::set_pwd()` API. It strips trailing slashes from paths for
convenience, but will reject otherwise bad paths, leaving PWD in a good
state. This should reduce the impact of faulty code incorrectly trying
to set PWD.
(https://github.com/nushell/nushell/pull/12760#issuecomment-2095393012)

The second part is implementing a PWD recovery mechanism. PWD can become
bad even when we did nothing wrong. For example, Unix allows you to
remove any directory when another process might still be using it, which
means PWD can just "disappear" under our nose. This PR makes it possible
to use `cd` to reset PWD into a good state. Here's a demonstration:

```sh
mkdir /tmp/foo
cd /tmp/foo

# delete "/tmp/foo" in a subshell, because Nushell is smart and refuse to delete PWD
nu -c 'cd /; rm -r /tmp/foo'

ls          # Error:   × $env.PWD points to a non-existent directory
            # help: Use `cd` to reset $env.PWD into a good state

cd /
pwd         # prints /
```

Also, auto-cd should be working again.
2024-05-10 11:06:33 -05:00
Ian Manske
72d3860d05
Refactor the CLI code a bit (#12782)
# Description
Refactors the code in `nu-cli`, `main.rs`, `run.rs`, and few others.
Namely, I added `EngineState::generate_nu_constant` function to
eliminate some duplicate code. Otherwise, I changed a bunch of areas to
return errors instead of calling `std::process::exit`.

# User-Facing Changes
Should be none.
2024-05-10 07:29:27 +08:00
YizhePKU
7a86b98f61
Migrate to a new PWD API (part 2) (#12749)
Refer to #12603 for part 1.

We need to be careful when migrating to the new API, because the new API
has slightly different semantics (PWD can contain symlinks). This PR
handles the "obviously safe" part of the migrations. Namely, it handles
two specific use cases:

* Passing PWD into `canonicalize_with()`
* Passing PWD into `EngineState::merge_env()`

The first case is safe because symlinks are canonicalized away. The
second case is safe because `EngineState::merge_env()` only uses PWD to
call `std::env::set_current_dir()`, which shouldn't affact Nushell. The
commit message contains detailed stats on the updated files.

Because these migrations touch a lot of files, I want to keep these PRs
small to avoid merge conflicts.
2024-05-07 18:17:49 +03:00
YizhePKU
bdb6daa4b5
Migrate to a new PWD API (#12603)
This is the first PR towards migrating to a new `$env.PWD` API that
returns potentially un-canonicalized paths. Refer to PR #12515 for
motivations.

## New API: `EngineState::cwd()`

The goal of the new API is to cover both parse-time and runtime use
case, and avoid unintentional misuse. It takes an `Option<Stack>` as
argument, which if supplied, will search for `$env.PWD` on the stack in
additional to the engine state. I think with this design, there's less
confusion over parse-time and runtime environments. If you have access
to a stack, just supply it; otherwise supply `None`.

## Deprecation of other PWD-related APIs

Other APIs are re-implemented using `EngineState::cwd()` and properly
documented. They're marked deprecated, but their behavior is unchanged.
Unused APIs are deleted, and code that accesses `$env.PWD` directly
without using an API is rewritten.

Deprecated APIs:

* `EngineState::current_work_dir()`
* `StateWorkingSet::get_cwd()`
* `env::current_dir()`
* `env::current_dir_str()`
* `env::current_dir_const()`
* `env::current_dir_str_const()`

Other changes:

* `EngineState::get_cwd()` (deleted)
* `StateWorkingSet::list_env()` (deleted)
* `repl::do_run_cmd()` (rewritten with `env::current_dir_str()`)

## `cd` and `pwd` now use logical paths by default

This pulls the changes from PR #12515. It's currently somewhat broken
because using non-canonicalized paths exposed a bug in our path
normalization logic (Issue #12602). Once that is fixed, this should
work.

## Future plans

This PR needs some tests. Which test helpers should I use, and where
should I put those tests?

I noticed that unquoted paths are expanded within `eval_filepath()` and
`eval_directory()` before they even reach the `cd` command. This means
every paths is expanded twice. Is this intended?

Once this PR lands, the plan is to review all usages of the deprecated
APIs and migrate them to `EngineState::cwd()`. In the meantime, these
usages are annotated with `#[allow(deprecated)]` to avoid breaking CI.

---------

Co-authored-by: Jakub Žádník <kubouch@gmail.com>
2024-05-03 14:33:09 +03:00
YizhePKU
52d99cc60c
Change environment variables to be case-preserving (#12701)
This PR changes `$env` to be **case-preserving** instead of
case-sensitive. That is, it preserves the case of the environment
variable when it is first assigned, but subsequent retrieval and update
ignores the case.

Notably, both `$env.PATH` and `$env.Path` can now be used to read or set
the environment variable, but child processes will always see the
correct case based on the platform.

Fixes #11268.

---

This feature was surprising simple to implement, because most of the
infrastructure to support case-insensitive cell path access already
exists. The `get` command extracts data using a cell path in a
case-insensitive way (!), but accepts a `--sensitive` flag. (I think
this should be flipped around?)
2024-05-01 17:22:34 -05:00
Ian Manske
9996e4a1f8
Shrink the size of Expr (#12610)
# Description
Continuing from #12568, this PR further reduces the size of `Expr` from
64 to 40 bytes. It also reduces `Expression` from 128 to 96 bytes and
`Type` from 32 to 24 bytes.

This was accomplished by:
- for `Expr` with multiple fields (e.g., `Expr::Thing(A, B, C)`),
merging the fields into new AST struct types and then boxing this struct
(e.g. `Expr::Thing(Box<ABC>)`).
- replacing `Vec<T>` with `Box<[T]>` in multiple places. `Expr`s and
`Expression`s should rarely be mutated, if at all, so this optimization
makes sense.

By reducing the size of these types, I didn't notice a large performance
improvement (at least compared to #12568). But this PR does reduce the
memory usage of nushell. My config is somewhat light so I only noticed a
difference of 1.4MiB (38.9MiB vs 37.5MiB).

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
2024-04-24 15:46:35 +00:00
Devyn Cairns
5c7f7883c8
Add ErrSpan extension trait for Result (#12626)
# Description
This adds an extension trait to `Result` that wraps errors in `Spanned`,
saving the effort of calling `.map_err(|err| err.into_spanned(span))`
every time. This will hopefully make it even more likely that someone
will want to use a spanned `io::Error` and make it easier to remove the
impl for `From<io::Error> for ShellError` because that doesn't have span
information.
2024-04-23 10:39:55 +02:00
Ian Manske
bae6d694ca
Refactor using ClosureEval types (#12541)
# Description
Adds two new types in `nu-engine` for evaluating closures: `ClosureEval`
and `ClosureEvalOnce`. This removed some duplicate code and centralizes
our logic for setting up, running, and cleaning up closures. For
example, in the future if we are able to reduce the cloning necessary to
run a closure, then we only have to change the code related to these
types.

`ClosureEval` and `ClosureEvalOnce` are designed with a builder API.
`ClosureEval` is used to run a closure multiple times whereas
`ClosureEvalOnce` is used for a one-shot closure.

# User-Facing Changes
Should be none, unless I messed up one of the command migrations.
Actually, this will fix any unreported environment bugs for commands
that didn't reset the env after running a closure.
2024-04-22 14:15:09 +08:00
Ian Manske
3b1d405b96
Remove the Value::Block case (#12582)
# Description
`Value` describes the types of first-class values that users and scripts
can create, manipulate, pass around, and store. However, `Block`s are
not first-class values in the language, so this PR removes it from
`Value`. This removes some unnecessary code, and this change should be
invisible to the user except for the change to `scope modules` described
below.

# User-Facing Changes
Breaking change: the output of `scope modules` was changed so that
`env_block` is now `has_env_block` which is a boolean value instead of a
`Block`.

# After Submitting
Update the language guide possibly.
2024-04-21 07:03:33 +02:00
Ian Manske
d7ba8872bf
Rename IoStream to OutDest (#12433)
# Description
I spent a while trying to come up with a good name for what is currently
`IoStream`. Looking back, this name is not the best, because it:
1. Implies that it is a stream, when it all it really does is specify
the output destination for a stream/pipeline.
2. Implies that it handles input and output, when it really only handles
output.

So, this PR renames `IoStream` to `OutDest` instead, which should be
more clear.
2024-04-09 16:48:32 +00:00
pwygab
04531357b4
Exposed the recursion limit value as a config option (#12308)
<!--
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.
-->

Closes #12253.

Exposes the option as "recursion_limit" under config.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

The config file now has a new option!

# 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.
-->

Nothing else...? Do let me know if there's something I've missed!
2024-03-28 15:40:45 -05:00
Ian Manske
8e763a2fd6
Fix file redirection for externals streams (#12321)
# Description
Fixes `open --raw file o> out.txt` and other instances where
`PipelineData::ExternalStream` is created from sources that are not
external commands.
2024-03-28 20:22:54 +00:00
Ian Manske
c747ec75c9
Add command_prelude module (#12291)
# Description
When implementing a `Command`, one must also import all the types
present in the function signatures for `Command`. This makes it so that
we often import the same set of types in each command implementation
file. E.g., something like this:
```rust
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
    record, Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData,
    ShellError, Signature, Span, Type, Value,
};
```

This PR adds the `nu_engine::command_prelude` module which contains the
necessary and commonly used types to implement a `Command`:
```rust
// command_prelude.rs
pub use crate::CallExt;
pub use nu_protocol::{
    ast::{Call, CellPath},
    engine::{Command, EngineState, Stack},
    record, Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, IntoSpanned,
    PipelineData, Record, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
};
```

This should reduce the boilerplate needed to implement a command and
also gives us a place to track the breadth of the `Command` API. I tried
to be conservative with what went into the prelude modules, since it
might be hard/annoying to remove items from the prelude in the future.
Let me know if something should be included or excluded.
2024-03-26 21:17:30 +00:00
Filip Andersson
b70766e6f5
Boxes record for smaller Value enum. (#12252)
<!--
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.
-->
Boxes `Record` inside `Value` to reduce memory usage, `Value` goes from
`72` -> `56` bytes after this change.
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
2024-03-26 17:17:44 +02:00
Stefan Holderbach
24d2c8dd8e
Follow API guidelines for public types (#12283)
# Description
Follow the [API guideline naming
conventions](https://rust-lang.github.io/api-guidelines/naming.html)
also for our externally exposed types

(See
[`clippy::wrong_self_convention`](https://rust-lang.github.io/rust-clippy/master/index.html#/wrong_self_convention)
with [`avoid-breaking-exported-api =
false`](https://doc.rust-lang.org/clippy/lint_configuration.html#avoid-breaking-exported-api)
)

Also be a good citizen around doccomments

- **Fix `Unit::to_value` to `Unit::build_value`**
- **Fix `PipelineData::is_external_failed` to `check_external_failed`**
- **Fix doccomment on `check_external_failed`**
- **Fix `Value::into_config` naming to `parse_as_config`**
- **Document `Value::parse_as_config`**

# Plugin-Author-Facing Changes
See renames above
2024-03-26 12:12:25 +01:00
Wind
87c5f6e455
ls, rm, cp, open, touch, mkdir: Don't expand tilde if input path is quoted string or a variable. (#12232)
# Description
Fixes:  #11887
Fixes: #11626

This pr unify the tilde expand behavior over several filesystem relative
commands. It follows the same rule with glob expansion:
|  command  |  result |
| ----------- |  ------ |
| ls ~/aaa  | expand tilde
| ls "~/aaa"  | don't expand tilde
| let f = "~/aaa"; ls $f | don't expand tilde, if you want to: use `ls
($f \| path expand)`
| let f: glob = "~/aaa"; ls $f | expand tilde, they don't expand on
`mkdir`, `touch` comamnd.

Actually I'm not sure for 4th item, currently it's expanding is just
because it followes the same rule with glob expansion.

### About the change
It changes `expand_path_with` to accept a new argument called
`expand_tilde`, if it's true, expand it, if not, just keep it as `~`
itself.

# User-Facing Changes
After this change, `ls "~/aaa"` won't expand tilde.

# Tests + Formatting
Done
2024-03-25 10:08:38 +08:00
Devyn Cairns
cf321ab510
Make EngineState clone cheaper with Arc on all of the heavy objects (#12229)
# Description
This makes many of the larger objects in `EngineState` into `Arc`, and
uses `Arc::make_mut` to do clone-on-write if the reference is not
unique. This is generally very cheap, giving us the best of both worlds
- allowing us to mutate without cloning if we have an exclusive
reference, and cloning if we don't.

This started as more of a curiosity for me after remembering that
`Arc::make_mut` exists and can make using `Arc` for mostly immutable
data that sometimes needs to be changed very convenient, and also after
hearing someone complain about memory usage on Discord - this is a
somewhat significant win for that.

The exact objects that were wrapped in `Arc`:

- `files`, `file_contents` - the strings and byte buffers
- `decls` - the whole `Vec`, but mostly to avoid lots of individual
`malloc()` calls on Clone rather than for memory usage
- `blocks` - the blocks themselves, rather than the outer Vec
- `modules` - the modules themselves, rather than the outer Vec
- `env_vars`, `previous_env_vars` - the entire maps
- `config`

The changes required were relatively minimal, but this is a breaking API
change. In particular, blocks are added as Arcs, to allow the parser
cache functionality to work.

With my normal nu config, running on Linux, this saves me about 15 MiB
of process memory usage when running interactively (65 MiB → 50 MiB).

This also makes quick command executions cheaper, particularly since
every REPL loop now involves a clone of the engine state so that we can
recover from a panic. It also reduces memory usage where engine state
needs to be cloned and sent to another thread or kept within an
iterator.

# User-Facing Changes
Shouldn't be any, since it's all internal stuff, but it does change some
public interfaces so it's a breaking change
2024-03-19 19:07:00 +01:00
Ian Manske
127c4a9e63
Fix wrong stdout with e>| (#12227)
# Description
Fixes a bug where stdout would not be the terminal if a `e>|` pipe was
used.
2024-03-18 15:03:30 +00:00
Ian Manske
b6c7656194
IO and redirection overhaul (#11934)
# Description
The PR overhauls how IO redirection is handled, allowing more explicit
and fine-grain control over `stdout` and `stderr` output as well as more
efficient IO and piping.

To summarize the changes in this PR:
- Added a new `IoStream` type to indicate the intended destination for a
pipeline element's `stdout` and `stderr`.
- The `stdout` and `stderr` `IoStream`s are stored in the `Stack` and to
avoid adding 6 additional arguments to every eval function and
`Command::run`. The `stdout` and `stderr` streams can be temporarily
overwritten through functions on `Stack` and these functions will return
a guard that restores the original `stdout` and `stderr` when dropped.
- In the AST, redirections are now directly part of a `PipelineElement`
as a `Option<Redirection>` field instead of having multiple different
`PipelineElement` enum variants for each kind of redirection. This
required changes to the parser, mainly in `lite_parser.rs`.
- `Command`s can also set a `IoStream` override/redirection which will
apply to the previous command in the pipeline. This is used, for
example, in `ignore` to allow the previous external command to have its
stdout redirected to `Stdio::null()` at spawn time. In contrast, the
current implementation has to create an os pipe and manually consume the
output on nushell's side. File and pipe redirections (`o>`, `e>`, `e>|`,
etc.) have precedence over overrides from commands.

This PR improves piping and IO speed, partially addressing #10763. Using
the `throughput` command from that issue, this PR gives the following
speedup on my setup for the commands below:
| Command | Before (MB/s) | After (MB/s) | Bash (MB/s) |
| --------------------------- | -------------:| ------------:|
-----------:|
| `throughput o> /dev/null` | 1169 | 52938 | 54305 |
| `throughput \| ignore` | 840 | 55438 | N/A |
| `throughput \| null` | Error | 53617 | N/A |
| `throughput \| rg 'x'` | 1165 | 3049 | 3736 |
| `(throughput) \| rg 'x'` | 810 | 3085 | 3815 |

(Numbers above are the median samples for throughput)

This PR also paves the way to refactor our `ExternalStream` handling in
the various commands. For example, this PR already fixes the following
code:
```nushell
^sh -c 'echo -n "hello "; sleep 0; echo "world"' | find "hello world"
```
This returns an empty list on 0.90.1 and returns a highlighted "hello
world" on this PR.

Since the `stdout` and `stderr` `IoStream`s are available to commands
when they are run, then this unlocks the potential for more convenient
behavior. E.g., the `find` command can disable its ansi highlighting if
it detects that the output `IoStream` is not the terminal. Knowing the
output streams will also allow background job output to be redirected
more easily and efficiently.

# User-Facing Changes
- External commands returned from closures will be collected (in most
cases):
  ```nushell
  1..2 | each {|_| nu -c "print a" }
  ```
This gives `["a", "a"]` on this PR, whereas this used to print "a\na\n"
and then return an empty list.

  ```nushell
  1..2 | each {|_| nu -c "print -e a" }
  ```
This gives `["", ""]` and prints "a\na\n" to stderr, whereas this used
to return an empty list and print "a\na\n" to stderr.

- Trailing new lines are always trimmed for external commands when
piping into internal commands or collecting it as a value. (Failure to
decode the output as utf-8 will keep the trailing newline for the last
binary value.) In the current nushell version, the following three code
snippets differ only in parenthesis placement, but they all also have
different outputs:

  1. `1..2 | each { ^echo a }`
     ```
     a
     a
     ╭────────────╮
     │ empty list │
     ╰────────────╯
     ```
  2. `1..2 | each { (^echo a) }`
     ```
     ╭───┬───╮
     │ 0 │ a │
     │ 1 │ a │
     ╰───┴───╯
     ```
  3. `1..2 | (each { ^echo a })`
     ```
     ╭───┬───╮
     │ 0 │ a │
     │   │   │
     │ 1 │ a │
     │   │   │
     ╰───┴───╯
     ```

  But in this PR, the above snippets will all have the same output:
  ```
  ╭───┬───╮
  │ 0 │ a │
  │ 1 │ a │
  ╰───┴───╯
  ```

- All existing flags on `run-external` are now deprecated.

- File redirections now apply to all commands inside a code block:
  ```nushell
  (nu -c "print -e a"; nu -c "print -e b") e> test.out
  ```
This gives "a\nb\n" in `test.out` and prints nothing. The same result
would happen when printing to stdout and using a `o>` file redirection.

- External command output will (almost) never be ignored, and ignoring
output must be explicit now:
  ```nushell
  (^echo a; ^echo b)
  ```
This prints "a\nb\n", whereas this used to print only "b\n". This only
applies to external commands; values and internal commands not in return
position will not print anything (e.g., `(echo a; echo b)` still only
prints "b").

- `complete` now always captures stderr (`do` is not necessary).

# After Submitting
The language guide and other documentation will need to be updated.
2024-03-14 15:51:55 -05:00
Ian Manske
26786a759e
Fix ignored clippy lints (#12160)
# Description
Fixes some ignored clippy lints.

# User-Facing Changes
Changes some signatures and return types to `&dyn Command` instead of
`&Box<dyn Command`, but I believe this is only an internal change.
2024-03-11 19:46:04 +01:00
Devyn Cairns
bc19be25b1
Keep plugins persistently running in the background (#12064)
# Description
This PR uses the new plugin protocol to intelligently keep plugin
processes running in the background for further plugin calls.

Running plugins can be seen by running the new `plugin list` command,
and stopped by running the new `plugin stop` command.

This is an enhancement for the performance of plugins, as starting new
plugin processes has overhead, especially for plugins in languages that
take a significant amount of time on startup. It also enables plugins
that have persistent state between commands, making the migration of
features like dataframes and `stor` to plugins possible.

Plugins are automatically stopped by the new plugin garbage collector,
configurable with `$env.config.plugin_gc`:

```nushell
  $env.config.plugin_gc = {
      # Configuration for plugin garbage collection
      default: {
          enabled: true # true to enable stopping of inactive plugins
          stop_after: 10sec # how long to wait after a plugin is inactive to stop it
      }
      plugins: {
          # alternate configuration for specific plugins, by name, for example:
          #
          # gstat: {
          #     enabled: false
          # }
      }
  }
```

If garbage collection is enabled, plugins will be stopped after
`stop_after` passes after they were last active. Plugins are counted as
inactive if they have no running plugin calls. Reading the stream from
the response of a plugin call is still considered to be activity, but if
a plugin holds on to a stream but the call ends without an active
streaming response, it is not counted as active even if it is reading
it. Plugins can explicitly disable the GC as appropriate with
`engine.set_gc_disabled(true)`.

The `version` command now lists plugin names rather than plugin
commands. The list of plugin commands is accessible via `plugin list`.

Recommend doing this together with #12029, because it will likely force
plugin developers to do the right thing with mutability and lead to less
unexpected behavior when running plugins nested / in parallel.

# User-Facing Changes
- new command: `plugin list`
- new command: `plugin stop`
- changed command: `version` (now lists plugin names, rather than
commands)
- new config: `$env.config.plugin_gc`
- Plugins will keep running and be reused, at least for the configured
GC period
- Plugins that used mutable state in weird ways like `inc` did might
misbehave until fixed
- Plugins can disable GC if they need to
- Had to change plugin signature to accept `&EngineInterface` so that
the GC disable feature works. #12029 does this anyway, and I'm expecting
(resolvable) conflicts with that

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

Because there is some specific OS behavior required for plugins to not
respond to Ctrl-C directly, I've developed against and tested on both
Linux and Windows to ensure that works properly.

# After Submitting
I think this probably needs to be in the book somewhere
2024-03-09 17:10:22 -06:00
Jakub Žádník
14d1c67863
Debugger experiments (#11441)
<!--
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 a new evaluator path with callbacks to a mutable trait
object implementing a Debugger trait. The trait object can do anything,
e.g., profiling, code coverage, step debugging. Currently,
entering/leaving a block and a pipeline element is marked with
callbacks, but more callbacks can be added as necessary. Not all
callbacks need to be used by all debuggers; unused ones are simply empty
calls. A simple profiler is implemented as a proof of concept.

The debugging support is implementing by making `eval_xxx()` functions
generic depending on whether we're debugging or not. This has zero
computational overhead, but makes the binary slightly larger (see
benchmarks below). `eval_xxx()` variants called from commands (like
`eval_block_with_early_return()` in `each`) are chosen with a dynamic
dispatch for two reasons: to not grow the binary size due to duplicating
the code of many commands, and for the fact that it isn't possible
because it would make Command trait objects object-unsafe.

In the future, I hope it will be possible to allow plugin callbacks such
that users would be able to implement their profiler plugins instead of
having to recompile Nushell.
[DAP](https://microsoft.github.io/debug-adapter-protocol/) would also be
interesting to explore.

Try `help debug profile`.

## Screenshots

Basic output:

![profiler_new](https://github.com/nushell/nushell/assets/25571562/418b9df0-b659-4dcb-b023-2d5fcef2c865)

To profile with more granularity, increase the profiler depth (you'll
see that repeated `is-windows` calls take a large chunk of total time,
making it a good candidate for optimizing):

![profiler_new_m3](https://github.com/nushell/nushell/assets/25571562/636d756d-5d56-460c-a372-14716f65f37f)

## Benchmarks

### Binary size

Binary size increase vs. main: **+40360 bytes**. _(Both built with
`--release --features=extra,dataframe`.)_

### Time

```nushell
# bench_debug.nu
use std bench

let test = {
    1..100
    | each {
        ls | each {|row| $row.name | str length }
    }
    | flatten
    | math avg
}

print 'debug:'
let res2 = bench { debug profile $test } --pretty
print $res2
```

```nushell
# bench_nodebug.nu
use std bench

let test = {
    1..100
    | each {
        ls | each {|row| $row.name | str length }
    }
    | flatten
    | math avg
}

print 'no debug:'
let res1 = bench { do $test } --pretty
print $res1
```

`cargo run --release -- bench_debug.nu` is consistently 1--2 ms slower
than `cargo run --release -- bench_nodebug.nu` due to the collection
overhead + gathering the report. This is expected. When gathering more
stuff, the overhead is obviously higher.

`cargo run --release -- bench_nodebug.nu` vs. `nu bench_nodebug.nu` I
didn't measure any difference. Both benchmarks report times between 97
and 103 ms randomly, without one being consistently higher than the
other. This suggests that at least in this particular case, when not
running any debugger, there is no runtime overhead.

## API changes

This PR adds a generic parameter to all `eval_xxx` functions that forces
you to specify whether you use the debugger. You can resolve it in two
ways:
* Use a provided helper that will figure it out for you. If you wanted
to use `eval_block(&engine_state, ...)`, call `let eval_block =
get_eval_block(&engine_state); eval_block(&engine_state, ...)`
* If you know you're in an evaluation path that doesn't need debugger
support, call `eval_block::<WithoutDebug>(&engine_state, ...)` (this is
the case of hooks, for example).

I tried to add more explanation in the docstring of `debugger_trait.rs`.

## TODO

- [x] Better profiler output to reduce spam of iterative commands like
`each`
- [x] Resolve `TODO: DEBUG` comments
- [x] Resolve unwraps
- [x] Add doc comments
- [x] Add usage and extra usage for `debug profile`, explaining all
columns

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Hopefully none.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
2024-03-08 20:21:35 +02:00
Devyn Cairns
626d597527
Replace panics with errors in thread spawning (#12040)
# Description
Replace panics with errors in thread spawning.

Also adds `IntoSpanned` trait for easily constructing `Spanned`, and an
implementation of `From<Spanned<std::io::Error>>` for `ShellError`,
which is used to provide context for the error wherever there was a span
conveniently available. In general this should make it more convenient
to do the right thing with `std::io::Error` and always add a span to it
when it's possible to do so.

# User-Facing Changes
Fewer panics!

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
2024-03-02 11:14:02 -06:00
Stefan Holderbach
7884de1941
Remove some unnecessary static Vecs (#11947)
Avoid unnecessary allocations or larger iterator structs

- Turn static `Vec`s into arrays when possible
- Use `std::iter::once`/`empty` where applicable
- Use `bool::then_some` in `detect column` `.chain`
- Drop in the bucket: de-vec-ing tests
2024-02-24 20:58:01 +01:00
Wind
f7d647ac3c
open, rm, umv, cp, rm and du: Don't globs if inputs are variables or string interpolation (#11886)
# Description
This is a follow up to
https://github.com/nushell/nushell/pull/11621#issuecomment-1937484322

Also Fixes: #11838 

## About the code change
It applys the same logic when we pass variables to external commands:


0487e9ffcb/crates/nu-command/src/system/run_external.rs (L162-L170)

That is: if user input dynamic things(like variables, sub-expression, or
string interpolation), it returns a quoted `NuPath`, then user input
won't be globbed
 
# User-Facing Changes
Given two input files: `a*c.txt`, `abc.txt`

* `let f = "a*c.txt"; rm $f` will remove one file: `a*c.txt`. 
~* `let f = "a*c.txt"; rm --glob $f` will remove `a*c.txt` and
`abc.txt`~
* `let f: glob = "a*c.txt"; rm $f` will remove `a*c.txt` and `abc.txt`

## Rules about globbing with *variable*
Given two files: `a*c.txt`, `abc.txt`
| Cmd Type | example | Result |
| ----- | ------------------ | ------ |
| builtin | let f = "a*c.txt"; rm $f | remove `a*c.txt` |
| builtin | let f: glob = "a*c.txt"; rm $f | remove `a*c.txt` and
`abc.txt`
| builtin | let f = "a*c.txt"; rm ($f \| into glob) | remove `a*c.txt`
and `abc.txt`
| custom | def crm [f: glob] { rm $f }; let f = "a*c.txt"; crm $f |
remove `a*c.txt` and `abc.txt`
| custom | def crm [f: glob] { rm ($f \| into string) }; let f =
"a*c.txt"; crm $f | remove `a*c.txt`
| custom | def crm [f: string] { rm $f }; let f = "a*c.txt"; crm $f |
remove `a*c.txt`
| custom | def crm [f: string] { rm $f }; let f = "a*c.txt"; crm ($f \|
into glob) | remove `a*c.txt` and `abc.txt`

In general, if a variable is annotated with `glob` type, nushell will
expand glob pattern. Or else, we need to use `into | glob` to expand
glob pattern

# Tests + Formatting
Done

# After Submitting
I think `str glob-escape` command will be no-longer required. We can
remove it.
2024-02-23 09:17:09 +08:00
Wind
1058707a29
make stderr works for failed external command (#11914)
# Description
Fixes: #11913

When running external command, nushell shouldn't consumes stderr
messages, if user want to redirect stderr.

# User-Facing Changes
NaN

# Tests + Formatting
Done

# After Submitting
NaN
2024-02-21 21:15:05 +08:00