Use core LSP TokenTypes where possible and validate token names #2548
Conversation
| // Use Core LSP token types where possible | ||
| value => _coreTokenMap.ContainsKey(value) | ||
| ? _coreTokenMap[value] | ||
| : new SemanticTokenType(MakeLSPCompatibleString(value.ToString()))); |
There was a problem hiding this comment.
Previously we had been using the Roslyn ClassificationTypeName which contained spaces. This changes us to use LSP TokenTypes when possible, otherwise SemanticHighlightClassification EnumMember names fixed up to begin with a lowercase letter.
| private readonly Mef.IRequestHandler<SemanticHighlightRequest, SemanticHighlightResponse> _definitionHandler; | ||
| private readonly DocumentSelector _documentSelector; | ||
|
|
||
| private static string MakeLSPCompatibleString(string str) | ||
| => char.ToLower(str[0]) + str.Substring(1); |
There was a problem hiding this comment.
| => char.ToLower(str[0]) + str.Substring(1); | |
| => char.ToLower(str[0]) + str[1..]; |
There was a problem hiding this comment.
I will also say that I spent a minute trying to figure out if there was a way to do this in one simple line without doing a substring at all, using spans, but I would have to make a local out of char.ToLower(str[0]) to get that to work.
There was a problem hiding this comment.
I would have to pull in Range to make your suggestion work for net472. Maybe we can clean this all up one day.
|
Very exciting! Any idea when we should expect this change to be included in a future OmniSharp release, or which release version will include the change? |
|
The next version will include it, but we don't have a regular release schedule. It's usually whenever there's something requiring a bugfix release. |
|
Isn't "making sure the software works out of the box again for nvim users" a reason for a bugfix release? |
|
That seems reasonable. @dibarbet, do you think we do a release? |
|
Please do release, I have been constantly seeing my neovim reporting Invalid character in group name, and I am sure many users been noticing this too. |
|
Thanks to @JoeRobich, 1.39.8 with this fix was released just now: https://github.com/OmniSharp/omnisharp-roslyn/releases/tag/v1.39.8 |
Closes #2483