Skip to content

Conversation

@apparebit
Copy link

As described in #140, the tokenizer already handles quoted identifiers but the current AST drops that information onto the floor again. This pull request adds the same kind of quoted identifier support to the AST, replacing the Ident type alias for String with a struct that closely resembles Word. The changes are relatively small, with most lines uplifting the tests.

This pull request closes #140. I am out of office next week (i.e., the one starting Monday 10/21). So please feel free to merge (or to make minor changes).

@benesch
Copy link
Contributor

benesch commented Oct 20, 2019

Thanks, Robert, this looks great! No review feedback whatsoever. I'm cargo fmting, squashing, and merging.

The Ident type was previously an alias for a String. Turn it into a full
fledged struct, so that the parser can preserve the distinction between
identifier value and quote style already made by the tokenizer's Word
structure.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 480

  • 121 of 122 (99.18%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ast/mod.rs 27 28 96.43%
Totals Coverage Status
Change from base Build 476: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

@benesch benesch merged commit 4cdd6e2 into apache:master Oct 20, 2019
@apparebit
Copy link
Author

Good to hear that it was acceptable. And thank you for doing the cleanup. I have CLion configured to rustfmt on Save but it appears to be unreliable. Sorry about that.

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.

Parser mishandles quoted column names

4 participants