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

[electron] Bind plugin components on the backend #4842

Merged
merged 2 commits into from
Apr 12, 2019
Merged

Conversation

paul-marechal
Copy link
Member

@paul-marechal paul-marechal commented Apr 8, 2019

The electron inversify container is missing a couple of bindings
required by the whole plugin system to work correctly.

This commit binds what is missing for plugins to function on electron.

Signed-off-by: Paul Maréchal paul.marechal@ericsson.com

Fixes #4802
Fixes #3651
Fixes #2210

@paul-marechal
Copy link
Member Author

paul-marechal commented Apr 8, 2019

@benoitf @evidolob hi, I was trying to enable plugins on Electron and I was wondering how to properly test that most of it is working as it should?

I tried the Ruby extension from VS Code so far, I could see snippets being contributed, but is there something for me to follow to more extensively test this change?

Also wanted feedback on the bindings, because I just naively hooked what seemed to be missing, but I am certainly missing some runtime assumptions (some things might be meant to run in a NodeJS env and not Electron, etc...).

I was expecting to be able to start with naive changes, and work my way there by running some tests and updating the bindings/implementations accordingly.

@paul-marechal paul-marechal added electron issues related to the electron target plug-in system issues related to the plug-in system labels Apr 8, 2019
@akosyakov
Copy link
Member

@marechal-p try to run something well tested like go-vscode extension and test with this project https://github.com/demo-apps/go-gin-app

It should suggest you to install tooling, provide language support and so on. Just take your time to learn this extension (maybe go as well :)) and check that it works nicely in Electron.

@paul-marechal
Copy link
Member Author

paul-marechal commented Apr 9, 2019

@akosyakov tried the go extension as you said, and everything looked fine :)

  • Tooling installation OK
  • Autocompletion/Hover
  • Navigation
  • Formatting
  • Errors are reported

Tried a few other extensions, all seem to work as expected.

@akosyakov
Copy link
Member

@marechal-p ok, let me verify as well

paul-marechal and others added 2 commits April 11, 2019 08:20
The electron inversify container is missing a couple of bindings
required by the whole plugin system to work correctly.

This commit binds what is missing for plugins to function on electron.

Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the mp/electron-plugins branch from c9e251d to f37b6ab Compare April 11, 2019 08:21
@akosyakov
Copy link
Member

@marechal-p I've rebased and added the missing option, please pull before force pushing next time. After Gitpod is green i will test Electron.

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

It works now, awesome!

Screen Shot 2019-04-11 at 10 50 58

@akosyakov
Copy link
Member

akosyakov commented Apr 11, 2019

Although it does not mean that it will work in the bundled app. It would be nice to test it as well after merging.

@akosyakov akosyakov merged commit edcf378 into master Apr 12, 2019
@akosyakov akosyakov deleted the mp/electron-plugins branch April 12, 2019 05:03
@paul-marechal
Copy link
Member Author

paul-marechal commented Apr 12, 2019

Although it does not mean that it will work in the bundled app. It would be nice to test it as well after merging.

I was trying to bundle an Electron application on Linux using electron-builder, but there seems to be a bug where process.env.THEIA_APP_PROJECT_PATH points to a location inside some app.asar, which is an archive. It then fails to execute this line.

I mean I will eventually make it work, but did anyone already made such a bundle on Linux?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target plug-in system issues related to the plug-in system
Projects
None yet
3 participants