[groovy-user] String.replaceAll and the closure value

[Jim White]

Before I open a JIRA and wind up in another lengthy debate, is there
anyone who wants to defend that String.replaceAll(String, Closure) is
passing the stringified closure result to Matcher.appendReplacement?

The consequence is that the resulting string is interpreted as a
"replacement string" which treats '$' and '\' specially so that group
replacements can be done. For my purposes that definitely undesireable.

The whole reason I see for the closure API is so that those
substitutions are done in the closure and that the closure expects it's
result string to be substituted literally.

An example of the problem is like this:

groovy> println 'x123z45'.replaceAll(/([^z]*)(z)/, { all, m, d -> m })
x12345

groovy> println '$123z45'.replaceAll(/([^z]*)(z)/, { all, m, d -> m })

$1232345

The fix is that quoteReplacement needs to be called on the result before
appendReplacement is called. This is similar to the problem we had with
minus.

http://jira.codehaus.org/browse/GROOVY-1637

Hmmm, there they avoided the problem that quoteReplacement is JDK 1.5 by
not using replaceAll. Looks like we need to implement
RegexUtils.quoteReplacement after all.

Jim


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email

[Paul King]

Post by Jim White
Hmmm, there they avoided the problem that quoteReplacement is JDK 1.5 by
not using replaceAll. Looks like we need to implement
RegexUtils.quoteReplacement after all.

I suspect the overall change is desirable.

Just on the last sentence. You are just meaning for the 1_5_X branch?
I presume we could (should?) remove RegexUtils from 1.6+.

Paul.

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email

[Jim White]

Post by Paul King

Post by Jim White
Hmmm, there they avoided the problem that quoteReplacement is JDK 1.5
by not using replaceAll. Looks like we need to implement
RegexUtils.quoteReplacement after all.

I suspect the overall change is desirable.
Just on the last sentence. You are just meaning for the 1_5_X branch?
I presume we could (should?) remove RegexUtils from 1.6+.

I agree with not using RegexUtils within Groovy itself, but I don't
think it is a good idea to remove it. It is groovy.text.RegexUtils and
therefore public API. You may deprecate at this time, but not delete.

JIRA in progress.

http://jira.codehaus.org/browse/GROOVY-3248

Jim


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email

[Ted Naleid]

I'd love to see this behavior changed/fixed as well. I ran across
this when I was writing an article for this month's groovymag on
regular expressions and call out this gotcha explicitly as it bit me a
couple of times in some examples I was trying to write.

In groovy 1.6, you have explicit access to all of the capture groups
already through the parameters passed to the closure, so there isn't
much point in this behavior and it ends up just being much more
confusing than helpful.

-Ted

Post by Jim White
Before I open a JIRA and wind up in another lengthy debate, is there
anyone who wants to defend that String.replaceAll(String, Closure)
is passing the stringified closure result to
Matcher.appendReplacement?
The consequence is that the resulting string is interpreted as a
"replacement string" which treats '$' and '\' specially so that
group replacements can be done. For my purposes that definitely
undesireable.
The whole reason I see for the closure API is so that those
substitutions are done in the closure and that the closure expects
it's result string to be substituted literally.
groovy> println 'x123z45'.replaceAll(/([^z]*)(z)/, { all, m, d ->
m })
x12345
groovy> println '$123z45'.replaceAll(/([^z]*)(z)/, { all, m, d ->
m })
$1232345
The fix is that quoteReplacement needs to be called on the result
before appendReplacement is called. This is similar to the problem
we had with minus.
http://jira.codehaus.org/browse/GROOVY-1637
Hmmm, there they avoided the problem that quoteReplacement is JDK
1.5 by not using replaceAll. Looks like we need to implement
RegexUtils.quoteReplacement after all.
Jim
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email