# Description
Adds the `AddEnvVar` plugin call, which allows plugins to set
environment variables in the caller's scope. This is the first engine
call that mutates the caller's stack, and opens the door to more
operations like this if needed.
This also comes with an extra benefit: in doing this, I needed to
refactor how context was handled, and I was able to avoid cloning
`EngineInterface` / `Stack` / `Call` in most cases that plugin calls are
used. They now only need to be cloned if the plugin call returns a
stream. The performance increase is welcome (5.5x faster on `inc`!):
```nushell
# Before
> timeit { 1..100 | each { |i| $"2.0.($i)" | inc -p } }
405ms 941µs 952ns
# After
> timeit { 1..100 | each { |i| $"2.0.($i)" | inc -p } }
73ms 68µs 749ns
```
# User-Facing Changes
- New engine call: `add_env_var()`
- Performance enhancement for plugin calls
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
- [x] Document env manipulation in plugins guide
- [x] Document `AddEnvVar` in plugin protocol
# Description
This adds three engine calls: `GetEnvVar`, `GetEnvVars`, for getting
environment variables from the plugin command context, and
`GetCurrentDir` for getting the current working directory.
Plugins are now launched in the directory of their executable to try to
make improper use of the current directory without first setting it more
obvious. Plugins previously launched in whatever the current directory
of the engine was at the time the plugin command was run, but switching
to persistent plugins broke this, because they stay in whatever
directory they launched in initially.
This also fixes the `gstat` plugin to use `get_current_dir()` to
determine its repo location, which was directly affected by this
problem.
# User-Facing Changes
- Adds new engine calls (`GetEnvVar`, `GetEnvVars`, `GetCurrentDir`)
- Runs plugins in a different directory from before, in order to catch
bugs
- Plugins will have to use the new engine calls if they do filesystem
stuff to work properly
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
- [ ] Document the working directory behavior on plugin launch
- [ ] Document the new engine calls + response type (`ValueMap`)
# Description
Adds support for the following operations on plugin custom values, in
addition to `to_base_value` which was already present:
- `follow_path_int()`
- `follow_path_string()`
- `partial_cmp()`
- `operation()`
- `Drop` (notification, if opted into with
`CustomValue::notify_plugin_on_drop`)
There are additionally customizable methods within the `Plugin` and
`StreamingPlugin` traits for implementing these functions in a way that
requires access to the plugin state, as a registered handle model such
as might be used in a dataframes plugin would.
`Value::append` was also changed to handle custom values correctly.
# User-Facing Changes
- Signature of `CustomValue::follow_path_string` and
`CustomValue::follow_path_int` changed to give access to the span of the
custom value itself, useful for some errors.
- Plugins using custom values have to be recompiled because the engine
will try to do custom value operations that aren't supported
- Plugins can do more things 🎉
# Tests + Formatting
Tests were added for all of the new custom values functionality.
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
- [ ] Document protocol reference `CustomValueOp` variants:
- [ ] `FollowPathInt`
- [ ] `FollowPathString`
- [ ] `PartialCmp`
- [ ] `Operation`
- [ ] `Dropped`
- [ ] Document `notify_on_drop` optional field in `PluginCustomValue`
# Description
This allows plugins to make calls back to the engine to get config,
evaluate closures, and do other things that must be done within the
engine process.
Engine calls can both produce and consume streams as necessary. Closures
passed to plugins can both accept stream input and produce stream output
sent back to the plugin.
Engine calls referring to a plugin call's context can be processed as
long either the response hasn't been received, or the response created
streams that haven't ended yet.
This is a breaking API change for plugins. There are some pretty major
changes to the interface that plugins must implement, including:
1. Plugins now run with `&self` and must be `Sync`. Executing multiple
plugin calls in parallel is supported, and there's a chance that a
closure passed to a plugin could invoke the same plugin. Supporting
state across plugin invocations is left up to the plugin author to do in
whichever way they feel best, but the plugin object itself is still
shared. Even though the engine doesn't run multiple plugin calls through
the same process yet, I still considered it important to break the API
in this way at this stage. We might want to consider an optional
threadpool feature for performance.
2. Plugins take a reference to `EngineInterface`, which can be cloned.
This interface allows plugins to make calls back to the engine,
including for getting config and running closures.
3. Plugins no longer take the `config` parameter. This can be accessed
from the interface via the `.get_plugin_config()` engine call.
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
Not only does this have plugin protocol changes, it will require plugins
to make some code changes before they will work again. But on the plus
side, the engine call feature is extensible, and we can add more things
to it as needed.
Plugin maintainers will have to change the trait signature at the very
least. If they were using `config`, they will have to call
`engine.get_plugin_config()` instead.
If they were using the mutable reference to the plugin, they will have
to come up with some strategy to work around it (for example, for `Inc`
I just cloned it). This shouldn't be such a big deal at the moment as
it's not like plugins have ever run as daemons with persistent state in
the past, and they don't in this PR either. But I thought it was
important to make the change before we support plugins as daemons, as an
exclusive mutable reference is not compatible with parallel plugin
calls.
I suggest this gets merged sometime *after* the current pending release,
so that we have some time to adjust to the previous plugin protocol
changes that don't require code changes before making ones that do.
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
I will document the additional protocol features (`EngineCall`,
`EngineCallResponse`), and constraints on plugin call processing if
engine calls are used - basically, to be aware that an engine call could
result in a nested plugin call, so the plugin should be able to handle
that.
# Description
This fixes a race condition where all interfaces to a plugin might have
been dropped, but both sides are still expecting input, and the
`PluginInterfaceManager` doesn't get a chance to see that the interfaces
have been dropped and stop trying to consume input.
As the manager needs to hold on to a writer, we can't automatically
close the stream, but we also can't interrupt it if it's in a waiting to
read. So the best solution is to send a message to the plugin that we
are no longer going to be sending it any plugin calls, so that it knows
that it can exit when it's done.
This race condition is a little bit tricky to trigger as-is, but can be
more noticeable when running plugins in a tight loop. If too many plugin
processes are spawned at one time, Nushell can start to encounter "too
many open files" errors, and not be very useful.
# User-Facing Changes
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
I will need to add `Goodbye` to the protocol docs
