Skip to content
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

Better plugin configuration #661

Closed
spacemaison opened this issue Aug 25, 2021 · 3 comments
Closed

Better plugin configuration #661

spacemaison opened this issue Aug 25, 2021 · 3 comments
Labels
config Improvements to the configuration system plugin system Enhancement or fixes related to plugin system

Comments

@spacemaison
Copy link
Contributor

spacemaison commented Aug 25, 2021

Before many of the items on the wish list for plugins can be completed Zellij needs a way for plugin metadata to be declared. Right now the only way to load a plugin is to specify a path in the context of a layout pane. Defining plugins inside of layout files feels insufficient for things like headless plugins which wont affect layout, or specifying allowed permissions for plugins.

I've got a pull request that implements a plugin manifest inside Zellij's config file. Additionally, I've also included the ability to pass configuration options into plugins. As well as headless plugins.

The API for designating plugins in the config.yaml file looks like this:

plugins:
  - path: /absolute/path/to/zellij-plugin-example/target/wasm32-wasi/debug/rust-plugin-example.wasm
    tag: example
    run: headless # Could also be once-per-pane, which is the default current behavour of plugins
    config:
      foo: 'bar'

While the updated API for the plugins looks like this:

#[derive(Default)]
struct Plugin;

#[derive(Deserialize, Default)]
struct ArbitraryOptions {
    foo: String
}

register_plugin!(Plugin);

impl ZellijPlugin<ArbitraryOptions> for Plugin {
    fn load(&mut self, options: ArbitraryOptions) {
        assert_eq!(options.foo, "bar".to_string());
    }
}

I'm still not happy with that pr though. All of the configuration for plugins is currently happening by the user consuming the plugin. It might be necessary for plugins to specify configuration of their own. For instance, when the permissions system is implemented, a plugin author is going to need the ability to request permissions, and the user of the plugin is going to have to grant/deny access to said permissions. For that, a separate manifest file is likely needed to be added in front of the wasm plugins that specifies configuration provided by the plugin author. For now though, it's not completely necessary.

Feedback on the API design would be appreciated.

@a-kenji a-kenji added plugin system Enhancement or fixes related to plugin system config Improvements to the configuration system labels Aug 25, 2021
@a-kenji
Copy link
Contributor

a-kenji commented Aug 30, 2021

Thank you for taking the time creating this issue and the pr!
This looks already very promising in my opinion.

plugins:
  - path: /absolute/path/to/zellij-plugin-example/target/wasm32-wasi/debug/rust-plugin-example.wasm
    tag: example
    run: headless # Could also be once-per-pane, which is the default current behavour of plugins
    config:
      foo: 'bar'

This looks good, I am not sure if that already maybe limits us in a way.
It is nice to have a list of plugins to configure, but what if I want to configure
multiple plugins at the same time. Do you think this could leave the
toplevel plugins: free for general config and a list of all the plugins,
or is that maybe not needed?

The tag alias is a great idea, so if I understand that correctly you could run
the same plugin with different configuration under a different tag here?

A future plan is also to make it possible to configure plugins in the config itself,
as to this issue and pr - as well as make the plugin config overridable in the layout itself,
basically it would be merged prior to loading the plugin.

All of the configuration for plugins is currently happening by the user consuming the plugin. It might be necessary for plugins to specify configuration of their own. For instance, when the permissions system is implemented, a plugin author is going to need the ability to request permissions, and the user of the plugin is going to have to grant/deny access to said permissions. For that, a separate manifest file is likely needed to be added in front of the wasm plugins that specifies configuration provided by the plugin author.

Agreed, if there is no issue for that already do you mind opening a separate one for that?

@spacemaison
Copy link
Contributor Author

spacemaison commented Aug 31, 2021

Hey @a-kenji, thanks for the feedback.

Do you think this could leave the toplevel plugins: free for general config and a list of all the plugins, or is that maybe not needed?

That's a good idea, even if only to leave room in the future for adding options for configuring all plugins. I can't think of anything to put there right now though. The only option that cuts across all plugins is the lookup directory for plugins, which is handled by the --data-dir option at the moment. It would be helpful to have more than one lookup directory for plugins though, maybe I can add that there? Or just leave it blank for future additions?

The tag alias is a great idea, so if I understand that correctly you could run the same plugin with different configuration under a different tag here?

Yup. Zellij will crash if the tag names aren't unique, but you can pass different tag names for the same wasm file to adjust the plugins for different configurations.

A future plan is also to make it possible to configure plugins in the config itself

I'm not sure what you mean here?

as well as make the plugin config overridable in the layout itself, basically it would be merged prior to loading the plugin.

Another good idea. Being able to specify a different plugin configuration in the layout files at the point of instantiation would be helpful.

I should ask though, given how unfinished the plugin system is, is there any hivemind consensus on how plugins should work when they're fully implemented? I'm sure my current draft pr is pretty off from that, most of what's implemented was just necessary to get my toy plugin working. I'm happy to further iterate on this design and the pull request.

@imsnif
Copy link
Member

imsnif commented Jul 25, 2023

Implemented in #2646

@imsnif imsnif closed this as completed Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Improvements to the configuration system plugin system Enhancement or fixes related to plugin system
Projects
None yet
Development

No branches or pull requests

3 participants