-
Notifications
You must be signed in to change notification settings - Fork 558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: simplify the plugin system #3930
Comments
Additional notes:
|
I think this could be an interesting topic to discuss on the Community call 👍 |
Not being able to mount commands in places like
And it will make it harder for us to extract functionality into independent plugins as well. |
Is it possible to enforce flags to be submitted after the command?
|
Yes that's probably doable, and after we can safely truncate. I just wonder if this isn't going to be complicated if the command also has arguments. That said, the plugin still needs to redefine the |
But arguments will follow the command, right? You can't have |
You're right, maybe it's not that complicated. |
The plugin system is quite complex, especially because of the
cobra.Command
serialization, which needs a lot of boilerplate code because ofpflags
which is not serializable at all and uses a go interface to store the flag value (see here).Like done in the
cli-plugin-network
(see here), we could only rely onos.Args
and rebuild thecobra.Command
in the plugin, instead of serialize it over the network. This only requires to truncateos.Args
first itemignite
, because in the plugin the command declared starts withnetwork
.This also has the benefit of using standard cobra code in the plugin, instead of a switch statement on
ExecutedCommand.Use
.But to achieve that we need to remove some features:
the ability to add a plugin command to a ignite sub command.
It's currently possible to have plugin that adds a command to
ignite scaffold
, likeignite scaffold wasm
. The simplification would allow to only add a command afterignite
, because it's not possible to truncateos.Args
properly in that case (imagine having to truncateignite scaffold --path ./xxx/yyy wasm
towasm --path ./xxx/yyy
, moreover you have to redeclare the--path
flag in the plugin).For the same reason
ignite
command should not have persistent flags at root (it currently doesn't have any).the ability to access the command flags from a hooked command.
Let's say a plugin adds a hook to
ignite chain build
, then inside the pluginExecuteHook(Pre|Post|CleanUp)
methods, it's possible to access the command flags viahook.ExecutedCommand.Flags()
. If we remove the command serialization, the plugin would have only access toos.Args
and so eventually repeat the hooked command declaration to read the flags.My feeling on this is mixed, that's why I would like to collect opinions, please share yours !
The text was updated successfully, but these errors were encountered: