Make sortText on completions be enum-typed

[Ben Lichtman (uniqueiniquity)]

We've had a couple issues in the past where the value of a member of the SortText enum has changed when adding a new member. This has caused bugs in editors that use TSServer.

Here, we ensure that the sortText property of completion entries is specifically typed with the SortText enum, so that it's properly represented in the protocol.

[TypeScript Bot (typescript-bot)]

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 Sheetal Nandi (@sheetalkamat), Andrew Casey (@amcasey), Matt Bierner (@mjbvz), Mine Starks (@minestarks) for you. Feel free to loop in other consumers/maintainers if necessary

[Andrew Casey (amcasey)]

Why are some "member" and others "members"?

[Wesley Wigham (weswigham)]

So if VS relies on specific string-numbers for the sort text, does that mean that VS/vscode couldn't handle LS plugins that provide arbitrary alphanumeric content for the sortText? I always assumed the content was an arbitrary relative "string to sort by".

[Wesley Wigham (weswigham)]

For reference: The angular language service plugin definitely passes full on arbitrary strings. The sort they map into sortText is the full alphanumeric name of a thing. I imagine that's currently fine, since they don't mix their template completions in with normal completion results?

[Matt Bierner (mjbvz)]

Wesley Wigham (@weswigham) VS code uses string sorting logic: https://github.com/microsoft/vscode/blob/9f474febfee5330f7a9a7456862ef73d0b6f0bf5/src/vs/editor/contrib/suggest/suggest.ts#L229

So you could use 'a', 'b', 'c', ... instead of '0', '1', ...

[Wesley Wigham (weswigham)]

Right - all that matters is that whoever is sending the sortText is internally consistent in that case. Ben Lichtman (@uniqueiniquity) how is it used elsewhere? And why is it not used as an opaque sortable string in those cases? As I've pointed out - the angular LS plugin does make use of it as not-internal-numbering-scheme, so this protocol change would definitely break it.

[Sheetal Nandi (sheetalkamat)]

agree with Wesley Wigham (@weswigham) here.. this needs to be string so that any string value can be passed in.. Its enum in services layer internally so that it can easily be labeled instead of making them "1" or "2"

[Ben Lichtman (uniqueiniquity)]

Wesley Wigham (@weswigham)Sheetal Nandi (@sheetalkamat) we don't generally care about the text - we specifically care about the JavaScriptIdentifiers entries, as since they're basically guesses, we want to handle those a little differently than the others. In all other cases, interpreting it as "a string to sort by" is fine.

My goal here is to either prevent these values from being changed or, if that's not feasible, to at least have a way to get notified when they get changed. I'd also be open to an additional property saying "this entry is a js-only guess" if that's preferred.

[Sheetal Nandi (sheetalkamat)]

I think making a property for js-only guess is better than having enum for the sorting text

[Ben Lichtman (uniqueiniquity)]

Superseded by #36063.

[TypeScript Bot (typescript-bot)]

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.