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

Add Regex::CompileOptions::MULTILINE_ONLY #14870

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ralsina
Copy link

@ralsina ralsina commented Aug 6, 2024

Adds a new MULTILINE_ONLY flag to regexes.

I created an issue here:

#14869

As discussed here:

https://forum.crystal-lang.org/t/regex-that-is-multiline-but-not-dotall-how/7054/12

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

issue: The translation needs to be implemented in pcre_compile_options as well.

Also would be nice to have some specs for testing the behaviour.

src/regex.cr Outdated Show resolved Hide resolved
src/regex.cr Outdated Show resolved Hide resolved
src/regex.cr Outdated Show resolved Hide resolved
src/regex/pcre2.cr Outdated Show resolved Hide resolved
src/regex.cr Show resolved Hide resolved
Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
@straight-shoota straight-shoota changed the title Add MULTILINE_ONLY flag for regex Add Regex::CompileOptions::MULTILINE_ONLY Aug 6, 2024
@ralsina
Copy link
Author

ralsina commented Aug 8, 2024

Re-added the special handling for MULTILINE (setting MULTILINE and DOTALL) because removing it broke a test.

The remaining test failure happens on main too (at spec/std/regex_spec.cr:19)

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

The case for MULTILINE is only necessary because the value of MULTILINE_ONLY is not a composite of MULTILINE. Change it to 0x0000_0004 and it should work.

And the test failure is in fact introduced by this change because the test uses 0x0000_0040 as an undefined option value (but is now defined).

src/regex.cr Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It's impossible to create a regex that is MULTILINE but not DOT_ALL
5 participants