[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-07 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. mostly good, please update the patch description. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:6 + +interface TokenColorRule { +

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-07 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/test/assets/firstIncludedTheme.jsonc:3 +// Some comment +"include": "secondIncludedTheme.jsonc", +"tokenColors": [ hokein wrote: > nit: any reason use

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

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

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-06 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. thanks, looks simpler now, just a few more nits. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:7 +interface TokenColorRule { + scope: string, textColor: string, +} nit: we need documentation

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-06 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 213568. jvikstrom added a comment. Clarified comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65738/new/ https://reviews.llvm.org/D65738 Files:

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-06 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 213567. jvikstrom marked 6 inline comments as done. jvikstrom added a comment. Renamed file to semantic-highlighting.ts. Added test for parsing theme files. Inlined the parsing into the loading function. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-06 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/TextMate.ts:6 + +export namespace SemanticHighlighting { +interface ScopeColorRule { I think we should not use the namespace in typescript. The `namespace` in TS refers

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-06 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom added a comment. In D65738#1614897 , @hokein wrote: > Haven't looked at the patch in details. > > Looks like the patch is doing different things, and the test just covers a > small set of the code. > > 1. find and parse the theme definition

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-06 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 213528. jvikstrom marked 2 inline comments as done. jvikstrom added a comment. Narrowed CL down to loading/parsing a text mate theme. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65738/new/

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Haven't looked at the patch in details. Looks like the patch is doing different things, and the test just covers a small set of the code. 1. find and parse the theme definition files `json` ; 2. define related data structures to save the TM scopes and do the

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

2019-08-05 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom created this revision. jvikstrom added reviewers: hokein, ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Adds a TextMate parser module to the vscode extension. It watches for changes to the vscode configuration