discussion: electron main fixup #8056
Conversation
Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Signed-off-by: Akos Kitta <kittaakos@typefox.io>
as the electron main cannot access the window object. alternatively, we could get the config from the window: `currentWindow.webContents.executeJavaScript` and get the config, but the window is not yet loaded :/ Signed-off-by: Akos Kitta <kittaakos@typefox.io>
it can be retrieved from the FE config Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Signed-off-by: Akos Kitta <kittaakos@typefox.io>
fixed the workspace location issue at app start. Signed-off-by: Akos Kitta <kittaakos@typefox.io>
| } | ||
|
|
||
| protected async onSecondInstance(event: ElectronEvent, argv: string[], cwd: string): Promise<void> { | ||
| await this.launch({ argv, cwd, secondInstance: true }); |
There was a problem hiding this comment.
Why remove cwd???
My approach was to pass everything Electron gives us because here's how the second instance API works:
- You can do
cd some/place && app .and it will open/some/placeas a workspace because the app knows what's the cwd when you doapp . - Now the app is already running, and you open a new terminal and do
cd somewhere/else && app .. Here it will trigger the second-instance mechanism and forward the cwd used to invoke the second instance to the already running process. If we don't have the new cwd we cannot resolveapp .to the correct location.
There was a problem hiding this comment.
Why remove cwd???
Work in progress; otherwise, I could not start the app without a workspace error on macOS.
There was a problem hiding this comment.
BTW, can you please fix the macOS behavior on your branch, and I'll just rebase this PR from yours and get back the cwd support. What you wrote 👆 sounds good to me, a very nice addition, but it does not work on macOS 😕 We must verify the bundled app behavior too.
There was a problem hiding this comment.
- You can do
cd some/place && app .and it will open/some/placeas a workspace because the app knows what's the cwd when you doapp .- Now the app is already running, and you open a new terminal and do
cd somewhere/else && app .. Here it will trigger the second-instance mechanism and forward the cwd used to invoke the second instance to the already running process. If we don't have the new cwd we cannot resolveapp .to the correct location.
@marechal-p, VS Code handles it differently, right?
cat `which code`
#!/usr/bin/env bash
#
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
function realpath() { python -c "import os,sys; print(os.path.realpath(sys.argv[1]))" "$0"; }
CONTENTS="$(dirname "$(dirname "$(dirname "$(dirname "$(realpath "$0")")")")")"
ELECTRON="$CONTENTS/MacOS/Electron"
CLI="$CONTENTS/Resources/app/out/cli.js"
ELECTRON_RUN_AS_NODE=1 "$ELECTRON" "$CLI" "$@"
exit $?
So the approach you have implemented does not work on macOS; when I start the app from a terminal in dev mode with yarn, or I launch the electron backend launch config from VS Code, without specifying any file my options will fall back to the electron process: "/Users/akos.kitta/Desktop/theia/node_modules/electron/dist/Electron.app/Contents/MacOS/Electron" and this cannot be opened for sure.
Is there a chance to support this new feature once it works in a follow-up PR?
CC: @akosyakov
What it does
How to test
Review checklist
Reminder for reviewers