Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Viral B. Shah
committed
Jan 30, 2012
1 parent
5b184ae
commit 2849191
Showing
2 changed files
with
9 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2849191
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little dubious about using this one. PCRE has a lot of configure-time options and the behavior of regular expression are pretty central to the language. Have you tried this out with the system PCRE? Does everything appear to work fine?
2849191
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2849191
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it's central in the sense that it's not a feature you can leave out. If PCRE is compiled with the wrong options or doesn't support the same options as the ones we expect, regex support will just break. For example, we definitely need Unicode support turned on. I have no idea why the hashing tests would fail with this change. Strange.
2849191
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2849191
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2849191
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they're definitely not sufficient. I'll take a crack at that.