String#index and String#rindex with Regex #3417
Conversation
|
|
||
| describe "with offset" do | ||
| assert { "abcDef".index(/[A-Z]/).should eq(3) } | ||
| assert { "foobarbaz".index(/ba/, -5).should eq(6) } |
zatherz
force-pushed
the
feature/string_regex_index
branch
2 times, most recently
from
c8af1a0 to
375cfad
Compare
|
Fixed, also improved the readability of the code (thanks @RX14). |
zatherz
force-pushed
the
feature/string_regex_index
branch
from
375cfad to
21960f8
Compare
zatherz
changed the title
|
Added String#rindex with Regex. |
|
|
||
| describe "with offset" do | ||
| assert { "bbbb".rindex(/b/, 1).should eq(2) } | ||
| assert { "bbbb".rindex(/b/, -1235).should be_nil } |
There was a problem hiding this comment.
Please run crystal tool format to ensure the formatting is correct.
See Use Crystal's code formatter section in the docs:
https://crystal-lang.org/docs/conventions/documenting_code.html
zatherz
force-pushed
the
feature/string_regex_index
branch
from
21960f8 to
d3381a8
Compare
| offset += size if offset < 0 | ||
| return nil unless 0 <= offset <= size | ||
|
|
||
| matchlist = self[0, self.size - offset].scan(search) |
There was a problem hiding this comment.
This can be done without storing all the matches in an array. Just use the block form and save the last match.
| offset += size if offset < 0 | ||
| return nil unless 0 <= offset <= size | ||
|
|
||
| if matchdata = self.match(search, offset) |
There was a problem hiding this comment.
This can simply be match(search, offset).try &.begin(0)
zatherz
force-pushed
the
feature/string_regex_index
branch
from
d3381a8 to
74d096f
Compare
| offset += size if offset < 0 | ||
| return nil unless 0 <= offset <= size | ||
|
|
||
| if index = self.match(search, offset).try &.begin |
There was a problem hiding this comment.
I mean, the if is not needed, because try already returns nil. So just self.match(search, offset).try &.begin at this point with nothing else following it is good :-)
There was a problem hiding this comment.
Fixed! Sorry for forgetting about that.
| matchresult = matchdata | ||
| end | ||
|
|
||
| if index = matchresult.try &.begin(0) |
There was a problem hiding this comment.
Same here, matchresult.try &.begin is good
Also, we prefer names like match_result and match_data instead of matchresult and matchdata, which are slightly harder to read
Includes specs.
zatherz
force-pushed
the
feature/string_regex_index
branch
from
74d096f to
4c8da90
Compare
|
@zatherz Thank you for your contribution and for your patience! :-) |
Includes specs.
Allows you to do things like
"ab d 64 aef".index(/\d+/).