<< Back to previous view

[CLJS-485] clojure.string/replace ignores regex flags Created: 12/Mar/13  Updated: 08/Jun/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: Esa Virtanen Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: bug, patch, test

Attachments: Text File 0001-Take-regex-flags-m-i-into-account-in-clojure.string-.patch     Text File CLJS-485.patch    
Patch: Code and Test

 Description   

The replace function in namespace clojure.string ignores regex flag provided in the match pattern. For example:

CLJS
clojure.string/replace "I am NOT matched" #"(?i)not " "")
=> "I am NOT matched"
CLJ
clojure.string/replace "I am NOT matched" #"(?i)not " "")
=> "I am matched"

The attached patch fixes this by parsing the m and i flags, if set, from the match object, instead of explicitly setting only "g".



 Comments   
Comment by Chas Emerick [ 19/Mar/14 9:29 AM ]

I can confirm the bug. The attached patch applies cleanly, and works as expected.

Esa, sorry for the long delay (this one must have slipped through the cracks)! Could you please submit a contributor's agreement, so that your patch can be merged? More info is here:

http://clojure.org/contributing

Comment by lvh [ 26/Jul/15 5:56 PM ]

I got bit by this bug. Working on figuring out if I can sign that agreement.

Comment by lvh [ 27/Jul/15 11:55 AM ]

This is a duplicate of CLJS-794.

Comment by Jake McCrary [ 04/Feb/16 6:58 PM ]

This patch changes string/replace-all to respect flags that were set on regexp passed as an argument.

I originally attached this to CLJS-794 and then noticed there was this older issue. I was unable to figure out how to edit at ticket to mark the patch as having "Code and Test" so I'm adding it to this issue instead.

I've signed a contributors agreement.

Comment by Mike Fikes [ 04/Feb/16 9:31 PM ]

There is a "sticky" flag y that could be conveyed.

http://www.ecma-international.org/ecma-262/6.0/index.html#sec-get-regexp.prototype.sticky

Comment by Jake McCrary [ 06/Feb/16 10:50 AM ]

Reading a bit more about it and looks like both 'u' and 'y' are newly supported in ECMA6. Is there a way to write tests that exercise this functionality? I've got changes locally to get support of 'y' but felt the need to write a test for it (as its a bit more complicated than simply looking for a flag) and am hung up on having EMCA6 support while running the tests.

I'm actually wondering if having 'u' and 'y' in there is a bit premature. Any guidance on whether adding code for 'u' and 'y' should be done (or removing 'u' from my patch) or testing ClojureScript with ECMAScript 6 support?

Comment by Mike Fikes [ 06/Feb/16 12:51 PM ]

One thought: Whatever this is exercising passes on recent versions of V8, JavaScriptCore, SpiderMonkey, Nashorn, and ChakraCore: https://github.com/clojure/clojurescript/blob/628d957f3ecabf8d26d57665abdef3dea765151e/src/test/cljs/cljs/core_test.cljs#L1472

It does seem tricky to write a robust RegExp clone implementation, and if you do some googling you see people dealing with u and y as special cases. The patch seemed OK to me with respect to this, but I'm not a JavaScript expert.

Generated at Sat Oct 01 10:38:31 CDT 2016 using JIRA 4.4#649-r158309.