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

Opening a workspace reloads theia #661

Closed
hexa00 opened this issue Oct 18, 2017 · 30 comments
Closed

Opening a workspace reloads theia #661

hexa00 opened this issue Oct 18, 2017 · 30 comments
Labels
bug bugs found in the application question user / developer questions

Comments

@hexa00
Copy link

hexa00 commented Oct 18, 2017

When you open a workspace in the navigator, Theia reloads calling localhost:3000

Thus losing all state in the frontend.

@hexa00 hexa00 added the bug bugs found in the application label Oct 18, 2017
@hexa00
Copy link
Author

hexa00 commented Oct 18, 2017

Looking more into it this comes from workspace-service.ts:

 protected async doOpen(uri: URI, options?: WorkspaceInput): Promise<void> {
+        const rootUri = uri.toString();
+        const valid = await this.isValidRoot(rootUri);
+        if (valid) {
+            // The same window has to be preserved too (instead of opening a new one), if the workspace root is not yet available and we are setting it for the first time.
+            const preserveWindow = !(await this.tryRoot);
+            await this.server.setRoot(rootUri);
+            this.openWindow(uri, Object.assign(options || {}, { preserveWindow }));
+            return;
+        }
+        throw new Error(`Invalid workspace root URI. Expected an existing directory location. URI: ${rootUri}.`);
+    }

Why do we need to reload the window ?

And if so we should restore the layout especially if there was no workspace there before.

@svenefftinge
Copy link
Contributor

We restore layout on a per workspace basis and reload the frontend when the workspace root changes. Both are intended and work as designed. What is the issue?

@kittaakos kittaakos added the question user / developer questions label Oct 19, 2017
@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

If you start a terminal or open a file with no workspace then open a workspace you lose your teriminal and opened file.

@akosyakov
Copy link
Member

akosyakov commented Oct 19, 2017

Maybe we just should not reuse the window but open a new one as usual.

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

Note i still don't get the relationship between the window and the workspace if anyone cares to elaborate ?

@akosyakov
Copy link
Member

akosyakov commented Oct 19, 2017

it is one to one, you cannot change the workspace, only open a new in another window

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

Why ? I think we should look ahead with what vscode is doing with multi workspaces all we need is ui widgets to listen on a work space change event no ?

@akosyakov
Copy link
Member

akosyakov commented Oct 19, 2017

Why ? I think we should look ahead with what vscode is doing with multi workspaces all we need is ui widgets to listen on a work space change event no ?

At some point in time probably yes, right now it will be a big effort. VS code took couple months to implement it. Before it was the same behavior: one workspace one window.

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

My point is that vscode took a lot of time because of the refactor involved. But in our case that may not be an issue. And we may do well to avoid having to do a refactor like that int the future.

So while we may not support multi workspace now I don't see why the infrastructure for theia could not be made with this in mind.

So my original question still stands what is the relationship between the window and the workspace ? Why is it there ?

@svenefftinge
Copy link
Contributor

Having different workspaces is the reason why I want to open multiple Theia apps. That's why there is the relationship.

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

Isn't it the opposite since there is a relationship we need to open multiple theia apps ?
Or we do not need do do that ? And we could choose to keep the same window ?

Seems that as it is now we do not open multiple apps we just reload for no reason ?

Also this may be a good idea if you open a new workspace, but if you had no workspace and open a one I don't think we should open a new app. WDYT ?

@akosyakov
Copy link
Member

Seems that as it is now we do not open multiple apps we just reload for no reason ?

Try to open a folder in an already opened workspace. In this case, you will get a new window.

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

It does not, nothing happens and I have to manually reload for the change to take effect. At least in the browser example.

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

See also #662 about this.

@akosyakov
Copy link
Member

Then it is a bug it used to work.

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

OK.

So when we open a workspace we should:

But when we open the first workspace should we not keep the current layout if any ?

And thus there is no reason to reload in this case?

@akosyakov
Copy link
Member

akosyakov commented Oct 19, 2017

It complicates the logic, i.e. frontend code should listen to the workspace root changes and react. The reason is to keep it simple.

We could make it even simpler and always open a new window regardless whether a workspace was opened before. It should solve your issue to preserve the layout.

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

We could make it even simpler and always open a new window regardless whether a workspace was opened before. It should solve your issue to preserve the layout.

