code review 5430046: strings: Add ContainsAny and ContainsRune to correspond...

strings: Add ContainsAny and ContainsRune to correspond to IndexAny etc.

[bytbox, 2011-11-22 08:45:57]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2011-11-23 00:50:26]

http://codereview.appspot.com/5430046/diff/3003/src/pkg/strings/strings.go
File src/pkg/strings/strings.go (right):

http://codereview.appspot.com/5430046/diff/3003/src/pkg/strings/strings.go#ne...
src/pkg/strings/strings.go:67: return Index(s, substr) != -1
please change all these tests to >= 0, which is simpler.

http://codereview.appspot.com/5430046/diff/3003/src/pkg/strings/strings.go#ne...
src/pkg/strings/strings.go:75: // ContainsFunc returns true if any unicode code
points within s satisfy f(c).
s/unicode/Unicode/

that said, i don't believe this function is worthwhile. it also reads funny: the
string doesn't contain the Func.

the other two might be worth it, but i'd like to think about it

[bytbox, 2011-11-23 17:28:19]

Hello golang-dev@googlegroups.com, r@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[bytbox, 2011-11-23 17:29:39]

Hello golang-dev@googlegroups.com, r@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[r, 2011-11-23 17:35:54]

http://codereview.appspot.com/5430046/diff/2002/src/pkg/strings/strings_test.go
File src/pkg/strings/strings_test.go (right):

http://codereview.appspot.com/5430046/diff/2002/src/pkg/strings/strings_test....
src/pkg/strings/strings_test.go:945: if ContainsFunc(ct.str, ct.f.f) !=
ct.expected {
this function is gone

[bytbox, 2011-11-23 17:36:38]

On 11/23/11 12:35, r@golang.org wrote:
> src/pkg/strings/strings_test.go:945: if ContainsFunc(ct.str, ct.f.f) !=
> ct.expected {
> this function is gone

Oh foo. That'll teach me to mail without testing.

-- 
Scott Lawrence

[bytbox, 2011-11-23 17:44:09]

Hello golang-dev@googlegroups.com, r@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[r, 2011-11-23 18:00:23]

http://codereview.appspot.com/5430046/diff/6005/src/pkg/strings/strings_test.go
File src/pkg/strings/strings_test.go (right):

http://codereview.appspot.com/5430046/diff/6005/src/pkg/strings/strings_test....
src/pkg/strings/strings_test.go:920: {"", "a", false},
you only need one set of tests, and they should include both strings and probes
that are empty, single byte, single multi-byte rune, multi-single-byte,
multi-single-rune, and mixed.

[bytbox, 2011-11-24 04:15:49]

Hello golang-dev@googlegroups.com, r@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[bytbox, 2011-11-24 04:15:54]

On 11/23/11 13:00, r@golang.org wrote:
>
>
http://codereview.appspot.com/5430046/diff/6005/src/pkg/strings/strings_test.go
>
> File src/pkg/strings/strings_test.go (right):
>
>
http://codereview.appspot.com/5430046/diff/6005/src/pkg/strings/strings_test....
>
> src/pkg/strings/strings_test.go:920: {"", "a", false},
> you only need one set of tests, and they should include both strings and
> probes that are empty, single byte, single multi-byte rune,
> multi-single-byte, multi-single-rune, and mixed.

Not sure I understand the first part - can you elaborate? (I'm 
immitating the tests for the Index methods.)

>
> http://codereview.appspot.com/5430046/


-- 
Scott Lawrence

[r, 2011-11-24 04:19:39]

LGTM

[r, 2011-11-24 04:20:15]

*** Submitted as http://code.google.com/p/go/source/detail?r=90bf4e91689b ***

strings: Add ContainsAny and ContainsRune to correspond to IndexAny etc.

R=golang-dev, r
CC=golang-dev
http://codereview.appspot.com/5430046

Committer: Rob Pike <r@golang.org>