-
Notifications
You must be signed in to change notification settings - Fork 51
Conversation
Don't pull yet, noticed other unscoped brackets: number(2) |
@50Wliu This is now ready for review. |
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.
Looks good, just some spec comments.
spec/grammar-spec.coffee
Outdated
@@ -167,3 +489,39 @@ describe "SQL grammar", -> | |||
expect(tokens[3]).toEqual value: ' WITH ', scopes: ['source.sql', 'comment.block.sql'] | |||
expect(tokens[4]).toEqual value: '*/', scopes: ['source.sql', 'comment.block.sql', 'punctuation.definition.comment.sql'] | |||
expect(tokens[6]).toEqual value: 'AND', scopes: ['source.sql', 'keyword.other.DML.sql'] | |||
|
|||
it 'tokenizes ()', -> |
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.
tokenizes parentheses
spec/grammar-spec.coffee
Outdated
expect(tokens[13]).toEqual value: ' employees', scopes: ['source.sql'] | ||
expect(tokens[14]).toEqual value: ')', scopes: ['source.sql', 'punctuation.definition.section.bracket.round.end.sql'] | ||
|
||
it 'tokenizes ,', -> |
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.
commas
spec/grammar-spec.coffee
Outdated
expect(tokens[1]).toEqual value: ',', scopes: ['source.sql', 'punctuation.separator.comma.sql'] | ||
expect(tokens[2]).toEqual value: ' year', scopes: ['source.sql'] | ||
|
||
it 'tokenizes .', -> |
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.
periods
spec/grammar-spec.coffee
Outdated
expect(tokens[1]).toEqual value: '.', scopes: ['source.sql', 'punctuation.separator.period.sql'] | ||
expect(tokens[2]).toEqual value: 'table', scopes: ['source.sql', 'constant.other.table-name.sql'] | ||
|
||
it 'tokenizes ;', -> |
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.
semicolons
spec/grammar-spec.coffee
Outdated
@@ -167,3 +489,39 @@ describe "SQL grammar", -> | |||
expect(tokens[3]).toEqual value: ' WITH ', scopes: ['source.sql', 'comment.block.sql'] | |||
expect(tokens[4]).toEqual value: '*/', scopes: ['source.sql', 'comment.block.sql', 'punctuation.definition.comment.sql'] | |||
expect(tokens[6]).toEqual value: 'AND', scopes: ['source.sql', 'keyword.other.DML.sql'] | |||
|
|||
it 'tokenizes ()', -> |
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.
Maybe this could be under a describe 'punctuation', ->
block as well.
spec/grammar-spec.coffee
Outdated
{tokens} = grammar.tokenizeLine('timetz (2)') | ||
expect(tokens[0]).toEqual value: 'timetz', scopes: ['source.sql', 'storage.type.sql'] | ||
expect(tokens[2]).toEqual value: '2', scopes: ['source.sql', 'constant.numeric.sql'] | ||
it 'tokenizes column types', -> |
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.
This is a rather massive test. We don't have to test for everything, just smoke-test 1-2 from each category (and consider separating into different tests).
()
aspunctuation.definition.section|parameters.bracket.round.begin|end.sql
,
aspunctuation.separator(.parameters).comma.sql
.
aspunctuation.separator.period.sql
;
aspunctuation.terminator.statement.semicolon.sql
nchar
andbit varying
, removedvar char
(with a space, couldn't find any documentation on this type)Fixes #68