You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The specs have one test for escaped backslashes in Regexp#inspect:
it"does not over escape"doRegexp.new('\\\/').inspect.should == "/\\\\\\//"end
The obvious implementation to make this spec pass is to simply escape every slash with another slash. This works in this case, but that causes an error in language/case_spec for something that is not related to the case statement:
it"tests with a regexp interpolated within another regexp"dodigits_regexp=/\d+/case"foo43"when/oo(#{digits_regexp})/
This results in a regexp /oo(?-mix:\\d+)/, which now expects a slash followed by one or more d characters. This does not match the input, but does not indicate the escaping as the issue.
I tested my implementation with the these specs added, but this was more or less brute forcing every possibility. Also, because Regexp.new takes a String which has an additional layer of escaping, I peferred the // notation.
I think it would be better to at least include the following cases:
/\d+/# Output should have 1 not escaped slash, since this should not be escaped/\\d+/# Output should have 1 escaped slash (so 2 slashes), this should be escaped/\\\d+/# Output should have 1 escaped and 1 not escaped slash (so 3 slashes)
Furthermore, I think it would be better to add an explicit test for the regexp embedded in regexp scenario like there is in the case spec now, just to make the test a bit more explicit and decouple it from the case statement.
I'm not that familiar with this project and how things are structured, can I just add a few tests to the inspect spec of regexp, and make a new spec file for the embedding tests?
The text was updated successfully, but these errors were encountered:
I'm not that familiar with this project and how things are structured, can I just add a few tests to the inspect spec of regexp, and make a new spec file for the embedding tests?
Yes, just make a PR and based on the changes we can find out what's the right place.
Specs testing interpolating a regexp in another regexp should probably move to language/regexp_spec.rb or core/regexp/to_s_spec.rb.
The specs have one test for escaped backslashes in
Regexp#inspect
:The obvious implementation to make this spec pass is to simply escape every slash with another slash. This works in this case, but that causes an error in
language/case_spec
for something that is not related to the case statement:This results in a regexp
/oo(?-mix:\\d+)/
, which now expects a slash followed by one or more d characters. This does not match the input, but does not indicate the escaping as the issue.I tested my implementation with the these specs added, but this was more or less brute forcing every possibility. Also, because
Regexp.new
takes a String which has an additional layer of escaping, I peferred the//
notation.I think it would be better to at least include the following cases:
Furthermore, I think it would be better to add an explicit test for the regexp embedded in regexp scenario like there is in the case spec now, just to make the test a bit more explicit and decouple it from the case statement.
I'm not that familiar with this project and how things are structured, can I just add a few tests to the inspect spec of regexp, and make a new spec file for the embedding tests?
The text was updated successfully, but these errors were encountered: