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

Fixed highlighting of brackets, punctuations and identifiers. #52

Merged
merged 5 commits into from
Jul 27, 2015
Merged

Fixed highlighting of brackets, punctuations and identifiers. #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc6e849
Fixed highlighting of brackets, punctuations and identifiers.
Jul 23, 2015
afdbccf
Adjustments after feedback from @svanharmelen
Jul 24, 2015
6023332
Forgot to revert specs.
Jul 24, 2015
02ec1f1
Removed `;` from operators list.
Jul 25, 2015
8ac94d6
Made package name regex more robust
Victorystick Jul 26, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 40 additions & 8 deletions grammars/go.cson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
}
{
'name': 'storage.type.go'
'match': '\\b(chan|map|bool|string|error|int|int8|int16|int32|int64|rune|byte|uint|uint8|uint16|uint32|uint64|uintptr|float32|float64|complex64|complex128)\\b'
'match': '\\b(bool|string|error|int|int8|int16|int32|int64|rune|byte|uint|uint8|uint16|uint32|uint64|uintptr|float32|float64|complex64|complex128)\\b'
Copy link
Contributor

Choose a reason for hiding this comment

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

Even through your absolutely correct that both chan and map are listed as reserved (key)words, they are also both types (https://golang.org/ref/spec#Map_types and https://golang.org/ref/spec#Channel_types) listed and explained right along side all other types (https://golang.org/ref/spec#Types).

Taking that into account and looking at how the words are used within the language (to define types) I think it makes much more sense to assign the storage.type.go scope instead of the keyword.go scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is true that chan and map are keywords used to define types; your opinion isn't unfounded.

However, the specification does list them as keywords. Additionally, all other type related keywords type, struct, interface and func are arguably also used to define types, but they aren't marked with the storage.type.go scope; they have the keyword.go scope. So should chan and map.

Consider this example using all type related keywords:

type oddMap map[*func(interface{}) int]<-chan struct{}

Copy link
Contributor

Choose a reason for hiding this comment

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

If you look it this with the theme I'm using, the current scoping still makes a lot more sense to me:

image
It makes very clear that I'm using the types map and chan. If you make them as keywords they fade away together with all the other keywords making the same line harder and less clear to read:

image

So IMO this should really be scoped as storage.type.go.

@joefitzgerald what's your take on this specific one? It seems the language spec leaves room for both keyword.go and storage.type.go to be used. So it comes down to usability and personal preference I think...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit of a silly example, of course. It's very unlikely to use all type related keywords together like that.

The difference with the new scopes on a more likely example is more appealing:
screen shot 2015-07-24 at 13 08 45
and
screen shot 2015-07-24 at 13 09 09
Builtin functions, keywords, primitive types and other types, are all highlighted differently.

}
{
'name': 'invalid.illegal.numeric.go'
Expand All @@ -105,12 +105,18 @@
}
{
'name': 'keyword.go'
'match': '\\b(func|var|const|type|struct|interface|case|default)\\b'
'match': '\\b(chan|map|func|var|const|type|struct|interface|case|default)\\b'
}
{
'name': 'keyword.directive.go'
'match': '\\b(package|import)\\b'
}
{
'match': '\\bpackage\\b\\s+([\\w_][\\w\\d_]*)'
'captures':
'1':
'name': 'entity.name.package.go'
}
{
'name': 'keyword.statement.go'
'match': '\\b(defer|go|goto|return|break|continue|fallthrough)\\b'
Expand All @@ -127,6 +133,10 @@
'name': 'constant.language.go'
'match': '\\b(iota|true|false|nil)\\b'
}
{
'match': '\\;'
'name': 'meta.terminator.semicolon.go'
}
{
'include': '#string_escaped_char'
}
Expand All @@ -142,6 +152,9 @@
{
'include': '#variables'
}
{
'include': '#delimiters'
}
]
'repository':
'string_escaped_char':
Expand All @@ -159,22 +172,41 @@
'patterns': [
{
"name": "keyword.operator.go",
"match": "(\\+|&|\\+=|&=|&&|==|!=|\\-|\\||\\-=|\\|=|\\|\\||<|<=|\\*|\\^|\\*=|\\^=|<\\-|>|>=|/|<<|/=|<<=|\\+\\+|=|:=|;|%|>>|%=|>>=|\\-\\-|!|\\.\\.\\.|&\\^|&\\^=)"
"match": "(\\+|&|\\+=|&=|&&|==|!=|\\-|\\||\\-=|\\|=|\\|\\||<|<=|\\*|\\^|\\*=|\\^=|<\\-|>|>=|/|<<|/=|<<=|\\+\\+|=|:=|%|>>|%=|>>=|\\-\\-|!|\\.\\.\\.|&\\^|&\\^=)"
}
{
'match': '\\{|\\}'
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the specs these are not delimiters, but operators. If you are keen on having them named more precise, I would suggest scopes like keyword.operator.brace.curly.go and keyword.operator.brace.square.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since braces aren't listed as keywords in the spec, I don't think they should have a scope name with keyword in them.

The braces are described in the section on Operators and Delimiters, but they are never mentioned in the Operators section. I concluded that they are are not operators, and must therefore be delimiters.

The reason for changing the scopes is to better align braces with how other packages (C++, C#, JavaScript, CSS, Perl, Shell, Python etc.) treat braces. The advantage of aligning with them is that a larger number of syntax highlighting themes will work well equally with Go's scopes. From these few examples of { in other packages, I concluded that meta, punctuation and scope are good scope names for curly braces, besides brace.curly of course.
screen shot 2015-07-24 at 12 10 49

What is your opinion on meta.scope.brace.curly.go?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since braces aren't listed as keywords in the spec, I don't think they should have a scope name with keyword in them.

The braces are described in the section on Operators and Delimiters, but they are never mentioned in the Operators section. I concluded that they are are not operators, and must therefore be delimiters.

Fair point... So I guess meta.brace.curly.go would make sense then...

'name': 'meta.brace.curly.go'
}
{
"name": "keyword.operator.bracket.go",
"match": "(\\(|\\)|\\[|\\]|\\{|\\})"
'match': '\\(|\\)'
'name': 'meta.brace.round.go'
}
{
"name": "keyword.operator.punctuation.go",
"match": "(\\.|,|:)"
'match': '\\[|\\]'
'name': 'meta.brace.square.go'
}
]
'variables':
'patterns': [
{
'name': 'variable.go'
'match': '\\b[\\w_][\\w\\d_]*\\b',
'match': '\\b[\\w_][\\w\\d_]*\\b'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would you remove the variable.go scope here? I would make sense to catch the package name so it doesn't get scopes as a variable, but for the rest this is perfectly correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be added back, if you prefer. We'd have to be careful around type declarations, etc. since a type name isn't a variable.

type User struct {
  name string
  yob  int
}

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true, but those (type's) it seem to be hard to scope them consistently as also discussed on Slack and here: #49

For the package name it should be no problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... Yes, that is true. While I understand that it's a lot more work, it would be really neat to have a highlighter which is context aware.

}
]
'delimiters':
Copy link
Contributor

Choose a reason for hiding this comment

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

For this also goes that these are operators, so we not scope it as such?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These should be delimiters for the same reason as braces.

Copy link
Contributor

Choose a reason for hiding this comment

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

Check... Even through these (and the braces) operators are not documented in more detail by the language specs, they really are still operators. They could (and maybe should) have been documented more along these lines for example: https://msdn.microsoft.com/en-us/library/6a71f45d.aspx

But then again, I agree that as they are not described and other language packages seem to describe them as delimiters I think the scopes like meta.delimiter.comma.go, meta.delimiter.period.go, meta.brace.curly.go and meta.brace.square.go these should be ok...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C# really documents those operators very neatly! While I love the simplicity of Go, including it's website and documentation, sometimes it's a bit too plain. It would have been nice to have something similar. 😉

'patterns': [
{
'match': ','
'name': 'meta.delimiter.comma.go'
}
{
'match': '\\.'
'name': 'meta.delimiter.period.go'
}
{
'match': ':'
'name': 'meta.delimiter.colon.go'
}
]
'printf_verbs':
Expand Down
96 changes: 62 additions & 34 deletions spec/go-spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ describe 'Go grammar', ->

it 'tokenizes types', ->
types = [
'chan', 'map', 'bool', 'string', 'error', 'int', 'int8', 'int16'
'bool', 'string', 'error', 'int', 'int8', 'int16'
'int32', 'int64', 'rune', 'byte', 'uint', 'uint8', 'uint16', 'uint32'
'uint64', 'uintptr', 'float32', 'float64', 'complex64', 'complex128'
]
Expand Down Expand Up @@ -185,7 +185,7 @@ describe 'Go grammar', ->

next = tokens[t.tokenPos + 1]
expect(next.value).toEqual '('
expect(next.scopes).toEqual ['source.go', 'keyword.operator.bracket.go']
expect(next.scopes).toEqual ['source.go', 'meta.brace.round.go']

it 'only tokenizes "func" when it is an exact match', ->
tests = ['myfunc', 'funcMap']
Expand All @@ -196,28 +196,33 @@ describe 'Go grammar', ->

it 'tokenizes func names in their declarations', ->
tests = [
{
'line': 'func f()'
'tokenPos': 2
}
# {
# 'line': 'func f()'
# 'tokenPos': 2
# }
{
'line': 'func (T) f()'
'tokenPos': 6
}
{
'line': 'func (t T) f()'
'tokenPos': 8
}
{
'line': 'func (t *T) f()'
'tokenPos': 9
}
# {
# 'line': 'func (t T) f()'
# 'tokenPos': 8
# }
# {
# 'line': 'func (t *T) f()'
# 'tokenPos': 9
# }
]

for t in tests
{tokens} = grammar.tokenizeLine t.line
expect(tokens[0].value).toEqual 'func'
expect(tokens[0].scopes).toEqual ['source.go', 'keyword.go']
expect(tokens[1].value).toEqual ' '
expect(tokens[2].value).toEqual '('
expect(tokens[3].value).toEqual 'T'
expect(tokens[4].value).toEqual ')'
expect(tokens[5].value).toEqual ' '

relevantToken = tokens[t.tokenPos]
expect(relevantToken).toBeDefined()
Expand All @@ -226,7 +231,7 @@ describe 'Go grammar', ->

next = tokens[t.tokenPos + 1]
expect(next.value).toEqual '('
expect(next.scopes).toEqual ['source.go', 'keyword.operator.bracket.go']
expect(next.scopes).toEqual ['source.go', 'meta.brace.round.go']

it 'tokenizes receiver types in method declarations', ->
tests = [
Expand Down Expand Up @@ -256,7 +261,7 @@ describe 'Go grammar', ->

next = tokens[t.tokenPos + 1]
expect(next.value).toEqual ')'
expect(next.scopes).toEqual ['source.go', 'keyword.operator.bracket.go']
expect(next.scopes).toEqual ['source.go', 'meta.brace.round.go']

it 'tokenizes numerics', ->
numerics = [
Expand Down Expand Up @@ -300,7 +305,7 @@ describe 'Go grammar', ->
opers = [
'+', '&', '+=', '&=', '&&', '==', '!=', '-', '|', '-=', '|=', '||', '<',
'<=', '*', '^', '*=', '^=', '<-', '>', '>=', '/', '<<', '/=',
'<<=', '++', '=', ':=', ';', '%', '>>', '%=', '>>=', '--', '!', '...',
'<<=', '++', '=', ':=', '%', '>>', '%=', '>>=', '--', '!', '...',
'&^', '&^='
]

Expand All @@ -317,33 +322,56 @@ describe 'Go grammar', ->

it 'tokenizes bracket operators', ->
opers = [
'[', ']', '(', ')', '{', '}'
{
values: [ '[', ']' ]
scope: 'meta.brace.square.go'
}
{
values: [ '(', ')' ]
scope: 'meta.brace.round.go'
}
{
values: [ '{', '}' ]
scope: 'meta.brace.curly.go'
}
]

for op in opers
{tokens} = grammar.tokenizeLine op
for ops in opers
for op in ops
{tokens} = grammar.tokenizeLine op

fullOp = tokens.map((tok) -> tok.value).join('')
expect(fullOp).toEqual op
fullOp = tokens.map((tok) -> tok.value).join('')
expect(fullOp).toEqual op

scopes = tokens.map (tok) -> tok.scopes
allKeywords = scopes.every (scope) -> 'keyword.operator.bracket.go' in scope
scopes = tokens.map (tok) -> tok.scopes
allKeywords = scopes.every (scope) -> ops.scope in scope

expect(allKeywords).toBe true
expect(allKeywords).toBe true

it 'tokenizes punctuation operators', ->
opers = [
'.', ',', ':'
{
value: ','
scope: 'meta.delimiter.comma.go'
}
{
value: '.'
scope: 'meta.delimiter.period.go'
}
{
value: ':'
scope: 'meta.delimiter.colon.go'
}
]

for op in opers
{tokens} = grammar.tokenizeLine op
{tokens} = grammar.tokenizeLine op.value

fullOp = tokens.map((tok) -> tok.value).join('')
expect(fullOp).toEqual op
expect(fullOp).toEqual op.value

scopes = tokens.map (tok) -> tok.scopes
allKeywords = scopes.every (scope) -> 'keyword.operator.punctuation.go' in scope
allKeywords = scopes.every (scope) -> op.scope in scope

expect(allKeywords).toBe true

Expand Down Expand Up @@ -400,7 +428,7 @@ describe 'Go grammar', ->

next = tokens[t.tokenPos + 1]
expect(next.value).toEqual '('
expect(next.scopes).toEqual ['source.go', 'keyword.operator.bracket.go']
expect(next.scopes).toEqual ['source.go', 'meta.brace.round.go']
else
expect(relevantToken.scopes).not.toEqual want

Expand All @@ -409,23 +437,23 @@ describe 'Go grammar', ->
expect(token.value).toBe 'var'
expect(token.scopes).toEqual ['source.go', 'keyword.go']

wantedScope = ['source.go', 'variable.go']
plainScope = ['source.go', 'variable.go']

testName = (token, name) ->
expect(token.value).toBe name
expect(token.scopes).toEqual wantedScope
expect(token.scopes).toEqual plainScope

testOp = (token, op) ->
expect(token.value).toBe op
expect(token.scopes).toEqual ['source.go', 'keyword.operator.go']

testOpBracket = (token, op) ->
expect(token.value).toBe op
expect(token.scopes).toEqual ['source.go', 'keyword.operator.bracket.go']
expect(token.scopes).toEqual ['source.go', 'meta.brace.round.go']

testOpPunctuation = (token, op) ->
expect(token.value).toBe op
expect(token.scopes).toEqual ['source.go', 'keyword.operator.punctuation.go']
expect(token.scopes).toEqual ['source.go', 'meta.delimiter.comma.go']

testType = (token, name) ->
expect(token.value).toBe name
Expand Down