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

feat: provide the zellij version to plugins #894

Merged
merged 3 commits into from
Nov 26, 2021
Merged

feat: provide the zellij version to plugins #894

merged 3 commits into from
Nov 26, 2021

Conversation

jaeheonji
Copy link
Member

resolves #888

@jaeheonji
Copy link
Member Author

I did some research on this feature.
The initial (my stupid) idea was to provide the zellij version when wasmer invokes the plugin's load function.

For some reason I changed the implementation direction.

  1. The data type that can be provided as a plugin through wasmer's call is only integer. Of course other types are possible with "slightly" complicated methods.
  2. Already in the wasm_vm.rs there is a nice way called plugin API !!

So I made a pluginAPI that simply outputs the zellij version. The plugin only needs to call the get_zellij_version function.

@jaeheonji jaeheonji marked this pull request as ready for review November 24, 2021 15:56
@jaeheonji jaeheonji changed the title wip: provide the zellij version to plugins feat: provide the zellij version to plugins Nov 24, 2021
Copy link
Contributor

@a-kenji a-kenji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but would like to have a 2nd pair of eyes here.

@TheLostLambda
Copy link
Member

I think we could improve the code here a little, let me experiment for a moment!

@TheLostLambda
Copy link
Member

Sorry for the delay in getting back to you!

I think we can reuse the object_from_stdin() function we already have, since String is deserializable!

Here is the diff that worked for me!

image

@TheLostLambda
Copy link
Member

Otherwise things look stellar! Thanks for doing this @jaeheonji !

* delete unnecessary function
@jaeheonji
Copy link
Member Author

@TheLostLambda Thanks for reviewing! I applied the part you suggested :)

Copy link
Contributor

@a-kenji a-kenji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

@a-kenji a-kenji merged commit c349586 into zellij-org:main Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: give plugins version information on load
3 participants