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

Support for properties, function/method calls #242

Merged
merged 11 commits into from Nov 17, 2015
Merged

Support for properties, function/method calls #242

merged 11 commits into from Nov 17, 2015

Conversation

MaximSokolov
Copy link
Contributor

Adds support for properties, function calls, method calls.

Fixes #37, fixes #241
Replaces: closes #217, closes #146
Refs #137

@50Wliu
Copy link
Contributor

50Wliu commented Oct 23, 2015

I've been pretty busy but I'll try to review this as soon as possible 😄.

@Stanzilla
Copy link

+1

'include': '#properties'
}
{
'match': '(?x) \\b[a-zA-Z_$]\\w* (?= \\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 really see the benefit of using (?x) here since the regex is pretty simple. Same goes for below.

@50Wliu
Copy link
Contributor

50Wliu commented Nov 1, 2015

👍 Looks good for the most part!

@MaximSokolov
Copy link
Contributor Author

atom-dark
Atom Dark

one-dark
One Dark

monokai
Monokai

Also syntax themes should be updated probably:

.meta.method-call, .meta.function-call {
  & > .entity.name.function {
      color: @syntax-text-color;
  }
}

or
Edit: Isn't a good option since not all lanugages wrap function declaration with meta.function

// Highlight function name only in function declaration
.meta.function {
  .entity.name.function {
      color: @color;
  }
}

/cc @simurai

@50Wliu
Copy link
Contributor

50Wliu commented Nov 2, 2015

This will probably warrant spec changes in Atom core - it did last time at least. I'll just wait for simurai and then merge and create a PR.

Conflicts:
	spec/javascript-spec.coffee
@simurai
Copy link
Contributor

simurai commented Nov 10, 2015

Tested with https://github.com/atom/language-examples/blob/master/languages/javascript.js

.meta.method-call, .meta.function-call {
  .entity.name.function {
      color: @syntax-text-color;
  }
}

screen shot 2015-11-11 at 12 02 24 am

// Highlight function name only in function declaration
.meta.function {
  .entity.name.function {
      color: @hue-6-2;
  }
}

screen shot 2015-11-11 at 12 00 42 am

Doesn't it make more sense to keep the function name and function call the same color? Basically without changing the themes.

Also, should it be scoped to JavaScript only? If not, I think option 2 would also affect other languages, like C.

@MaximSokolov
Copy link
Contributor Author

Screenshot 2 should be the same as screenshot 1:
scs2
By option 2 I meant .entity.name.function {...} -> .meta.function .entity.name.function {...}
I thought all languages wrap a function name in declaration with meta.function (not all of them do)

Doesn't it make more sense to keep the function name and function call the same color? Basically without changing the themes.

Well, this can be merged, if you think it looks ok

@MaximSokolov MaximSokolov changed the title Properties Support for properties, function/method calls Nov 10, 2015
simurai added a commit that referenced this pull request Nov 17, 2015
Support for properties, function/method calls
@simurai simurai merged commit 34ea8d4 into atom:master Nov 17, 2015
@50Wliu
Copy link
Contributor

50Wliu commented Nov 17, 2015

👏 🎉

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.

Constants with accessors not highlighted Hightlight functions and methods
4 participants