Split the plugin crate #12563
Merged
Split the plugin crate #12563
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sholderbach
reviewed
Apr 18, 2024
There was a problem hiding this comment.
That was quick and seemingly relatively painless.
Not quite sure if we need the nu-plugin-protocol / nu-plugin-core split yet but there is probably not much harm.
Is there a specificly raised need to remove the socket support, otherwise I am inclined to say that added feature causes complexity for testing and ensuring compatibility between nu and plugin builds.
|
With your recent polishing here, are we good to go to land this and prep the release script? |
|
I think I'd probably like to see #12607 merge too first, then I can fix all of the conflicts from all of that effort |
|
It's ok for now, but will probably conflict with some of the other PRs |
|
And the guaranteed conflicts came 馃槅 |
maxim-uvarov
pushed a commit
to maxim-uvarov/nushell
that referenced
this pull request
May 1, 2024
# Description This breaks `nu-plugin` up into four crates: - `nu-plugin-protocol`: just the type definitions for the protocol, no I/O. If someone wanted to wire up something more bare metal, maybe for async I/O, they could use this. - `nu-plugin-core`: the shared stuff between engine/plugin. Less stable interface. - `nu-plugin-engine`: everything required for the engine to talk to plugins. Less stable interface. - `nu-plugin`: everything required for the plugin to talk to the engine, what plugin developers use. Should be the most stable interface. No changes are made to the interface exposed by `nu-plugin` - it should all still be there. Re-exports from `nu-plugin-protocol` or `nu-plugin-core` are used as required. Plugins shouldn't ever have to use those crates directly. This should be somewhat faster to compile as `nu-plugin-engine` and `nu-plugin` can compile in parallel, and the engine doesn't need `nu-plugin` and plugins don't need `nu-plugin-engine` (except for test support), so that should reduce what needs to be compiled too. The only significant change here other than splitting stuff up was to break the `source` out of `PluginCustomValue` and create a new `PluginCustomValueWithSource` type that contains that instead. One bonus of that is we get rid of the option and it's now more type-safe, but it also means that the logic for that stuff (actually running the plugin for custom value ops) can live entirely within the `nu-plugin-engine` crate. # User-Facing Changes - New crates. - Added `local-socket` feature for `nu` to try to make it possible to compile without that support if needed. # Tests + Formatting - 馃煝 `toolkit fmt` - 馃煝 `toolkit clippy` - 馃煝 `toolkit test` - 馃煝 `toolkit test stdlib`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This breaks
nu-pluginup into four crates:nu-plugin-protocol: just the type definitions for the protocol, no I/O. If someone wanted to wire up something more bare metal, maybe for async I/O, they could use this.nu-plugin-core: the shared stuff between engine/plugin. Less stable interface.nu-plugin-engine: everything required for the engine to talk to plugins. Less stable interface.nu-plugin: everything required for the plugin to talk to the engine, what plugin developers use. Should be the most stable interface.No changes are made to the interface exposed by
nu-plugin- it should all still be there. Re-exports fromnu-plugin-protocolornu-plugin-coreare used as required. Plugins shouldn't ever have to use those crates directly.This should be somewhat faster to compile as
nu-plugin-engineandnu-plugincan compile in parallel, and the engine doesn't neednu-pluginand plugins don't neednu-plugin-engine(except for test support), so that should reduce what needs to be compiled too.The only significant change here other than splitting stuff up was to break the
sourceout ofPluginCustomValueand create a newPluginCustomValueWithSourcetype that contains that instead. One bonus of that is we get rid of the option and it's now more type-safe, but it also means that the logic for that stuff (actually running the plugin for custom value ops) can live entirely within thenu-plugin-enginecrate.User-Facing Changes
local-socketfeature fornuto try to make it possible to compile without that support if needed.Tests + Formatting
toolkit fmttoolkit clippytoolkit testtoolkit test stdlib