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

Adds the scanner rules for punctuation #30

Merged
merged 2 commits into from
Jun 3, 2021
Merged

Conversation

almann
Copy link
Contributor

@almann almann commented Jun 2, 2021

Adds the Pest grammar rules for these tokens and particularly
adds the lookahead assertions for the various edge cases around the
punctuation tokens that interact with each other and comments.

Adds special Content enum variants for ./*/? and a basic variant
for Operator and Delimiter.

This should the final pre-requisite for all of the terminal parse rules in
the PEG and allow us to start adding the parser rules for expressions.

An explicit TODO is around modeling the various operators as their own
enum or enum variants, right now they are returned as normalized string
content.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Adds the Pest grammar rules for these tokens and particularly
adds the lookahead assertions for the various edge cases around the
punctuation tokens that interact with each other and comments.

Adds special `Content` enum variants for `.`/`*`/`?` and a basic variant
for `Operator` and `Delimiter`.

This should the final pre-requisite for all of the terminal parse rules in
the PEG and allow us to start adding the parser rules for expressions.

An explicit TODO is around modeling the various operators as their own
enum or enum variants, right now they are returned as normalized string
content.
@almann almann requested review from marcbowes and alancai98 June 2, 2021 05:01
@codecov
Copy link

codecov bot commented Jun 2, 2021

Codecov Report

Merging #30 (b7e2e91) into main (d309746) will increase coverage by 1.63%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   88.97%   90.60%   +1.63%     
==========================================
  Files           7        7              
  Lines         408      479      +71     
==========================================
+ Hits          363      434      +71     
  Misses         45       45              
Impacted Files Coverage Δ
partiql-parser/src/scanner.rs 95.07% <100.00%> (+1.37%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d309746...b7e2e91. Read the comment docs.

@almann
Copy link
Contributor Author

almann commented Jun 2, 2021

An explicit TODO is around modeling the various operators as their own
enum or enum variants, right now they are returned as normalized string
content.

See #31 for details on this TODO.

Comment on lines +246 to +248
Rule::Dot_ => Content::Dot,
Rule::Star_ => Content::Star,
Rule::Parameter => Content::Parameter,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason tests haven't been added for Dot, Star, and Parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, got so caught up in my other punctuation tests especially around potentially ambiguous cases that I forgot. will add.

Fixes ambiguity around decimal and dot.
@almann almann merged commit d257e3e into partiql:main Jun 3, 2021
@almann almann deleted the punctuation branch June 3, 2021 19:51
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