Faces for variables (all instances) and property access

[nicolasartman]

For those of us who like lots of syntax highlighting, it would be great to have all variable instances highlightable with a face, not just params and declarations (so both bars in let bar; bar += 3;). It would also be great to have a face for properties, so one could apply styling to bar in foo.bar.

Also, I'm a bit surprised by the face used in keys in object literals. If there was a face for properties, I think that might be appropriate, e.g. for foo in {foo: 'bar'} and {'foo': 'bar'} (though probably not for the foo in the ES6 form [foo]: 7.)

Thanks!

[dgutov]

I wouldn't mind accepting a patch along these lines, but why do you think it's important?

bar in bar += 3 is obviously a variable, because it's not a number, and it's not highlighted otherwise. And if you're going to highlight all the places where a variable is referenced or introduced, you'll also need to handle the properties and references inside destructuring expressions.

{foo: 'bar'} looks more troublesome. I'd prefer picking a face for properties from the default set, though. Maybe font-lock-string-face? All properties are strings, after all.

[nicolasartman]

I find syntax highlighting nearly everything allows me to scan and navigate code faster, so that's where my request comes from. You're totally right that one can easily see bar is a variable if they read the meaning of the line, and that it would ideally need to handle assignment via destructuring expressions too.

I think using the string face for properties makes logical sense—It'd be cool to have a separate face just for flexibility, but I don't think that's super important.

Here are examples from my textmate theme vs my emacs theme (the changes for keywords and funcalls are deliberate and desired), with properties set with the same face as strings.

[dgutov]

... and that it would ideally need to handle assignment via destructuring expressions too.

That change, as a whole, would need to be implemented by someone else.

I think using the string face for properties makes logical sense—It'd be cool to have a separate face just for flexibility, but I don't think that's super important.

I'm now on the fence about this. Take this example:

var a = {
  foo: 'bar',
  tee() { }
};

Highlighting tee with font-lock-function-name-face and foo with font-lock-string-face looks kinda odd.

I've looked at what other editors do about this. Vim and Sublime don't highlight keys at all. Atom highlights tee as a function name, but doesn't highlight foo (GitHub code snippets, unsurprisingly, look the same). Maybe we should take after Atom.

Here are examples from my textmate theme vs my emacs theme (the changes for keywords and funcalls are deliberate and desired), with properties set with the same face as strings.

Is it how TextMate devs chosen to highlight keys (always the same as strings), or is it the decision of the theme author?

[nicolasartman]

Ah, good example! I'd personally be quite happy coloring that foo with ...string-face and tee with ...function-name-face, but perhaps I'm an outlier. I currently have similar functionality for ES5 code (example). I specifically chose colors for everything that I felt were harmonious when in proximity to each other.

Textmate's default JS highlighting looks something like this, and it has a special scope for keys in object literals that is not used many other places, entity.name.attribute-name.js, that you can apply styles to (same as strings or different). All of the additions in the previous screenshots are a result of me augmenting the default language definition.

Of course, if any of this sounds worth implementing to you, I'll be grateful. For what you don't have time for, I am certainly planning on taking a stab at it myself once I further improve my elisp skill.

[dgutov]

it has a special scope for keys in object literals that is not used many other places, entity.name.attribute-name.js, that you can apply styles to (same as strings or different)

Let's do it the same way, then. I've added a new face, js2-object-property.

[nicolasartman]

Thank you! What do you think about applying that same face to bar in foo.bar?

[dgutov]

Does TextMate use the same scope for it?

[dgutov]

Overall, the result looks too fruit-salady to me. But you can try it out:

