[CLJ-753] clojure.string/replace-first returns nil with replacement fn when regex doesn't match Created: 11/Mar/11  Updated: 26/Jul/13  Resolved: 24/Aug/12

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 2
Labels: None

Attachments: Text File 0001-clojure.string-replace-first-now-returns-the-origina.patch     Text File clojure-string-with-no-match-returns-original-2.patch     Text File clojure-string-with-no-match-returns-original.patch    
Patch: Code and Test
Approval: Vetted


Originally reported by Takahiro Hozumi

With a function as the "replacement" argument, clojure.string/replace-first returns nil if there is no match, instead of returning the original string unchanged.

user=> (use 'clojure.string)
user=> (replace-first "abcdef" "ghi" "jkl")
user=> (replace-first "abcdef" #"ghi" "jkl")
user=> (replace-first "abcdef" #"ghi" (fn [a] "jkl"))

Comment by Federico Brubacher [ 21/Mar/11 7:36 AM ]

This is my patch for this issue. I have a CA signed. Any suggestions and i can try again. Federico

Comment by Chris Perkins [ 28/Nov/11 9:31 AM ]

Same fix as Frederico's patch, but also removes unnecessary allocation of a StringBuffer when there is no match, for both replace and replace first.

Comment by Chris Perkins [ 28/Nov/11 9:46 AM ]

Same again, but with docstring typo fixes too.

Comment by Steve Miner [ 28/Nov/11 10:12 AM ]

You should use StringBuilder instead of StringBuffer.


Comment by Chris Perkins [ 28/Nov/11 10:37 AM ]

Yup, that's what I thought, but it turns out it doesn't work because java's Matcher/appendReplacement requires a StringBuffer, unfortunately.

Comment by Andy Fingerhut [ 28/Nov/11 5:09 PM ]

I've tested Chris's clojure-string-with-no-match-returns-original-2.patch against latest github Clojure as of this morning, and the tests pass. Cool that he found a straightforward performance improvement for the no match case. The code appears correct. I also verified that for the reason Chris mentions, StringBuilder will not work as a replacement for StringBuffer. At least there will never be any contention on the locks implementing the StringBuffer synchronization the way they are used here.

Comment by Stuart Sierra [ 29/Nov/11 8:45 AM ]

Vetted & moved to Approved Backlog.

Comment by Andy Fingerhut [ 12/Jan/12 12:06 AM ]

CLJ-870 has an attached proposed combined patch for this issue and that one.

Comment by Andy Fingerhut [ 18/Aug/12 12:09 PM ]

This issue should now be corrected with the patch, attached to CLJ-870, that was applied on Aug 18, 2012.

Comment by Stuart Sierra [ 24/Aug/12 7:41 AM ]

CLJ-753 patch applied for CLJ-870 addresses this one, too, applied Aug 18, 2012

Generated at Sun Mar 24 10:25:19 CDT 2019 using JIRA 4.4#649-r158309.