Skip to content

Conversation

@naitoh
Copy link
Contributor

@naitoh naitoh commented Jan 8, 2024

fix #70
If there is no substring matching the group (s[3]), the behavior is different.

If there is no substring matching the group, the corresponding element (s[3]) should be nil.

Before

s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
s[0]           #=> "foobar"
s[1]           #=> "foo"
s[2]           #=> "bar"
s[3]           #=> nil
s.captures #=> ["foo", "bar", ""]
s.captures.compact #=> ["foo", "bar", ""]

After

s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
s[0]           #=> "foobar"
s[1]           #=> "foo"
s[2]           #=> "bar"
s[3]           #=> nil
s.captures #=> ["foo", "bar", nil]
s.captures.compact #=> ["foo", "bar"]

FYI

https://docs.ruby-lang.org/ja/latest/method/MatchData/i/captures.html

/(foo)(bar)(BAZ)?/ =~ "foobarbaz" #=> 0
$~.to_a        #=> ["foobar", "foo", "bar", nil]
$~.captures #=> ["foo", "bar", nil]
$~.captures.compact #=> ["foo", "bar"]

@naitoh naitoh changed the title fix captures nil case Fixed problem with nil in captures Jan 8, 2024
@naitoh naitoh marked this pull request as ready for review January 8, 2024 06:07
@naitoh naitoh requested a review from kou January 8, 2024 06:25
Comment on lines 909 to 910
adjustRegisterPosition(REGION_ADAPTER.getBeg(regs, i)),
adjustRegisterPosition(REGION_ADAPTER.getEnd(regs, i)));
Copy link
Member

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

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

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.

@kou kou changed the title Fixed problem with nil in captures StringScanner#captures: Return nil not "" for unmached capture Jan 12, 2024
@kou kou merged commit 1fbfdd3 into ruby:master Jan 12, 2024
@headius
Copy link
Contributor

headius commented Jan 14, 2024

And thank you for patching the JRuby code!

@naitoh naitoh deleted the fix_captures_nil branch January 14, 2024 05:53
kou pushed a commit to kou/ruby that referenced this pull request Jan 14, 2024
unmached capture
(ruby/strscan#72)

fix ruby/strscan#70
If there is no substring matching the group (s[3]), the behavior is
different.

If there is no substring matching the group, the corresponding element
(s[3]) should be nil.

```
s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
s[0]           #=> "foobar"
s[1]           #=> "foo"
s[2]           #=> "bar"
s[3]           #=> nil
s.captures #=> ["foo", "bar", ""]
s.captures.compact #=> ["foo", "bar", ""]
```

```
s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
s[0]           #=> "foobar"
s[1]           #=> "foo"
s[2]           #=> "bar"
s[3]           #=> nil
s.captures #=> ["foo", "bar", nil]
s.captures.compact #=> ["foo", "bar"]
```

https://docs.ruby-lang.org/ja/latest/method/MatchData/i/captures.html
```
/(foo)(bar)(BAZ)?/ =~ "foobarbaz" #=> 0
$~.to_a        #=> ["foobar", "foo", "bar", nil]
$~.captures #=> ["foo", "bar", nil]
$~.captures.compact #=> ["foo", "bar"]
```

* StringScanner#captures is not yet documented.
https://docs.ruby-lang.org/ja/latest/class/StringScanner.html

ruby/strscan@1fbfdd3c6f
kou pushed a commit to ruby/ruby that referenced this pull request Jan 14, 2024
unmached capture
(ruby/strscan#72)

fix ruby/strscan#70
If there is no substring matching the group (s[3]), the behavior is
different.

If there is no substring matching the group, the corresponding element
(s[3]) should be nil.

```
s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
s[0]           #=> "foobar"
s[1]           #=> "foo"
s[2]           #=> "bar"
s[3]           #=> nil
s.captures #=> ["foo", "bar", ""]
s.captures.compact #=> ["foo", "bar", ""]
```

```
s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
s[0]           #=> "foobar"
s[1]           #=> "foo"
s[2]           #=> "bar"
s[3]           #=> nil
s.captures #=> ["foo", "bar", nil]
s.captures.compact #=> ["foo", "bar"]
```

https://docs.ruby-lang.org/ja/latest/method/MatchData/i/captures.html
```
/(foo)(bar)(BAZ)?/ =~ "foobarbaz" #=> 0
$~.to_a        #=> ["foobar", "foo", "bar", nil]
$~.captures #=> ["foo", "bar", nil]
$~.captures.compact #=> ["foo", "bar"]
```

* StringScanner#captures is not yet documented.
https://docs.ruby-lang.org/ja/latest/class/StringScanner.html

ruby/strscan@1fbfdd3c6f
k0kubun added a commit to ruby/ruby that referenced this pull request Jun 4, 2024
	Revert "Rollback to released version numbers of stringio and strscan"

	This reverts commit 6a79e53.

	[ruby/strscan] StringScanner#captures: Return nil not "" for unmached capture (ruby/strscan#72)

	fix ruby/strscan#70
	If there is no substring matching the group (s[3]), the behavior is
	different.

	If there is no substring matching the group, the corresponding element
	(s[3]) should be nil.

	```
	s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
	s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
	s[0]           #=> "foobar"
	s[1]           #=> "foo"
	s[2]           #=> "bar"
	s[3]           #=> nil
	s.captures #=> ["foo", "bar", ""]
	s.captures.compact #=> ["foo", "bar", ""]
	```

	```
	s = StringScanner.new('foobarbaz') #=> #<StringScanner 0/9 @ "fooba...">
	s.scan /(foo)(bar)(BAZ)?/  #=> "foobar"
	s[0]           #=> "foobar"
	s[1]           #=> "foo"
	s[2]           #=> "bar"
	s[3]           #=> nil
	s.captures #=> ["foo", "bar", nil]
	s.captures.compact #=> ["foo", "bar"]
	```

	https://docs.ruby-lang.org/ja/latest/method/MatchData/i/captures.html
	```
	/(foo)(bar)(BAZ)?/ =~ "foobarbaz" #=> 0
	$~.to_a        #=> ["foobar", "foo", "bar", nil]
	$~.captures #=> ["foo", "bar", nil]
	$~.captures.compact #=> ["foo", "bar"]
	```

	* StringScanner#captures is not yet documented.
	https://docs.ruby-lang.org/ja/latest/class/StringScanner.html

	ruby/strscan@1fbfdd3c6f

	[ruby/strscan] Bump version

	ruby/strscan@d6f97ec102
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Response when there is no substring matching the group in StringScanner#captures

3 participants