-
Notifications
You must be signed in to change notification settings - Fork 23
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
Improve set handling #55
Conversation
# Conflicts: # lib/regexp_parser/expression/subexpression.rb # lib/regexp_parser/lexer.rb # lib/regexp_parser/parser.rb # lib/regexp_parser/scanner/scanner.rl
# Conflicts: # lib/regexp_parser/syntax/ruby/1.8.6.rb # lib/regexp_parser/syntax/ruby/1.9.1.rb # lib/regexp_parser/syntax/ruby/2.0.0.rb # test/syntax/versions/test_1.8.rb
Turns out I should have read the docs... Intersections apply to all expressions in their set, not just adjacent ones. 'abc1'.scan(/[a b \d && b c [:digit:]]/x) # => ["b", "1"]
'abc1'.scan(/[^a b \d && b c [:digit:]]/x) # => ["a", "c"] So maybe Intersection parse results need to look somewhat like this:
Now that would require quite a bit of tree restructuring while parsing. Not to mention that there can be more than one intersection: 'abc1&'.scan(/[abc && ab && bc]/x) # => ["b"] Another option could be to treat Sets as group of Hmmm ... |
I'm reasonably happy with this now ... |
... this fixes the `>` and `l` #strfregexp_tree parts of Alternation (currently broken on master) and the new Intersection expression. Maybe #level should be renamed #group_level and #nesting_level should become #level instead, for clarity sake?
# Conflicts: # ChangeLog # test/scanner/test_sets.rb # test/warnings.yml
This makes
CharacterSet
a standardSubexpression
as suggested in #47 (comment)All equivalent tokens result in the same
Scanner
andParser
emissions as outside of sets.New
CharacterSet::Range
andCharacterSet::Intersection
expressions represent respective trees.Other notable changes are:
@ammar What do you think? The commit messages provide a bit more explanation if you are wondering about some of the changes, but feel free to suggest any other solution.