electron: restore previous workspace

[vince-fugnitto]

What it does

Fixes: #9990.
Fixes: #9997.
Closes: #9993

The commit fixes logic when determining whether to open a default window or existing workspace on electron startup. The update now correctly restores the workspace which would previously never restore properly. As a consequence, the fix also resolves the window-state not being properly restored.

How to test

Workspace Restoration:

1. start the example-electron application, and select a workspace folder.
2. close the application.
3. the application should re-open with the previous workspace folder (master would reload to the default window).
4. execute the command new window - a new window should open with no workspace present.
5. start the application and specify a workspace folder (ex: (cd examples/electron/ && yarn start ~/workspaces/theia)) - the application should start and open with the specified workspace.

Test Size + Position:

1. start the example-electron application
2. resize the window, move the position
3. close the application, and re-open
4. confirm the window size and position is restored
5. opening new folders (which spawns new windows) should not overlap eachother

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

Signed-off-by: vince-fugnitto [email protected]

[msujew]

I can confirm that the issue exists on master and is addressed by this change nicely.

[ccorn]

I hereby confirm that git cherry-pick origin/vf/electron-restore solves #9990 for me as well as immediately opening the previous workspace instead of showing a welcome page. Well done!

[vince-fugnitto]

One limitation with the approach is if the default window (no workspace) was the last opened window, then we will still attempt to re-open the last workspace. For comparison, vscode restores the default window successfully for such a case.

[ccorn]

Question: This would be the first non-maintenance commit to master since v1.17.0. Could we have a release tag (e.g. v1.17.1) so packagers do not need to cherry-pick?

[vince-fugnitto]

Question: This would be the first non-maintenance commit to master since v1.17.0. Could we have a release tag (e.g. v1.17.1) so packagers do not need to cherry-pick?

@ccorn still confirming if the patch is proper, or if additional work is required to handle the other use-cases. As for creating a patch release, we generally do not do it for each non-maintenance commits to master but only if a major bug is resolved (we can think about if this warrants a patch release). Else, you can always use a next version of the @theia dependencies once the commit is merged for your application.

[ccorn]

One limitation with the approach is if the default window (no workspace) was the last opened window, then we will still attempt to re-open the last workspace. For comparison, vscode restores the default window successfully in such a case.

In that case, opening the welcome page should clear Theia's the persistent Electron store's idea of latest workspace (without clearing the list of recent workspaces, of course), or there should be a distinction between latest window and latest workspace, I think.

[ccorn]

One limitation with the approach is if the default window (no workspace) was the last opened window, then we will still attempt to re-open the last workspace. For comparison, vscode restores the default window successfully in such a case.

Interesting. With this patch:

1. yarn start:electron opens previous workspace with previous window coordinates (as expected)
2. New Window opens the welcome page with new window coordinates (as expected)
3. Close the workspace window, leaving only the welcome page, then close that as well.
4. yarn start:electron now opens previous workspace with latest welcome page's coordinates. Hmmm. Not expected, but not such a show stopper as the behavior without this patch.

[paul-marechal]

Code LGTM.

Using this patch I am able to restore the last workspace as well as the case where the workspace is not set (the window re-opens without a workspace).

[paul-marechal]

@ccorn can you also confirm that the behavior is fixed?

[ccorn]

@ccorn can you also confirm that the behavior is fixed?

The slightly odd behavior described above remains.
@paul-marechal: How have you got rid of workspaces so the next start shows the welcome page instead (apart from deleting persistent storage)?

[paul-marechal]

@ccorn I followed the same procedure as you mentioned.

Vincent force-pushed, maybe you are still using the old changes? git show should show commit c4fff9e.

[ccorn]

@ccorn I followed the same procedure as you mentioned.

Vincent force-pushed, maybe you are still using the old changes? git show should show commit c4fff9e.

I basically do a git fetch origin vf/electron-restore (which has got the forced update) and apply that on a branch starting from v1.17.0 using git cherry-pick FETCH_HEAD. The latest such action reported 4 files changed, 9 insertions(+), 15 deletions(-), and git show FETCH_HEAD showed commit c4fff9ecbf2fa17c6fe31b01a10455b0d3acd653. That should be it.

[ccorn]

Update: A clean rebuild fixed it. I can confirm that closing opening the welcome page at the end will get it reopened at the next start, otherwise the latest workspace gets restored.

Excellent issue handling! I cannot comment on the code, but I ran across a Linter warning for one of the affected files (Update: seems unrelated):

@theia/workspace: /home/ccorn/BUILD/theia/packages/workspace/src/browser/workspace-service.ts
@theia/workspace:   434:16  warning  'getTemporaryWorkspaceFileUri' is deprecated. since 1.4.0 - because of https://github.com/eclipse-theia/theia/wiki/Coding-Guidelines#di-function-export, use `WorkspaceService.getUntitledWorkspace` instead  deprecation/deprecation

[paul-marechal]

We'll release tomorrow, just need to give a heads up during the dev meeting as there's at least one other commit I'd like to include.