multi-root workspace support, minimal patch #2117

elaihau · 2018-06-15T20:54:24Z

What is a workspace / workspace folder? It is a folder on the file system that serves as a container for:

workspace configuration: in sub-folder .theia
- preferences, in .theia/settings.json
- root folders list, in .theia/roots.json
optional content in the form of other files/folders

For backward compatibility and simplicity, a workspace folder that contains no explicit root folder list configuration will use its own folder as the single root. This permits using any folder as a workspace.
If subsequently an additional root folder is added to such a workspace, then both the newly added folder and the original implicit root are added to the root folders list configuration. Either can later be removed. If all roots are removed form the configuration, the workspace falls-back to using its own folder as implicit root.

This PR is the minimal patch for #1660

What's included in this PR:

a workspace's root folders list can be modified using the navigator context menu "add/remove folder" items to add or remove a folder
preference 'workspace.supportMultiRootWorkspace', with default value false, can be used to control if the context menu entries, used to modify the list of root folders, are shown or not. Since this is a new feature that is not supported by most Theia extensions yet, it probably makes sense to hide the UI by default for now
previously the "current root" was a static value, obtained by each extension that needs it, at the startup of a Theia client. To change it (e.g. to switch workspace), a restart of the client was required. For this feature we did not want to have to restart the client each time the list of root folders is updated, so we introduced a new "onWorkspaceRootChanged" event. Extensions that need to know when the list of root folders is modified can react to this event, e.g. to update their widgets. It's still necessary to reload the Theia frontend when switching workspace in a given tab or when closing a workspace.
the root folder list comprised of 1 to many folders can be displayed from the file navigator widget.
the root folders will be scanned for git repositories that the user can select, stage and commit files into.

What's not included:

users should have the choice of naming the workspace and choosing where to store the config file
preference extension should be able to handle the settings from multiple project
search-in-workspace and file-search extension should be able to perform the search across all folders in the workspace, and / or provide filtering mechanism to reduce the amount of information being displayed
output and problems widgets should display data from all root folders, and / or provide filtering mechanism to reduce the amount of information being displayed
task extension should be able to run tasks defined under all root folders
users should be able to open the terminal from any root folder in the workspace

Signed-off-by: elaihau liang.huang@ericsson.com

paul-marechal

I might have missed some things but it looks nice !

paul-marechal · 2018-06-26T18:46:56Z

packages/filesystem/src/browser/file-tree/file-tree-model.ts

