Skip to content

Add new Roslyn JSON classifications#6390

Merged
allisonchou merged 4 commits intomainfrom
allichou/AddNewRoslynTokenTypes
May 17, 2022
Merged

Add new Roslyn JSON classifications#6390
allisonchou merged 4 commits intomainfrom
allichou/AddNewRoslynTokenTypes

Conversation

@allisonchou
Copy link
Contributor

Summary of the changes

@davidwengier
Copy link
Member

We should definitely share these somehow

@ryzngard
Copy link
Contributor

We should definitely share these somehow

I wonder if that would be a bad idea. If a user specifies different colors for classifications (which is completely allowed) then we wouldn't respect them right?

@allisonchou
Copy link
Contributor Author

We have a tracking issue for exposing C#'s semantic tokens legend to Razor: dotnet/roslyn#61237

If a user specifies different colors for classifications (which is completely allowed) then we wouldn't respect them right?

I think what David is saying (feel free to correct me if I'm wrong) is that instead of hardcoding the C# token types we should be pulling the types in from Roslyn (maybe via reflection?). I'm pretty sure we can do this via external access (tracked by the work item above)

@davidwengier
Copy link
Member

Yeah, just to remove the fragility of us needing to chase around after Roslyn every time they add new token types.

RazorCommentType,
RazorCommentTransitionType,
RazorCommentStarType, // 90
RazorCommentTransitionType, // 100
Copy link
Contributor

Choose a reason for hiding this comment

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

These were mostly for helping me back when I was doing a lot more of this by hand back in the day, if the comment numbers are annoying to maintain feel free to destroy them.

new SemanticTokenType("extension method name"), // 30
new SemanticTokenType("field name"),
new SemanticTokenType("interface name"),
new SemanticTokenType("json - array"),

Choose a reason for hiding this comment

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

Ya totally agree, this doesn't seem like a sustainable way to add more support for embedded embedded languages in C#.

@CyrusNajmabadi how would you feel about generalizing C# embedded language classifications? Aka, "Embedded array, embedded comment etc."? Asking because if we continue to add this type of support then in VS, VSCode (basically any platform trying trying to consume these changes) there will be a requirement for the consumers of the C# side to chase down these classifications in order to populate their classifications properly.

@ryanbrandenburg
Copy link
Contributor

Here's the existing issue tracking programmatic inclusion of C# classifications.

@allisonchou
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@allisonchou allisonchou merged commit 6ac021d into main May 17, 2022
@allisonchou allisonchou deleted the allichou/AddNewRoslynTokenTypes branch May 17, 2022 22:31
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.

5 participants