-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Comments
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. |
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? |
If you start a terminal or open a file with no workspace then open a workspace you lose your teriminal and opened file. |
Maybe we just should not reuse the window but open a new one as usual. |
Note i still don't get the relationship between the window and the workspace if anyone cares to elaborate ? |
it is one to one, you cannot change the workspace, only open a new in another window |
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. |
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 ? |
Having different workspaces is the reason why I want to open multiple Theia apps. That's why there is the relationship. |
Isn't it the opposite since there is a relationship we need to open multiple theia apps ? 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 ? |
Try to open a folder in an already opened workspace. In this case, you will get a new window. |
It does not, nothing happens and I have to manually reload for the change to take effect. At least in the browser example. |
See also #662 about this. |
Then it is a bug it used to work. |
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? |
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. |
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? |
In the long run however thinking about multi-workspace I think we should consider having frontend code react to workspace root changes. |
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. |
Also, let's keep multiple root directories for one workspace different from multiple workspaces. 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. |
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:
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:
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. |
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. |
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. |
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 ? |
As part of the #838, we discussed the followings:
Both apply to electron and browser. |
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. |
My only problem with that is losing the no-workspace state... is it saved like the workspaces states are saved on close? |
Yes, I added a special prefix for that. You can verify by reloading a browser in that context. |
OK closing this |
When you open a workspace in the navigator, Theia reloads calling localhost:3000
Thus losing all state in the frontend.
The text was updated successfully, but these errors were encountered: