Skip to content

Commit

Permalink
make replace with count=0 a no-op (#22325)
Browse files Browse the repository at this point in the history
  • Loading branch information
rfourquet authored Jul 4, 2017
1 parent c4f2737 commit bf83397
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 7 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ Deprecated or removed
Ternaries must now include some amount of whitespace, e.g. `x ? a : b` rather than
`x? a : b` ([#22523]).

* The method `replace(s::AbstractString, pat, r, count)` with `count <= 0` is deprecated
in favor of `replace(s::AbstractString, pat, r, typemax(Int))` ([#22325]).


Julia v0.6.0 Release Notes
==========================
Expand Down
16 changes: 16 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,22 @@ function bkfact!(A::StridedMatrix, uplo::Symbol, symmetric::Bool = issymmetric(A
return bkfact!(symmetric ? Symmetric(A, uplo) : Hermitian(A, uplo), rook)
end

# PR #22325
# TODO: when this replace is removed from deprecated.jl:
# 1) rename the function replace_new from strings/util.jl to replace
# 2) update the replace(s::AbstractString, pat, f) method, below replace_new
# (see instructions there)
function replace(s::AbstractString, pat, f, n::Integer)
if n <= 0
depwarn(string("`replace(s, pat, r, count)` with `count <= 0` is deprecated, use ",
"`replace(s, pat, r, typemax(Int))` or `replace(s, pat, r)` instead"),
:replace)
replace(s, pat, f)
else
replace_new(String(s), pat, f, n)
end
end

# END 0.7 deprecations

# BEGIN 1.0 deprecations
Expand Down
21 changes: 14 additions & 7 deletions base/strings/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,10 @@ _replace(io, repl, str, r, pattern) = print(io, repl)
_replace(io, repl::Function, str, r, pattern) =
print(io, repl(SubString(str, first(r), last(r))))

function replace(str::String, pattern, repl, limit::Integer)
# TODO: rename to `replace` when `replace` is removed from deprecated.jl
function replace_new(str::String, pattern, repl, count::Integer)
count == 0 && return str
count < 0 && throw(DomainError())
n = 1
e = endof(str)
i = a = start(str)
Expand All @@ -384,25 +387,29 @@ function replace(str::String, pattern, repl, limit::Integer)
end
r = search(str,pattern,k)
j, k = first(r), last(r)
n == limit && break
n == count && break
n += 1
end
write(out, SubString(str,i))
String(take!(out))
end

"""
replace(string::AbstractString, pat, r[, n::Integer=0])
replace(s::AbstractString, pat, r, [count::Integer])
Search for the given pattern `pat`, and replace each occurrence with `r`. If `n` is
provided, replace at most `n` occurrences. As with search, the second argument may be a
Search for the given pattern `pat` in `s`, and replace each occurrence with `r`.
If `count` is provided, replace at most `count` occurrences.
As with [`search`](@ref), the second argument may be a
single character, a vector or a set of characters, a string, or a regular expression. If `r`
is a function, each occurrence is replaced with `r(s)` where `s` is the matched substring.
If `pat` is a regular expression and `r` is a `SubstitutionString`, then capture group
references in `r` are replaced with the corresponding matched text.
"""
replace(s::AbstractString, pat, f, n::Integer) = replace(String(s), pat, f, n)
replace(s::AbstractString, pat, r) = replace(s, pat, r, 0)
replace(s::AbstractString, pat, f) = replace_new(String(s), pat, f, typemax(Int))
# TODO: change this to the following when `replace` is removed from deprecated.jl:
# replace(s::AbstractString, pat, f, count::Integer=typemax(Int)) =
# replace(String(s), pat, f, count)


# hex <-> bytes conversion

Expand Down
12 changes: 12 additions & 0 deletions test/strings/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,18 @@ end
# Issue 13332
@test replace("abc", 'b', 2.1) == "a2.1c"

# test replace with a count for String and GenericString
# check that replace is a no-op if count==0
for s in ["aaa", Base.Test.GenericString("aaa")]
# @test replace("aaa", 'a', 'z', 0) == "aaa" # enable when undeprecated
@test replace(s, 'a', 'z', 1) == "zaa"
@test replace(s, 'a', 'z', 2) == "zza"
@test replace(s, 'a', 'z', 3) == "zzz"
@test replace(s, 'a', 'z', 4) == "zzz"
@test replace(s, 'a', 'z', typemax(Int)) == "zzz"
@test replace(s, 'a', 'z') == "zzz"
end

# chomp/chop
@test chomp("foo\n") == "foo"
@test chop("fooε") == "foo"
Expand Down

2 comments on commit bf83397

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

Please sign in to comment.