added the workspace symbol provider for markdown #46406#47610
Conversation
mjbvz
left a comment
There was a problem hiding this comment.
Thanks for looking into this! Just a few changes before we can merge this in. Let me know if you have any questions about my comments
|
|
||
| public constructor(symbolProvider: MDDocumentSymbolProvider) { | ||
| this.symbolProvider = symbolProvider; | ||
| this.populateSymbolCache(); |
There was a problem hiding this comment.
This should be done lazily when workspace symbols are first requested
| } | ||
|
|
||
| private registerOnSaveEvent(): void { | ||
| workspace.onDidSaveTextDocument(async document => { |
There was a problem hiding this comment.
This registration returns a registration handle that handles unregistering the event handler. Add a dispose method on MarkdownWorkspaceSymbolProvider and then call .dipose() on the the registration handle to make sure it is properly cleaned up
There was a problem hiding this comment.
@mjbvz I have one question here. I have made the changes though.. Excuse me if my question is naive..
Who calls the dispose method? I couldn't find the same in the WorkspaceSymbolProvider interface as well.. I assume there should be a common place to check if the method "dispose" exists on an object and then invoke this.
Can you kindly point me to that.
|
@mjbvz got a chance to look into the changes.. ? |
|
Thanks! Will be in the next insiders build and in VS Code 1.23 |
|
@mjbvz Thanks for merging.. I had asked a question in the review comment. It will be really helpful if you could answer the same. Who calls the dispose method? I couldn't find the same in the WorkspaceSymbolProvider interface as well.. I assume there should be a common place to check if the method "dispose" exists on an object and then invoke this. Can you kindly point me to that. |
|
In this case it will never get called. The extension would have to register the provider as one of its disposables but this is pointless because the extension itself never gets disposed. It's still good practice to clean up any resources in a |
|
@pradeepmurugesan By the way, do you have a twitter handle that you would like mentioned when we call out this feature? Tweeting can help get some additional testing on the feature and I want to make sure proper credit is given for your work |
|
@mjbvz Sure.. This is my twitter profile https://twitter.com/pradeepm_14. Handle- @pradeepm_14 |
#46406