Refactored pretty much everything #6
Conversation
| case class Group(term: RegularExpression) extends RegularExpression | ||
| case class Substitution(index: Int) extends RegularExpression | ||
| case class NonCapturingGroup(term: RegularExpression) extends RegularExpression | ||
| final case class Group(term: RegularExpression, rest: Option[RegularExpression] = None, capturing: Boolean = true) extends Term |
There was a problem hiding this comment.
Do you need to encode the difference between a capturing group and a non capturing group. When it comes to printing they both print the same.
| } | ||
|
|
||
| case class CharacterClass(terms: CharacterClass.Term*) extends RegularExpression | ||
|
|
||
| sealed trait CharacterClass extends CharacterClass.Group.Term |
There was a problem hiding this comment.
This encoding would not allow a character class to be quantified. e.g
[a-z]+
[abc]{0,5}
Could not be represented.
It may be the same for intersections but I'm not sure
| } | ||
|
|
||
| def parse(string: String): RegularExpression = { | ||
| def parse(string: String): RegularExpression = | ||
| regularExpression(new PackratReader(new CharSequenceReader(string))).get |
There was a problem hiding this comment.
Outside the scope of this but any ideas how meaningful parse errors can be given to users?
|
|
||
| import CharacterClass._ | ||
|
|
||
| val digits: data.Group[Char] = Inclusion((48.toChar to 57.toChar).toSet) |
There was a problem hiding this comment.
Why are you using
(48.toChar to 57.toChar)
instead of
('0' to '9')
| } yield (min, max) | ||
|
|
||
| forAll(gen) { | ||
| case (min, max) => | ||
| GenParser.parse(s"[$min-$max]") mustEqual CharacterClass(CharacterClass.CharRange(min, max)) | ||
| GenParser.parse(s"[$min-$max]") mustEqual CharacterClass.Group(CharacterClass.Range(min, max)) | ||
| } | ||
| } | ||
|
|
||
| "fail to parse an out of order lowercase char range" ignore { |
There was a problem hiding this comment.
Currently test always passes. If you change:
an[Exception] mustBe thrownBy(GenParser.parse(s"[$min-$max"))
to
an[Exception] mustBe thrownBy(GenParser.parse(s"[$min-$max]"))
It will fail for the right reasons and you can remove ignore
| case class OneOrMore(term: Term) extends Quantified | ||
| case class Length(term: Term, length: Int) extends Quantified | ||
| case class RangeFrom(term: Term, min: Int) extends Quantified | ||
| case class Range(term: Term, min: Int, max: Int) extends Quantified |
There was a problem hiding this comment.
Could add require(min <= max) to range, this would allow you to unignore the fail to parse a range test in GenParserSpec file
| @@ -282,7 +237,7 @@ class GenParserSpec extends WordSpec with MustMatchers with PropertyChecks { | |||
|
|
|||
| forAll(gen) { | |||
| case (min, max) => | |||
| GenParser.parse(s"a{$min,$max}") mustEqual Range(Literal("a"), min, max) | |||
| GenParser.parse(s"a{$min,$max}") mustEqual Range(Literal('a'), min, max) | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I can't comment the test below as it hasn't changed but if you make the change to Range you can unignore this test, it will still fail until you change max <- Gen.chooseNum(0, min) to max <- Gen.chooseNum(0, min - 1) as {1, 1} for example is a valid range
| @@ -1,56 +1,49 @@ | |||
| package wolfendale.scalacheck.regexp.ast | |||
|
|
|||
| sealed trait RegularExpression { | |||
| sealed trait RegularExpression | |||
There was a problem hiding this comment.
a sealed abstract class should be used instead of a sealed trait as they can break binary compatibility
No description provided.