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

Clean up grammar source #407

Merged
merged 3 commits into from
Aug 23, 2016
Merged

Clean up grammar source #407

merged 3 commits into from
Aug 23, 2016

Conversation

Alhadis
Copy link
Contributor

@Alhadis Alhadis commented Aug 19, 2016

Trivial changes to code formatting that were prompted by feedback to an earlier PR.

Summary:

  • Simplified character classes for matching variable names:

    [a-zA-Z_$][a-zA-Z_$0-9]*
    [a-zA-Z_$][\\w$]*
    
  • Extended patterns now use triple-quoted blocks consistently

  • Lengthy patterns converted into extended patterns to clarify

@@ -126,7 +126,11 @@
}
{
'comment': 'ES6 export: `export default (variable|class|function, etc.)`'
'match': '(?x) \\b(export)\\b \\s* \\b(default)\\b (?:\\s*) \\b((?!\\bfunction\\b|\\bclass\\b|\\blet\\b|\\bvar\\b|\\bconst\\b)[a-zA-Z_$][a-zA-Z_$0-9]*)?\\b'
'match': '''(?x)
\\b(export)\\b \\s*
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these spaces are needed, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. The space only serves to distinguish one column from another:

Figure 1

Of course, I can always remove it if you prefer. I just found it pronounced the pertinent parts better for new readers.

* Simplified character classes for matching variable names:
  [a-zA-Z_$][a-zA-Z_$0-9]* -> [a-zA-Z_$][\\w$]*

* Extended patterns now use triple-quoted blocks consistently

* Lengthy patterns converted into extended patterns for readability
@Alhadis
Copy link
Contributor Author

Alhadis commented Aug 20, 2016

@50Wliu I've shot the spaces into orbit, disregard my earlier comment. =)

@Alhadis
Copy link
Contributor Author

Alhadis commented Aug 20, 2016

@MaximSokolov Can you check if there're any conflicts between this PR and #408?

@Alhadis
Copy link
Contributor Author

Alhadis commented Aug 20, 2016

... okay, I literally asked that the same time you submitted your PR to my branch. Flawless timing.

@50Wliu There're keywords missing from the support-constant groups, which I'm planning on submitting in a follow-up PR (which fixes a few other bugs as well). I've isolated all cosmetic changes to this branch in an effort to keep a cleaner revision history.

I also fixed a few typos in the matching groups and eliminated the nested subpatterns. Oniguruma processes fixed-length lists faster than nested ones, so stuff like borderWidth|borderHeight|borderColour will match faster than border(?:Width|Height|Colour) or what-have-you. I benchtested this myself, although I've forgotten what grammar/repo I did it in...

@MaximSokolov
Copy link
Contributor

There is no conflict

@Alhadis
Copy link
Contributor Author

Alhadis commented Aug 20, 2016

Stellar. Спасибо!

@winstliu winstliu merged commit d6238db into atom:master Aug 23, 2016
@Alhadis Alhadis deleted the lint branch August 23, 2016 04:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants