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.

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

str_replace errors when using "$" as a replacement. #83

Closed
dkesh opened this Issue Jun 25, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@dkesh

dkesh commented Jun 25, 2015

The code below works for me on some boxes, but not others and I am attempting to figure out what the difference is:

> stringr::str_replace("aba", "b", "$" )
Error in stri_replace_all_regex(x, c("\\$", "\\\\(\\d)"), c("\\\\$", "\\$$1"),  : 
  Unknown ICU error. (U_REGEX_INVALID_CAPTURE_GROUP_NAME)

Enter a frame number, or 0 to exit   

1: stringr::str_replace("aba", "b", "$")
2: fix_replacement(replacement)
3: stri_replace_all_regex(x, c("\\$", "\\\\(\\d)"), c("\\\\$", "\\$$1"), vecto
@dkesh

This comment has been minimized.

dkesh commented Jun 25, 2015

stringr version 1.0.0 and stringi version 0.5.2 on both working and non-working boxes.

@dkesh

This comment has been minimized.

dkesh commented Jun 25, 2015

On the boxes that fail (both Ubuntu and OS X):

> stringi::stri_install_check()
stringi_0.5.2; en_US.UTF-8; ICU4C 55.1; Unicode 7.0
All tests completed successfully.

On the box that works:

> stringi::stri_install_check()
stringi_0.5.2; en_US.UTF-8; ICU4C 52.1; Unicode 6.3
All tests completed successfully.
@gagolews

This comment has been minimized.

Contributor

gagolews commented Jun 25, 2015

$ in a replacement string (in a regex search task) has a special meaning - it refers to a capture group. It should be followed by an integer. So to use $, it shall be escaped.

stringr::str_replace("aba", "b", "\\$" )

you may also want to consider using fixed("b") as a search patter in this example.

From syntactic view point, this was an error too in ICU 52.1, but apparently in ICU developers decided to throw an error and no longer ignore it in a later ICU release (like 55).

In other words, this is not a bug.

@dkesh

This comment has been minimized.

dkesh commented Jun 25, 2015

Running the escaped example returns this:

stringr::str_replace( "aba", "b", "$")
[1] "a$a"

Perhaps stringr:::fix_replacement is no longer necessary on newer versions of ICU?

@dkesh

This comment has been minimized.

dkesh commented Jun 25, 2015

The second backslashes in each of the code line got gobbled up by markdown; just imagine they are there.

@gagolews

This comment has been minimized.

Contributor

gagolews commented Jun 25, 2015

Hmmm.. I didn't know that stringr is "fixing" replacement strings.. I will look at that tomorrow.

@gagolews

This comment has been minimized.

Contributor

gagolews commented Jun 27, 2015

All righty, I filed a patch for that: #84.

@dkesh

This comment has been minimized.

dkesh commented Jun 27, 2015

Thanks!

@dkesh dkesh closed this Jun 27, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment