Add Regex.to_embed/2

[demerphq]

This patch, which works on 1.18.x but does not work on 1.19 is an attempt to implement String.Chars protocol and also a Regex.to_string() and Regex.modifiers() and Regex.to_string!() and Regex.modifiers!() functions.

The idea is to make it possible to safely embed precompiled regexes into other regexes in a similar way as that supported by perl. The general idea is that ~r/foo/x turns into "(?x-ims:foo\n)", and etc. Thus it should match the same as it would have in its original form when it is embedded into a pattern which has a different set of modifiers.

For review by Jose.

[josevalim]

Thank you for the PR. Given Regex.to_string cannot fully convert all regular expressions created by ~r/.../, only a subset of modifiers are supported, my suggestion is to simplify this considerably:

1. There is no need to introduce Regex.modifiers, given the implementation in Inspect and String.Chars will be different anyway

2. Generally speaking, our conversion functions (to_*) do not return {:ok, _} tuples, they simply return the result

3. Do not implement String.Chars, if it can only convert a subset of the created regexes

So my suggestion is to add a single function, called Regex.to_embed/1, that receives a regex and either returns a string or raises an ArgumentError, and then test this function directly. This function will have its own implementation of Inspect.Range.translate_options tailored to its needs. Then in a separate pull request, I can make it so ~r/#{...}/ works by doing AST traversal in the sigil macro.

[demerphq]

Hi, I have updated the change as you requested. I made one tweak, adding a "strict" option for to_embed!(regex,strict) and make to_embed(regex) be the same as to_embed!(regex,false). When the pattern has been compiled with an option that cant be represented in an embeddable form an ArgumentError will be raised if strict is true, if strict is false such options will be ignored. The non strict mode is particularly helpful if the pattern was compiled with /u and is to be embedded in a pattern that is also compiled with /u.

FWIW, More modern versions of PCRE may one day support more options that just 'imsx'. Perl itself supports 'u' in embeddable form (although it has slightly different meaning, in Elixir/Erlang/PCRE the /u flag means "string encoded as unicode" and also "use unicode semantics". In the Perl /u means "use unciode semantics regardless of the encoding". This is why the exceptions mentions the current version of PCRE.

Hope this is what you had in mind with your feedback!

[josevalim]

Looks great, I have dropped only some minor suggestions now and we can ship it!

[demerphq]

Suggestions turned to commits, with one caveat about a compromise wording as noted, and I followed up on your point about to_string(). I didnt squash so its easier for you to review, you said previously you didnt mind doing that yourself.

[josevalim]

💚 💙 💜 💛 ❤️