use Char replacement function for Charpattern
#25815
Conversation
test/strings/util.jl
Outdated
| @@ -261,6 +261,15 @@ end | |||
|
|
|||
| # Issue 25741 | |||
| @test replace("abc", ['a', 'd'] => 'A') == "Abc" | |||
|
|
|||
| # for Char pattern call Char replacement function | |||
| @test replace("a", "a" => sizeof) == "1" | |||
There was a problem hiding this comment.
Rather than sizeof, better use typeof.
base/strings/util.jl
Outdated
| @@ -390,6 +390,8 @@ end | |||
| _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)))) | |||
| _replace(io, repl::Function, str, r, pattern::Function) = | |||
| print(io, repl(getindex(str, first(r)))) | |||
base/strings/util.jl
Outdated
| is a function, each occurrence is replaced with `r(s)` where `s` is the matched substring. | ||
| If `pat` is a `Char`, a collection of `Char` or a predicate function and `r` |
There was a problem hiding this comment.
Instead of repeating the full sentence, maybe just change where `s` is the matched substring. to where `x` is the matched substring (when `pat` is a `Regex` or `AbstractString`) or character (when `pat` is a `Char` or a collection of `Char`).?
|
This is a breaking change, yes? I can't think of any way to do a better deprecation process. Should be listed under breaking changes in the NEWS. |
NEWS.md
Outdated
| @@ -196,6 +196,11 @@ Breaking changes | |||
|
|
|||
| This section lists changes that do not have deprecation warnings. | |||
|
|
|||
| * `replace(s::AbstractString, pat=>repl)` if repl is a function, now calls | |||
There was a problem hiding this comment.
This isn't clear about what the change was. Maybe:
`replace(s::AbstractString, pat=>repl)` for function `repl` arguments formerly passed a substring
to `repl` in all cases. It now passes substrings for string patterns `pat`, but a `Char`
for character patterns (when `pat` is a `Char`, collection of `Char`, or a character predicate).
|
@stevengj Merge if that's OK for you? |
NEWS.md
Outdated
| @@ -196,6 +196,11 @@ Breaking changes | |||
|
|
|||
| This section lists changes that do not have deprecation warnings. | |||
|
|
|||
| * `replace(s::AbstractString, pat=>repl)` if repl is a function, now calls | |||
| `repl(s[pos]::Char)`, when pat is a `Char` or collection of `Char` or predicate, | |||
There was a problem hiding this comment.
Seems easier to just say now calls repl(c) for each matched character c::Char, instead of a substring as it did previously. No need to define pos, and it would be clearer to say what the old behavior was.
|
Looks okay to merge. We might want to implement a faster |
According to a proposal of @nalimilan in #25396 .
If the replacement pattern is a function, it should be a function of
Char, if the pattern is aChar, a collection ofChars, or a predicate function (which acts on a singleChar), and a function of the matched substring otherwise.