New scope name for preprocessors

50Wliu commented 7 years ago

Not including prerequisites as they don't apply.

In #188, there was some discussion that entity.name.function.preprocessor.c was not correct. Some alternatives that were proposed, as well as some that I'm proposing right now:

entity.name.macro.preprocessor.c
entity.name.identifier.preprocessor.c
entity.name.expression.preprocessor.c
entity.name.preprocessor.c
entity.name.macro.c

None are widely supported by themes, and also have no precedent.

Textmate docs are here, though they don't cover this as well. Proposed Atom naming conventions also doesn't cover this.

cc: @thomasjo @alpyre @simurai @MaximSokolov

thomasjo commented 7 years ago

My votes are, in order of preference;

entity.name.identifier.preprocessor.c
entity.name.expression.preprocessor.c
entity.name.preprocessor.c

jeff-hykin commented 5 years ago

I'm working on maintaining the TextMate grammar for VS Code, so I'd like to coordinate this naming change with you all just to keep theming for both VS Code and Atom somewhat in sync.

I agree there isn't going to be a perfect fix. For most themes I think it is just going to end up being colored with the default color for entity.name.

My vote preference would be for one of

entity.name.other.preprocessor.macro
entity.name.identifier.preprocessor.macro

Why `other`?

I prefer other because several languages (including C++ in other places) could benefit from having entity.name.other. For example, rust, go, php, sass, and r all have non-standard entity.name tags.

rust has entity.name.lifetime
go has entity.name.package and entity.name.import
sass has entity.name.namespace-prefix etc

It would be a shame to just pile on another edge case without solving the general case. Even C++ namespaces are not really entity.name.type and should probably be labeled entity.name.other.namespace.

Why `preprocessor.macro` vs `macro.preprocessor` or `macro`?

In general, and in the future, there might be preprocessor entities that are not macros. Additionally I think:

entity.name.function.preprocessor should be changed to entity.name.function.macro
variable.parameter.preprocessor should be changed to variable.parameter.macro

The rust grammar uses entity.name.function.macro so making the above change would also make things slightly more standard.

In the VS Code grammar, macro arguments like ##vars (from https://github.com/atom/language-c/issues/318) are marked as variable.other.macro.argument.cpp .

Why not expression/constant?

Macros can be/do almost anything:

constants e.g. 10
variables e.g. test_var
expressions test_var++
class names vector<string>
syntax abbreviations while (true)

Labeling them as expressions/constants would be misleading.

Why not `identifier`?

entity.name is basically the cross-language word for "identifier that is not a variable". I don't think there is a entity.name that is not an identifier.

atom / language-c