Skip to content
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 - Optional Unicode properties support, and prefer brewed libpcre on MacOS #4712

Closed
wants to merge 4 commits into from

Conversation

akzhan
Copy link
Contributor

@akzhan akzhan commented Jul 15, 2017

Current crystal depends on pcre instead of libpcre. It breaks linking to newest PCRE under MacOS.

I have changed link attribute and add specs for UCP support.

/(*UCP)[[:alnum:]]/.match "à"

UCP support disabled by default due to bad performance, but can be used by "(*UCP)" flag or by Regex::Options::UCP option.

This pull request depends on PCRE 8.10 or above.

Fixes #4704.

@akzhan
Copy link
Contributor Author

akzhan commented Jul 15, 2017

CrcleCI has too old PCRE.

@RX14
Copy link
Contributor

RX14 commented Jul 15, 2017

We should find a way to degrade gracefully as we do with openssl. Use pkgconfig to query the library version etc.

@akzhan
Copy link
Contributor Author

akzhan commented Jul 15, 2017

@RX14 thanks, I'll do. Anyway it should be optional flag.

@akzhan akzhan changed the title Regexp - Unicode codepoints support [WIP] Regexp - Unicode codepoints support Jul 16, 2017
2) prefer libpcre instead of pcre to build correctly on MacOS with newer brewed libpcre.
@akzhan akzhan changed the title [WIP] Regexp - Unicode codepoints support Regex - Optional Unicode properties support, and prefer brewed libpcre on MacOS Jul 23, 2017
@akzhan
Copy link
Contributor Author

akzhan commented Jul 23, 2017

CircleCI is strange again. std spec built ok, but all_spec not.

(Regex.new("\\w", Regex::Options::UCP).match "à").should be_truthy
end
else
pending "PCRE built without Unicode properties support" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary. Is it done anywhere else in specs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nowhere. It's just declaration that sometimes Crystal will be built with UCP-friendly PCRE only.

@akzhan
Copy link
Contributor Author

akzhan commented Jul 25, 2017

I'll close this pull request because want to change regex engine to successor (PCRE2) that built with UCP support by default everywhere and has well known stable API.

@akzhan akzhan closed this Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants