Skip to content

Commit

Permalink
Improved JS constant pattern (#1737)
Browse files Browse the repository at this point in the history 
This changes the JS constant pattern so that all WebGL constants (like `FLOAT_MAT2x4`) are matched.
  • Loading branch information
@RunDevelopment
RunDevelopment authored Feb 28, 2019
1 parent 8378ac8 commit 7bcec58
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 24 deletions.
2 changes: 1 addition & 1 deletion components/prism-javascript.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Prism.languages.insertBefore('javascript', 'keyword', {
inside: Prism.languages.javascript
}
],
'constant': /\b[A-Z][A-Z\d_]*\b/
'constant': /\b[A-Z](?:[A-Z_]|\dx?)*\b/
});

Prism.languages.insertBefore('javascript', 'string', {
Expand Down
2 changes: 1 addition & 1 deletion components/prism-javascript.min.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion prism.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ Prism.languages.insertBefore('javascript', 'keyword', {
inside: Prism.languages.javascript
}
],
'constant': /\b[A-Z][A-Z\d_]*\b/
'constant': /\b[A-Z](?:[A-Z_]|\dx?)*\b/
});

Prism.languages.insertBefore('javascript', 'string', {
Expand Down
38 changes: 17 additions & 21 deletions tests/languages/javascript/constant_feature.test
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
var FOO;
const FOO_BAR;
const BAZ42;

----------------------------------------------------

[
["keyword", "var"],
["constant", "FOO"],
["punctuation", ";"],
["keyword", "const"],
["constant", "FOO_BAR"],
["punctuation", ";"],
["keyword", "const"],
["constant", "BAZ42"],
["punctuation", ";"]
]

----------------------------------------------------

Checks for constants.
var FOO;
const FOO_BAR;
const BAZ42;
gl.FLOAT_MAT2x4;

----------------------------------------------------

[
["keyword", "var"], ["constant", "FOO"], ["punctuation", ";"],
["keyword", "const"], ["constant", "FOO_BAR"], ["punctuation", ";"],
["keyword", "const"], ["constant", "BAZ42"], ["punctuation", ";"],
"\ngl", ["punctuation", "."], ["constant", "FLOAT_MAT2x4"], ["punctuation", ";"]
]

----------------------------------------------------

Checks for constants.

5 comments on commit 7bcec58

@joshgoebel
Copy link

Choose a reason for hiding this comment

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

Isn't this more of a coding style choice though? That fact that JS has constant as a rule at all and that it has nothing to do with const? I'm asking because I actually like this type of thing and trying to think about how I might justify adding it to Highlight.js also. I wonder does Prism have a philosophy on this kind of thing or is it just pretty much whatever anyone contributes?

@RunDevelopment
Copy link
Member Author

Choose a reason for hiding this comment

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

I can't really say anything about Prism's philosophy but I can tell you about the one I'm using when writing language definitions.

I usually try to highlight concepts consistently across languages. constant is a good example. A constant is usually a value that does not change throughout the runtime of your program (e.g. gl.FLOAT_MAT2x4 in JS or NULL in C). There is the naming convention across a large number of languages (Java, C, C++, Rust to name a few) that constant names should be all uppercase. This naming convention makes it easy to detect constants and, IMO, it's useful to highlight them.
JS const variables aren't always constants since local variables might live only very briefly. So while unchanging, they aren't constants in that sense. However, an all-uppercase name expresses the intent of the author that this (maybe const) variable is a constant, so I think we should highlight that intent.

To answer your question: Yes it's just a coding style choice, but one which works pretty well since many developers actually use this naming convention to express their intent.

@joshgoebel
Copy link

Choose a reason for hiding this comment

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

Thanks for clarifying your thoughts! The trick for us is right now we don't have a concept of "constant" at all... :-( We end up mapping them to symbol or builtin, or some such.

@RunDevelopment
Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah... Missing concepts are a problem here too. E.g. we usually map preprocessors/macros to property.

@joshgoebel
Copy link

Choose a reason for hiding this comment

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

We have meta for such things are preprocessors.

Please sign in to comment.