Normalize the argument order in lstrip and rstrip

[ararslan]

This puts the function argument first, as dictated in the style guide.

[simonbyrne]

Link to old discussion in #27211, #27232 and #27309.

[ararslan]

There was general agreement that we should at least allow the function argument first.

As I said in one of those, this will just be a weird case of "oh yeah, functions come first everywhere but this one oddball case," like we had then fixed with finalizer.

[JeffBezanson]

This very closely resembles the drop-while function found in many functional languages, where the predicate (as always!) is the first argument.

I believe I've read all the threads on this, and I don't see a single good argument for why the function should go second in just this case. @KristofferC said he finds it more natural to put the function second, and that's it. That doesn't seem like enough to overrule a strongly established convention here. If people want the option to pass the function second as well, and that doesn't cause any ambiguities, then I guess that's fine in addition.

[strickek]

Maybe it is also useful to have a method for 3 arguments: Strip if function is true or in chars. eg

lstrip(isnumeric, "0123abc", ['a', 'b']) == "c"

[ararslan]

Seems that could be written more clearly as

lstrip(c->isnumeric(c) || c in ['a','b'], "0123abc")

[strickek]

You are right, it would be better to do it in this way.

[StefanKarpinski]

It's not allowed to supply both pred and chars so this should be two separate signatures, something like this:

lstrip([pred=isspace,] str::AbstractString)
lstrip(str::AbstractString[, chars::Base.Chars])

[StefanKarpinski]

Yes, I think this is the right move.

[ararslan]

I also removed the reference in DelimitedFiles to _default_delims, which no longer exists, that's causing a warning during the build.

[strickek]

I think chars in 2nd signiture should not be optional, because for str only the first one with default pred=isspace is used: lstrip([pred=isspace,] str::AbstractString)lstrip(str::AbstractString, chars::Base.Chars) 2018-06-18 5:02 GMT+02:00 Stefan Karpinski <[email protected]>:

…

***@***.**** commented on this pull request. ------------------------------ In base/strings/util.jl <#27605 (comment)>: > @@ -135,15 +135,16 @@ function chomp(s::String) end """ - lstrip(str::AbstractString[, chars]) + lstrip([pred,] str::AbstractString[, chars]) It's not allowed to supply both pred and chars so this should be two separate signatures, something like this: lstrip([pred=isspace,] str::AbstractString)lstrip(str::AbstractString[, chars::Base.Chars]) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#27605 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AXv3kVTdhmufu1t-MDgShG_dR1ynseJOks5t9xhKgaJpZM4UqPVk> .

[StefanKarpinski]

Yes, of course, you're right, @strickek.

[ararslan]

Good catch, @strickek, thanks!

[ararslan]

The CI failures are unrelated: FreeBSD froze and Circle got OOM killed. Good to merge?