[CLJ-753] clojure.string/replace-first returns nil with replacement fn when regex doesn't match Created: 11/Mar/11 Updated: 01/Mar/13 Resolved: 24/Aug/12 |
|
| Status: | Closed |
| Project: | Clojure |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | Approved Backlog |
| Type: | Defect | Priority: | Minor |
| Reporter: | Stuart Sierra | Assignee: | Stuart Sierra |
| Resolution: | Completed | Votes: | 2 |
| Labels: | None | ||
| Attachments: |
0001-clojure.string-replace-first-now-returns-the-origina.patch
clojure-string-with-no-match-returns-original-2.patch
clojure-string-with-no-match-returns-original.patch
|
| Patch: | Code and Test |
| Approval: | Vetted |
| Description |
|
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) nil user=> (replace-first "abcdef" "ghi" "jkl") "abcdef" user=> (replace-first "abcdef" #"ghi" "jkl") "abcdef" user=> (replace-first "abcdef" #"ghi" (fn [a] "jkl")) nil |
| Comments |
| 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. http://stackoverflow.com/questions/355089/stringbuilder-and-stringbuffer-in-java |
| 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 ] |
|
|
| Comment by Andy Fingerhut [ 18/Aug/12 12:09 PM ] |
|
This issue should now be corrected with the patch, attached to |
| Comment by Stuart Sierra [ 24/Aug/12 7:41 AM ] |
|
|