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

Support informal delimiter literals #64

Merged
merged 2 commits into from Jun 7, 2020
Merged

Support informal delimiter literals #64

merged 2 commits into from Jun 7, 2020

Conversation

@ammar
Copy link
Owner

ammar commented Jun 6, 2020

Interprets the {, }, and ] delimiters as literals depending on the context.

Notes

  • Reduce ambiguity in the quantifier_interval pattern by replacing the optional matchers with 4 fixed patterns.
  • Remove the premature_end_error final state handler from quantifier_interval to permit valid literal sequences.

Closes #63

@ammar ammar requested a review from jaynetics Jun 6, 2020
Copy link
Collaborator

jaynetics left a comment

looks great!

i've checked some more edge cases and found no issues. maybe we should add one example to spec/parser/quantifiers_spec.rb to document that leading zeros are correctly ignored as in MRI:

include_examples 'quantifier', /a{004}+b/, '{004}+', :possessive, :interval, 4, 4

quantifier_exact = range_open . (digit+) . range_close . quantifier_mode?;
quantifier_minimum = range_open . (digit+) . ',' . range_close . quantifier_mode?;
quantifier_maximum = range_open . ',' . (digit+) . range_close . quantifier_mode?;
quantifier_range = range_open . (digit+) . ',' . (digit+) .
range_close . quantifier_mode?;

quantifier_interval = quantifier_exact | quantifier_minimum |
quantifier_maximum | quantifier_range;

Comment on lines 65 to 73

This comment has been minimized.

Copy link
@jaynetics

jaynetics Jun 6, 2020

Collaborator

slightly more concise:

Suggested change
quantifier_exact = range_open . (digit+) . range_close . quantifier_mode?;
quantifier_minimum = range_open . (digit+) . ',' . range_close . quantifier_mode?;
quantifier_maximum = range_open . ',' . (digit+) . range_close . quantifier_mode?;
quantifier_range = range_open . (digit+) . ',' . (digit+) .
range_close . quantifier_mode?;
quantifier_interval = quantifier_exact | quantifier_minimum |
quantifier_maximum | quantifier_range;
quantity_exact = (digit+);
quantity_minimum = (digit+) . ',';
quantity_maximum = ',' . (digit+);
quantity_range = (digit+) . ',' . (digit+);
quantifier_interval = range_open . ( quantity_exact | quantity_minimum |
quantity_maximum | quantity_range ) . range_close .
quantitfier_mode?;

but this is more a matter of taste, ragel probably optimises away the redundancy (?)

This comment has been minimized.

Copy link
@ammar

ammar Jun 7, 2020

Author Owner

I like it a lot. The intention is clearer. There's a typo in the name of the mode pattern so I will make the change manually.

@ammar
Copy link
Owner Author

ammar commented Jun 7, 2020

Thank you for the review. Very helpful.

i've checked some more edge cases and found no issues. maybe we should add one example to spec/parser/quantifiers_spec.rb to document that leading zeros are correctly ignored as in MRI:

include_examples 'quantifier', /a{004}+b/, '{004}+', :possessive, :interval, 4, 4

Good call. I will add it.

I will wrap this up and push out a new release (1.8) this afternoon.

@ammar
Copy link
Owner Author

ammar commented Jun 7, 2020

On second thought, I'll consider this a bug fix, and release as 1.7.1.

@ammar ammar merged commit ae68d66 into master Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.