This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Added highlighting to enum constants.
ClosedPublic

Authored by jvikstrom on Jul 12 2019, 2:57 AM.

Details

Summary

VSCode does not have a scope for enum constants. So they were placed under "constant.other.enum" as that seems to be the most correct scope for enum constants. However, this makes theia color them blue (the same color it uses for keywords).

Diff Detail

Repository
rL LLVM

Event Timeline

jvikstrom created this revision.Jul 12 2019, 2:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2019, 2:57 AM

mostly good.

clang-tools-extra/clangd/SemanticHighlighting.cpp
122 ↗(On Diff #209445)

nit: clang-format.

257 ↗(On Diff #209445)

could you check the tm scope on vscode? They seem to use variable.other.enummember.

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
107 ↗(On Diff #209445)

since we have a dedicated testcase for enum, could you extend the testcase here?

jvikstrom updated this revision to Diff 209514.Jul 12 2019, 9:30 AM
jvikstrom marked 2 inline comments as done.

Address comments.

hokein accepted this revision.Jul 12 2019, 1:54 PM
hokein added inline comments.
clang-tools-extra/clangd/SemanticHighlighting.cpp
257 ↗(On Diff #209445)
This revision is now accepted and ready to land.Jul 12 2019, 1:54 PM
jvikstrom marked an inline comment as done.Jul 15 2019, 12:31 AM
jvikstrom added inline comments.
clang-tools-extra/clangd/SemanticHighlighting.cpp
257 ↗(On Diff #209445)

Oh yeah, I was going to write a comment about keeping it as constant.other.enum or something similar to it. Because it's not really a variable. But as we've stuck to the same TM scopes as vscode we should probably keep doing that so I'll change to variable.other.enummember and land.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2019, 12:41 AM