Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Enhanced preprocessor rules #188

Merged
merged 13 commits into from
Feb 1, 2017
Merged

Enhanced preprocessor rules #188

merged 13 commits into from
Feb 1, 2017

Conversation

alpyre
Copy link
Contributor

@alpyre alpyre commented Dec 13, 2016

This PR resolves all preprocessor related spec errors and adds enhanced preprocessor line highlighting.
(may also resolve some other preprocessor related issues... please refer them if any)
before-after

@50Wliu
Copy link
Contributor

50Wliu commented Dec 13, 2016

Nice. Can you fix the one failing spec please?

#warning and #error preprocessor lines should tokenize quoted strings to highlight them...
...and should end at a comment (as they do in the new conditional preprocessor implementation).
Copy link
Contributor Author

@alpyre alpyre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#warning and #error preprocessor lines should highlight quoted strings and following comments.
warningerror

#preprocessor-rule-other-block is no more...
it has been changed to :
#preprocessor-rule-conditional-block
@50Wliu 50Wliu requested a review from thomasjo December 15, 2016 15:24
@alpyre
Copy link
Contributor Author

alpyre commented Dec 18, 2016

Block scopes should end not only at #endif's... but also at #elif's and #else's.
(Following commit provides this)
before-after

Block scopes should end not only at #endif's... but also at #elif and #else's.
Copy link
Contributor

@thomasjo thomasjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, but I left a comment regarding an inappropriate scope name.

