Skip to content

Adding signature help tooltip coloring support for VS#3821

Merged
navya9singh merged 11 commits into
mainfrom
navyasingh/VSSupport
May 15, 2026
Merged

Adding signature help tooltip coloring support for VS#3821
navya9singh merged 11 commits into
mainfrom
navyasingh/VSSupport

Conversation

@navya9singh
Copy link
Copy Markdown
Member

How Strada does it

  • tsserver tags text with SymbolDisplayPartKind values (TypeScript's own enum)
  • TypeScript-VS C# middleware translates these to Roslyn ClassificationTypeNames via GetClassificationName()
  • Middleware builds a ClassifiedTextElement and attaches it as ColorizedLabel on SignatureInformation
  • VS uses the Roslyn classification names to render colored text

Why Corsa needs a different approach

  • In Corsa, VS talks directly to tsgo over LSP — no C# middleware exists in this path
  • Without middleware, there's nothing to translate display part kinds or build the ClassifiedTextElement
  • Result: signature help renders as plain uncolored text

New displayPartsWriter

  • The Go printer's internal writer discards classification info — WriteKeyword, WriteSymbol, etc. all just append plain text
  • Modifying the internal writer would add overhead to every print operation across the codebase
  • displayPartsWriter implements EmitTextWriter as an external writer passed to printer.Write(), intercepting classified write calls and recording Roslyn classification names alongside text
  • classificationForSymbol() collapses Strada's 2-step translation (SymbolFlagsSymbolDisplayPartKind → Roslyn name) into one step, verified against both displayPartKind() and GetClassificationName()

Changes

  • Modified: internal/ls/signaturehelp.go - builds _vs_colorizedLabel with classified runs per signature

  • Modified: internal/lsp/lsproto/_generate/generate.mts— added ClassifiedTextRun, ClassifiedTextElement types; patched SignatureInformation

  • 23 baseline files updated with new _vs_colorizedLabel data

  • displayPartsWriter is reusable for hover, FAR, completions — any feature needing VS colorized output

Fixes Bug 2800005

@navya9singh navya9singh force-pushed the navyasingh/VSSupport branch from 946ffcb to 40cb407 Compare May 13, 2026 03:38
@navya9singh navya9singh marked this pull request as ready for review May 13, 2026 17:55
Copilot AI review requested due to automatic review settings May 13, 2026 17:55
@navya9singh navya9singh changed the title adding signature help tooltip coloring support for VS Adding signature help tooltip coloring support for VS May 13, 2026
Comment thread internal/ls/signaturehelp.go Outdated
Comment thread internal/ls/signaturehelp.go Outdated
Comment thread internal/ls/signaturehelp.go Outdated
Comment thread internal/ls/signaturehelp.go Outdated
Comment thread internal/fourslash/_scripts/failingTests.txt
Comment thread internal/ls/displaypartswriter.go Outdated
Comment thread internal/lsp/lsproto/_generate/generate.mts
Comment on lines +45 to +51
"parameters": [],
"_vs_colorizedLabel": {
"Runs": [
{
"ClassificationTypeName": "method name",
"Text": "foo"
},
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we just need to enable the VS cap in fourslash?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if we enable the capability in fourslash then these responses will include the _vs_colorizedLabel. Should I do that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I assume we want to make sure it's working, right?

Copy link
Copy Markdown
Member Author

@navya9singh navya9singh May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll add it. I've tried it with VS and it works there and fixes the coloring issue.

@navya9singh navya9singh requested a review from jakebailey May 15, 2026 18:02
@jakebailey
Copy link
Copy Markdown
Member

I think we should try to test this, but otherwise LGTM

TestAutoImportTypeImport4
TestAutoImportTypeOnlyPreferred3
TestAutoImportVerbatimTypeOnly1
TestCalledUnionsOfDissimilarTyeshaveGoodDisplay
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did TestQualifyModuleTypeNames go?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm running these tests again to check

@navya9singh navya9singh added this pull request to the merge queue May 15, 2026
Merged via the queue into main with commit f4a1d2a May 15, 2026
21 checks passed
@navya9singh navya9singh deleted the navyasingh/VSSupport branch May 15, 2026 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants