Added support for master electron packages #3431

bogthe · 2018-11-07T18:01:03Z

Signed-off-by: Bogdan Stolojan petre.stolojan@arm.com

This PR fixes: #3364
By adding a new extension method called backendMasterElectron to modules which will be required into node inside of the master electron process and enabling packages like electron-updater to work with Theia.

thegecko · 2019-01-16T10:42:00Z

Any feedback on this PR?

bogthe · 2019-01-18T09:39:14Z

Hi @kittaakos this seems to have been floating for a while now. Any feedback on having this merged?

akosyakov · 2019-01-21T07:46:05Z

dev-packages/application-package/src/extension-package.ts

@@ -24,6 +24,7 @@ export interface Extension {
    frontendElectron?: string;
    backend?: string;
    backendElectron?: string;
+    backendMasterElectron?: string;


What are expectations about js fie, the same as for other entry points? Does it return ContainerModule and use inversify to allow fine grained customizations?

As it stands it doesn't use ContainerModule or inversify.
Reason being I would like to see how the use cases for this entry point evolve and didn't want to impose any restrictions. For now each backendMasterElectron package can use it's own inversify containers but a simple js entry point would do for electron-updates (or any modules required to run from a different thread) to work.

It's decided to Theia to use inversify in all entry points to allow customizations. I don't think this entry point should deviate.

Can we use this entry point as a short term solution and replace it with an inversify method later?

kittaakos · 2019-01-21T13:06:22Z

@bogthe, thank you for the patch! Could you please provide an example I can try?

bogthe · 2019-01-21T13:57:55Z

Hi @kittaakos , no worries! Glad I could help.

Check this branch out, I created a small example here.

kittaakos

I did the review, it worked as expected. 👍 Could you please tell me why did you pick the name electron master modules, maybe it is correct, I just do not see what does it do with master.

kittaakos · 2019-01-21T14:45:00Z

dev-packages/application-manager/src/generator/backend-generator.ts

@@ -72,6 +79,12 @@ module.exports = (port, host, argv) => Promise.resolve()${this.compileBackendMod
    });`;
    }

+    protected compileElectronMaster(backendMasterModules: Map<string, string>) {
+        return `// @ts-check
+${[...backendMasterModules.values()].map(module => `require('${module}')`)


${[...backendMasterModules.values()].map(module => `require('${module}')`) .join('\n')}`;

should be

${[...backendMasterModules.values()].map(module => `require('${module}');`).join('\n')}`;

(You have missed the semi-colons after the require.)

kittaakos · 2019-01-21T14:45:40Z

dev-packages/application-manager/src/generator/frontend-generator.ts

+        }${
+            this.pck.targetElectronMasterModules.size > 0 ?
+                `
+        require('../backend/electronMaster.js')` : ''}


