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

Distinguishing from comparison operators #188

Closed
smaili opened this issue Aug 4, 2015 · 17 comments · Fixed by #225
Closed

Distinguishing from comparison operators #188

smaili opened this issue Aug 4, 2015 · 17 comments · Fixed by #225

Comments

@smaili
Copy link

smaili commented Aug 4, 2015

I've noticed that these two rules use the same grouping:

  {
    'match': '(?<!\\.)\\b(delete|in|of|instanceof|new|typeof|void)(?!\\s*:)\\b'
    'name': 'keyword.operator.js'
  }
  {
    'match': '!=|!==|<=|>=|<<=|>>=|>>>=|\\*=|(?<!\\()/=|%=|\\+=|\\-=|&=|\\^=|!|%|&|\\*|\\-\\-|\\-|\\+\\+|\\+|~|===|==|=|<>|<|>|!|&&|\\|\\||\\?|\\:|\\^'
    'comment': 'match 2-character operator first'
    'name': 'keyword.operator.js'
  }

By using the same groups for both, we can't have syntax highlighting be applied to keywords like delete and typeof. What would be the best way to distinguish these? Maybe remove the keyword class from the second group of operators?

@chbk
Copy link

chbk commented Aug 25, 2015

How about:

  • keyword.operator.language.js for delete, in, of, instanceof, new, typeof, void
  • keyword.operator.js for =, !, <, >, +, -, %, and other symbols

Since the language class is used by true, false, null, super, this, Infinity, NaN, and undefined, it would fit nicely with the operators.

@smaili
Copy link
Author

smaili commented Aug 25, 2015

I like it 👍

pchaigno added a commit to pchaigno/language-javascript that referenced this issue Sep 7, 2015
pchaigno added a commit to pchaigno/language-javascript that referenced this issue Sep 7, 2015
pchaigno added a commit to pchaigno/language-javascript that referenced this issue Sep 7, 2015
@MaximSokolov
Copy link
Contributor

Since the language class is used by true, false, null, super, this, Infinity, NaN, and undefined, it would fit nicely with the operators.

Disagree. variable.language scope separate user defined variables from language variables. Scopes should be semantic. I don't see any reason why delete, in, of, instanceof, new, typeof, void are language operators and *=, !, <, >, +, -, %, and other symbols are not.

I proposed keyword.operator.comparison , keyword.operator.arithmetic , keyword.operator.logical (and others) scopes. Seems these scopes are suitable for this goal.

variable

  • parameter
  • language — reserved language variables like this, super, self, etc.
  • other

@chbk
Copy link

chbk commented Sep 7, 2015

I don't see any reason why delete, in, of, instanceof, new, typeof, void are language operators and *=, !, <, >, +, -, %, and other symbols are not.

In that case why use a "language" scope at all, if it's so broad? A "reserved" scope would be less ambiguous, and it could be shared with non-arithmetic operators.

IMO the doc should take into account which categories are commonly highlighted together, since that's the main purpose of scopes. delete, in, of, instanceof, new, typeof, void are usually highlighted because they are reserved words not just operators.

@MaximSokolov
Copy link
Contributor

In that case why use a "language" scope at all, if it's so broad?

language scope isn't broad. variable.language and constant.language scopes have specific goal: match variables/contants provided by language. e.g true is language constant while 123 is not (it's constant.numeric).

IMO the doc should take into account which categories are commonly highlighted together, since that's the main purpose of scopes. delete, in, of, instanceof, new, typeof, void are usually highlighted because they are reserved words not just operators.

I agree. Scopes should have common attribute. However these scopes don't have nothing in common. Some of these "operators" aren't even a keyword.operator. I think we shouldn't change such scopes until we would have a clear documentation. Otherwise would be much more troubles with syntax highlighting because of misunderstanding like in this issue.

@VitaliyR
Copy link

@MaximSokolov hello. Actually there are already troubles with highlighting. I really wanna make logical operators to be white (e.g.) and instanceof/delete etc to be orange but for now - I can't (actually I have a start script which make that job for me, but it is not a solution)

@MaximSokolov
Copy link
Contributor

