Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate moving cpp grammar to https://github.com/jeff-hykin/cpp-textmate-grammar #68392

Closed
aeschli opened this issue Feb 11, 2019 · 15 comments
Assignees
Labels
grammar Syntax highlighting grammar
Milestone

Comments

@aeschli
Copy link
Contributor

aeschli commented Feb 11, 2019

https://github.com/jeff-hykin/cpp-textmate-grammar is a c++ grammar derived from https://github.com/atom/language-c fixing several of the issues..

Given that https://github.com/atom/language-c is no longer actively maintained (atom switched to tree-sitter), we could consider to use https://github.com/jeff-hykin/cpp-textmate-grammar as the built-in text mate grammar.
@jeff-hykin Would that be ok for you?

@alexr00 alexr00 added the grammar Syntax highlighting grammar label Feb 11, 2019
@alexr00 alexr00 added this to the February 2019 milestone Feb 11, 2019
@Astrantia
Copy link

I have another suggestion. We could consider moving to tree sitter too @aeschli

@sean-mcmanus
Copy link
Contributor

Yeah, the C/C++ extension team would like VS Code to switch to that better TextMate grammar, i.e. so that all the C/C++ users can benefit "out-of-the-box" without needing to install another extension.

@jeff-hykin
Copy link

jeff-hykin commented Feb 14, 2019

I'd be fine with this! I'm a university student so my availability may fluctuate with exams, but I'd be happy adding collaborators and I certainly plan to continue rapidly adding improvements until the code is easily maintainable.

There's a lot of behind the scenes work that needs to be done to get it to to that point, but the code should always be a strict improvement from what it is now.

@jeff-hykin
Copy link

@Astrantia Tree sitters are really cool, I've been looking into them. It'd be great to move towards that or something even better. It's going to take some time though, and some of the issues in the TextMate grammar were solved by literally just backspacing non-existent keywords; thats how easy is to improve the existing code.

Textmate grammars have fundamental problems, we need to switch at some point if we want things like custom class tagging and other advanced highlighting. But there's incredible room for improvement within Textmate and lots of the improvements are easy to implement.

The json/cson file itself is unmaintainable, but we can use an actual language like Ruby to generate the TextMate grammar. It'll make it maintainable and we can get 50% of the tree sitter benefits right now, without changing VS Code at all.

@alexr00 alexr00 modified the milestones: February 2019, March 2019 Feb 14, 2019
@alexr00
Copy link
Member

alexr00 commented Feb 14, 2019

I've moved this to March so that we can have it for longer in insiders before it releases to stable. That means I'll try pulling in the new grammar around March 4th. Thanks @jeff-hykin!

@jeff-hykin
Copy link

jeff-hykin commented Feb 14, 2019

That sounds good, glad I could help! This weekend I'll do an update to make the repo more collaborator friendly and update the project board with a roadmap.

@sean-mcmanus
Copy link
Contributor

@jeff-hykin FYI, the bug atom/language-c#127 got a couple more upvotes (#67161) if you have time to fix that.

@jeff-hykin
Copy link

jeff-hykin commented Feb 23, 2019

Alright I went ahead and created an issue for it there. jeff-hykin/better-cpp-syntax#4

I made some progress on it and pushed an update with the improvements; there's only one edge case its failing on now. The last edge case is going to be a little harder to fix because its conflicting with the single-character (ex: '1') syntax.

I went ahead an finished adding support, all of them should work now.

screen shot 2019-02-23 at 5 20 53 am

The C99 hexadecimal floating constant syntax (ex: 0x0.5p10) was also missing so I added that as well. There were a lot of other changes in the v1.4 today too. Next week I'll try to copy over all the still-unfixed atom issues into https://github.com/jeff-hykin/cpp-textmate-grammar.

@sean-mcmanus
Copy link
Contributor

@jeff-hykin Cool, that is really awesome. Thanks.

@alexr00
Copy link
Member

alexr00 commented Mar 4, 2019

With a1481e3 I've made the switch. Thanks @jeff-hykin!

@jeff-hykin
Copy link

jeff-hykin commented Mar 16, 2019

All relevant atom/language-c issues have been moved to
https://github.com/jeff-hykin/cpp-textmate-grammar/issues

All the following from atom/language-c have been fixed:
atom/language-c#320
atom/language-c#318
atom/language-c#312
atom/language-c#309
atom/language-c#289
atom/language-c#270
atom/language-c#260
atom/language-c#264
atom/language-c#246
atom/language-c#240
atom/language-c#233
atom/language-c#215
atom/language-c#177
atom/language-c#153
atom/language-c#121
atom/language-c#101

A framework for unit testing patterns has been implemented.

And lambda support was added today 🎉

[ a, b, c ] (int thing1) mutable -> Ret { }
[ a, b, c ] (Args... args) { }
[=]() mutable { return; }
[=] -> int { }
[ a, b, c ] { }

@rbx
Copy link

rbx commented Apr 11, 2019

How can I revert to the old grammar in 1.33 ?!

@jeff-hykin
Copy link

@rbx I can probably make an extension this weekend that would revert it. Is there anything in particular that you do not like though?

The syntax should only be adding information, all color differences can be changed without changing the syntax (but not vice-versa).

@rbx
Copy link

rbx commented Apr 11, 2019

@jeff-hykin in particular I encountered several inconsistent highlighting issues which were mostly already reported (I also opened an issue today here: jeff-hykin/better-cpp-syntax#87).

Since I didn't have any problems with the old grammar, I would like to be able to revert until new one is fixed.

Anyway, thanks for your effort, I hope this will end up being better. I like the improved namespace highlighting.

@alexr00
Copy link
Member

alexr00 commented Apr 11, 2019

The grammar will be reverted in 1.33.1 since that is the lowest risk fix for C++ syntax highlighting. In Insiders 1.34, the new grammar is still in place. I've pulled in @jeff-hykin's recent changes, and I've started updating the default themes to better handle some of the changes in the new grammar. Things that should be fixed in the next Insiders: using, new, delete, typedef.

No one is maintaining the old grammar, so once we've stablaized the grammar + theme combos it should be a significant improvement!

@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
grammar Syntax highlighting grammar
Projects
None yet
Development

No branches or pull requests

6 participants