cli

cli copied to clipboard

The plugin system is quite complex, especially because of the cobra.Command serialization, which needs a lot of boilerplate code because of pflags 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 on os.Args and rebuild the cobra.Command in the plugin, instead of serialize it over the network. This only requires to truncate os.Args first item ignite, because in the plugin the command declared starts with network. 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, like ignite scaffold wasm. The simplification would allow to only add a command after ignite, because it's not possible to truncate os.Args properly in that case (imagine having to truncate ignite scaffold --path ./xxx/yyy wasm to wasm --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 plugin ExecuteHook(Pre|Post|CleanUp) methods, it's possible to access the command flags via hook.ExecutedCommand.Flags(). If we remove the command serialization, the plugin would have only access to os.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 !

Additional notes:

• Since the serialization of cobra.Command is not complete, issues like ignite/cli#3350 can raise on a regular basis. With the simplification this is no longer a problem.
• The simplification will break existing plugins. Authors will have to update their plugin code to make it works with the CLI version that has the simplification.

I think this could be an interesting topic to discuss on the Community call 👍

Not being able to mount commands in places like ignite chain and ignite scaffold means there are limits to how plugins can really "extend" CLI from users' point of view. Each org extending the CLI would have to mount under their own top-level name.

ignite acme scaffold foo instead of ignite scaffold foo.

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?

❌ ignite scaffold --path ./xxx/yyy wasm

✅ ignite scaffold wasm --path ./xxx/yyy

Is it possible to enforce flags to be submitted after the command?

❌ ignite scaffold --path ./xxx/yyy wasm

✅ ignite scaffold wasm --path ./xxx/yyy

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 path flag internally to be able to read it, whereas this flag is defined in the parent comment scaffold.

But arguments will follow the command, right? You can't have ignite scaffold arg1 arg2 foo, it's always ignite scaffold foo arg1 arg2.

You're right, maybe it's not that complicated.