@@ -124,6 +131,10 @@ export class FileTreeModel extends TreeModelImpl implements LocationService {
                    await this.fileSystem.move(sourceUri, targetUri, { overwrite: true });
                    // to workaround https://github.com/Axosoft/nsfw/issues/42
                    this.refresh(target);
+
+                    if (source.parent) {
+                        this.refresh(source.parent);


This wasn't here before, is it a bug fix packed along the multi-root feature ?

Correct me if i am wrong: i believe this is related to Axosoft/nsfw#42.
When files move from one root to the other, the code is not getting enough information to find out the source.

Thanks for the explanation !

Could you elaborate on how can I reproduce an issue without this code? refresh supposed to handle Axosoft/nsfw#42.

@elaihau could you comment on previous comment please

Sorry I missed it. The way to reproduce it is having more than one root folders added to the workspace (say, folder1 and folder2), and moving a folder by drag-and-drop from folder2 to folder1, and same folder back from folder1 to folder2.

paul-marechal · 2018-06-26T19:00:12Z

packages/filesystem/src/browser/file-tree/file-tree.ts

@@ -78,6 +90,38 @@ export class FileTree extends TreeImpl {
        };
    }

+    protected isVirtualRoot(node: CompositeTreeNode): boolean {
+        return node.parent === undefined && this.hasVirtualRoot && node.name === 'WorkspaceRoot';


Is it possible to simply test the node based on a field it has ?
Like instead of checking if the tree knows if it holds a virtual root and check the name, have a field virtual and test like !node.parent && node.virtual. Or even !node.parent && VirtualNode.is(node) to match the way the check is done on other nodes ?

Maybe the tree doesn't have to know if it holds a virtual node, that's my thought. Tell me if I missed something.

I believe I don't event need the flag hasVirtualRoot in FileTree class. Like you said, the tree shouldn't have the knowledge pertaining to whether its root is virtual or not.

I will make the change you suggested.

paul-marechal · 2018-06-26T19:29:48Z

packages/navigator/src/browser/navigator-widget.ts

+        await this.refreshWidget(workspaceChangeData.root, workspaceChangeData.roots, workspaceChangeData.workspaceConfigFile);
+    }
+
+    protected async refreshWidget(root: FileStat | undefined, roots: FileStat[], workspaceConfig: FileStat | undefined) {


Would it be better to just rely on roots and not root at the same time ?
I feel like the prototype should be:

refreshWidget(roots: FileStat[], workspaceConfig: FileStat | undefined)

paul-marechal · 2018-06-26T19:35:07Z

packages/workspace/src/browser/workspace-service.ts

+                this.startWatch(valid);
+                this.onWorkspaceRootChangeEmitter.fire(this._workspaceRootChange);
+            }
+            return Promise.resolve();


paul-marechal · 2018-06-26T19:41:48Z

packages/workspace/src/browser/workspace-uri-contribution.ts

@@ -19,7 +19,8 @@ export class WorkspaceUriLabelProviderContribution extends DefaultUriLabelProvid
    constructor(@inject(WorkspaceService) wsService: WorkspaceService,
        @inject(FileSystem) protected fileSystem: FileSystem) {
        super();
-        wsService.root.then(root => {
+        wsService.workspaceRootChange.then(data => {


It is minor, but maybe this could be in an async postConstruct method, to convert this part to attribute injection like the documentation suggests to do ?

paul-marechal · 2018-06-26T19:42:43Z

packages/workspace/src/node/default-workspace-server.ts

+            newConfig.roots = [workspaceFolderUri, rootUriToAdd];
+        }
+        await this.writeToWorkspaceFolder(newConfig);
+        return Promise.resolve();


You can remove this line unless you think it adds clarity.

paul-marechal · 2018-06-26T19:42:54Z

packages/workspace/src/node/default-workspace-server.ts

+        if (configContent) {
+            await this.writeToWorkspaceFolder({ roots: configContent.roots.filter(r => r !== rootUriToRemove) });
+        }
+        return Promise.resolve();


You can remove this line unless you think it adds clarity.

paul-marechal · 2018-06-26T19:52:41Z

Also for some reason, when I finish to build your branch, I see that there is a diff on yarn.lock, did you forget to check it in ?

marcdumais-work · 2018-06-26T22:31:17Z

packages/workspace/src/browser/workspace-service.ts

+                    firstRootNameDiv.textContent = rootName;
+                    firstRootMsg.appendChild(firstRootNameDiv);
+                    const remainder = document.createElement('span');
+                    remainder.textContent = ' and its subfolders cannot be removed as it is the first root folder of the workspace.';


As discussed, it may no longer be necessary to treat the first root folder in a special way, at least as far as removal goes.

marcdumais-work · 2018-06-26T22:52:56Z

I tried this and was not able to switch workspace, using "File -> open". It worked when I first closed the current workspace and tried again.

marcdumais-work · 2018-06-27T00:58:05Z

@elaihau I think the description of this PR and commit message should be updated/enhanced. Here's my draft suggestion; we can discuss tomorrow.

What is a workspace / workspace folder? A folder on the file system that serves as a container for:

workspace configuration: in sub-folder .theia
- preferences, in .theia/settings.json
- root folders list, in .theia/roots.json
optionally content in the form of other files/folders

Theia and its extensions will generally only consider folders that are part of the root folders list. e.g. only those folders:

will be shown in the file navigator
will be scanned for git repositories that the user can select, stage and commit files into
[similarly for other extensions, to be updated later]

For extensions that are not yet updated to support multiple root folders, they will temporarily default to using the first configured root folder as fallback.

For backward compatibility and simplicity, a workspace folder that contains no explicit root folder list configuration will use its own folder as the single root. This permits using any folder as a workspace.
If subsequently an additional root folder is added to such a workspace, then both the newly added folder and the original implicit root are added to the root folders list configuration. Either can later be removed. If all roots are removed form the configuration, the workspace falls-back to using its own folder as implicit root.

What's in this PR:

a workspace's root folders list can be modified using the navigator context menu "add/remove folder" items to add or remove a folder
preference 'workspace.supportMultiRootWorkspace', with default value false, can be used to control if the context menu entries, used to modify the list of root folders, are shown or not. Since this is a new feature that is not supported by most Theia extensions yet, it probably makes sense to hide the UI by default for now
previously the "current root" was a static value, obtained by each extension that needs it, at the startup of a Theia client. To change it (e.g. to switch workspace), a restart of the client was required. For this feature we did not want to have to restart the client each time the list of root folders is updated, so we introduced a new "onWorkspaceRootChanged" event. Extensions that need to know when the list of root folders is modified can react to this event, e.g. to update their widgets. It's still necessary to reload the Theia frontend when switching workspace in a given tab or when closing a workspace.
extensions updated: workspace, git, & navigator

marcdumais-work · 2018-06-27T11:04:56Z

One little bug I noticed: If I enable the preference, I can see and execute the multi-root related commands. If I then disable the preference, the commands are still visible but disabled, until I restart the client. After client restart, the commands are no longer visible.

Update: not sure it's always the case, but when I just added the preference globally, I needed a client restart before I could see the commands in the context menu,

lmcbout · 2018-06-27T12:59:15Z

Looking at the following statement mentioned before: If subsequently an additional root folder is added to such a workspace, then both the newly added folder and the original implicit root are added to the root folders list configuration. Either can later be removed. If all roots are removed form the configuration, the workspace falls-back to using its own folder as implicit root. For the initial contribution, will we be able to remove the initial root folders !!

…

________________________________ De : marcdumais-work <notifications@github.com> Envoyé : 26 juin 2018 20:58:07 À : theia-ide/theia Cc : Subscribed Objet : Re: [theia-ide/theia] multi-root workspace support, minimal patch (#2117) @elaihau<https://github.com/elaihau> I think the description of this PR and commit message should be updated/enhanced. Here's my draft suggestion; we can discuss tomorrow.

________________________________ What is a workspace / workspace folder? A folder on the file system that serves as a container for: * workspace configuration: in sub-folder .theia * preferences, in .theia/settings.json * root folders list, in .theia/roots.json * optionally content in the form of other files/folders Theia and its extensions will generally only consider folders that are part of the root folders list. e.g.: * will be shown in the file navigator * will be scanned for git repositories that the user can select, stage and commit files into * [similarly for other extensions, to be updated later] For extensions that are not yet updated to support multiple root folders, they will temporarily default to using the first configured root folder as fallback. For backward compatibility and simplicity, a workspace folder that does not contain no explicit root folder list configuration will use its own folder as the single root. This permits using any folder as a workspace. If subsequently an additional root folder is added to such a workspace, then both the newly added folder and the original implicit root are added to the root folders list configuration. Either can later be removed. If all roots are removed form the configuration, the workspace falls-back to using its own folder as implicit root. What's in this PR: * a workspace's root folders list can be modified using the navigator context menu "add/remove folder" items to add or remove a folder * preference 'workspace.supportMultiRootWorkspace', with default value false, can be used to control if the context menu entries, used to modify the list of root folders, are shown or not. Since this is a new feature that is not supported by most Theia extensions yet, it probably makes sense to hide the UI by default for now * previously the "current root" was a static value, obtained by each extension that needs it, at the startup of a Theia client. To change it (e.g. to switch workspace), a restart of the client was required. For this feature we did not want to have to restart the client each time the list of root folders is updated, so we introduced a new "onWorkspaceRootChanged" event. Extensions that need to know when the list of root folders is modified can react to this event, e.g. to update their widgets. It's still necessary to reload the Theia frontend when switching workspace in a given tab or when closing a workspace. * extensions updated: workspace, git, & navigator — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#2117 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAZk69UDp3Eo6W4jpreDpF1VuD0eM9Ccks5uAtifgaJpZM4UqIyF>.

marcdumais-work · 2018-06-27T13:05:47Z

For the initial contribution, will we be able to remove the initial root folders !!

Yes, with a couple of minor adjustments to the current PR.

akosyakov · 2018-06-27T13:31:24Z

packages/filesystem/src/browser/file-tree/file-tree-model.ts

-        const parent = this.getNode(change.uri.parent.toString());
-        if (DirNode.is(parent) && parent.expanded) {
-            accept(parent);
+        const parentNodes = this.getNodes((node: FileStatNode) => change.uri.parent.toString() === node.uri.toString());


Is there a way to look up by an id still? An idea was to have fast look up, now it seems to travers the whole tree to find a node.

akosyakov · 2018-06-27T13:32:37Z

packages/core/src/browser/tree/tree.ts

@@ -204,6 +208,17 @@ export class TreeImpl implements Tree {
        return id !== undefined ? this.nodes[id] : undefined;
    }

+    getNodes(test: (node: TreeNode) => boolean): TreeNode[] {


It looks like a very expensive operation. Is there a way to replace clients with getNode(id) by redesigning their tree structure?

Then what id would you like to use ? Two different nodes can hold the same file uri (which was the old id).

example: I have the following roots:

- /home/x/projects/theia/ - /home/x/projects/

(it is something possible on vscode)

getNode(id) function is still in the interface. getNodes() is only called when there is a need. Having multiple roots opened in the file navigator means there could be more than one file associated with the same uri (like the scenario that @marechal-p described above).

Suppose there is file://foo/bar/baz/x.txt file, and 2 workspaces file://foo and file://foo/bar.

If a file tree node have an id of form ${workspaceUri}:${fileUri} or something like that (so file://foo:file://foo/bar/baz/x.txt and file://foo/bar:file://foo/bar/baz/x.txt), then when a file is changed one can take a list of all workspaces, loop over them to create node ids and call getNode for each.

I assume that a list of workspaces will be short in most of cases (i have maximum 10 at the time), in comparison the file tree only for node_modules folder has over 800 children.

Having an id in the form of ${workspaceUri}:${fileUri} is very close to I already did in this PR. With the ${workspaceUri} part of the id, we could easily figure out which root folder that the node comes from. No issues. :)

The getNodes() function is only called in the scenario where the ${workspaceUri}:${fileUri} is unavailable. Imagine we have file://foo:file://foo/bar/baz/x.txt and file://foo/bar:file://foo/bar/baz/x.txt in the workspace, and a user opens the file://foo/bar/baz/x.txt from the quick open (Ctrl + p). When this happens we have no context of this x.txt and therefore would have no idea which node to highlight in the file tree.

I see the reason that you think this operation is expensive. I would be happy if we could find a work around of calling getNodes().

When this happens we have no context of this x.txt and therefore would have no idea which node to highlight in the file tree.

I don't understand how getNodes() makes difference? We always know workspace roots, so it is not an issue to create file://foo:file://foo/bar/baz/x.txt and file://foo/bar:file://foo/bar/baz/x.txt and call getNode for them.

Also sorry i meant ${workspaceRootUri}:${fileUri}, not ${workspaceUri}:...

Concatenating the absolute root uris with the absolute file uris seems to be the most simple way to craft an id indeed. The getNodes was to make the loop you talked about part of the implementation, but thinking back it doesn't make really much sense for the Tree interface indeed.

@akosyakov fixed

elaihau · 2018-06-27T14:08:53Z

packages/navigator/src/browser/navigator-widget.ts

-                this.update();
+                const icon = await this.labelProvider.getIcon(root);
+                this.model.root = DirNode.createRoot(root, label, icon);
+            } else if (roots.length > 1) { // more than one folder


Ideally having a single root in the workspace is just one scenario that multi-root workspace supports.
I actually don't have to have

if (roots.length === 1) { // only one folder is opened ... else if (roots.length > 1) { // more than one folder

And the logic under the else if should be good enough to have the file tree displayed in both cases.

The reason that I am separating the one-root and multi-root is that, the file tree would collapse due to #1781 and the tree wouldn't look as nice as it should, in the multi-root setting.
I will keep the single root and multi root display logic separated until #1781 is fixed.

marcdumais-work · 2018-06-27T18:25:29Z

@elaihau as briefly discussed, the event added to notify extensions that the workspace has changed (added or removed a root folder) seems to be sent only in the frontend where it was triggered.So if we have more than one client using that workspace, the other one(s) will not know to update their widgets/state to reflect the change. e.g. the navigator of other clients will not show the latest roots

marcdumais-work · 2018-06-29T15:33:25Z

packages/filesystem/src/browser/file-tree/file-tree-model.ts

@@ -121,6 +121,10 @@ export class FileTreeModel extends TreeModelImpl implements LocationService {
     * Move the given source file or directory to the given target directory.
     */
    async move(source: TreeNode, target: TreeNode) {
+        if (source.parent && source.parent.name === 'WorkspaceRoot') {
+            // do not support moving the root folder


That tests for any root folder, correct? If so, the comment could be adjusted. i.e. "the root" -> "a root"

marcdumais-work · 2018-06-29T17:30:39Z

packages/workspace/src/browser/workspace-service.ts

+
+        if (validUris.length > 0 && this._root) {
+            const messageContainer = document.createElement('div');
+            messageContainer.textContent = 'Do you really want to remove the following roots and their subfolders from the workspace?';


Would some users be scared that the content of the removed roots might be deleted? Since there is already an appropriate title on the dialog (Remove Folder From Workspace), maybe we could shorten the message and mention nothing is physically erased? e.g.:
"Remove the following folders from workspace? (note: nothing will be erased from disk)"

elaihau · 2018-07-26T17:47:43Z

I find it is strange that settings for one project (workspace root) are considered to be workspace settings. In VS Code they solved it by separating workspace file from projects and a workspace file contain workspace settings next to roots, not a random project from a workspace. Each project should has their own settings and workspace its own independent from projects.

agreed. settings from different root folders should be separated.
like the commit message and the PR description mentioned, changes made to preferences package are not included. This PR is just the minimal patch of the #1660

There will be a separated PR in near future that addresses preference issues introduced by having multiple roots in the same workspace.
Right now, there are only two levels of prefs: UserPref & WorkspacePref. With the multi-root feature added to theia, I am going to add FolderPreferences, where settings defined in a project folder will only be applied to that particular folder only.

akosyakov · 2018-07-27T05:24:39Z

packages/monaco/src/browser/monaco-workspace.ts

            if (rootStat) {
                this._rootUri = rootStat.uri;
                this.resolveReady();
            }
        });
+        this.workspaceService.onWorkspaceChanged(event => {


workspaceFolder cannot change right?

akosyakov · 2018-07-27T05:27:15Z

packages/navigator/src/browser/navigator-tree.ts

+        return CompositeTreeNode.is(node) && node.name === WorkspaceNode.name;
+    }
+
+    export function createRoot(children: DirNode[]): WorkspaceNode {


DirNode -> WorkspaceRootNode

akosyakov · 2018-07-27T05:36:05Z

packages/workspace/src/browser/workspace-frontend-contribution.ts

    ) { }

    registerCommands(commands: CommandRegistry): void {
        commands.registerCommand(WorkspaceCommands.OPEN, {
            isEnabled: () => true,
-            execute: () => this.showFileDialog()
+            execute: () => this.workspaceService.workspace.then(async data => {


please extract a protected method showFileDialog as before

akosyakov · 2018-07-27T05:46:06Z

packages/workspace/src/browser/workspace-uri-contribution.ts

-    constructor(@inject(WorkspaceService) wsService: WorkspaceService,
-        @inject(FileSystem) protected fileSystem: FileSystem) {
+
+    constructor() {


can be removed?

akosyakov · 2018-07-27T05:59:24Z

please rebase the PR, build failures probably are caused by a new release

please also look at #2117 (comment)

elaihau · 2018-07-27T13:32:00Z

we should remove it and use _workspaceDescription.workspaceFolder instead, as a single source of truth

done.

please rebase the PR, build failures probably are caused by a new release

just did the rebase. let me see if it builds this time

svenefftinge · 2018-07-30T08:19:00Z

There will be a separated PR in near future that addresses preference issues introduced by having multiple roots in the same workspace.

In that case, we could simply use roots[0] for now to obtain the location of preferences.

elaihau · 2018-07-30T13:06:14Z

@svenefftinge

In that case, we could simply use roots[0] for now to obtain the location of preferences.
roots[0] is not necessarily where the preferences retrieves the settings. Think about this use case:

user opens atom as the workspace folder
user adds vsCode to the workspace. At this point, theia gets prefs from .../atom/.theia/settings.json. And roots.json looks like roots: ['atom', 'vsCode']. Everything is working properly.
user removes atom from the workspace. roots.json becomes roots: ['vsCode'].
user reloads theia. When this happens preferences extension would get prefs from .../vsCode/.theia/settings.json (instead of from .../atom/.theia/settings.json).

You might want to argue that "we could expose both atom and vsCode even if atom is removed from the workspace". If we do that, preferences extension works fine, however other extensions would get wrong roots data, e.g., navigator would render 2 roots as it doesn't know atom is not in the workspace anymore.

svenefftinge · 2018-07-30T18:12:03Z

I don't really understand why it would be proper to use the atom-settings when working on the vsCode root? I think this is wrong and we need to support multiple settings, which are obtain using a URI as context. I.e. support settings per root.

But since you want to keep this PR small, we should tackle the preferences separately.
Therefore, I think we can go with the first root, what ever that is.

elaihau · 2018-07-30T18:58:08Z

I don't really understand why it would be proper to use the atom-settings when working on the vsCode root? I think this is wrong and we need to support multiple settings, which are obtain using a URI as context. I.e. support settings per root.

In this use case atom-settings should be used as the workspace settings, while the vsCode-settings should be used as the folder settings. workspace settings are applied to the entire workspace, while folder settings are applied to the folder / project (i.e., atom or vsCode), and folder settings should override the workspace settings in the folder where they take effect. Therefore, settings under the atom are still important, even if atom is no longer in the workspace.

But since you want to keep this PR small, we should tackle the preferences separately.
Therefore, I think we can go with the first root, what ever that is.

That would introduce inconsistencies if we don't change preferences package much:

when settings.json has atom and vsCode, workspace folder is atom, theia uses settings from atom
when atom is removed, settings.json has vsCode, workspace folder becomes vsCode, theia still uses settings from atom.
after user refreshes the browser, settings.json has vsCode, workspace folder stays vsCode, theia start using settings from vsCode.
once user adds atom back, settings.json has vsCode and atom, workspace folder stays vsCode, theia uses settings from vsCode.

what i dislike is the 2) and 3) mentioned above, where theia settings silently change after the browser refresh.

svenefftinge · 2018-07-30T19:19:36Z

I'm confused, we don't have something like folder settings, do we?
Also it sounds like you are distinguishing between the first root ("workspace folder") and the others. Why is that? I would say they are all just roots.

elaihau · 2018-07-30T19:53:49Z

I'm confused, we don't have something like folder settings, do we?

at the moment we don't have folder settings. it is something that I am going to add.

Also it sounds like you are distinguishing between the first root ("workspace folder") and the others. Why is that? I would say they are all just roots.

because not all theia extensions are updated (as of today) to embrace the multi-root change. preferences is just one of them.

And this is why i think "go with the first root, what ever that is" wouldn't keep this PR small:

we would have to refresh all the settings when atom gets removed from the workspace, as all the atom settings become obsolete. Also, do we want to implement something in preferences package in this PR, which would get changed in the next PR in a month?
using the first root of the roots array breaks the rule of workspace folder never changes (until the next workspace gets opened), and makes the logic behind multi-root workspace conceptually difficult to understand:
If atom is removed from the workspace, is the workspace folder still atom ?
If the workspace folder becomes vsCode after atom is gone, should we move the roots.json from atom to vsCode?
And what do we do to the roots.json that already exists under vsCode ? .../vsCode/.theia/roots.json might have been used for another workspace where vsCode is the workspace folder.

free feel to disagree: i think keeping the workspaceFolder consistent is the easiest to achieve the backward compatibility, keep the PR small, and make concept of workspace easy to understand.

we could discuss this in tomorrow's meeting, if you would be there :)

svenefftinge · 2018-08-07T08:50:56Z

packages/git/src/browser/git-repository-provider.ts

@@ -74,14 +77,21 @@ export class GitRepositoryProvider {
    }

    async refresh(options?: GitRefreshOptions): Promise<void> {
-        const root = await this.workspaceService.root;
+        const roots = await this.workspaceService.roots;
+        const root = roots[0];


The test for the first root should be removed. We are dealing with an array, which might be empty, but that's fine.

you are right. changes done.

svenefftinge · 2018-08-07T09:04:18Z

packages/preferences/src/browser/workspace-preference-provider.ts

-        const root = await this.workspaceService.root;
-        if (root) {
-            const rootUri = new URI(root.uri);
+        const workspaceFolder = (await this.workspaceService.roots)[0];


Should we put a TODO comment to all the locations where we just fetch the first root, so we don't miss them?

no worries :) I will review all references of WorkspaceService before creating my next PR

svenefftinge · 2018-08-07T09:17:33Z

The tree navigation doesn't behave nicely, when I use the cursor keys going up:

elaihau · 2018-08-07T13:02:48Z

@svenefftinge

I tested my change on top of the latest master, and looks like the tree navigation works. Please see the attached GIF. Could you please tell me how to reproduce the problem you observed? Maybe i have missed something in my test. Thanks !

What is a workspace / workspace folder? It is a folder on the file system that serves as a container for: - workspace configuration: in sub-folder ".theia" - preferences, in ".theia/settings.json" - root folders list, in ".theia/roots.json" - optionally content in the form of other files/folders For backward compatibility and simplicity, a workspace folder that contains no explicit root folder list configuration will use its own folder as the single root. This permits using any folder as a workspace. If subsequently an additional root folder is added to such a workspace, then both the newly added folder and the original implicit root are added to the root folders list configuration. Either can later be removed. If all roots are removed form the configuration, the workspace falls-back to using its own folder as implicit root. This PR is the minimal patch for eclipse-theia#1660 What's included in this PR: - a workspace's root folders list could be modified using the navigator context menu "add/remove folder" items to add or remove a folder - preference 'workspace.supportMultiRootWorkspace', with default value false, can be used to control if the context menu entries, used to modify the list of root folders, are shown or not. Since this is a new feature that is not supported by most Theia extensions yet, it probably makes sense to hide the UI by default for now - previously the "current root" was a static value, obtained by each extension that needs it, at the startup of a Theia client. To change it (e.g. to switch workspace), a restart of the client was required. For this feature we did not want to have to restart the client each time the list of root folders is updated, so we introduced a new "onWorkspaceRootChanged" event. Extensions that need to know when the list of root folders is modified can react to this event, e.g. to update their widgets. It's still necessary to reload the Theia frontend when switching workspace in a given tab or when closing a workspace. - the root folder list comprised of 1 to many folders can be displayed from the file navigator widget. - the root folders will be scanned for git repositories that the user can select, stage and commit files into. What's not included: - users should have the choice of naming the workspace and choosing where to store the config file - preference extension should be able to handle the settings from multiple projects - search-in-workspace and file-search extension should be able to perform the search across all folders in the workspace, and / or provide filtering mechanism to reduce the amount of information being displayed - output and problems widgets should display data from all root folders, and / or provide filtering mechanism to reduce the amount of information being displayed - task extension should be able to run tasks defined under all root folders - users should be able to open the terminal from any root folder in the workspace Signed-off-by: elaihau <liang.huang@ericsson.com>

elaihau force-pushed the T1660-1st branch 7 times, most recently from 37f2c75 to a98a351 Compare June 22, 2018 20:10

elaihau changed the title ~~[wip] multi-root workspace support, minimal patch~~ multi-root workspace support, minimal patch Jun 22, 2018

elaihau requested review from kittaakos and akosyakov June 22, 2018 20:10

tsmaeder mentioned this pull request Jun 25, 2018

Multi-root workspaces #1660

Closed

paul-marechal reviewed Jun 26, 2018

View reviewed changes

marcdumais-work reviewed Jun 26, 2018

View reviewed changes

akosyakov reviewed Jun 27, 2018

View reviewed changes

elaihau commented Jun 27, 2018

View reviewed changes

elaihau force-pushed the T1660-1st branch 4 times, most recently from b8869ba to a0248a5 Compare June 29, 2018 14:52

marcdumais-work reviewed Jun 29, 2018

View reviewed changes

elaihau force-pushed the T1660-1st branch from a0248a5 to b892749 Compare June 29, 2018 16:15

marcdumais-work reviewed Jun 29, 2018

View reviewed changes

elaihau force-pushed the T1660-1st branch 2 times, most recently from 7027290 to d153165 Compare July 26, 2018 18:45

akosyakov reviewed Jul 27, 2018

View reviewed changes

elaihau force-pushed the T1660-1st branch from d153165 to 3fe79d8 Compare July 27, 2018 13:29

elaihau force-pushed the T1660-1st branch 5 times, most recently from 71a12e5 to a0fe5c5 Compare August 3, 2018 12:24

svenefftinge approved these changes Aug 7, 2018

View reviewed changes

elaihau force-pushed the T1660-1st branch 2 times, most recently from 3ea11aa to d4896cf Compare August 7, 2018 14:31

elaihau force-pushed the T1660-1st branch from d4896cf to 1299094 Compare August 7, 2018 15:20

elaihau merged commit 80f402c into eclipse-theia:master Aug 7, 2018

elaihau deleted the T1660-1st branch August 7, 2018 17:08

multi-root workspace support, minimal patch #2117

multi-root workspace support, minimal patch #2117

Conversation

elaihau commented Jun 15, 2018 • edited Loading

paul-marechal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elaihau Jun 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-marechal commented Jun 26, 2018

Choose a reason for hiding this comment

marcdumais-work commented Jun 26, 2018

marcdumais-work commented Jun 27, 2018 • edited Loading

marcdumais-work commented Jun 27, 2018 • edited Loading

lmcbout commented Jun 27, 2018 via email

marcdumais-work commented Jun 27, 2018

Choose a reason for hiding this comment

akosyakov Jun 27, 2018 • edited Loading

Choose a reason for hiding this comment

paul-marechal Jun 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akosyakov Jun 27, 2018 • edited Loading

Choose a reason for hiding this comment

elaihau Jun 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elaihau Jun 27, 2018 • edited Loading

Choose a reason for hiding this comment

marcdumais-work commented Jun 27, 2018 • edited Loading

marcdumais-work Jun 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elaihau commented Jul 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akosyakov commented Jul 27, 2018 • edited Loading

elaihau commented Jul 27, 2018

svenefftinge commented Jul 30, 2018

elaihau commented Jul 30, 2018

svenefftinge commented Jul 30, 2018 • edited Loading

elaihau commented Jul 30, 2018 • edited Loading

svenefftinge commented Jul 30, 2018

elaihau commented Jul 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svenefftinge commented Aug 7, 2018 • edited Loading

elaihau commented Aug 7, 2018 • edited Loading

elaihau commented Jun 15, 2018 •

edited

Loading

elaihau Jun 27, 2018 •

edited

Loading

marcdumais-work commented Jun 27, 2018 •

edited

Loading

marcdumais-work commented Jun 27, 2018 •

edited

Loading

akosyakov Jun 27, 2018 •

edited

Loading

paul-marechal Jun 27, 2018 •

edited

Loading

akosyakov Jun 27, 2018 •

edited

Loading

elaihau Jun 27, 2018 •

edited

Loading

elaihau Jun 27, 2018 •

edited

Loading

marcdumais-work commented Jun 27, 2018 •

edited

Loading

marcdumais-work Jun 29, 2018 •

edited

Loading

akosyakov commented Jul 27, 2018 •

edited

Loading

svenefftinge commented Jul 30, 2018 •

edited

Loading

elaihau commented Jul 30, 2018 •

edited

Loading

elaihau commented Jul 30, 2018 •

edited

Loading

svenefftinge commented Aug 7, 2018 •

edited

Loading

elaihau commented Aug 7, 2018 •

edited

Loading