better-cpp-syntax
better-cpp-syntax copied to clipboard
jeff-hykin
C++20 keywords don't seem to be highlighted
Checklist
- [x] This problem exists even with the setting
"C_Cpp.enhancedColorization": "Disabled" - [ ] This bug exists for C
- [x] This bug exists for C++
- [ ] This bug exists for Objective-C
- [ ] This bug exists for Objective-C++
The code with a problem is:
module example;
import <vector>
import some.other.module;
export template<typename From, typename To>
concept convertible_to =
std::is_convertible_v<From, To> and
requires(From (&f)()) {
static_cast<To>(f());
};
In the above example, module, import, <vector>, some.other.module, concept, and requires should all be highlighted, but they're not.
It looks like:
Theme: peaceshi
Theme: XD
Theme: One Monokai
It should look like:
module,import,concept, andrequiresshould all be highlighted like the other keywords. (Note:moduleis contextually-dependent.)<vector>andsome.other.moduleshould probably look like<vector>in#include <vector>
Just keyword highlighting, no context awareness, could be added in easily enough. Import as an alternative to #include should also be easy enough to add.
I've been trying to work out how to do this locally, but without success. Looks like the above words are already in the JSON file, which has confused me.
Often those are in negative lookbehinds and lookaheads.
import: the import keyword needs to be pulled out of the check for #, But if the semicolon is required it might be best if its pulled into its own pattern.
Adding keywords: This should be just as simple as adding the correct flags to the keywords in https://github.com/jeff-hykin/cpp-textmate-grammar/blob/master/source/languages/cpp/tokens.rb#L261-L274
Thanks. I was definitely barking up the wrong tree, trying to add what's below to generate.rb.
grammar[:concept_definition] = Pattern.new(
tag_as: "meta.declaration.concept",
should_fully_match: ["concept A = B;"]
match: Pattern.new(
match:/concept/,
tag_as: "keyword.other.concept",
).maybe(@spaces).then(
identifier
).maybe(@spaces).then(
assignment_operator
).maybe(@spaces).then(
/[^;]+/
).maybe(@spaces).then(@semicolon.or(/\n/)),
)
Looks good, couple changes.
- use
.then(std_space)rather than.maybe(@spaces). This is for a couple reasons:std_spacesupports inline commentsstd_spaceis automatically optional.maybe(@spaces)can catastrophically backtrack.
.then(/[^;]+/)should be.oneOrMoreOf(match: /[^;]/, dont_backtrack?: true)- use
@end_of_lineinstead of/\n/ - You still want a fallback to highlight
/concept/if the context aware pattern fails.
Do concept, requires, and import need logic like what's above? I get module probably does, but the others aren't context-dependent keywords.
If a keyword doesn't influence any other highlighting, then all that is need is to add the correct flags inside tokens.rb. In the new PR, I added support for import.
I think this is a rookie mistake, but I'm having issues with npm run build.
cjdb: npm test
> [email protected] pretest /mnt/c/Users/Chris/projects/cpp-textmate-grammar
> npm run build
> [email protected] build /mnt/c/Users/Chris/projects/cpp-textmate-grammar
> ruby scripts/generate.rb
scripts/generate.rb:6: warning: Insecure world writable dir /mnt/c/Users/Chris/projects/cpp-textmate-grammar/node_modules/.bin in PATH, mode 040777
Traceback (most recent call last):
4: from /mnt/c/Users/Chris/projects/cpp-textmate-grammar/source/languages/c/generate.rb:3:in `<main>'
3: from /mnt/c/Users/Chris/projects/cpp-textmate-grammar/source/languages/c/generate.rb:3:in `require_relative'
2: from /mnt/c/Users/Chris/projects/cpp-textmate-grammar/source/textmate_tools.rb:4:in `<top (required)>'
1: from /usr/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in `require'
/usr/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in `require': cannot load such file -- deep_clone (LoadError)
Generating the syntax for 'c' failed: 256
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] build: `ruby scripts/generate.rb`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR! /home/cjdb/.npm/_logs/2019-10-11T04_10_51_315Z-debug.log
npm ERR! Test failed. See above for more details.
Are there any prerequisites I should be installing? I've run npm install and npm install top to no avail.
Yes
bundle install, I should probably add a post install hook.
I went ahead and added that to the CONTRIBUTING.md for next time. Sorry, about that @cjdb, and thanks for taking the initiative to get things running on your own machine.
I've noticed that since pulling 9f50b88, my custom requires highlighting has stopped working. Highlighting doesn't show even when it's configured identical to consteval, but interestingly, it doesn't highlight at all when I try to use it like a normal function.
It is expected that requires is not usable as a function name due to, avoid_invalid_function_names = @cpp_tokens.lookBehindToAvoidWordsThat(:isWord, not(:isPreprocessorDirective), not(:isValidFunctionName)).
Can you share what you have for the requires highlighting? As well as what context you added it to.
Can you share what you have for the requires highlighting?
# source/language/cpp/tokens.rb
{ representation: "consteval", name: "consteval", isLambdaSpecifier: true },
{ representation: "requires", name: "requires", isLambdaSpecifier: true },
Same as consteval for now. Figured I'd start small, and work my way up.
As well as what context you added it to.
[]() consteval x {}; // consteval highlighted
[]() requires x {}; // requires not highlighted
If you are using the debugger could you check that you are using the "Launch and Build All" configuration, the default "Launch Extension" does not usually rebuild the syntax files.
I've been manually running npm test, but I just tried Launch and Build All: no difference.
That is very strange, If I add the requires token directly after consteval (line 221) everything seems to work.
Could you remove the other requires definition (on line 263) just to be sure?
Is the lambda definition inside of a macro?
@jeff-hykin Any thoughts on why it wouldn't work?
Still nothing. Is there any chance that the customised file isn't being loaded, and the default one is?
There shouldn't be, npm test has no knowledge of what vscode loads normally, and the extension development host replaces the extension, so that the default one should never be loaded.
Is the pattern being recognized as a lambda? (Is the scope at { meta.function.definition.body.lambda.cpp when using Developer: Inspect TM Scopes
Could you try to reproduce this from a clean clone of the repo to try to determine if this is a build issue or if other changes might be affecting it?
Could you try to reproduce this from a clean clone of the repo to try to determine if this is a build issue or if other changes might be affecting it?
This didn't fix it. Perhaps we should jump on an IM channel? I'm available on cpplang.slack or #include<C++> Discord under @cjdb.
Is the pattern being recognized as a lambda? (Is the scope at
{ meta.function.definition.body.lambda.cppwhen usingDeveloper: Inspect TM Scopes
See attached.
I've been manually running
npm test, but I just tried Launch and Build All: no difference.
Is it opening a separate window? Running npm test will generate the files, but it won't change any visuals. To see the changes VS Code has to compile the extension, create a new window, and then sudo-install the newly compiled extension inside that window. And most times: when a change is made to the code the entire new-window needs to be reloaded.
Then new visuals and Developer: Inspect TM Scopes will only work inside that new window. Everywhere else will be the old version.
This didn't fix it. Perhaps we should jump on an IM channel?
Not a bad idea. Although if other people have this problem, we do want it to be findable.
just an update on this problem: in v1.14.20 (pushed just now) should add at least basic highlighting for those keywords
just an update on this problem: in
v1.14.20(pushed just now) should add at least basic highlighting for those keywords
Awesome, thanks!
(Note: it does not highlight the second requires keyword from this example (the one that comes from a requires-clause rather than a requires-expression), but perhaps that's a known issue.)
yeah... thanks for pointing that out. That case in particular is more of a templating problem see #246: Template definition needs multiple fixes
