Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

replace(::String, count=1) with empty string regression #36953

Closed
mortenpi opened this issue Aug 6, 2020 · 3 comments · Fixed by #36959
Closed

replace(::String, count=1) with empty string regression #36953

mortenpi opened this issue Aug 6, 2020 · 3 comments · Fixed by #36959
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version

Comments

@mortenpi
Copy link
Contributor

mortenpi commented Aug 6, 2020

On 1.4.2:

julia> replace("xyx", "" => '_', count=1)
"_xyx"

vs. 1.5.0 and master:

julia> replace("xyx", "" => '_', count=1)
"_x_y_x_"
@mortenpi mortenpi mentioned this issue Aug 6, 2020
Closed
@mortenpi
Copy link
Contributor Author

mortenpi commented Aug 7, 2020

Git bisect blames a6e27b3 from #32348. I assume though that it's just revealing a bug in replace.

cc @sostock

@sostock
Copy link
Contributor

sostock commented Aug 7, 2020

The problem is this line:

julia/base/strings/util.jl

Line 524 in 7c0cb30

r == 0:-1 || n == count && break

r == 0:-1 was never true with the old behavior, but now 0:-1 is equal to any empty range. There was a similar bug with split (which was caught by the tests and thus fixed in #32348). I can prepare a fix.

@KristofferC KristofferC added bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version labels Aug 7, 2020
sostock added a commit to sostock/julia that referenced this issue Aug 7, 2020
@sostock
Fix replace (JuliaLang#36953)
ebb0996
@sostock sostock mentioned this issue Aug 7, 2020
Merged
@vtjnash
Copy link
Member

vtjnash commented Aug 7, 2020

This is a side-effect of the larger bug #36768

@KristofferC KristofferC mentioned this issue Aug 10, 2020
Merged
25 tasks
KristofferC pushed a commit that referenced this issue Aug 10, 2020
@sostock
Fix replace (#36953) (#36959)
00f2133
KristofferC pushed a commit that referenced this issue Aug 10, 2020
@sostock @KristofferC
Fix replace (#36953) (#36959)
dc767cf 
(cherry picked from commit 00f2133)
simeonschaub pushed a commit to simeonschaub/julia that referenced this issue Aug 11, 2020
@sostock @simeonschaub
Fix replace (JuliaLang#36953) (JuliaLang#36959)
7fe031b
KristofferC pushed a commit that referenced this issue Aug 18, 2020
@sostock @KristofferC
Fix replace (#36953) (#36959)
992b562 
(cherry picked from commit 00f2133)
KristofferC pushed a commit that referenced this issue Aug 19, 2020
@sostock @KristofferC
Fix replace (#36953) (#36959)
1f2b5f4 
(cherry picked from commit 00f2133)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix replace(::String, count=1) with empty pattern (issue #36953) sostock/julia
4 participants
@mortenpi @vtjnash @KristofferC @sostock