Remove long deprecated completions protocol commands#52346
Conversation
|
Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, and @minestarks for you. Feel free to loop in other consumers/maintainers if necessary |
4bebbfe to
377a819
Compare
| Change = "change", | ||
| Close = "close", | ||
| /** @deprecated Prefer CompletionInfo -- see comment on CompletionsResponse */ | ||
| Completions = "completions", |
There was a problem hiding this comment.
This scared me but, doesn't seem like anyone's actually using it.
There was a problem hiding this comment.
I wonder if there are any clients sending the string "completions" to TSServer.
There was a problem hiding this comment.
I thought of that immediately after closing the tab, I have no idea how to check for that one...
Searching for executeCommand("completions", it seems not.
There was a problem hiding this comment.
I think this change is going to break too many editors.
There was a problem hiding this comment.
Actually, looking again:
- Atom-TypeScript seems updatedish: https://github.com/TypeStrong/atom-typescript/blob/master/lib/client/commandArgsResponseMap.d.ts#L8-L9
- nvim-typescript is deprecated entirely, but latest version is fixed: https://github.com/mhartington/nvim-typescript/blob/44f3db3331632003ed95d963b0753ef94abc8b90/rplugin/node/nvim_typescript/src/client.ts#L20
- oni is deprecated entirely, and oni2 seems to just reuse some VS Code integration? https://github.com/onivim/oni2
But
- tide actually does need to be updated: https://github.com/ananthakumaran/tide/blob/4cf6a0d89da7f946565a425a632ee2410a40c7da/tide.el#L1702-L1717
- typescript.java actually does need to be updated https://github.com/angelozerr/typescript.java/blob/8a6759ca825c2b0ae14cb8e09e5ba1778aae24d3/core/ts.core/src/ts/client/CommandNames.java#L25
- editor-cells actually does need to be updated: https://github.com/MarcWeber/editor-cells/blob/3cb70ffd5e09681b9159bc7527110281ff8660e4/py3/site-packages/cells/asyncio/tsserver.py#L168
- The Sublime Plugin needs to be updated: https://github.com/microsoft/TypeScript-Sublime-Plugin/blob/fb61dcd11cd3f1807a4bc454148cb59b199e8f3b/typescript/libs/service_proxy.py#L73-L80
There was a problem hiding this comment.
But does this mean we will never remove the long deprecated server/services API/commands ?
There was a problem hiding this comment.
Eg Occurrences is another such command
There was a problem hiding this comment.
I just think that if we want to remove these, we should be willing to send PRs to fix these plugins up.
There was a problem hiding this comment.
I'd say we can merge this if we send PRs by Monday to the above packages.
|
Adding @zkat to confirm the protocol changes are OK for VS |
|
Reading the comments, it looks like this PR is blocked on sending PRs to update a number of projects that still use the old commands. |
|
This has been sitting for 2.5 years. Is it still worth merging? Otherwise I'd like to close it. |
|
Given the new codebase will be using LSP I would think we could skip this |
No description provided.