diff --git a/js2-mode.el b/js2-mode.el
index f38e1a5..01c97f9 100644
--- a/js2-mode.el
+++ b/js2-mode.el
@@ -6758,6 +6758,8 @@ Shown at or above `js2-highlight-level' 3.")
                 (prop
                  (if (string-match js2-ecma-object-props prop-name)
                      'font-lock-constant-face))))))
+        (when (and (not face) prop-name)
+          (setq face 'js2-object-property))
         (when face
           (let ((pos (+ (js2-node-pos parent)  ; absolute
                         (js2-node-pos prop)))) ; relative
@@ -9973,9 +9975,9 @@ Returns an expression tree that includes PN, the parent node."
         (setq pn (js2-parse-tagged-template pn (make-js2-string-node :type tt))))
        (t
         (js2-unget-token)
-        (setq continue nil))))
-    (if (>= js2-highlight-level 2)
-        (js2-parse-highlight-member-expr-node pn))
+        (setq continue nil)))
+      (if (>= js2-highlight-level 2)
+          (js2-parse-highlight-member-expr-node pn)))
     pn))

 (defun js2-parse-tagged-template (tag-node tpl-node)

[nicolasartman]

Awsome, thank you! Textmate doesn't give it a special scope at all, so I had to make my own for it. If there's a way to assign another face to it perhaps that'd work so it can be unstyled by default? (can you assign multiple faces in emacs? Not experienced enough with the system yet to know for sure)

[dgutov]

But js2-object-property is already unstyled by default. Do you need another face?

[nicolasartman]

Oh, I don't, I just thought others might want one so they could color bar in {bar: 3} but not foo.bar. Very happy with either, sorry for the confusion!

[dgutov]

Textmate doesn't give it a special scope at all, so I had to make my own for it.

How does one create a special scope there?

[nicolasartman]

Textmate has a language grammar definition based on regexes for start/ending scopes, which is how it colors things (described quite thoroughly here)—I forked the default grammar file for javascript and now use my own with those few additions. Since scopes are nestable I just added my own on top of the existing syntax identification it did so I could specially target properties.

As far as I understand, Atom uses a system based on textmate's grammar definition. Not sure what sublime uses for parsing. I'm happy to try to answer any question's about textmate's system if it'd be helpful, though as far as I understand, a regex based system like that is far less robust than the proper parser js2-mode has.

[dgutov]

Oh, I see. It's extensible that way.

The change above is pushed now.

[nicolasartman]

Awesome, thank you so much!

If I get time soon I'll look into colorizing all vars, at least in the es5 cases, to see how hard that'd be.

[nicolasartman]

@dgutov is this change working properly for you? I just updated and it seems to be overriding the function call face in all instances (both something.fun() and doFun() on its own. I switched between versions and I don't think it's something else in my environment.

[dgutov]

Indeed, thanks for noticing. Should be fine now.

[nicolasartman]

Looks great, thank you so much!

[nicolasartman]

One slight glitch I noticed when checking this more now—it's unrelated I think so let me know if you'd rather I file another issue: global functions are colored as external vars even when called. so someGlobalFun() ends up colored differently than window.someGlobalFun() which seems inconsistent with the other highlighting choices. I suppose if one wanted to be super detailed, that could have a separate face for external function invocation, which would mirror the behavior for variables (local declarations and external uses are highlighted with different faces).

[dgutov]

global functions are colored as external vars even when called

That's intentional: they are external vars, as well as functions. They get two faces applied, one through text property, and another through an overlay.

[nicolasartman]

Ahh I see now, sorry for the false report!

[dgutov]

By the way, you can rid of the "external var" highlighting, among other ways, by declaring said vars at the top of the file with JSLint-style /*global declaration.

[ksjogo]

I can recommend https://github.com/Fanael/rainbow-identifiers for anybody interested in having more color on his screen. It really helps me to see what is matching/different.

[dgutov]

https://github.com/jacksonrayhamilton/context-coloring is another related project.

[ksjogo]

Interesting. Would be great to have a combination of booth.

[nicolasartman]

@ksjogo that is fantastic, thank you for pointing me to that.

@dgutov with the properties change in place now, simply using rainbow identifiers allows me to get the highlighting I was hoping for, so i don't think there's any need to build the additional var usage highlighting into js2-mode. Please feel free to close this issue as completed and accept my sincere thanks for your excellent work.

[dgutov]

@nicolasartman All right then. You're welcome.