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

[workspace] misleading isMultiRootWorkspaceOpened implementation #4811

Closed
vince-fugnitto opened this issue Apr 4, 2019 · 2 comments · Fixed by #5498
Closed

[workspace] misleading isMultiRootWorkspaceOpened implementation #4811

vince-fugnitto opened this issue Apr 4, 2019 · 2 comments · Fixed by #5498
Labels
bug bugs found in the application help wanted issues meant to be picked up, require help workspace issues related to the workspace

Comments

@vince-fugnitto
Copy link
Member

The following implementation for the method isMultiRootWorkspaceOpened() is misleading.
https://github.com/theia-ide/theia/blob/a27cbd0bfcc03ccc36044e51c6bd341d7f5eb93a/packages/workspace/src/browser/workspace-service.ts#L267-L269

The method checks whether there is a workspace opened and that the preference is on.
This is a false positive and does not accurately describe if a multi-root workspace is in fact opened.

Additional checks should be made to verify the source of the workspace
(for example being a *.theia-workspace file) or a directory (single root).

@elaihau your thoughts?

@vince-fugnitto vince-fugnitto added bug bugs found in the application workspace issues related to the workspace labels Apr 4, 2019
@elaihau
Copy link
Contributor

elaihau commented Apr 4, 2019

you are right. isMultiRootWorkspaceOpened does not return what its name indicates. it should be renamed into something like isMultiRootWorkspaceEnabled.

@akosyakov
Copy link
Member

We can keep though isMultiRootWorkspaceOpened with a workspace file stat check, but should still add a breaking change note in CHANGELOG, since semantics is changed.

@akosyakov akosyakov added the help wanted issues meant to be picked up, require help label Apr 5, 2019
elaihau pushed a commit to elaihau/theia that referenced this issue Jun 17, 2019
- fixed eclipse-theia#4811

Signed-off-by: elaihau <liang.huang@ericsson.com>
elaihau pushed a commit to elaihau/theia that referenced this issue Jun 17, 2019
- fixed eclipse-theia#4811

Signed-off-by: elaihau <liang.huang@ericsson.com>
elaihau pushed a commit to elaihau/theia that referenced this issue Jun 17, 2019
- fixed eclipse-theia#4811

Signed-off-by: elaihau <liang.huang@ericsson.com>
elaihau pushed a commit to elaihau/theia that referenced this issue Jun 17, 2019
- fixed eclipse-theia#4811

Signed-off-by: elaihau <liang.huang@ericsson.com>
elaihau pushed a commit to elaihau/theia that referenced this issue Jun 25, 2019
- fixed eclipse-theia#4811

Signed-off-by: elaihau <liang.huang@ericsson.com>
elaihau pushed a commit to elaihau/theia that referenced this issue Jun 26, 2019
- fixed eclipse-theia#4811

Signed-off-by: elaihau <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Jun 26, 2019
- fixed #4811

Signed-off-by: elaihau <liang.huang@ericsson.com>
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 help wanted issues meant to be picked up, require help workspace issues related to the workspace
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants