<!--
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:
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):
## 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.
-->
# Description
When nushell calls a plugin it now sends a configuration `Value` from
the nushell config under `$env.config.plugins.PLUGIN_SHORT_NAME`. This
allows plugin authors to read configuration provided by plugin users.
The `PLUGIN_SHORT_NAME` must match the registered filename after
`nu_plugin_`. If you register `target/debug/nu_plugin_config` the
`PLUGIN_NAME` will be `config` and the nushell config will loook like:
$env.config = {
# ...
plugins: {
config: [
some
values
]
}
}
Configuration may also use a closure which allows passing values from
`$env` to a plugin:
$env.config = {
# ...
plugins: {
config: {||
$env.some_value
}
}
}
This is a breaking change for the plugin API as the `Plugin::run()`
function now accepts a new configuration argument which is an
`&Option<Value>`. If no configuration was supplied the value is `None`.
Plugins compiled after this change should work with older nushell, and
will behave as if the configuration was not set.
Initially discussed in #10867
# User-Facing Changes
* Plugins can read configuration data stored in `$env.config.plugins`
* The plugin `CallInfo` now includes a `config` entry, existing plugins
will require updates
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
- [ ] Update [Creating a plugin (in
Rust)](https://www.nushell.sh/contributor-book/plugins.html#creating-a-plugin-in-rust)
[source](https://github.com/nushell/nushell.github.io/blob/main/contributor-book/plugins.md)
- [ ] Add "Configuration" section to [Plugins
documentation](https://www.nushell.sh/contributor-book/plugins.html)
# Description
Replace `.to_string()` used in `GenericError` with `.into()` as
`.into()` seems more popular
Replace `Vec::new()` used in `GenericError` with `vec![]` as `vec![]`
seems more popular
(There are so, so many)
# Description
The `PluginSignature` type supports extra usage but this was not
available in `plugin_name --help`. It also supports search terms but
these did not appear in `help commands`
New behavior show below is the "Extra usage for nu-example-1" line and
the "Search terms:" line
```
❯ nu-example-1 --help
PluginSignature test 1 for plugin. Returns Value::Nothing
Extra usage for nu-example-1
Search terms: example
Usage:
> nu-example-1 {flags} <a> <b> (opt) ...(rest)
Flags:
-h, --help - Display the help message for this command
-f, --flag - a flag for the signature
-n, --named <String> - named string
Parameters:
a <int>: required integer value
b <string>: required string value
opt <int>: Optional number (optional)
...rest <string>: rest value string
Examples:
running example with an int value and string value
> nu-example-1 3 bb
```
Search terms are also available in `help commands`:
```
❯ help commands | where name == "nu-example-1" | select name search_terms
╭──────────────┬──────────────╮
│ name │ search_terms │
├──────────────┼──────────────┤
│ nu-example-1 │ example │
╰──────────────┴──────────────╯
```
# User-Facing Changes
Users can now see plugin extra usage and search terms
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
N/A
Elide the reference for `Copy` type (`usize`)
Use the canonical deref where possible.
* `&Box` -> `&`
* `&String` -> `&str`
* `&PathBuf` -> `&Path`
Skips the ctrl-C handler for now.
# Description
As part of the refactor to split spans off of Value, this moves to using
helper functions to create values, and using `.span()` instead of
matching span out of Value directly.
Hoping to get a few more helping hands to finish this, as there are a
lot of commands to update :)
# 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.
-->
---------
Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
Co-authored-by: WindSoilder <windsoilder@outlook.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.
-->
Added comments to support API docs for the `nu-plugin` crate. Removed a
few items that I'd expect should only be used internally to Nushell from
the documentation and reduced the visibility of some items that did not
need to be public.
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
There should be no user facing impact.
# 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 -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` 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
> ```
-->
Standard tests run. Additionally numerous doctests were added to the
`nu-plugin` crate.
# 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.
-->
No changes to the website necessary.
# Description
As title, we can't provide examples for plugin commands, this pr would
make it possible
# User-Facing Changes
Take plugin `nu-example-1` as example:
```
❯ nu-example-1 -h
PluginSignature test 1 for plugin. Returns Value::Nothing
Usage:
> nu-example-1 {flags} <a> <b> (opt) ...(rest)
Flags:
-h, --help - Display the help message for this command
-f, --flag - a flag for the signature
-n, --named <String> - named string
Parameters:
a <int>: required integer value
b <string>: required string value
(optional) opt <int>: Optional number
...rest <string>: rest value string
Examples:
running example with an int value and string value
> nu-example-1 3 bb
```
The examples session is newly added.
## Basic idea behind these changes
when nushell query plugin signatures, plugin just returns it's signature
without any examples, so nushell have no idea about the examples of
plugin commands.
To adding the feature, we just making plugin returns it's signature with
examples.
Before:
```
1. get signature
---------------->
Nushell ------------------ Plugin
<-----------------
2. returns Vec<Signature>
```
After:
```
1. get signature
---------------->
Nushell ------------------ Plugin
<-----------------
2. returns Vec<PluginSignature>
```
When writing plugin signature to $nu.plugin-path:
Serialize `<PluginSignature>` rather than `<Signature>`, which would
enable us to serialize examples to `$nu.plugin-path`
## Shortcoming
It's a breaking changes because `Plugin::signature` is changed, and it
requires plugin authors to change their code for new signatures.
Fortunally it should be easy to change, for rust based plugin, we just
need to make a global replace from word `Signature` to word
`PluginSignature` in their plugin project.
Our content of plugin-path is really large, if one plugin have many
examples, it'd results to larger body of $nu.plugin-path, which is not
really scale. A solution would be save register information in other
binary formats rather than `json`. But I think it'd be another story.
# 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 -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass
# 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
Lint: `clippy::uninlined_format_args`
More readable in most situations.
(May be slightly confusing for modifier format strings
https://doc.rust-lang.org/std/fmt/index.html#formatting-parameters)
Alternative to #7865
# User-Facing Changes
None intended
# Tests + Formatting
(Ran `cargo +stable clippy --fix --workspace -- -A clippy::all -D
clippy::uninlined_format_args` to achieve this. Depends on Rust `1.67`)
This is an attempt to implement a new `Value::LazyRecord` variant for
performance reasons.
`LazyRecord` is like a regular `Record`, but it's possible to access
individual columns without evaluating other columns. I've implemented
`LazyRecord` for the special `$nu` variable; accessing `$nu` is
relatively slow because of all the information in `scope`, and [`$nu`
accounts for about 2/3 of Nu's startup time on
Linux](https://github.com/nushell/nushell/issues/6677#issuecomment-1364618122).
### Benchmarks
I ran some benchmarks on my desktop (Linux, 12900K) and the results are
very pleasing.
Nu's time to start up and run a command (`cargo build --release;
hyperfine 'target/release/nu -c "echo \"Hello, world!\""' --shell=none
--warmup 10`) goes from **8.8ms to 3.2ms, about 2.8x faster**.
Tests are also much faster! Running `cargo nextest` (with our very slow
`proptest` tests disabled) goes from **7.2s to 4.4s (1.6x faster)**,
because most tests involve launching a new instance of Nu.
### Design (updated)
I've added a new `LazyRecord` trait and added a `Value` variant wrapping
those trait objects, much like `CustomValue`. `LazyRecord`
implementations must implement these 2 functions:
```rust
// All column names
fn column_names(&self) -> Vec<&'static str>;
// Get 1 specific column value
fn get_column_value(&self, column: &str) -> Result<Value, ShellError>;
```
### Serializability
`Value` variants must implement `Serializable` and `Deserializable`, which poses some problems because I want to use unserializable things like `EngineState` in `LazyRecord`s. To work around this, I basically lie to the type system:
1. Add `#[typetag::serde(tag = "type")]` to `LazyRecord` to make it serializable
2. Any unserializable fields in `LazyRecord` implementations get marked with `#[serde(skip)]`
3. At the point where a `LazyRecord` normally would get serialized and sent to a plugin, I instead collect it into a regular `Value::Record` (which can be serialized)
* when spawned process during register plugin, pass env to child process
* tweak comment
* tweak comment
* remove trailing whitespace
Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
* Skeleton implementation
Lots and lots of TODOs
* Bootstrap simple CustomValue plugin support test
* Create nu_plugin_custom_value
* Skeleton for nu_plugin_custom_values
* Return a custom value from plugin
* Encode CustomValues from plugin calls as PluginResponse::PluginData
* Add new PluginCall variant CollapseCustomValue
* Handle CollapseCustomValue plugin calls
* Add CallInput::Data variant to CallInfo inputs
* Handle CallInfo with CallInput::Data plugin calls
* Send CallInput::Data if Value is PluginCustomValue from plugin calls
* Remove unnecessary boxing of plugins CallInfo
* Add fields needed to collapse PluginCustomValue to it
* Document PluginCustomValue and its purpose
* Impl collapsing using plugin calls in PluginCustomValue::to_base_value
* Implement proper typetag based deserialization for CoolCustomValue
* Test demonstrating that passing back a custom value to plugin works
* Added a failing test for describing plugin CustomValues
* Support describe for PluginCustomValues
- Add name to PluginResponse::PluginData
- Also turn it into a struct for clarity
- Add name to PluginCustomValue
- Return name field from PluginCustomValue
* Demonstrate that plugins can create and handle multiple CustomValues
* Add bincode to nu-plugin dependencies
This is for demonstration purposes, any schemaless binary seralization
format will work. I picked bincode since it's the most popular for Rust
but there are defintely better options out there for this usecase
* serde_json::Value -> Vec<u8>
* Update capnp schema for new CallInfo.input field
* Move call_input capnp serialization and deserialization into new file
* Deserialize Value's span from Value itself instead of passing call.head
I am not sure if this was correct and I am breaking it or if it was a
bug, I don't fully understand how nu creates and uses Spans. What should
reuse spans and what should recreate new ones?
But yeah it felt weird that the Value's Span was being ignored since in
the json serializer just uses the Value's Span
* Add call_info value round trip test
* Add capnp CallInput::Data serialization and deserialization support
* Add CallInfo::CollapseCustomValue to capnp schema
* Add capnp PluginCall::CollapseCustomValue serialization and deserialization support
* Add PluginResponse::PluginData to capnp schema
* Add capnp PluginResponse::PluginData serialization and deserialization support
* Switch plugins::custom_values tests to capnp
Both json and capnp would work now! Sadly I can't choose both at the
same time :(
* Add missing JsonSerializer round trip tests
* Handle plugin returning PluginData as a response to CollapseCustomValue
* Refactor plugin calling into a reusable function
Many less levels of indentation now!
* Export PluginData from nu_plugin
So plugins can create their very own serve_plugin with whatever
CustomValue behavior they may desire
* Error if CustomValue cannot be handled by Plugin
