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

String.split(x: Array[Char]) produces bad regexes #105

Closed
gzm0 opened this Issue Jan 11, 2014 · 7 comments

Comments

Projects
None yet
3 participants
@gzm0
Copy link
Contributor

gzm0 commented Jan 11, 2014

Just like split(x: Char) (see #65), StringLike relies on the \Q, \E escapes for Regex creation. We now have to deal with the following pattern:

[\Q<char>\E\Q<char>\E<etc>]

This produces regexes such as:

[\Q-\E]

Which then cause an exception in the JavaScript regex matcher.

@sjrd Should we handle this as a hack just like #65, knowing that this is substantially more complex?

@ghost ghost assigned gzm0 Jan 11, 2014

@sjrd

This comment has been minimized.

Copy link
Member

sjrd commented Jan 11, 2014

Why is this more complex?

@gzm0

This comment has been minimized.

Copy link
Contributor

gzm0 commented Jan 11, 2014

Sorry, I wasn't expressing myself very clearly: I meant the regex creation at runtime will become more complex and hence slower.

@sjrd

This comment has been minimized.

Copy link
Member

sjrd commented Jan 11, 2014

Hum ... I don't know. I wouldn't care if that particular method was slow, but IIUC, in order to support it, we would have to slow down all regex creations, which is not a good idea.

@gzm0

This comment has been minimized.

Copy link
Contributor

gzm0 commented Jan 11, 2014

Exactly. We already do this to support split(x: Char), but what we have to match against is less complex. We might have no choice but to re-implement StringLike. :(

@jonas

This comment has been minimized.

Copy link
Contributor

jonas commented Jan 12, 2014

FYI, to avoid duplicate work, I am creating a test suite for the Java regexp emulation. My plan is to post a pull request today.

@gzm0

This comment has been minimized.

Copy link
Contributor

gzm0 commented Jan 13, 2014

Originating partest: run/t0325.scala

@gzm0

This comment has been minimized.

Copy link
Contributor

gzm0 commented Jan 13, 2014

As discussed with @sjrd, we will currently not support this. Leaving open until properly documented.

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