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

Regex improvements break Java 6 compatibility #836

Closed
olivergierke opened this Issue Jul 1, 2014 · 6 comments

Comments

Projects
None yet
4 participants
@olivergierke

This commit breaks compatibility with Java 6 runtimes trying to load the TemplateFactory:

Caused by: java.util.regex.PatternSyntaxException: Unknown look-behind group near index 15
\{(?<premod>%?%?)(?<index>\d+)(?<postmod>[slu%]?%?)\}
               ^
    at java.util.regex.Pattern.error(Pattern.java:1713)
    at java.util.regex.Pattern.group0(Pattern.java:2505)
    at java.util.regex.Pattern.sequence(Pattern.java:1806)
    at java.util.regex.Pattern.expr(Pattern.java:1752)
    at java.util.regex.Pattern.compile(Pattern.java:1460)
    at java.util.regex.Pattern.<init>(Pattern.java:1133)
    at java.util.regex.Pattern.compile(Pattern.java:823)
    at com.mysema.query.types.TemplateFactory.<clinit>(TemplateFactory.java:38)
    ... 64 more

This happens on a JDK 6 on Mac:

Serendipity:spring-data-jpa @ master olivergierke $ java -version
java version "1.6.0_65"
Java(TM) SE Runtime Environment (build 1.6.0_65-b14-462)
Java HotSpot(TM) 64-Bit Server VM (build 20.65-b04-462, mixed mode)

as well as on Linux boxes (see this build log for example). Unfortunately the commit doesn't contain any explanation what the context of this improvement is or what the drivers behind it were.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Jul 1, 2014

Member

I see, I am greatly sorry, as I was just focusing on the behavior of the regular expression.
It used to be dynamic sub-stringing, where regular expression seemed to be much more clear in what was happening.
I didn't know that Java 6 doesn't support named capturing groups, but after researching found this to be true.
I will change it to numbered groups instead of named groups.
Thanks for the issue report.

Member

Shredder121 commented Jul 1, 2014

I see, I am greatly sorry, as I was just focusing on the behavior of the regular expression.
It used to be dynamic sub-stringing, where regular expression seemed to be much more clear in what was happening.
I didn't know that Java 6 doesn't support named capturing groups, but after researching found this to be true.
I will change it to numbered groups instead of named groups.
Thanks for the issue report.

@olivergierke

This comment has been minimized.

Show comment
Hide comment
@olivergierke

olivergierke Jul 1, 2014

No worries, I wish we could leave Java 6 behind us as well :).

No worries, I wish we could leave Java 6 behind us as well :).

@timowest timowest closed this in #837 Jul 6, 2014

@timowest timowest reopened this Jul 6, 2014

@timowest timowest added bug labels Jul 6, 2014

@timowest timowest added this to the 3.4.2 milestone Jul 9, 2014

@timowest timowest modified the milestone: 3.4.2 Jul 19, 2014

@timowest timowest removed the fixed label Jul 19, 2014

@timowest timowest closed this Jul 19, 2014

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jul 29, 2014

Member

Released in 3.4.2

Member

timowest commented Jul 29, 2014

Released in 3.4.2

@cmhuynh

This comment has been minimized.

Show comment
Hide comment
@cmhuynh

cmhuynh Jul 30, 2014

Love QueryDSL so much. I am about to submit the same thing, and found you just release 3.4.2. Awesome. Would be terrific if you add the release notes http://www.querydsl.com/releases.html

cmhuynh commented Jul 30, 2014

Love QueryDSL so much. I am about to submit the same thing, and found you just release 3.4.2. Awesome. Would be terrific if you add the release notes http://www.querydsl.com/releases.html

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jul 30, 2014

Member

Yes, sorry, I will update the release notes today.

Member

timowest commented Jul 30, 2014

Yes, sorry, I will update the release notes today.

@cmhuynh

This comment has been minimized.

Show comment
Hide comment
@cmhuynh

cmhuynh Aug 2, 2014

That's awesome. The new release will be discovered easier. Cheers
On Jul 30, 2014 10:56 PM, "Timo Westkämper" notifications@github.com
wrote:

Yes, sorry, I will update the release notes today.


Reply to this email directly or view it on GitHub
#836 (comment).

cmhuynh commented Aug 2, 2014

That's awesome. The new release will be discovered easier. Cheers
On Jul 30, 2014 10:56 PM, "Timo Westkämper" notifications@github.com
wrote:

Yes, sorry, I will update the release notes today.


Reply to this email directly or view it on GitHub
#836 (comment).

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