-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Improved JS constant pattern (#1737)
This changes the JS constant pattern so that all WebGL constants (like `FLOAT_MAT2x4`) are matched.
- Loading branch information
1 parent
8378ac8
commit 7bcec58
Showing
4 changed files
with
20 additions
and
24 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
7bcec58
There was a problem hiding this comment.
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 withconst
? 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?7bcec58
There was a problem hiding this comment.
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 orNULL
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 (maybeconst
) 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.
7bcec58
There was a problem hiding this comment.
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
orbuiltin
, or some such.7bcec58
There was a problem hiding this comment.
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
.7bcec58
There was a problem hiding this comment.
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.