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

Declarative contribution of custom Tree Explorer #14048

Merged
merged 126 commits into from
Nov 10, 2016
Merged

Declarative contribution of custom Tree Explorer #14048

merged 126 commits into from
Nov 10, 2016

Conversation

octref
Copy link
Contributor

@octref octref commented Oct 20, 2016

PR for #12163.

Main changes:

  • contributes.explorer section for letting extension add custom tree explorers
  • TreeExplorerNodeProvider<T> in vscode namespace
  • registerTreeExplorerNodeProvider(providerId: string, provider: TreeExplorerNodeProvider<any>): Disposable in vscode namespace
  • vs/workbench/parts/explorers/*
    • The viewlet for tree explorers
    • Command to toggle each tree explorer

Not yet implemented:

  • Handle long running TreeExplorerNodeProvider.provideRootNode and TreeExplorerNodeProvider.resolveChildren by adding a progress bar.
  • Handle restoring external viewlet if last active viewlet is external.
    • Construct blank sidebar and restore external viewlet after it gets registered
    • Show progress bar at top of the blank sidebar while waiting for external viewlet getting registered
  • Refresh button at top of external viewlet
  • Show error popup for handling errors in TreeExplorerNodeProvider's methods.
  • Show error popup for no matching TreeExplorerNodeProvider found
  • Use -webkit-filter and opacity to style user defined activity bar icons to indicate activation.
  • Add extension that test against the API

High level flow

  • Workbench get constructed
  • Stock Viewlets get registered
  • Extensions' package.json get read.
  • ViewletDescriptor for external viewlets get registered at ViewletRegistry
  • ActivitybarPart adds icons for external viewlets when external viewlets finish registration
  • When icon for external viewlet get clicked, construct the viewlet and hook it to the registered provider

Examples

  • Deps: easiest to get running.
  • Avatar: need to get a GH token. See src/token.ts.
  • Redis: need to install redis.

@mention-bot
Copy link

@octref, thanks for your PR! By analyzing the history of the files in this pull request, we identified @isidorn, @egamma and @joaomoreno to be potential reviewers.

@joaomoreno
Copy link
Member

joaomoreno commented Oct 20, 2016

Forwarding to @bpasero since he owns the Explorer and to @Tyriar since he is your buddy.

@jrieken jrieken self-assigned this Oct 20, 2016
@bpasero
Copy link
Member

bpasero commented Oct 20, 2016

@octref can you please resolve the merge conflicts?

@octref
Copy link
Contributor Author

octref commented Oct 20, 2016

@bpasero Rebased on master.

@octref
Copy link
Contributor Author

octref commented Nov 9, 2016

@bpasero

Addressed all comments, and some of your additional feedbacks. Will leave progress bar and badge for post merge.

Let me know what else you want to me to change.

@bpasero
Copy link
Member

bpasero commented Nov 10, 2016

@octref looks a lot better, there is only one thing I am not sure about in workbench.ts you seem to have changed the logic about how to restore a viewlet.

Previously: the check for shouldRestoreSidebar is used to find out if the default viewlet (explorer) should be opened or actually the last viewlet that was open. shouldRestoreSidebar will be true when running out of sources (to speed up development) or when a refresh of the window was initiated (e.g. after installing an extension).

Now: you seem to never open a viewlet if shouldRestoreSidebar is false. That would mean anytime I open Code I would see the sidebar collapsed. That would be pretty bad and a bug. Can we get back to the previous behaviour?

Also I wonder if we can move the logic about extension viewlets into the viewlet service. IViewletService.openViewlet() should be the one that checks if a viewlet is external and defer its opening until it is loaded. Otherwise everyone that wants to open a viewlet has to be aware of the fact that the viewlet can be external and not yet loaded.

@octref
Copy link
Contributor Author

octref commented Nov 10, 2016

If I understand correctly:

  • if shouldRestoreSidebar is true
    • restore last opened viewlet if possible
    • restore file explorer in cases like last opened viewlet is disabled/uninstalled
  • if shouldRestoreSidebar is false
    • restore file explorer

is this what you meant?

move the logic about extension viewlets into the viewlet service

Sure, will do.

@bpasero
Copy link
Member

bpasero commented Nov 10, 2016

@octref I realize that the name shouldRestoreSidebar is confusing so feel free to change it. Yes, the only time we do NOT open a viewlet is when the sidebar is hidden. Otherwise we restore the explorer if shouldRestoreSidebar is false and the last used viewlet otherwise.

@octref
Copy link
Contributor Author

octref commented Nov 10, 2016

@bpasero Made changes as you requested. Decided to put the logic in a dedicated restoreViewlet, as the logic is heavy & need to call registry multiple times. openViewlet should be used after editor startup when all extensions are loaded.

@octref octref merged commit 2d75d95 into microsoft:master Nov 10, 2016
@LiPengfei19820619
Copy link

Good News. Thank you.

@egamma egamma mentioned this pull request Dec 20, 2016
56 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants