Skip to content

StringScanner#captures: Return nil not "" for unmached capture #72

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.

Sign up for GitHub

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

Merged
kou merged 1 commit into from
Jan 12, 2024
Merged

StringScanner#captures: Return nil not "" for unmached capture #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ext/jruby/org/jruby/ext/strscan/RubyStringScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,9 +901,14 @@ public IRubyObject captures(ThreadContext context) {
newAry = RubyArray.newArray(runtime, numRegs);

for (i = 1; i < numRegs; i++) {
IRubyObject str = extractRange(runtime,
IRubyObject str;
if (REGION_ADAPTER.getBeg(regs, i) == -1) {
str = context.nil;
} else {
str = extractRange(runtime,
adjustRegisterPosition(REGION_ADAPTER.getBeg(regs, i)),
adjustRegisterPosition(REGION_ADAPTER.getEnd(regs, i)));
Comment on lines 909 to 910
Copy link
Member

@kou kou Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@headius Should we indent them?

Copy link
Member

@kou kou Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this. If we need to fix style here, we can do it as a follow-up PR.

Copy link
Contributor

@headius headius Jan 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally just try to match existing style when making other changes. We can do a separate PR to clean up formatting.

}
newAry.push(str);
}

Expand Down
18 changes: 11 additions & 7 deletions ext/strscan/strscan.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1243,10 +1243,10 @@ strscan_size(VALUE self)
* If nothing was priorly matched, it returns nil.
*
* s = StringScanner.new("Fri Dec 12 1975 14:39")
* s.scan(/(\w+) (\w+) (\d+) /) # -> "Fri Dec 12 "
* s.captures # -> ["Fri", "Dec", "12"]
* s.scan(/(\w+) (\w+) (\d+) /) # -> nil
* s.captures # -> nil
* s.scan(/(\w+) (\w+) (\d+) (1980)?/) # -> "Fri Dec 12 "
* s.captures # -> ["Fri", "Dec", "12", nil]
* s.scan(/(\w+) (\w+) (\d+) (1980)?/) # -> nil
* s.captures # -> nil
*/
static VALUE
strscan_captures(VALUE self)
Expand All @@ -1262,9 +1262,13 @@ strscan_captures(VALUE self)
new_ary = rb_ary_new2(num_regs);

for (i = 1; i < num_regs; i++) {
VALUE str = extract_range(p,
adjust_register_position(p, p->regs.beg[i]),
adjust_register_position(p, p->regs.end[i]));
VALUE str;
if (p->regs.beg[i] == -1)
str = Qnil;
else
str = extract_range(p,
adjust_register_position(p, p->regs.beg[i]),
adjust_register_position(p, p->regs.end[i]));
rb_ary_push(new_ary, str);
}

Expand Down
4 changes: 2 additions & 2 deletions test/strscan/test_stringscanner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,8 +737,8 @@ def test_size
def test_captures
s = create_string_scanner("Timestamp: Fri Dec 12 1975 14:39")
s.scan("Timestamp: ")
s.scan(/(\w+) (\w+) (\d+) /)
assert_equal(["Fri", "Dec", "12"], s.captures)
s.scan(/(\w+) (\w+) (\d+) (1980)?/)
assert_equal(["Fri", "Dec", "12", nil], s.captures)
s.scan(/(\w+) (\w+) (\d+) /)
assert_nil(s.captures)
end
Expand Down