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

Tokenize 'this' as a keyword #39

Merged
merged 3 commits into from
Sep 11, 2014
Merged

Tokenize 'this' as a keyword #39

merged 3 commits into from
Sep 11, 2014

Conversation

lee-dohm
Copy link
Contributor

@lee-dohm lee-dohm commented Sep 8, 2014

Per the specification, this is a JavaScript keyword that is forbidden from being used as a variable or property name. So we should tokenize it as a keyword and not a variable.

See: http://coffeescript.org/documentation/docs/lexer.html#section-69

Fixes #36

Per the specification, `this` is a JavaScript keyword that is forbidden
from being used as a variable or property name. So we should tokenize it
as a keyword and not a variable.

See: http://coffeescript.org/documentation/docs/lexer.html#section-69

Fixes #36
@@ -244,7 +244,7 @@
'name': 'constant.language.null.coffee'
}
{
'match': '\\b(?<!\\.)(this|extends)(?!\\s*[:=])\\b'
'match': '\\b(?<!\\.)(extends)(?!\\s*[:=])\\b'
Copy link
Contributor

Choose a reason for hiding this comment

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

Mind 🔥 the parens around extends since they are no longer needed?

@lee-dohm
Copy link
Contributor Author

lee-dohm commented Sep 8, 2014

Done. Thanks!

@kevinsawicki
Copy link
Contributor

So, looking at this a bit more, this PR would make CoffeeScript different than several other languages with this. C++ and Java both use variable.language for this.

Similarly PHP uses variable.language for $this and Ruby and Python use variable.language for self.

Also, this changes makes instance variable assignments that use this. look different than @ even though they are functionally the same which feels a bit weird.

screen shot 2014-09-08 at 10 51 28 am

@lee-dohm
Copy link
Contributor Author

lee-dohm commented Sep 8, 2014

Yeah, there is definitely a tension between the fact that it is a keyword but it is used as an implicitly-declared variable.

Perhaps we should add another signifier so that people can color this differently than other variables? So instead of variable.language.coffee it could be variable.language.this.coffee? We could do the same with the other languages as well.

@kevinsawicki
Copy link
Contributor

@lee-dohm That sounds good to me, adding another class is a great way to make it targetable

@lee-dohm
Copy link
Contributor Author

lee-dohm commented Sep 8, 2014

@kevinsawicki Changed it as discussed and updated the test. Going to open Issues on the other languages you mentioned.

@kevinsawicki
Copy link
Contributor

Looking good 👍

kevinsawicki added a commit that referenced this pull request Sep 11, 2014
Tokenize 'this' as a keyword
@kevinsawicki kevinsawicki merged commit f0e6596 into atom:master Sep 11, 2014
Victorystick pushed a commit to Victorystick/language-c that referenced this pull request Jul 29, 2015
Victorystick pushed a commit to Victorystick/language-java that referenced this pull request Jul 29, 2015
See: atom/language-coffee-script#39

Fixes atom#6

Added minimal specs which were completely absent.
infininight pushed a commit to textmate/coffee-script.tmbundle that referenced this pull request Aug 22, 2016
This pull request was closed.
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.

this is not a keyword
2 participants