IMO sub-scopes can be:
in, of, &&, ||, !keyword.operator.logical
~, &, ^, |keyword.operator.bitwise
*, -, +, %keyword.operatorarithmetic
<=, >=, !=, !==, ===, ==, <, >keyword.operator.comparison
=keyword.operator.assignment
/=, %=, +=, -=, *=, **=keyword.operator.assignment.augmentedcompound
|=, &=, ^=, <<=, >>=, >>>=keyword.operator.assignment.~~augmented~~compound.bitwise --keyword.operator.decrement ++keyword.operator.increment delete, new, instanceof, typeof— don't known what common sub-scope shoud be for them. But not akeyword.operator.language! It can be keyword.operator.(e.g.keyword.operator.new) ...keyword.operator.spread` #291

@Pauan
Copy link

Pauan commented Sep 16, 2015

@MaximSokolov I generally agree with your list, except... I'm curious why you listed in and of as being keyword.operator.logical

In my opinion, in and of should not be keyword.operator.logical, but should have the same rule as delete, new, typeof, etc.

@MaximSokolov
Copy link
Contributor

The in operator returns true if the specified property is in the specified object.

result = property in object

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/in
https://msdn.microsoft.com/en-us/library/9k25hbz2(v=vs.94).Aspx

Couldn't find any specific for "of" operator. I suppose i of arr returns boolean:

for (let i of arr) {}

https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Statements/for...of

@chbk
Copy link

chbk commented Sep 16, 2015

I agree with @Pauan, why put in and of in keyword.operator.logical but leave out instanceof?
Anyway, those aren't even logical operators: JavaScript Logical Operators

@chbk
Copy link

chbk commented Sep 16, 2015

@MaximSokolov I suggest you replace augmented with alteration. It would fit better with the other scope names.

@chbk
Copy link

chbk commented Sep 16, 2015

@MaximSokolov JavaScript Reserved Keywords
I suggest adding a reserved scope that can be shared by different tokens.

@MaximSokolov
Copy link
Contributor

why put in and of in keyword.operator.logical but leave out instanceof?

I was mistaken because of Python definition of logical operators. According formal difinition in, of, instanceof are not logical operators.

but why weren't delete and new added to keyword.control?

'new', 'delete' aren't related to flow control.

I suggest you replace augmented with alteration...

Ruby, Python, Go already use keyword.operator.assignment.augmented

...It would fit better with the other scope names.

What other scope names?

I suggest adding a reserved scope that can be shared by different tokens.

Give examples of unreserved tokens please.

@chbk
Copy link

chbk commented Sep 16, 2015

new, delete aren't related to flow control.

Yes I agree that delete and new aren't flow control keywords, I edited my comment shortly after posting.

Ruby, Python, Go already use keyword.operator.assignment.augmented

I wasn't suggesting changing it right now. I feel it's something to consider while working on the docs.

What other scope names?

I mean that comparison, assignment, decrement, increment, etc. are all nouns, yet augmented is an adjective. Why? But more importantly augmented is too specific, using alteration would be wiser as it doesn't imply the variable is being increased.

Give examples of unreserved tokens please.

IMO all keywords that are listed here should share a reserved scope. Anything not in that list is "unreserved" in the sense that it's not a word used for language functionality.

@MaximSokolov
Copy link
Contributor

augmented is too specific, using alteration would be wiser as it doesn't imply the variable is being increased.

It means "assignment augmented by another operator" Why it shouldn't be specific?

Give examples of unreserved tokens please.

IMO all keywords that are listed here should share a reserved scope. Anything not in that list is "unreserved" in the sense that it's not a word used for language functionality.

"Reserved" keywords there means they cannot be used as identifiers. reserved will not append any useful information about language symbol.

@chbk
Copy link

chbk commented Sep 16, 2015

It means "assignment augmented by another operator"

That makes much more sense, I didn't interpret it that way.

"Reserved" keywords there means that they cannot be used as identifiers. reserved will not append any useful information about language symbol.

I see what you mean, but you also said scopes should have common attributes, and even though it doesn't add information, it does separate delete, in, instanceof, new, typeof from other operators, and makes it extremely convenient to apply a single syntax coloring rule to all reserved words at once.

@MaximSokolov
Copy link
Contributor

All keywords from grammar are "reserved". You can't use && as identifier either.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants