search

jeff-hykin / better-cpp-syntax

💾 The source of VS Code's C++ syntax highlighting
GNU General Public License v3.0
154 stars 29 forks source link

Major Changes for Downstream #64

Open alexr00 opened 5 years ago

alexr00 commented 5 years ago

It looks like new used to be keyword.control.cpp. Now it is keyword.operator.memory.new.cpp. Is this intentional? If it's intentional, we can add the it into the themes in VS Code, but ideally there wouldn't be too many changes like this since it requires constant updating of the themes.

alexr00 commented 5 years ago

Original issue: https://github.com/Microsoft/vscode/issues/71821

jeff-hykin commented 5 years ago

Yes it was intentional, and I agree we will try to keep changes like this to a minimum since many themes already setup with the old way.

From here on out, I'll put any similar changes on this post.

C++ Log as of August 12th 2019

jeff-hykin commented 5 years ago

The reason it was changed is because new and delete are completely unrelated to control flow. I picked operator since that's what what I believed it was classified under the C++ spec.

@matter123 pointed out in his pull request its only an operator under a special case. Additionally its debatable that the coloration would be improved if it were labeled with keyword.other.

@alexr00 should we go ahead and make the change from keyword.operator to keyword.other?

alexr00 commented 5 years ago

Having the big changes here is perfect, thank you. I took a quick look at how other languages handle new: coffee keyword.operator.new.coffee

JS keyword.operator.new.js

groovy keyword.control.new.groovy

java keyword.control.new.java

There isn't a real consensus. Based on what the VS Code themes already have, keyword.operator.new would be good. keyword.other, like keyword.operator.memory would still require changes to the theme. I'm happy to make minor updates to themes though. If you prefer keyword.operator.memory more, then feel free to leave it and I'll make the update to include it here and in the equivalent places in the other themes.

matter123 commented 5 years ago

keyword.operator.new is probably the best. I noticed on [expr.new]/4 the standard refers to the new operator. Though change is still needed for keyword.operator.delete

jeff-hykin commented 5 years ago

Thats interesting @alexr00 I'll try to get a complete list of changes by the end of tonight. There is a file in this repo syntax_examples.cpp it is designed to visually debug the syntax highlighter changes. You can probably use it to quickly see how everything looks.

jeff-hykin commented 5 years ago

@alexr00 The list isn't 100% complete, but I believe it covers everything that would be considered an issue. Many of the changes were made tonight to try to get all of the changes done now. I probably won't be able to make changes for about a week or more, but I'll be around daily for GitHub communication.

Do note that these changes are C++ exclusive. We are working on an ideal way to port them to C.

I believe the new keyword.operator.wordlike, keyword.operator.functionlike should take care of most of the issues.

jeff-hykin commented 5 years ago

Today I added a tags.txt, that gets auto generated with the syntax. It just sorts and lists out all of the tags that are in the grammar, it should be useful for keeping track of when things are added and removed. @matter123

jeff-hykin commented 5 years ago

@alexr00 The Objective-C and Objective-C++ are fully restored/fixed. Yesterday we expanded the repo's structure to accommodate multiple languages. We added both the Objective-C and Objective-C++ syntaxes along with integration tests for both of them. Neither of them are dependent on C and C++ repos anymore. There's also been some work done on improving the Objective-C++ syntax, but to be on the safe side those changes are not included.

Other Info About the Problem

If you want to know more about why this happened in the first place, see here: https://github.com/jeff-hykin/cpp-textmate-grammar/issues/107#issue-434543514

Once the updated Objective-C and Objective-C++ are in VS Code, we can actually instantly fix some long standing bugs related to https://github.com/atom/language-c/issues/146

Scope Change Guide

To make pulling the latest changes easy, here's a scope-change guide for the "dark+" and "dark" themes.

Making a pattern specific to C++

I'd recommend adding source.cpp keyword.operator.new rather than keyword.operator.new.cpp The second way makes it where additional specificity like keyword.operator.new.overload.cpp will break the theme

Might want to change "Control flow keywords"

The only scope in the "Control flow keywords" that is control flow is keyword.control, all of the others (keyword.operator.new.cpp, keyword.operator.delete, keyword.other.using, keyword.other.operator) are unrelated to control flow. So you might want to rename that group to something like "Special Keywords"

Scope Names

dark plus
    // probably make these white #d4d4d4 (same as keyword.operator)
        "storage.modifier.pointer",
        "storage.modifier.reference"
    // probably put these with the "Control flow keywords"/"Special Keywords"
        "keyword.operator.functionlike keyword.operator", // typeid(), alignas(), etc
        "keyword.operator.wordlike keyword.operator",  // and, or, xor, not, etc
    // C++ attributes, maybe color them purple, blue, or light blue
        "entity.other.attribute",
        "punctuation.section.attribute.begin",
        "punctuation.section.attribute.end",
    // might want to put the overloadee in "Function declarations"
        "entity.name.operator.overloadee"
    // might want to add this to "Types declaration and references"
    // since its almost always a class or namespace that is the scope-resolution
        "entity.name.scope-resolution"

dark theme:
    // probably want to color blue:
        "support.type"
        "keyword.operator.wordlike keyword.operator"
    // probably make these white #d4d4d4 (same as keyword.operator)
        "storage.modifier.pointer"
        "storage.modifier.reference"
alexr00 commented 5 years ago

@jeff-hykin thank you for the detailed description of the issue and for all the fixes and suggestions! We are planning on releasing soon, and I want all these changes to be in insiders for a while before they go out in stable. As soon as we have branched for release (probably in the next couple week days) I'll bring in your Objective C and Objective C++ changes.

jeff-hykin commented 5 years ago

Great 👍 and I agree, I'd be more comfortable with them staying in the insiders branch for awhile.

jeff-hykin commented 4 months ago

Starting at v1.22.0 you might get downstream complaints about no SQL highlighting.