[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-10 Thread Johan Vikström via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL365602: [clangd] Added highlighting for class and enum types. (authored by jvikstrom, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-10 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D64257#1577347 , @nridge wrote: > are you testing these patches against a client-side implementation of > semantic highlighting? If so, which one? Looks like Theia is the only LSP client supporting the semantic highlighting

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-10 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. thanks, looks good. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:83 +// `Class` type. The destructor decls are handled in `VisitTypeLoc` (we will +

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-10 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom added a comment. In D64257#1577347 , @nridge wrote: > @jvikstrom out of curiosity, are you testing these patches against a > client-side implementation of semantic highlighting? If so, which one? Yes, I am testing against Theia. Just modified

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. @jvikstrom out of curiosity, are you testing these patches against a client-side implementation of semantic highlighting? If so, which one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64257/new/

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. (Thanks for addressing my comments earlier. I don't have more concerns, but haven't been following the last revisions in detail so will leave approval to Haojian) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64257/new/

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208708. jvikstrom marked 4 inline comments as done. jvikstrom added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64257/new/ https://reviews.llvm.org/D64257 Files:

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:38 bool VisitNamedDecl(NamedDecl *ND) { -// FIXME: (De)Constructors/operator need to be highlighted some other way. +// Constructors have a TypePtr TagDecl that is a Function,

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208671. jvikstrom added a comment. Changed comment in addToken. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64257/new/ https://reviews.llvm.org/D64257 Files:

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:38 bool VisitNamedDecl(NamedDecl *ND) { -// FIXME: (De)Constructors/operator need to be highlighted some other way. +// Constructors have a TypePtr TagDecl that is a

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 208670. jvikstrom marked 4 inline comments as done. jvikstrom added a comment. No longer highlighting anonymous structs as types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64257/new/

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:81 void addToken(SourceLocation Loc, const Decl *D) { +if (isa(D)) { + addToken(Loc, HighlightingKind::Class); jvikstrom wrote: > hokein wrote: > > nit: move

[PATCH] D64257: [clangd] Added highlighting for class and enum types

2019-07-09 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:81 void addToken(SourceLocation Loc, const Decl *D) { +if (isa(D)) { + addToken(Loc, HighlightingKind::Class); hokein wrote: > nit: move this around `if