code review 135760043: strings, bytes: document behavior of Replace when old i...

strings, bytes: document behavior of Replace when old is empty

Fixes issue 8143.

[cespare, 2014-08-25 17:03:25]

Please review.

I found it hard to accurately and concisely describe the behavior. Suggestions
for better wording are welcome. I considered and rejected:

- If old is empty, Replace splits s into UTF-8 sequences. // vague, imprecise

- If old is empty, it matches an empty string around each UTF-8 sequence. // is
new inserted twice between two UTF-8 sequences?

- If old is empty, new is inserted at the beginning, end, and between each UTF-8
sequence up to n times. // long, redundant with preceding sentence

[rsc, 2014-08-25 17:30:47]

please run 'hg mail' to send this out for review. that command is what actually
sends it to the right mailing list and gets it on our dashboards. thanks.

[cespare, 2014-08-25 17:46:31]

Hello rsc@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[cespare, 2014-08-25 17:47:36]

On 2014/08/25 17:30:47, rsc wrote:
> please run 'hg mail' to send this out for review. that command is what
actually
> sends it to the right mailing list and gets it on our dashboards. thanks.

Mailed.

I had thought the 'Publish+Mail Comments' action in the web view was equivalent.

[bradfitz, 2014-08-25 17:52:22]

https://codereview.appspot.com/135760043/diff/40001/src/pkg/bytes/bytes.go
File src/pkg/bytes/bytes.go (right):

https://codereview.appspot.com/135760043/diff/40001/src/pkg/bytes/bytes.go#ne...
src/pkg/bytes/bytes.go:608: // If old is empty, it matches one empty slice at
the beginning,
I think I'd say "an" instead of "one".  Or "it matches at the beginning, end,
and between ..."

[bradfitz, 2014-08-25 17:52:39]

R=r for words



On Mon, Aug 25, 2014 at 10:46 AM, <cespare@gmail.com> wrote:

> Hello rsc@golang.org (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> https://codereview.appspot.com/135760043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[r, 2014-08-25 18:05:21]

https://codereview.appspot.com/135760043/diff/40001/src/pkg/bytes/bytes.go
File src/pkg/bytes/bytes.go (right):

https://codereview.appspot.com/135760043/diff/40001/src/pkg/bytes/bytes.go#ne...
src/pkg/bytes/bytes.go:608: // If old is empty, it matches one empty slice at
the beginning,
On 2014/08/25 17:52:21, bradfitz wrote:
> I think I'd say "an" instead of "one".  Or "it matches at the beginning, end,
> and between ..."

perhaps

at the beginning of the string and after each UTF-8-encoded character, yielding
n+1 replacements for an n-rune string.

[cespare, 2014-08-25 21:09:12]

Hello rsc@golang.org, bradfitz@golang.org, r@golang.org (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[cespare, 2014-08-25 21:09:25]

https://codereview.appspot.com/135760043/diff/40001/src/pkg/bytes/bytes.go
File src/pkg/bytes/bytes.go (right):

https://codereview.appspot.com/135760043/diff/40001/src/pkg/bytes/bytes.go#ne...
src/pkg/bytes/bytes.go:608: // If old is empty, it matches one empty slice at
the beginning,
On 2014/08/25 18:05:21, r wrote:
> On 2014/08/25 17:52:21, bradfitz wrote:
> > I think I'd say "an" instead of "one".  Or "it matches at the beginning,
end,
> > and between ..."
> 
> perhaps
> 
> at the beginning of the string and after each UTF-8-encoded character,
yielding
> n+1 replacements for an n-rune string.

Good suggestions.

I stuck with "UTF-8 sequence" rather than "UTF-8-encoded character" as that
seems to be the phrase used elsewhere in strings and bytes (see the Split family
of functions, for instance) but I'm happy to change if that's not the best
wording.

[r, 2014-08-25 21:41:59]

LGTM

[r, 2014-08-25 21:42:30]

*** Submitted as https://code.google.com/p/go/source/detail?r=0a730a1a53e2 ***

strings, bytes: document behavior of Replace when old is empty

Fixes issue 8143.

LGTM=r
R=rsc, bradfitz, r
CC=golang-codereviews
https://codereview.appspot.com/135760043

Committer: Rob Pike <r@golang.org>

[gobot, 2014-08-26 00:59:44]

This CL appears to have broken the linux-386 builder.
See http://build.golang.org/log/e2060e2b298d7b8d5597f3763bfde48738e4f8be