Repository navigation
Allows plugin to register commands - #44291
Nathan Shively-Sanders (sandersn) merged 8 commits into
Conversation
TypeScript Bot (typescript-bot)
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
Sheetal Nandi (@sheetalkamat) this seems about right to me. Thoughts? |
There was a problem hiding this comment.
I would rather prefer if enablePlugins passes session (along with project which is already passed in) to the plugin rather than having these extra methods and we having to deal with the additions etc
b411c08 to
fad1c05
Compare
|
I'm not pretty sure what's different about these kind of projects. |
Sheetal Nandi (sheetalkamat)
left a comment
Sheetal Nandi (sheetalkamat)
left a comment
There was a problem hiding this comment.
Also add test for plugin where session is passed through and you call addProtocolHandler and test that functionality
Tests are in src/testRunner/unittests/tsserver/plugins.ts
| private packageJsonFilesMap: ESMap<Path, FileWatcher> | undefined; | ||
|
|
||
| /*@internal*/ | ||
| readonly session: Session<unknown> | undefined; |
There was a problem hiding this comment.
I think you should wire TMessage thing here and PluginCreateInfo too,
There was a problem hiding this comment.
I thought the unknown is better:
If we add type parameter TMessage into editorService, We should also add the type parameter into Project and etc. Too many changes.
And we are not really care about the type of message in editorService. We just forward session into pluin.
And the handlers or addProtocolHandler does not have TMessage too. Thay are raw string.
There was a problem hiding this comment.
If that’s the case we shouldn’t add it to project service options either
Sheetal Nandi (sheetalkamat)
left a comment
Sheetal Nandi (sheetalkamat)
left a comment
There was a problem hiding this comment.
need to update tsserverlibrary.d.ts baseline but otherwise change looks good. Thank you for your patience and sorry for going back and forth on this one as it was little unclear if TypeParameter was needed or not until you made those changes.
|
Never mind. |
|
Wenlu Wang (@Kingwl) are you OK if I un-draft and merge this? |
Sure. Please feel free to merge. |
|
Just noticed this in the PR backlog. I'm going to merge it since it looks ready to go. |
#43449