That would feel weird however since just opening theia and clicking open would open a new one.

I suggest the workaround for now would be to warn the user if he has stuff open. WDTY?

@hexa00
Copy link
Author

hexa00 commented Oct 19, 2017

In the long run however thinking about multi-workspace I think we should consider having frontend code react to workspace root changes.

@svenefftinge
Copy link
Contributor

svenefftinge commented Oct 29, 2017

Please explain why. Both Atom and Vs Code do it like that, too. I don't see a use-case to change the workspace root without opening a new window or reloading.

@svenefftinge
Copy link
Contributor

Also, let's keep multiple root directories for one workspace different from multiple workspaces.
A workspace really is meant to be the place a user works in for a specific scope. We should restore layout for that no matter it has multiple roots or not. And, yes, when we allow multiple roots, we should allow adding and removing them to the current workspace, which should not change the layout or any other frontend state as much as possible.

For the case that no workspace or file has been specified to open, I still don't understand the use case, in which a user would first create state (opening terminals and files), then at a later point decide to open a workspace and expect to keep the state from before.

@hexa00
Copy link
Author

hexa00 commented Oct 30, 2017

For multi-root/ workspace:

My point is that if we're going to allow multi-root we need to make sure that what is now the workspaceRoot can be changed. Thus components need to react to this change.

And if we do design this this way we will be able to support:

  • Multi-root
  • Whatever window behavior we want, (new window/ replace the windows/ don't replace and just add a directory in the workspace)

However if we don't design it like that and make it so that we have a direct relationship between the window and the workspaceRoot. We can only support:

  • One workspace root
  • New windows for any new workspace root

So while I agree that it simplifies the code now. I think it complicates things a lot more for the future. And that we should think about this more ?

For the case of no workspace, Theia is unique from other ides/editors in that it has a backend/frontend design. And I've seen interest by many people to use it as an editor for sysadmin like tasks like opening a config file on a remote server or things like that.

It could also be used as a an editor while using git commands for example git commit could bring up theia.

So some people may use it without a workspace and I think we should support that.

Now about keeping the state, I think in general we should not clear state without any warning to the user in any scenario.

The last thing a user wants is to lose some data or whatever he was doing before.

@svenefftinge
Copy link
Contributor

svenefftinge commented Nov 7, 2017

We have a direct relationship between "window and workspace". While atm a workspace can only have one root, we can for sure allow multiple roots for a single! workspace in future.

@svenefftinge
Copy link
Contributor

Now about keeping the state, I think in general we should not clear state without any warning to the user in any scenario.

I did not say we should lose state. The state is just associated to another context (no workspace context). The whole point is to use a 'workspace' as a scope for a certain workbench state, such that I get the context I was in back when I reopen a previously used WS.

So when you initially open without a workspace and create state we keep it in that context, such that when the sysadmin comes back to 'no workspace' she will find the same context.

@hexa00
Copy link
Author

hexa00 commented Nov 7, 2017

For the case that no workspace or file has been specified to open, I still don't understand the use case, in which a user would first create state (opening terminals and files), then at a later point decide to open a workspace and expect to keep the state from before.

So when you initially open without a workspace and create state we keep it in that context, such that when the sysadmin comes back to 'no workspace' she will find the same context.

These 2 for me are contradictory ? But I like the later.

So opening a workspace from no-workspace context would open a new window correct ?

@kittaakos
Copy link
Contributor

As part of the #838, we discussed the followings:

  • When the WS root is not set and the user selects one with the Open..., we reload the current window.
  • When the WS root is set, selecting a new one with the Open... will open a new window.

Both apply to electron and browser.

@kittaakos
Copy link
Contributor

There is one significant difference between the WS root opening behavior in VSCode and Theia: VSCode does not open a new window when the user opens a new WS.

@hexa00
Copy link
Author

hexa00 commented Nov 17, 2017

When the WS root is not set and the user selects one with the Open..., we reload the current window.

My only problem with that is losing the no-workspace state... is it saved like the workspaces states are saved on close?

@svenefftinge
Copy link
Contributor

Yes, I added a special prefix for that. You can verify by reloading a browser in that context.

@hexa00
Copy link
Author

hexa00 commented Nov 20, 2017

OK closing this

@hexa00 hexa00 closed this as completed Nov 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application question user / developer questions
Projects
None yet
Development

No branches or pull requests

4 participants