bytes, strings: add ReplaceAll

[HaraldNordgren]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

What did you expect to see?

What did you see instead?

[cznic]

That would be strings.Replace(s, old, new, -1)

[HaraldNordgren]

Thanks for the downvote. I created a PR here https://go-review.googlesource.com/c/go/+/137456 and the implementation is exactly to call strings.Replace(s, old, new, -1).

My problem with the current state of affairs is that I find myself looking up the syntax of this every time I use it. Or if I am reviewing someone else's code I stumble over the -1 because I forget why it's there. So in the spirit of keeping Go simple I want to introduce this helper.

[bradfitz]

Numbers would be interesting here.

What percentage of strings.Replace calls in the wild hard-code -1?

[cznic]

> What percentage of strings.Replace calls in the wild hard-code -1?

I guestimate that on the small sample of my own code it's about 99%. Yet I don't think a function that just sets a default value to one of the parameters of another stdlib function belongs to the stdlib.

[HaraldNordgren]

@bradfitz From my company's codebase, strings.Replace is called 12 times and all but one of them uses -1 as n. That also includes helper functions that are called in lots of places, so the total ratio shifts higher toward almost exclusively the 'ReplaceAll' behavior.

[HaraldNordgren]

@cznic Btw, there are these helper in the standard library already:

from powser2:

func Print(U PS) {
	Printn(U, 1000000000)
}

from draw:

func Draw(dst Image, r image.Rectangle, src image.Image, sp image.Point, op Op) {
	DrawMask(dst, r, src, sp, nil, image.Point{}, op)
}

from ast:

func Inspect(node Node, f func(Node) bool) {
	Walk(inspector(f), node)
}

[magical]

@HaraldNordgren powser2.go is a test, not library code. draw.Draw seems like a good example though.

[bradfitz]

Or strings.Fields and strings.FieldFunc.

Or http.ListenAndServe vs http.Server.ListenAndServe.

Or regexp.Match vs regexp.Compile+Match.

Or regexp.MustCompile vs regexp.Compile, or template.Parse vs template.MustParse.

I don't agree with @cznic thumbs down for this; the standard library is full of convenient-vs-lower-level versions of things.

I'd rather see numbers.

[jimmyfrasche]

in my personal/work code: 16 calls, all -1.

[bradfitz]

In Perkeep, 60 use -1 and 43 use 1. No other values.

[dpinela]

There are 265 calls to strings.Replace in the main Go repo, of which 216 use -1.

[dominikh]

In the Go corpus, excluding vendored code, 1034 of 2213 calls to strings.Replace hard-code -1.

I'm with cznic on this one. Nearly all of the examples provided by bradfitz are more complex (syntactically and semantically) than providing a trivial default value for an argument. Panicking instead of returning an error is a significant change in API. regexp.Match encapsulates two separate operations. strings.Field is far more complex, providing an optimized implementation and pulling in additional dependencies.

In all of these cases, there's really no question as to which function should be used. I would rather not, however, tell people to use ReplaceAll instead of Replace, and I wouldn't want to see a mix of old and new code, either.