Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP #122 support CQL comments #139

Merged
merged 6 commits into from
Feb 1, 2017

Conversation

Eman-Shaaban
Copy link
Member

@@ -38,7 +38,7 @@ object CqlParser extends JavaTokenParsers
parse(phrase(selectStatement <~ semicolon.?), input)

def parseDML(input: String): ParseResult[DataManipulation] =
parse(phrase(dmlDefinition <~ semicolon.?), input)
parse(phrase(dmlDefinition <~ semicolon.? <~ comment.?), input)
Copy link
Member

Choose a reason for hiding this comment

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

Comments wont't be supported inside queries. Instead they should be supported inside schema.cql file only.

def comment: Parser[Comment] = {
import Comment._

def str = "[0-9a-fA-F]".r
Copy link
Member

Choose a reason for hiding this comment

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

I am sure we have this regex defined somewhere else we can reuse

def str = "[0-9a-fA-F]".r
def dash = s"--$str".r ^^ SingleLineDash
def slash = s"//$str".r ^^ SingleLineSlash
def multi = s"/*$str*/".r ^^ MultiLine
Copy link
Member

Choose a reason for hiding this comment

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

This won't work, the regex won't allow new lines.
Maybe write a test case first

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 88.649% when pulling 6e277cb on Eman-Shaaban:feature/122-cql-comments into 368459f on cassandra-scala:master.


// Comments Parser
protected override val whiteSpace = """(\s|//.*|--.*|(?m)/\*(\*(?!/)|[^*])*\*/)+""".r
def comment = ident+
Copy link
Member

Choose a reason for hiding this comment

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

Is this line used?

@@ -74,11 +74,15 @@ object CqlParser extends JavaTokenParsers

str | blob | uuid | floats | int | boolean | nullConst
}

// Comments Parser
Copy link
Member

Choose a reason for hiding this comment

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

This comment can be something line "Ignores comments"

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 87.482% when pulling 7292dd8 on Eman-Shaaban:feature/122-cql-comments into 368459f on cassandra-scala:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 87.77% when pulling efe33cb on Eman-Shaaban:feature/122-cql-comments into 368459f on cassandra-scala:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 88.201% when pulling f126eff on Eman-Shaaban:feature/122-cql-comments into 368459f on cassandra-scala:master.

user_time time,
user_status boolean,
user_balance double,
user_trans_id bigint
Copy link
Member

Choose a reason for hiding this comment

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

What about adding test for comments similar to those?

Copy link
Member Author

Choose a reason for hiding this comment

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

in the same file CreateTableParserTest.scala ??

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 88.201% when pulling 748e47f on Eman-Shaaban:feature/122-cql-comments into 368459f on cassandra-scala:master.

@tabdulradi tabdulradi merged commit 836950a into schemasafe:master Feb 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants