Better contextual filtering for `typescript-basics` snippets

[andrewbranch]

VS Code ships with an extension called typescript-basics which contains static snippet definitions for things like control flow constructs and basic declarations. When TS Server serves a request for completions, it includes a boolean switch that controls whether these snippets are added to the completion items in its response, but it can’t say which ones are relevant for the requesting location—it’s all or nothing. This leads to a pretty poor UX, like offering a for-loop in the body of a class:

But we can’t disable that without disabling the useful method declaration snippet:

But equally embarrassingly, the method declaration snippets are offered outside of classes as well:

Now that TS Server has native support for generating snippets, we can easily do better. The simplest change would be to copy these snippets from typescript-basics into TS Server and let TS Server include only the relevant ones in a given response. (I say “copy” and not “move” so as not to change the behavior for JS/TS users who have completely disabled TS Server, or during times when TS Server is not available.) Another approach could be to let TS Server’s completions response include some sort of filtering information instead of an all-or-nothing switch to mix the snippets in from the typescript-basics source, but this sounds more complicated, more error-prone, and less backward compatible to me—but in general I’m open to other ways of solving the same problem.

[amcasey]

Does the protocol currently have a way to express "this completion item is a snippet" so that the icon can be changed? Maybe it's implied by the presence of a cursor marker?

[andrewbranch]

Yes: https://github.com/microsoft/TypeScript/blob/449aaa118fbbed36bfd886cc05957a49682111e6/src/server/protocol.ts#L2273

[amcasey]

Sounds like the right approach to me then.

[andrewbranch]

Notes from editor meeting:

• Static snippets from typescript-basics can be individually disabled and bound to custom keyboard shortcuts. We don't want to lose this functionality.
  • This could be an argument for the “TS Server filters typescript-basics” model
  • VS Code counterpart issue: https://github.com/microsoft/vscode/issues/58112
  • Trying to serialize into a context like this feels fragile, like we would have to update this DSL all the time. It might be just as good to identify each snippet individually.
  • This model is VS Code specific—tighter coupling of editor / TS Server than we prefer.
  • Other option would be to ensure snippets coming from TS Server have human readable stable identifiers so the disabling/keybinding can still happen.
  • This seems like the better way forward.
  • Next steps would be to figure out what protocol changes are necessary to support this. We would at least need to attach some extra data to the snippet CompletionEntry.