Missing semi-colon after require('../backend/electronMaster.js'').

kittaakos · 2019-01-21T14:48:56Z

dev-packages/application-manager/src/generator/backend-generator.ts

+
+        if (this.pck.targetElectronMasterModules.size > 0) {
+            await this.write(
+                this.pck.backend('electronMaster.js'),


Please use the preferred file naming convention.

Also, why is it called electron master modules? I do not have a better idea, but electronMaster does not sound good to me.

Sorry 'bout that!

I have to say it doesn't sound that good to me either but I didn't know what else to name it. They are packages which will be ran from the master process of electron not the workers, and main process would've been confusing... so went with master! ¯_(ツ)_/¯

No, not the master process of electron, but the main process of electron. And later, the electron main process forks the backend process, and the forked backend process will create the workers...

I would go with electron main (process) contributions... something like that. We can agree that the feature you want works 👍 but I would like to find a proper name for it before we merge it.

bogthe · 2019-01-21T15:02:28Z

@kittaakos thanks for checking it! Added the changes you requested

kittaakos · 2019-01-21T15:21:44Z

dev-packages/application-package/src/application-package.ts

@@ -220,6 +228,10 @@ export class ApplicationPackage {
        return this.ifBrowser(this.backendModules, this.backendElectronModules);
    }

+    get targetElectronMasterModules(): Map<string, string> {
+        return this.ifElectron(this.backendElectronMasterModules, new Map<string, string>());
+    }


This method isn't necessary here, right? I can understand the targetFrontendModules and the targetBackendModules but not this one. I think it is sufficient to have the other method (backendElectronMasterModules), and if not electron, then return with an empty map. You do not have electron master modules in the browser, so no need to mimic a symmetric API. What do you think?

You're right, I'll get rid of the method.

paul-marechal · 2019-01-21T15:25:06Z

Looking at this PR, it reminded me that I had been thinking about using inversify for the electron application in the same way that we do for the backend and frontend apps.

Do you think it would be worth to do sometime? Right now I really think it would be nice to have it modular like BE/FE, would allow clients have more control on the Electron feel without contributing to the generators every time.

bogthe · 2019-01-21T16:34:22Z

@marechal-p are you referring to the way BE and FE exchange messages through JsonRpcConnectionHandler and WebSocketConnectionProvider?
As it is you can still pass messages from any FE to a backendMasterElectron package through the basic ipcRenderer / ipcMain, which I admit is not ideal 😄 because it's different from the standard way. So yeah, would like to see this evolve into something like that, but for now it looks like a nice playground for things like update mechanisms, etc.

…view widget Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

So let's say you access the workspace /tmp/moo on server potato.com, the URL will look like http(s?)://potato.com/#/tmp/moo Changing workspace using the interface will update the path in the fragment. The goal is that it always the url fragment always reflect whatever workspace you are using. If you find a situation where it doesn't, it's a bug. The motivations of this change: - We can create browser bookmarks for different workspaces on the same server, or access a particular workspace using the browser history. - This makes the "reload" feature more robust. Right now, when you reload, it works because Theia saves (just before exiting) the workspace in the front of ~/recentworkspace.json, and we happen to automatically open the most recent workspace at startup. After this patch, it will work because we will pick up the workspace path from the fragment after reloading). - I want to introduce an option for not opening the most recent workspace at startup (just show the welcome page which shows the list of recent workspaces and let me choose which one I want to open). Enabling this option would break reloading, since reloading relies on opening the most recent workspace. - "Power user" feature: you can easily copy paste a directory path in the URL to open it as a workspace. Change-Id: I73f54451fcf9312110a1f87f3c236c1e4eef7ebd Signed-off-by: Simon Marchi <simon.marchi@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

…path in electron Closes: eclipse-theia#3297 Signed-off-by: Akos Kitta <kittaakos@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

macOS GUI applications does not inherit the `PATH` by default. Signed-off-by: Akos Kitta <kittaakos@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Nigel Westbury <nigelipse@miegel.org> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

…t scopes. `sinon.restore` ran for other tests. E.g.: `file-uri.spec` test module. Signed-off-by: Akos Kitta <kittaakos@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

When building a menu, we now check if the item is visible. If not, then we do not add it to the menu. This allow us to filter items with invisible children, but that have children nonetheless. Fix eclipse-theia#2669. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Casey Flynn <caseyflynn@google.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Fixes breaking import 'WidgetTracker' found in `master` Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: elaihau <liang.huang@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Added missing mocked `CorePreferences` to the `navigator-model.spec.ts` file which is required for the tests to successfully pass. Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Mykola Morhun <mmorhun@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

…ave code The simple example is `bat` extension: https://github.com/Microsoft/vscode/blob/f9dac5925447293678b81e817fa077a5bc1b9d3c/extensions/bat/package.json#L1 Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

`fireWillSaveModel` is triggering every listener at the same time, each providing edits from the same original document, which can lead to concurrency issues. Changed the implementation to match what VS Code is doing when triggering the same listeners: sequentially, and blaming listeners that directly make modifications to the model. Add `Emitter.sequence` to process each listener one after the other. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Change e4bcba5 is reverted, as it pops too many notifications especially when the auto_save is on. Signed-off-by: elaihau <liang.huang@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

I've also added relevant mime types. Signed-off-by: Jan Keromnes <jan.keromnes@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Josh Bradley <jgbradley1@gmail.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

…nges Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

+ load snippets async Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Sven Efftinge <sven.efftinge@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

An error about some `execute("return window.__coverage__")` kept appearing in the test logs. Fixed. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

When we want to create a folder, we usually do not care if the folder already exists or not, if we did, we could always query it using `filesystem.exists`. Currently, `filesystem.createFolder` will fail if it already exists. This commit makes it so `createFolder` will not fail if the file that exists is also a folder. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Some path returned by the filesystem was forced as non-undefined in the code, while at runtime the value actually ended up undefined. Hopefully improved filesystem access. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Oleksii Orel <oorel@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

So it is matching VSCode command as well https://code.visualstudio.com/docs/getstarted/keybindings#_navigation Change-Id: I2044bcfbf67c71f981cce50f7aeb363c12cfb166 Signed-off-by: Florent Benoit <fbenoit@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Change-Id: Id29e37f5ab62a00f8982ef049b718f49124f8cd3 Signed-off-by: Florent Benoit <fbenoit@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Change-Id: I15b3db94dae4d1607926d1e01d5d3e417ea037d6 Signed-off-by: Florent Benoit <fbenoit@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

bogthe · 2019-01-21T16:39:57Z

woops

paul-marechal · 2019-01-21T17:16:04Z

@bogthe I wasn't talking about the communication FE/BE, but rather the way they are constructed using inversify's container modules, which allows for contribution providers and symbol rebinding.

Currently the only way to extend the way the electron app part feels is by changing the generators, in the main repo. Another solution would be to forget about our generators and create your own. That could be different, that's my point.

paul-marechal requested review from akosyakov and paul-marechal November 7, 2018 18:03

akosyakov requested a review from svenefftinge November 8, 2018 07:23

thegecko requested a review from kittaakos January 16, 2019 10:42

akosyakov reviewed Jan 21, 2019

View reviewed changes

kittaakos reviewed Jan 21, 2019

View reviewed changes

akosyakov and others added 16 commits January 21, 2019 16:37

[mini-browser] one widget per a URI + aligning behaviour with the pre…

7e99fb1

…view widget Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

open handlers should not contribute visible commands

f5c76f2

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

fix eclipse-theia#3506: split terminal command

3af673a

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[shell] get rid of WidgetTracker

0242b22

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

render tab bar toolbar with react

5f6dc00

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

eclipse-theiaGH-3297: Set and use an env variable as the app project …

0bef364

…path in electron Closes: eclipse-theia#3297 Signed-off-by: Akos Kitta <kittaakos@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

eclipse-theiaGH-3297: Added fix-path.

c16e9a3

macOS GUI applications does not inherit the `PATH` by default. Signed-off-by: Akos Kitta <kittaakos@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Fix Windows drive root conversion

b035dd9

Signed-off-by: Nigel Westbury <nigelipse@miegel.org> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

eclipse-theiaGH-633: Fixed the incorrect beforeEach/afterEach tes…

ebb7a0f

…t scopes. `sinon.restore` ran for other tests. E.g.: `file-uri.spec` test module. Signed-off-by: Akos Kitta <kittaakos@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Editor Preview widget implementation.

4641989

Signed-off-by: Casey Flynn <caseyflynn@google.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Fix breaking import 'WidgetTracker'

78aaa71

Fixes breaking import 'WidgetTracker' found in `master` Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

unit tests for navigator-model and navigator-tree

810b61a

Signed-off-by: elaihau <liang.huang@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[Plug-in] Fix workspace folder uri transfer

3a7fd39

Signed-off-by: Mykola Morhun <mmorhun@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

akosyakov and others added 22 commits January 21, 2019 16:37

Inital commit for adding API to apply workspace edit

2559a9c

Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

revert "prompt on read/write a corrupted settings.json"

f4cf7fa

Change e4bcba5 is reverted, as it pops too many notifications especially when the auto_save is on. Signed-off-by: elaihau <liang.huang@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Enable INI syntax highlighting for .properties and .toml files

06f0cbc

I've also added relevant mime types. Signed-off-by: Jan Keromnes <jan.keromnes@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Changed README to refer feedback to Spectrum

4ad2f42

Signed-off-by: Josh Bradley <jgbradley1@gmail.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

fix eclipse-theia#3991: update settings schema resource on schema cha…

70c1da1

…nges Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[plugin] fix eclipse-theia#3976: resolve VS Code from sources

62774cf

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[plugin] fix eclipse-theia#3972: basic support of snippets

a3734c1

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[monaco] deprecate TextmateSnippetCompletionProvider

753f3d7

+ load snippets async Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[java] Allow rebinding the JavaContribution

4e5ac7c

Signed-off-by: Sven Efftinge <sven.efftinge@typefox.io> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[plug-in]Add dummy implementation to emit onWillSaveTextDocument event

c58a0c0

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

[tests][example-browser] Catch coverage error

a3fa11e

An error about some `execute("return window.__coverage__")` kept appearing in the test logs. Fixed. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

CHE-11443 fix treeItemId for the callback

73c9582

Signed-off-by: Oleksii Orel <oorel@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

replace all occurences (and not the first one) of theia-resource:

5699f35

Change-Id: Id29e37f5ab62a00f8982ef049b718f49124f8cd3 Signed-off-by: Florent Benoit <fbenoit@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Allow to grab os type of client connected to plug-ins

0122973

Change-Id: I15b3db94dae4d1607926d1e01d5d3e417ea037d6 Signed-off-by: Florent Benoit <fbenoit@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

implement 'registerRenameProvider' of Languages API

e5de537

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com> Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Requested changes

f5c9dc5

Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

Removed obsolete method

96e4408

Signed-off-by: Bogdan Stolojan <petre.stolojan@arm.com>

bogthe force-pushed the electron-master-extension branch from e194adc to 96e4408 Compare January 21, 2019 16:38

bogthe requested review from benoitf and evidolob as code owners January 21, 2019 16:38

bogthe mentioned this pull request Jan 21, 2019

Electron master extension #4127

Closed

bogthe closed this Jan 21, 2019

bogthe deleted the electron-master-extension branch January 22, 2019 12:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for master electron packages #3431

Added support for master electron packages #3431

bogthe commented Nov 7, 2018

thegecko commented Jan 16, 2019

bogthe commented Jan 18, 2019

akosyakov Jan 21, 2019 •

edited

Loading

bogthe Jan 21, 2019

akosyakov Jan 22, 2019

thegecko Jan 22, 2019

kittaakos commented Jan 21, 2019

bogthe commented Jan 21, 2019

kittaakos left a comment

kittaakos Jan 21, 2019

kittaakos Jan 21, 2019

kittaakos Jan 21, 2019

bogthe Jan 21, 2019

kittaakos Jan 21, 2019

kittaakos Jan 21, 2019

bogthe commented Jan 21, 2019

kittaakos Jan 21, 2019

bogthe Jan 21, 2019

paul-marechal commented Jan 21, 2019 •

edited

Loading

bogthe commented Jan 21, 2019

bogthe commented Jan 21, 2019

paul-marechal commented Jan 21, 2019

Added support for master electron packages #3431

Added support for master electron packages #3431

Conversation

bogthe commented Nov 7, 2018

thegecko commented Jan 16, 2019

bogthe commented Jan 18, 2019

akosyakov Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kittaakos commented Jan 21, 2019

bogthe commented Jan 21, 2019

kittaakos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogthe commented Jan 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-marechal commented Jan 21, 2019 • edited Loading

bogthe commented Jan 21, 2019

bogthe commented Jan 21, 2019

paul-marechal commented Jan 21, 2019

akosyakov Jan 21, 2019 •

edited

Loading

paul-marechal commented Jan 21, 2019 •

edited

Loading