-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
Declarative contribution of custom Tree Explorer #14048
Changes from 75 commits
2657a9d
aa1ffcd
c2bdd6a
d03ea81
d8c0af7
5276b19
8081e2e
10af4d7
6f4de8b
61639c0
c8bbaaa
f364538
154cdf9
ec1cc0d
e88e3f0
373829a
7f5e03f
230ac46
eced815
8e9982c
fb032eb
29c03ea
f366857
09703d2
c7f2d3d
30e3be2
b8ce9ad
668aa65
f660ca6
f659fdd
6bb8278
b474369
0cc3fec
c24338d
af5af72
6300fbf
13d5690
4eab0df
944101c
1c28702
940a7fa
2e0f8d0
1364159
88e869a
0031078
0471662
5b1ba3c
31afc03
09eebc5
857411d
0c18979
045aedd
f19446e
ec7f077
7750596
c49e7bc
7642976
b654b53
4cc76f5
16dd5f9
313df77
b141507
6fd1c54
267aab6
6e868d6
a5adf50
d579cb8
0b84a56
37398ea
bc573a7
0e9aecd
60574ba
a21ac50
39ecfd3
e2f4a6b
794ad90
8f777ab
4ed359a
a3d577c
a196d7f
70be684
f5d4ede
09c11fe
f18fa6e
c476937
31960a7
84dd43b
a52cfbb
d153d96
894c3cf
355f115
1ed2912
68284bb
a04f317
4660455
50eb471
3870173
a6f8b94
3b5e3ea
9cf01e8
b205a4c
1cdbe70
13f2dad
5342705
65f43cc
347c2e9
ff63a4d
11486b8
29346ad
307037c
15bf2ec
e2b7b52
6793e08
c57973f
85f4204
0b15cbb
b9cf15d
902137c
06b82f5
4d25517
dd19d59
d111426
25885c6
80a0cbf
ae720e9
a585a33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/*--------------------------------------------------------------------------------------------- | ||
* Copyright (c) Microsoft Corporation. All rights reserved. | ||
* Licensed under the MIT License. See License.txt in the project root for license information. | ||
*--------------------------------------------------------------------------------------------*/ | ||
'use strict'; | ||
|
||
import { localize } from 'vs/nls'; | ||
import { join } from 'vs/base/common/paths'; | ||
import { createCSSRule } from 'vs/base/browser/dom'; | ||
import { IJSONSchema } from 'vs/base/common/jsonSchema'; | ||
import { ExtensionsRegistry } from 'vs/platform/extensions/common/extensionsRegistry'; | ||
import { Registry } from 'vs/platform/platform'; | ||
import { ViewletRegistry, Extensions as ViewletExtensions, ViewletDescriptor } from 'vs/workbench/browser/viewlet'; | ||
import { VIEWLET_ID_ROOT } from 'vs/workbench/parts/explorers/common/treeExplorer'; | ||
|
||
namespace schema { | ||
|
||
export interface IExplorer { | ||
treeExplorerNodeProviderId: string; | ||
treeLabel: string; | ||
icon: string; | ||
} | ||
|
||
export const explorerContribtion: IJSONSchema = { | ||
description: localize('vscode.extension.contributes.explorer', "Contributes custom tree explorer viewlet to the sidebar"), | ||
type: 'object', | ||
properties: { | ||
treeExplorerNodeProviderId: { | ||
description: localize('vscode.extension.contributes.explorer.treeExplorerNodeProviderId', 'Unique id used to identify provider registered through vscode.workspace.registerTreeExplorerNodeProvider'), | ||
type: 'string' | ||
}, | ||
treeLabel: { | ||
description: localize('vscode.extension.contributes.explorer.treeLabel', 'Human readable string used to render the custom tree viewlet'), | ||
type: 'string' | ||
}, | ||
icon: { | ||
description: localize('vscode.extension.contributes.explorer.icon', 'Path to the viewlet icon on the activity bar'), | ||
type: 'string' | ||
} | ||
} | ||
}; | ||
} | ||
|
||
ExtensionsRegistry.registerExtensionPoint<schema.IExplorer>('explorer', schema.explorerContribtion).setHandler(extensions => { | ||
let baseOrder = 200; // Stock viewlet order goes up to 100 | ||
let descriptors = []; | ||
|
||
for (let extension of extensions) { | ||
const { treeExplorerNodeProviderId, treeLabel, icon } = extension.value; | ||
|
||
const getIconRule = (iconPath) => { return `background-image: url('${iconPath}')`; }; | ||
if (icon) { | ||
const iconClass = `.monaco-workbench > .activitybar .monaco-action-bar .action-label.${treeExplorerNodeProviderId}`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need styles for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, activity bar doesn't change with themes for now (I'd like it to though) |
||
const iconPath = join(extension.description.extensionFolderPath, icon); | ||
createCSSRule(iconClass, getIconRule(iconPath)); | ||
// Coerce the icon into a style similar to stock icons | ||
createCSSRule(iconClass, '-webkit-filter: grayscale(1) invert(1)'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one could be pulled into CSS as a general rule There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean pull it into the CSS file, not done in typescript since it's static. |
||
} | ||
|
||
descriptors.push(new ViewletDescriptor( | ||
'vs/workbench/parts/explorers/browser/treeExplorerViewlet', | ||
'TreeExplorerViewlet', | ||
VIEWLET_ID_ROOT + treeExplorerNodeProviderId, | ||
treeLabel, | ||
treeExplorerNodeProviderId, | ||
baseOrder++, | ||
true | ||
)); | ||
} | ||
Registry.as<ViewletRegistry>(ViewletExtensions.Viewlets).registerExternalViewlets(descriptors); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,6 +74,12 @@ export interface ITheme { | |
label: string; | ||
} | ||
|
||
export interface ITreeExplorer { | ||
treeExplorerNodeProviderId: string; | ||
treeLabel: string; | ||
icon: string; | ||
} | ||
|
||
export interface IExtensionContributions { | ||
commands?: ICommand[]; | ||
configuration?: IConfiguration; | ||
|
@@ -85,6 +91,7 @@ export interface IExtensionContributions { | |
menus?: { [context: string]: IMenu[] }; | ||
snippets?: ISnippet[]; | ||
themes?: ITheme[]; | ||
explorer?: ITreeExplorer; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only one tree per extension? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [needs thinking] The problem is that this means no extension can ever add more than one tree-viewlet There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even if we change this part later, it would be an easy update from extension side. I found use cases for multiple viewlets less unlikely than a single multi-section viewlet. |
||
} | ||
|
||
export interface IExtensionManifest { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1348,6 +1348,72 @@ declare module 'vscode' { | |
provideTextDocumentContent(uri: Uri, token: CancellationToken): string | Thenable<string>; | ||
} | ||
|
||
/** | ||
* A node provider for the tree explorer contributed by extension. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Drop "by extension" as it's implied being in this file. How about this:
|
||
* | ||
* Providers are registered through (#workspace.registerTreeExplorerNodeProvider) with a | ||
* `providerId` that corresponds to the `treeExplorerNodeProviderId` in the extension's | ||
* `contributes.explorer` section. | ||
* | ||
* The contributed tree explorer will ask the corresponding provider to provide the root | ||
* node and resolve children for each node. In addition, the provider could **optionally** | ||
* provide the following information for each node: | ||
* - label: A human-readable label used for rendering the node. | ||
* - hasChildren: Whether the node has children and is expandable. | ||
* - clickCommand: A command to execute when the node is clicked. | ||
*/ | ||
export interface TreeExplorerNodeProvider<T> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some high level feedback on the API
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. General comment on these; keep in mind that one of the goals was to make the API as slim and solid as possible, opting to iterate on feedback/user need on extras. Icons, updates, reveal, select, focus, etc. were all considered but put on hold initially as we would prefer @octref to ship something solid and useful that can be iterated on over not shipping at all. This strategy worked great imo for the terminal API which was slim almost to the point that it was not usable for a month, however the following month filled all the major gaps. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the feedback!
Agreed. I also wanted this. It's not there as Kai was initially averse to it, but he now seems to support it. We have a refresh button on top of the viewlet.
Decided to leave that for next iteration. Wanted to get some feedback from users before deciding how to do it.
We are showing the progress bar if the Better to wait for some feedback before adding it, I think.
Yep. But while I was writing my 3 examples I never found the need for it. And I can't think of a use case for DB Explorer, which we are designing this for. So better wait for next iteration when we understand the use cases more. Will sync with Kai/Daniel this afternoon and talk through your concerns. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need to up-cast anything when your define it as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We actually had it earlier as you're requesting and @kieferrm convinced us to go in this direction.
There are a few benefits to doing it this way:
There's some more context on this in https://github.com/octref/vscode/issues/4 and https://github.com/octref/vscode/issues/5. I was also originally opposed to this way but it's definitely grown on me and I think it's quite a nice way to work on a tree structure. I agree consistency very important - just some background/insights into the current situation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of It all boils down to where we want to put the responsibility of providing Imagine a more complex In the current approach, all logic can be in
Whereas if we put logic in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having worked on Eclipse before I am very well aware of how the LabelProvider and ContentProvider patterns work. The question I am asking and not getting feedback on is how does it align with QuickPick, how would a tree'ish quick pick data structure look like, and how does it generally fit into the API? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I admit it is very different from QuickPick API. I think it's more aligned to other Providers like
That's what I like about the current design. It doesn't care what your TreeItem data-structure look like. As long as your provider can
It doesn't resemble any existing API. Previously we did have a version like this: export interface TreeExplorerNodeProvider {
provideRootNode(): Thenable<TreeExplorerNode>;
resolveChildren(node: TreeExplorerNode): Thenable<TreeExplorerNode[]>;
}
export interface TreeExplorerNode {
label: string;
shouldInitiallyExpand: boolean;
onClickCommand: string;
} but we switched to the |
||
|
||
/** | ||
* Provide the root node. This function will be called when the tree explorer is activated | ||
* for the first time. | ||
* The root node is hidden and its direct children will be displayed on the first level of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Attach this to the previous line. |
||
* the tree explorer. | ||
* | ||
* @return The root node. | ||
*/ | ||
provideRootNode(): T | Thenable<T>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jrieken currently this returns the invisible root node, what do you think about that versus a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
/** | ||
* Resolve the children of `node`. | ||
* | ||
* @param node The node from which the provider resolves children. | ||
* @return Children of `node`. | ||
*/ | ||
resolveChildren(node: T): T[] | Thenable<T[]>; | ||
|
||
/** | ||
* Provide a human-readable string that will be used for rendering the node. | ||
* | ||
* Default to use `node.toString()` if not provided. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd attach this sentence to the first line. |
||
* | ||
* @param node The node from which the provider computes label. | ||
* @return A human-readable label. | ||
*/ | ||
getLabel?(node: T): string; | ||
|
||
/** | ||
* Determine if `node` has children and is expandable. | ||
* | ||
* Default to return `true` if not provided. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd attach this sentence to the first line. |
||
* | ||
* @param node The node to determine if it has children and is expandable. | ||
* @return A boolean that determines if `node` has children and is expandable. | ||
*/ | ||
getHasChildren?(node: T): boolean; | ||
|
||
/** | ||
* Get the command to execute when `node` is clicked. | ||
* | ||
* Commands can be registered through (#commands.registerCommand). `node` will be provided | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
* as the first argument to the command's callback function. | ||
* | ||
* @param node The node that the command is associated with. | ||
* @return The command to execute when `node` is clicked. | ||
*/ | ||
getClickCommand?(node: T): string; | ||
} | ||
|
||
/** | ||
* Represents an item that can be selected from | ||
* a list of items. | ||
|
@@ -3803,6 +3869,15 @@ declare module 'vscode' { | |
*/ | ||
export function registerTextDocumentContentProvider(scheme: string, provider: TextDocumentContentProvider): Disposable; | ||
|
||
/** | ||
* Register a [tree explorer node provider](#TreeExplorerNodeProvider). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tree explorer node provider -> TreeExplorerNodeProvider |
||
* | ||
* @param providerId A unique id that identifies the provider. | ||
* @param provider A [TreeExplorerNodeProvider](#TreeExplorerNodeProvider). | ||
* @return A [disposable](#Disposable) that unregisters this provider when being disposed. | ||
*/ | ||
export function registerTreeExplorerNodeProvider(providerId: string, provider: TreeExplorerNodeProvider<any>): Disposable; | ||
|
||
/** | ||
* An event that is emitted when a [text document](#TextDocument) is opened. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import { ExtHostDocuments } from 'vs/workbench/api/node/extHostDocuments'; | |
import { ExtHostDocumentSaveParticipant } from 'vs/workbench/api/node/extHostDocumentSaveParticipant'; | ||
import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; | ||
import { ExtHostDiagnostics } from 'vs/workbench/api/node/extHostDiagnostics'; | ||
import { ExtHostTreeExplorers } from 'vs/workbench/api/node/extHostTreeExplorers'; | ||
import { ExtHostWorkspace } from 'vs/workbench/api/node/extHostWorkspace'; | ||
import { ExtHostQuickOpen } from 'vs/workbench/api/node/extHostQuickOpen'; | ||
import { ExtHostHeapService } from 'vs/workbench/api/node/extHostHeapService'; | ||
|
@@ -65,6 +66,7 @@ export function createApiFactory(threadService: IThreadService, extensionService | |
const extHostDocumentSaveParticipant = col.define(ExtHostContext.ExtHostDocumentSaveParticipant).set<ExtHostDocumentSaveParticipant>(new ExtHostDocumentSaveParticipant(extHostDocuments, threadService.get(MainContext.MainThreadWorkspace))); | ||
const extHostEditors = col.define(ExtHostContext.ExtHostEditors).set<ExtHostEditors>(new ExtHostEditors(threadService, extHostDocuments)); | ||
const extHostCommands = col.define(ExtHostContext.ExtHostCommands).set<ExtHostCommands>(new ExtHostCommands(threadService, extHostEditors, extHostHeapService)); | ||
const extHostExplorers = col.define(ExtHostContext.ExtHostExplorers).set<ExtHostTreeExplorers>(new ExtHostTreeExplorers(threadService, extHostCommands)); | ||
const extHostConfiguration = col.define(ExtHostContext.ExtHostConfiguration).set<ExtHostConfiguration>(new ExtHostConfiguration(threadService.get(MainContext.MainThreadConfiguration))); | ||
const extHostDiagnostics = col.define(ExtHostContext.ExtHostDiagnostics).set<ExtHostDiagnostics>(new ExtHostDiagnostics(threadService)); | ||
const languageFeatures = col.define(ExtHostContext.ExtHostLanguageFeatures).set<ExtHostLanguageFeatures>(new ExtHostLanguageFeatures(threadService, extHostDocuments, extHostCommands, extHostHeapService, extHostDiagnostics)); | ||
|
@@ -332,6 +334,9 @@ export function createApiFactory(threadService: IThreadService, extensionService | |
onWillSaveTextDocument: (listener, thisArgs?, disposables?) => { | ||
return extHostDocumentSaveParticipant.onWillSaveTextDocumentEvent(listener, thisArgs, disposables); | ||
}, | ||
registerTreeExplorerNodeProvider(providerId: string, provider: vscode.TreeExplorerNodeProvider<any>) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [needs change] This function must be marked as proposed - look at the |
||
return extHostExplorers.registerTreeExplorerNodeProvider(providerId, provider); | ||
}, | ||
onDidChangeConfiguration: (listener: () => any, thisArgs?: any, disposables?: extHostTypes.Disposable[]) => { | ||
return extHostConfiguration.onDidChangeConfiguration(listener, thisArgs, disposables); | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,8 @@ import { SaveReason } from 'vs/workbench/services/textfile/common/textfiles'; | |
import { IWorkspaceSymbol } from 'vs/workbench/parts/search/common/search'; | ||
import { IApplyEditsOptions, TextEditorRevealType, ITextEditorConfigurationUpdate, IResolvedTextEditorConfiguration, ISelectionChangeEvent } from './mainThreadEditorsTracker'; | ||
|
||
import { InternalTreeExplorerNode } from 'vs/workbench/parts/explorers/common/treeExplorerViewModel'; | ||
|
||
export interface InstanceSetter<T> { | ||
set<R extends T>(instance: T): R; | ||
} | ||
|
@@ -115,6 +117,11 @@ export abstract class MainThreadEditorsShape { | |
$tryApplyEdits(id: string, modelVersionId: number, edits: editorCommon.ISingleEditOperation[], opts: IApplyEditsOptions): TPromise<boolean> { throw ni(); } | ||
} | ||
|
||
export abstract class MainThreadTreeExplorersShape { | ||
$registerTreeExplorerNodeProvider(providerId: string): void { throw ni(); } | ||
$showMessage(severity: Severity, message: string): void { throw ni(); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [needs change] Why is this here? It duplicates functionality we already have in other places (ExtHostMessageService). Also, failure to resolve a tree item shouldn't automatically result in an error message. I propose to let the error bubble (as rejected promise) and handle the error in the tree, like showing an error node. |
||
} | ||
|
||
export abstract class MainThreadErrorsShape { | ||
onUnexpectedExtHostError(err: any): void { throw ni(); } | ||
} | ||
|
@@ -257,6 +264,12 @@ export abstract class ExtHostEditorsShape { | |
$acceptTextEditorRemove(id: string): void { throw ni(); } | ||
} | ||
|
||
export abstract class ExtHostTreeExplorersShape { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that but the practice is to be explicit about API and implementation. The protocol is data-driven and therefore we are explicit about only using primitive types and plain (data) interfaces that map directly to JSON. This is more a convention thing than a problem There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops sorry. Actually I designed with this in mind and should've used |
||
$provideRootNode(providerId: string): TPromise<InternalTreeExplorerNode> { throw ni(); }; | ||
$resolveChildren(providerId: string, node: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode[]> { throw ni(); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned above, instead of |
||
$executeCommand(providerId: string, node: InternalTreeExplorerNode): TPromise<void> { throw ni(); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [needs change] Related to To preserve the semantics wrt to the node bing the command argument, make sure to use |
||
} | ||
|
||
export abstract class ExtHostExtensionServiceShape { | ||
$localShowMessage(severity: Severity, msg: string): void { throw ni(); } | ||
$activateExtension(extensionDescription: IExtensionDescription): TPromise<void> { throw ni(); } | ||
|
@@ -331,6 +344,7 @@ export const MainContext = { | |
MainThreadDocuments: createMainId<MainThreadDocumentsShape>('MainThreadDocuments', MainThreadDocumentsShape), | ||
MainThreadEditors: createMainId<MainThreadEditorsShape>('MainThreadEditors', MainThreadEditorsShape), | ||
MainThreadErrors: createMainId<MainThreadErrorsShape>('MainThreadErrors', MainThreadErrorsShape), | ||
MainThreadExplorers: createMainId<MainThreadTreeExplorersShape>('MainThreadExplorers', MainThreadTreeExplorersShape), | ||
MainThreadLanguageFeatures: createMainId<MainThreadLanguageFeaturesShape>('MainThreadLanguageFeatures', MainThreadLanguageFeaturesShape), | ||
MainThreadLanguages: createMainId<MainThreadLanguagesShape>('MainThreadLanguages', MainThreadLanguagesShape), | ||
MainThreadMessageService: createMainId<MainThreadMessageServiceShape>('MainThreadMessageService', MainThreadMessageServiceShape), | ||
|
@@ -351,6 +365,7 @@ export const ExtHostContext = { | |
ExtHostDocuments: createExtId<ExtHostDocumentsShape>('ExtHostDocuments', ExtHostDocumentsShape), | ||
ExtHostDocumentSaveParticipant: createExtId<ExtHostDocumentSaveParticipantShape>('ExtHostDocumentSaveParticipant', ExtHostDocumentSaveParticipantShape), | ||
ExtHostEditors: createExtId<ExtHostEditorsShape>('ExtHostEditors', ExtHostEditorsShape), | ||
ExtHostExplorers: createExtId<ExtHostTreeExplorersShape>('ExtHostExplorers', ExtHostTreeExplorersShape), | ||
ExtHostFileSystemEventService: createExtId<ExtHostFileSystemEventServiceShape>('ExtHostFileSystemEventService', ExtHostFileSystemEventServiceShape), | ||
ExtHostHeapService: createExtId<ExtHostHeapServiceShape>('ExtHostHeapMonitor', ExtHostHeapServiceShape), | ||
ExtHostLanguageFeatures: createExtId<ExtHostLanguageFeaturesShape>('ExtHostLanguageFeatures', ExtHostLanguageFeaturesShape), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/*--------------------------------------------------------------------------------------------- | ||
* Copyright (c) Microsoft Corporation. All rights reserved. | ||
* Licensed under the MIT License. See License.txt in the project root for license information. | ||
*--------------------------------------------------------------------------------------------*/ | ||
'use strict'; | ||
|
||
import { TreeExplorerNodeProvider } from 'vscode'; | ||
import { TPromise } from 'vs/base/common/winjs.base'; | ||
import { Disposable } from 'vs/workbench/api/node/extHostTypes'; | ||
import { IThreadService } from 'vs/workbench/services/thread/common/threadService'; | ||
import { MainContext, ExtHostTreeExplorersShape, MainThreadTreeExplorersShape } from './extHost.protocol'; | ||
import { InternalTreeExplorerNode } from 'vs/workbench/parts/explorers/common/treeExplorerViewModel'; | ||
import { ExtHostCommands } from 'vs/workbench/api/node/extHostCommands'; | ||
import { asWinJsPromise } from 'vs/base/common/async'; | ||
import { Severity } from 'vs/platform/message/common/message'; | ||
|
||
export class ExtHostTreeExplorers extends ExtHostTreeExplorersShape { | ||
private _proxy: MainThreadTreeExplorersShape; | ||
|
||
private _treeExplorerNodeProviders: { [providerId: string]: TreeExplorerNodeProvider<any> }; | ||
private _externalNodeMaps: { [providerId: string]: { [id: number]: any } }; | ||
|
||
constructor( | ||
threadService: IThreadService, | ||
private commands: ExtHostCommands | ||
) { | ||
super(); | ||
|
||
this._proxy = threadService.get(MainContext.MainThreadExplorers); | ||
|
||
this._treeExplorerNodeProviders = Object.create(null); | ||
this._externalNodeMaps = Object.create(null); | ||
} | ||
|
||
registerTreeExplorerNodeProvider(providerId: string, provider: TreeExplorerNodeProvider<any>): Disposable { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw
So I just followed the pattern. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both types are equal - |
||
this._proxy.$registerTreeExplorerNodeProvider(providerId); | ||
this._treeExplorerNodeProviders[providerId] = provider; | ||
|
||
return new Disposable(() => { | ||
delete this._treeExplorerNodeProviders[providerId]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you need to clean up |
||
}); | ||
} | ||
|
||
$provideRootNode(providerId: string): TPromise<InternalTreeExplorerNode> { | ||
const provider = this.getProvider(providerId); | ||
|
||
return asWinJsPromise(() => provider.provideRootNode()).then(externalRootNode => { | ||
const treeNodeMap = Object.create(null); | ||
this._externalNodeMaps[providerId] = treeNodeMap; | ||
|
||
const internalRootNode = new InternalTreeExplorerNode(externalRootNode, provider); | ||
this._externalNodeMaps[providerId][internalRootNode.id] = externalRootNode; | ||
return internalRootNode; | ||
}, err => { | ||
this.showErrorMessage(`TreeExplorerNodeProvider '${providerId}' failed to provide root node.`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [needs change] Replace with |
||
}); | ||
} | ||
|
||
$resolveChildren(providerId: string, mainThreadNode: InternalTreeExplorerNode): TPromise<InternalTreeExplorerNode[]> { | ||
const provider = this.getProvider(providerId); | ||
|
||
const externalNodeMap = this._externalNodeMaps[providerId]; | ||
const externalNode = externalNodeMap[mainThreadNode.id]; | ||
|
||
return asWinJsPromise(() => provider.resolveChildren(externalNode)).then(children => { | ||
return children.map(externalChild => { | ||
const internalChild = new InternalTreeExplorerNode(externalChild, provider); | ||
externalNodeMap[internalChild.id] = externalChild; | ||
return internalChild; | ||
}); | ||
}, err => { | ||
this.showErrorMessage(`TreeExplorerNodeProvider '${providerId}' failed to resolve children.`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [needs change] Like above |
||
}); | ||
} | ||
|
||
$executeCommand(providerId: string, mainThreadNode: InternalTreeExplorerNode): TPromise<void> { | ||
if (mainThreadNode.clickCommand) { | ||
const externalNode = this._externalNodeMaps[providerId][mainThreadNode.id]; | ||
return asWinJsPromise(() => this.commands.executeCommand(mainThreadNode.clickCommand, externalNode)).then(() => { | ||
return null; | ||
}, err => { | ||
this.showErrorMessage(`Failed to execute command '${mainThreadNode.clickCommand}' provided by TreeExplorerNodeProvider '${providerId}'.`); | ||
}); | ||
} | ||
|
||
return TPromise.as(null); | ||
} | ||
|
||
getProvider(providerId: string): TreeExplorerNodeProvider<any> { | ||
const provider = this._treeExplorerNodeProviders[providerId]; | ||
if (!provider) { | ||
this.showErrorMessage(`No TreeExplorerNodeProvider with id '${providerId}' registered.`); | ||
} | ||
|
||
return provider; | ||
} | ||
|
||
private showErrorMessage(message: string): void { | ||
this._proxy.$showMessage(Severity.Error, message); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could probably make ExtHostMessageService a dependency and call into that from here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thought about that, mainly worried about this recent change: Don't know what @jrieken meant by |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you reuse the one in extensionManagement.ts instead of duplicating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.