Skip to content

Conversation

@gluax
Copy link
Contributor

@gluax gluax commented May 20, 2021

Closes #945.
Eventually, strings should use "..." instead of .... Similarly, in the inputs, it should use "..." instead of a ['a'...'b']. But, these changes can be done in feature/string-parsing once inputs are merged in, and we refactor string parsing to use "...".

Also found a bug with char input parsing that it cannot parse the space character - ' '. Tried messing with it for a while, but I am kind of stuck on why it won't work. Maybe someone more familiar with pest can take a look?

@gluax gluax added this to the Leo Developer Preview III milestone May 20, 2021
@gluax gluax requested review from acoglio, collinc97 and damirka May 20, 2021 18:36
@gluax gluax self-assigned this May 20, 2021
@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #979 (c8e63a2) into feature/string-parsing (a5f994a) will increase coverage by 0.04%.
The diff coverage is 97.56%.

Impacted file tree graph

@@                    Coverage Diff                     @@
##           feature/string-parsing     #979      +/-   ##
==========================================================
+ Coverage                   75.20%   75.24%   +0.04%     
==========================================================
  Files                         434      434              
  Lines                       17513    17539      +26     
==========================================================
+ Hits                        13170    13197      +27     
+ Misses                       4343     4342       -1     
Impacted Files Coverage Δ
parser/src/tokenizer/lexer.rs 90.90% <ø> (ø)
ast/src/reducer/canonicalization.rs 72.87% <90.00%> (+0.40%) ⬆️
ast/src/reducer/reconstructing_director.rs 91.61% <100.00%> (+0.39%) ⬆️
ast/src/reducer/reconstructing_reducer.rs 85.93% <100.00%> (ø)
compiler/src/phases/reducing_director.rs 48.34% <100.00%> (-0.15%) ⬇️
compiler/src/value/value.rs 63.86% <100.00%> (+1.25%) ⬆️
compiler/tests/canonicalization/mod.rs 100.00% <100.00%> (ø)
ast/src/expression/value.rs 72.97% <0.00%> (+2.70%) ⬆️

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 a5f994a...c8e63a2. Read the comment docs.

@acoglio
Copy link
Collaborator

acoglio commented May 20, 2021

Which Pest file?

@gluax
Copy link
Contributor Author

gluax commented May 20, 2021

@acoglio we only have one pest file - input/src/leo-input.pest.

@acoglio
Copy link
Collaborator

acoglio commented May 20, 2021

That narrows it down 😀

Perhaps you need to make this rule
https://github.com/AleoHQ/leo/blob/ca59ff3177ae34ceba2151b977d9fbe398583684/input/src/leo-input.pest#L151
atomic:

value_char = ${ "\'" ~ char_types ~ "\'" }

otherwise the implicit WHITESPACE rule may take precedence and eat the space. Using ${ and not @{ because, if I understand the Pest documentation correctly, we want the inner rule char_types to produce a token.

@gluax
Copy link
Contributor Author

gluax commented May 21, 2021

@acoglio good catch that fixed the issue!

Copy link
Collaborator

@collinc97 collinc97 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@acoglio acoglio left a comment

Choose a reason for hiding this comment

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

Looks good.

@gluax
Copy link
Contributor Author

gluax commented May 21, 2021

@acoglio We can merge this in since it's just going to another branch, and the Clippy failure exists there as well! We won't check it off the list on the string epic till it's fully merged into master.

@acoglio acoglio merged commit c63a549 into feature/string-parsing May 21, 2021
@acoglio acoglio deleted the feature/string-canonicalization branch May 21, 2021 19:11
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.

4 participants