'patterns': [
{
'match': '[a-zA-Z_$][\\w$]*'
'name': 'entity.name.function.preprocessor.c'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is function being used here? According to the C++ specification grammar, the token following #if is a constant expression, whereas the token following #ifdef and #ifndef is an identifier.

I'd prefer that we handle #if differently from #ifdef and #ifndef, but to keep things simple, I propose we change the function part of the current scope to expression, identifier, or something equally suitable. Using function is certainly not appropriate.

/cc @50Wliu for scope name suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. (I was afraid you'd never turn back) :)

The current implementation tokenizes the macro name following a #define as entity.name.function.preprocessor.c (please check lines 78-79).

Though I too don't find it appropriate, yet I decided to use the same scope name just to keep consistent with the current code.

If the desicion will be to change it, we should consider changing the other one too since they are the same entities (in C programming all variable values in an #if expression are just defined macro's).

BTW, I'd like the scope name to be: entity.name.macro.preprocessor.c

On the other hand, changing it to something appropriate will make it un-highlighted once again until we have that scope name covered in one-dark syntax.

NOTE: While waiting for a review, I've worked on some other submitted issues and succesfully resolved them. After deciding what to do with your change request, I can commit them in this PR if you wish.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply.
I do like entity.name.macro.preprocessor.c - that seems fairly appropriate to me.

I'm going to cc @MaximSokolov for his work on standardizing scope names and the C grammar and @simurai for potential syntax changes (down the cc chain we go!).

If we decide that the macro won't work, then entity.name.identifier.preprocessor.c would be my fallback.

I can commit them in this PR if you wish.

Please open separate PRs so that we can review each one individually.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing it to entity.name.macro.preprocessor.c in one-dark-syntax would be no problem but there are hundreds of other syntax themes that probably don't highlight macro. So for now, it's probably better to keep using function or something from the existing Textmate conventions. Or at least keep function as fallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello there...

Time is passing. I have lots and lots of other fixes for this repository waiting on the line...
What is the verdict on the scope name?

Let's conclude this PR please. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we've lost enough time debating over the scope name. Let's keep it function for this PR and open a new issue.

{
'begin': '^'
'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
'contentName': 'comment.block.preprocessor.if-branch'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

{
'begin': '^'
'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
'contentName': 'comment.block.preprocessor.if-branch.in-block'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

{
'begin': '^'
'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
'contentName': 'comment.block.preprocessor.elif-branch'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

'2':
'name': 'punctuation.definition.directive.c'
'end': '(?=^\\s*((#)\\s*endif\\b))'
'contentName': 'comment.block.preprocessor.else-branch'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

'2':
'name': 'punctuation.definition.directive.c'
'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
'contentName': 'comment.block.preprocessor.if-branch'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

'2':
'name': 'punctuation.definition.directive.c'
'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
'contentName': 'comment.block.preprocessor.if-branch.in-block'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

'2':
'name': 'punctuation.definition.directive.c'
'end': '(?=^\\s*((#)\\s*endif\\b))'
'contentName': 'comment.block.preprocessor.elif-branch'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

'2':
'name': 'punctuation.definition.directive.c'
'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
'contentName': 'comment.block.preprocessor.elif-branch'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

'2':
'name': 'punctuation.definition.directive.c'
'end': '(?=^\\s*((#)\\s*endif\\b))'
'contentName': 'comment.block.preprocessor.elif-branch.in-block'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

'2':
'name': 'punctuation.definition.directive.c'
'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
'contentName': 'comment.block.preprocessor.elif-branch'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a .c at the end.

- Additionally it fixes the specs that fail because of this change
- Also a new spec added cover the ternary operator usage in preprocessor conditionals.
Copy link
Contributor

@50Wliu 50Wliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last comment.

{
'include': '#numbers'
}
{ #Catch ternary operators here and implement a custom one
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this comment be moved to a new line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries.

@50Wliu
Copy link
Contributor

50Wliu commented Jan 20, 2017

Can you comment on the changes in d2d20c2? I'm not sure I follow.

@@ -455,7 +455,7 @@
'beginCaptures':
'0':
'name': 'punctuation.definition.comment.cpp'
'end': '\\n'
'end': '(?=\\n)'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change on line 458 does not make an observable change on the tokenizing behaviour of single comments at the moment. It just does not consume the newline character.

The changes on lines 1004, 1088, 1128, 1217, 1302, 1338, 1412 are to change the logic for capturing the area between a preprocessor conditional and its matching endif (or elif/else). Capturing that area is crucial to comment out with "#if 0".

Using '^' used to work well too. But if one day some other package wants to inject its rules at the beginning of source.c (using L:) that '^' may be consumed by those rules and this would break "#if 0" commenting.

Matching the new line char (\n) instead its following '^' is much smarter and safer on this regard.
But that \n shoudn't be consumed by a possible single line comment.

This change seamlessly handles it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I'm not sure about this change. I feel like if languages are injecting using L: then for the most part they should be assuming responsibility (I'm open to discussion on this). Would you be ok with backing out this change so that this can be merged, and then including it in a different PR where we can discuss it more?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm 100% confident that this particular change does not and will not break anything about single line comments ever.

And matching '^' turned out to be problematic not only with L: injected rules but also will be problematic with every possible new rule added above 'preprocessor-rule-disabled' in this grammar.
Matching '\n' here is a much much better logic in all cases (just with the exception of possible single line comments consuming them - which is a 100% unnecessary behaviour anyway).

It is always ok for me to back out and/or create new PR's but I find it quite unnecessary for this particular one since this is a very seamless touch. I'll happily do it anyway if you don't get convinced. ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just that I believe this is the first time I've ever seen \n used as a begin regex. I'm with you that this most likely doesn't break anything, and improves syntax highlighting, but I'd also like to do some of my own investigation for alternative solutions, and don't want that to hold up this PR :).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand you. The implementation seems quiet strange but I can assure you that using ^ was as strange as \n. :)

Either of them are both to match the point where the #if... conditional line ends and the following line starts. Actually I was planning to use \G there (which may make sense) but the location it matches changes when it recurses. So I had decided to match the first new line that follows it using '^'.

Now I've realized that it may be consumed by other - innocent - syntax, I had to think of a better way, which is to match the first newline character that follows.

I really don't think that there is any better alternative for this. If you think that this will obfuscate the code, maybe we can add a descriptive comment into the code.

BTW: The PR is held up by @thomasjo anyway, maybe we can discuss this here more as we wait for his response :)

Copy link
Contributor

@50Wliu 50Wliu Jan 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it-works
Try this! There was a spot I missed in my previous diff (suspiciously the elif section)...

diff --git a/grammars/c.cson b/grammars/c.cson
index 443b6da..b16e456 100644
--- a/grammars/c.cson
+++ b/grammars/c.cson
@@ -865,7 +865,7 @@
       }
     ]
   'preprocessor-rule-conditional-line-parens-innards':
-    'patterns':[
+    'patterns': [
       {
         #the valid "defined" keyword usage
         'match': '(?:\\bdefined\\b\\s*$)|(?:\\bdefined\\b(?=\\s*\\(*\\s*(?:(?!defined\\b)[a-zA-Z_$][\\w$]*\\b)\\s*\\)*\\s*(?:\\n|//|/\\*|\\?|\\:|&&|\\|\\||\\\\\\s*\\n)))'
@@ -935,15 +935,8 @@
   'preprocessor-rule-disabled':
     'patterns': [
       {
-        'begin': '^\\s*((#)\\s*if\\b)(?=\\s*\\(*\\b0+\\b\\)*\\s*(?:$|//|/\\*))'
-        'beginCaptures':
-          '0':
-            'name': 'meta.preprocessor.c'
-          '1':
-            'name': 'keyword.control.directive.conditional.c'
-          '2':
-            'name': 'punctuation.definition.directive.c'
-        'end': '^\\s*((#)\\s*endif\\b)'
+        'begin': '^\\s*(?=((#)\\s*if)\\s+\\(*0+\\)*\\s*(?:$|//|/\\*))'
+        'end': '^\\s*((#)\\s*endif)\\b'
         'endCaptures':
           '0':
             'name': 'meta.preprocessor.c'
@@ -953,8 +946,13 @@
             'name': 'punctuation.definition.directive.c'
         'patterns': [
           {
-            'begin': '\\G'
-            'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?=\\n)'
+            'begin': '((#)\\s*if)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+            'beginCaptures':
+              '1':
+                'name': 'keyword.control.directive.conditional.c'
+              '2':
+                'name': 'punctuation.definition.directive.c'
+            'end': '(?!\\G)'
             'name': 'meta.preprocessor.c'
             'patterns': [
               {
@@ -975,19 +973,17 @@
             'include': '#preprocessor-rule-disabled-elif'
           }
           {
-            'begin': '^\\s*((#)\\s*elif\\b)'
-            'beginCaptures':
-              '0':
-                'name': 'meta.preprocessor.c'
-              '1':
-                'name': 'keyword.control.directive.conditional.c'
-              '2':
-                'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*(?:elif|else|endif)\\b))'
+            'begin': '^\\s*(?=((#)\\s*elif)\\b)'
+            'end': '(?=^\\s*((#)\\s*(?:elif|else|endif))\\b)'
             'patterns': [
               {
-                'begin': '\\G'
-                'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?<!\\\\)(?=\\n)'
+                'begin': '((#)\\s*elif)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+                'beginCaptures':
+                  '1':
+                    'name': 'keyword.control.directive.conditional.c'
+                  '2':
+                    'name': 'punctuation.definition.directive.c'
+                'end': '(?!\\G)'
                 'name': 'meta.preprocessor.c'
                 'patterns': [
                   {
@@ -1001,8 +997,8 @@
             ]
           }
           {
-            'begin': '\\n'
-            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
+            'begin': '(?!\\G)'
+            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif))\\b)'
             'contentName': 'comment.block.preprocessor.if-branch.c'
             'patterns': [
               {
@@ -1019,15 +1015,8 @@
   'preprocessor-rule-disabled-block':
     'patterns': [
       {
-        'begin': '^\\s*((#)\\s*if\\b)(?=\\s*\\(*\\b0+\\b\\)*\\s*(?:$|//|/\\*))'
-        'beginCaptures':
-          '0':
-            'name': 'meta.preprocessor.c'
-          '1':
-            'name': 'keyword.control.directive.conditional.c'
-          '2':
-            'name': 'punctuation.definition.directive.c'
-        'end': '^\\s*((#)\\s*endif\\b)'
+        'begin': '^\\s*(?=((#)\\s*if)\\s+\\(*0+\\)*\\s*(?:$|//|/\\*))'
+        'end': '^\\s*((#)\\s*endif)\\b'
         'endCaptures':
           '0':
             'name': 'meta.preprocessor.c'
@@ -1037,8 +1026,13 @@
             'name': 'punctuation.definition.directive.c'
         'patterns': [
           {
-            'begin': '\\G'
-            'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?=\\n)'
+            'begin': '((#)\\s*if)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+            'beginCaptures':
+              '1':
+                'name': 'keyword.control.directive.conditional.c'
+              '2':
+                'name': 'punctuation.definition.directive.c'
+            'end': '(?!\\G)'
             'name': 'meta.preprocessor.c'
             'patterns': [
               {
@@ -1059,19 +1053,17 @@
             'include': '#preprocessor-rule-disabled-elif'
           }
           {
-            'begin': '^\\s*((#)\\s*elif\\b)'
-            'beginCaptures':
-              '0':
-                'name': 'meta.preprocessor.c'
-              '1':
-                'name': 'keyword.control.directive.conditional.c'
-              '2':
-                'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*(?:elif|else|endif)\\b))'
+            'begin': '^\\s*(?=((#)\\s*elif)\\b)'
+            'end': '(?=^\\s*((#)\\s*(?:elif|else|endif))\\b)'
             'patterns': [
               {
-                'begin': '\\G'
-                'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?<!\\\\)(?=\\n)'
+                'begin': '((#)\\s*elif)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+                'beginCaptures':
+                  '1':
+                    'name': 'keyword.control.directive.conditional.c'
+                  '2':
+                    'name': 'punctuation.definition.directive.c'
+                'end': '(?!\\G)'
                 'name': 'meta.preprocessor.c'
                 'patterns': [
                   {
@@ -1085,8 +1077,8 @@
             ]
           }
           {
-            'begin': '\\n'
-            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
+            'begin': '(?!\\G)'
+            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif))\\b)'
             'contentName': 'comment.block.preprocessor.if-branch.in-block.c'
             'patterns': [
               {
@@ -1101,19 +1093,17 @@
       }
     ]
   'preprocessor-rule-disabled-elif':
-    'begin': '^\\s*((#)\\s*elif\\b)(?=\\s*\\(*\\b0+\\b\\)*\\s*(?:$|//|/\\*))'
-    'beginCaptures':
-      '0':
-        'name': 'meta.preprocessor.c'
-      '1':
-        'name': 'keyword.control.directive.conditional.c'
-      '2':
-        'name': 'punctuation.definition.directive.c'
-    'end': '(?=^\\s*((#)\\s*(?:elif|else|endif)\\b))'
+    'begin': '^\\s*(?=((#)\\s*elif)\\s+\\(*0+\\)*\\s*(?:$|//|/\\*))'
+    'end': '(?=^\\s*((#)\\s*(?:elif|else|endif))\\b)'
     'patterns': [
       {
-        'begin': '\\G'
-        'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?<!\\\\)(?=\\n)'
+        'begin': '((#)\\s*elif)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+        'beginCaptures':
+          '1':
+            'name': 'keyword.control.directive.conditional.c'
+          '2':
+            'name': 'punctuation.definition.directive.c'
+        'end': '(?!\\G)'
         'name': 'meta.preprocessor.c'
         'patterns': [
           {
@@ -1125,7 +1115,7 @@
         'include': '#comments'
       }
       {
-        'begin': '\\n'
+        'begin': '(?!\\G)'
         'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
         'contentName': 'comment.block.preprocessor.elif-branch.c'
         'patterns': [
@@ -1141,17 +1131,8 @@
   'preprocessor-rule-enabled':
     'patterns': [
       {
-        'begin': '^\\s*((#)\\s*if\\b)(?=\\s*\\(*\\b0*1\\b\\)*\\s*(?:$|//|/\\*))'
-        'beginCaptures':
-          '0':
-            'name': 'meta.preprocessor.c'
-          '1':
-            'name': 'keyword.control.directive.conditional.c'
-          '2':
-            'name': 'punctuation.definition.directive.c'
-          '3':
-            'name': 'constant.numeric.preprocessor.c'
-        'end': '^\\s*((#)\\s*endif\\b)'
+        'begin': '^\\s*(?=((#)\\s*if)\\s+\\(*0*1\\)*\\s*(?:$|//|/\\*))'
+        'end': '^\\s*((#)\\s*endif)\\b'
         'endCaptures':
           '0':
             'name': 'meta.preprocessor.c'
@@ -1161,8 +1142,13 @@
             'name': 'punctuation.definition.directive.c'
         'patterns': [
           {
-            'begin': '\\G'
-            'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?=\\n)'
+            'begin': '((#)\\s*if)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+            'beginCaptures':
+              '1':
+                'name': 'keyword.control.directive.conditional.c'
+              '2':
+                'name': 'punctuation.definition.directive.c'
+            'end': '(?!\\G)'
             'name': 'meta.preprocessor.c'
             'patterns': [
               {
@@ -1174,7 +1160,7 @@
             'include': '#comments'
           }
           {
-            'begin': '^\\s*((#)\\s*else\\b)'
+            'begin': '^\\s*((#)\\s*else)\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1182,7 +1168,7 @@
                 'name': 'keyword.control.directive.conditional.c'
               '2':
                 'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*endif\\b))'
+            'end': '(?=^\\s*((#)\\s*endif)\\b)'
             'contentName': 'comment.block.preprocessor.else-branch.c'
             'patterns': [
               {
@@ -1194,7 +1180,7 @@
             ]
           }
           {
-            'begin': '^\\s*((#)\\s*elif\\b)'
+            'begin': '^\\s*((#)\\s*elif)\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1202,7 +1188,7 @@
                 'name': 'keyword.control.directive.conditional.c'
               '2':
                 'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
+            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif))\\b)'
             'contentName': 'comment.block.preprocessor.if-branch.c'
             'patterns': [
               {
@@ -1214,7 +1200,7 @@
             ]
           }
           {
-            'begin': '\\n'
+            'begin': '(?!\\G)'
             'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
             'patterns': [
               {
@@ -1228,15 +1214,8 @@
   'preprocessor-rule-enabled-block':
     'patterns': [
       {
-        'begin': '^\\s*((#)\\s*if\\b)(?=\\s*\\(*\\b0*1\\b\\)*\\s*(?:$|//|/\\*))'
-        'beginCaptures':
-          '0':
-            'name': 'meta.preprocessor.c'
-          '1':
-            'name': 'keyword.control.directive.conditional.c'
-          '2':
-            'name': 'punctuation.definition.directive.c'
-        'end': '^\\s*((#)\\s*endif\\b)'
+        'begin': '^\\s*(?=((#)\\s*if)\\s+\\(*0*1\\)*\\s*(?:$|//|/\\*))'
+        'end': '^\\s*((#)\\s*endif)\\b'
         'endCaptures':
           '0':
             'name': 'meta.preprocessor.c'
@@ -1246,8 +1225,13 @@
             'name': 'punctuation.definition.directive.c'
         'patterns': [
           {
-            'begin': '\\G'
-            'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?=\\n)'
+            'begin': '((#)\\s*if)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+            'beginCaptures':
+              '1':
+                'name': 'keyword.control.directive.conditional.c'
+              '2':
+                'name': 'punctuation.definition.directive.c'
+            'end': '(?!\\G)'
             'name': 'meta.preprocessor.c'
             'patterns': [
               {
@@ -1259,7 +1243,7 @@
             'include': '#comments'
           }
           {
-            'begin': '^\\s*((#)\\s*else\\b)'
+            'begin': '^\\s*((#)\\s*else)\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1267,7 +1251,7 @@
                 'name': 'keyword.control.directive.conditional.c'
               '2':
                 'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*endif\\b))'
+            'end': '(?=^\\s*((#)\\s*endif)\\b)'
             'contentName': 'comment.block.preprocessor.else-branch.in-block.c'
             'patterns': [
               {
@@ -1279,7 +1263,7 @@
             ]
           }
           {
-            'begin': '^\\s*((#)\\s*elif\\b)'
+            'begin': '^\\s*((#)\\s*elif)\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1287,7 +1271,7 @@
                 'name': 'keyword.control.directive.conditional.c'
               '2':
                 'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
+            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif))\\b)'
             'contentName': 'comment.block.preprocessor.if-branch.in-block.c'
             'patterns': [
               {
@@ -1299,7 +1283,7 @@
             ]
           }
           {
-            'begin': '\\n'
+            'begin': '(?!\\G)'
             'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
             'patterns': [
               {
@@ -1311,19 +1295,17 @@
       }
     ]
   'preprocessor-rule-enabled-elif':
-    'begin': '^\\s*((#)\\s*elif\\b)(?=\\s*\\(*\\b0*1\\b\\)*\\s*(?:$|//|/\\*))'
-    'beginCaptures':
-      '0':
-        'name': 'meta.preprocessor.c'
-      '1':
-        'name': 'keyword.control.directive.conditional.c'
-      '2':
-        'name': 'punctuation.definition.directive.c'
+    'begin': '^\\s*(?=((#)\\s*elif)\\s+\\(*0*1\\)*\\s*(?:$|//|/\\*))'
     'end': '(?=^\\s*((#)\\s*endif\\b))'
     'patterns': [
       {
-        'begin': '\\G'
-        'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?<!\\\\)(?=\\n)'
+        'begin': '((#)\\s*elif)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+        'beginCaptures':
+          '1':
+            'name': 'keyword.control.directive.conditional.c'
+          '2':
+            'name': 'punctuation.definition.directive.c'
+        'end': '(?!\\G)'
         'name': 'meta.preprocessor.c'
         'patterns': [
           {
@@ -1335,11 +1317,11 @@
         'include': '#comments'
       }
       {
-        'begin': '\\n'
-        'end': '(?=^\\s*((#)\\s*(?:endif)\\b))'
+        'begin': '(?!\\G)'
+        'end': '(?=^\\s*((#)\\s*(?:endif))\\b)'
         'patterns': [
           {
-            'begin': '^\\s*((#)\\s*(else)\\b)'
+            'begin': '^\\s*((#)\\s*(else))\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1347,7 +1329,7 @@
                 'name': 'keyword.control.directive.conditional.c'
               '2':
                 'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*endif\\b))'
+            'end': '(?=^\\s*((#)\\s*endif)\\b)'
             'contentName': 'comment.block.preprocessor.elif-branch.c'
             'patterns': [
               {
@@ -1359,7 +1341,7 @@
             ]
           }
           {
-            'begin': '^\\s*((#)\\s*(elif)\\b)'
+            'begin': '^\\s*((#)\\s*(elif))\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1385,19 +1367,17 @@
       }
     ]
   'preprocessor-rule-enabled-elif-block':
-    'begin': '^\\s*((#)\\s*elif\\b)(?=\\s*\\(*\\b0*1\\b\\)*\\s*(?:$|//|/\\*))'
-    'beginCaptures':
-      '0':
-        'name': 'meta.preprocessor.c'
-      '1':
-        'name': 'keyword.control.directive.conditional.c'
-      '2':
-        'name': 'punctuation.definition.directive.c'
-    'end': '(?=^\\s*((#)\\s*endif\\b))'
+    'begin': '^\\s*(?=((#)\\s*elif)\\s+\\(*0*1\\)*\\s*(?:$|//|/\\*))'
+    'end': '(?=^\\s*((#)\\s*endif)\\b)'
     'patterns': [
       {
-        'begin': '\\G'
-        'end': '(?=//)|(?=/\\*(?!.*\\\\\\s*\\n))|(?<!\\\\)(?=\\n)'
+        'begin': '((#)\\s*ellif)\\s*' # trailing \s* is needed so the ending (?!\G) capture isn't matched early
+        'beginCaptures':
+          '1':
+            'name': 'keyword.control.directive.conditional.c'
+          '2':
+            'name': 'punctuation.definition.directive.c'
+        'end': '(?!\\G)'
         'name': 'meta.preprocessor.c'
         'patterns': [
           {
@@ -1409,11 +1389,11 @@
         'include': '#comments'
       }
       {
-        'begin': '\\n'
-        'end': '(?=^\\s*((#)\\s*(?:endif)\\b))'
+        'begin': '(?!\\G)'
+        'end': '(?=^\\s*((#)\\s*(?:endif))\\b)'
         'patterns': [
           {
-            'begin': '^\\s*((#)\\s*(else)\\b)'
+            'begin': '^\\s*((#)\\s*(else))\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1421,7 +1401,7 @@
                 'name': 'keyword.control.directive.conditional.c'
               '2':
                 'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*endif\\b))'
+            'end': '(?=^\\s*((#)\\s*endif)\\b)'
             'contentName': 'comment.block.preprocessor.elif-branch.in-block.c'
             'patterns': [
               {
@@ -1433,7 +1413,7 @@
             ]
           }
           {
-            'begin': '^\\s*((#)\\s*(elif)\\b)'
+            'begin': '^\\s*((#)\\s*(elif))\\b'
             'beginCaptures':
               '0':
                 'name': 'meta.preprocessor.c'
@@ -1441,7 +1421,7 @@
                 'name': 'keyword.control.directive.conditional.c'
               '2':
                 'name': 'punctuation.definition.directive.c'
-            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif)\\b))'
+            'end': '(?=^\\s*((#)\\s*(?:else|elif|endif))\\b)'
             'contentName': 'comment.block.preprocessor.elif-branch.c'
             'patterns': [
               {
@@ -1459,7 +1439,7 @@
       }
     ]
   'preprocessor-rule-enabled-else':
-    'begin': '^\\s*((#)\\s*else\\b)'
+    'begin': '^\\s*((#)\\s*else)\\b'
     'beginCaptures':
       '0':
         'name': 'meta.preprocessor.c'
@@ -1467,14 +1447,14 @@
         'name': 'keyword.control.directive.conditional.c'
       '2':
         'name': 'punctuation.definition.directive.c'
-    'end': '(?=^\\s*((#)\\s*endif\\b))'
+    'end': '(?=^\\s*((#)\\s*endif)\\b)'
     'patterns': [
       {
         'include': '$base'
       }
     ]
   'preprocessor-rule-enabled-else-block':
-    'begin': '^\\s*((#)\\s*else\\b)'
+    'begin': '^\\s*((#)\\s*else)\\b'
     'beginCaptures':
       '0':
         'name': 'meta.preprocessor.c'
@@ -1482,7 +1462,7 @@
         'name': 'keyword.control.directive.conditional.c'
       '2':
         'name': 'punctuation.definition.directive.c'
-    'end': '(?=^\\s*((#)\\s*endif\\b))'
+    'end': '(?=^\\s*((#)\\s*endif)\\b)'
     'patterns': [
       {
         'include': '#block_innards'

Copy link
Contributor Author

@alpyre alpyre Jan 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry. Still not working on my Atom (v1.14.0-beta0)
Yet I accidentally discovered that it works when there is indentation:
sorry gif

Copy link
Contributor

@50Wliu 50Wliu Jan 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm...I wonder why it's working for me then. 🤔
EDIT: Because I forgot to reload Atom, that's why 🙈

Copy link
Contributor Author

@alpyre alpyre Jan 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Atom's single line regex matching mechanism is quite limited to support such a complex multiline pattern. I believe we should consent that we were able to find an implementation that at least works whether it's ugly or not. :)

So can we please compromise on \nusage for this one and move on?

Note: I can apply your changes on the \bs however. ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree - let's merge this PR and then iterate on it as necessary.

Note: I can apply your changes on the \bs however. ;)

Don't worry about this. I can do it.

@50Wliu 50Wliu merged commit a5dd739 into atom:master Feb 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants