Add Regex.import/1 #14910
Add Regex.import/1 #14910
Conversation
| Regex.import(~r/foo/E) | ||
| ~r/foo/ |
There was a problem hiding this comment.
Real doc test doesn't work because of ref comparison
lib/elixir/lib/regex.ex
Outdated
|
|
||
| {:re_pattern, _, _, _, _} -> | ||
| if Code.ensure_loaded?(:re) and function_exported?(:re, :import, 1) do | ||
| raise ArgumentError, "Expected an exported Regex, got: #{inspect(regex)}" |
There was a problem hiding this comment.
I think already imported regexes should be a no-op. This will allow consumers to write code that always get an imported regex at the end, without having to worry if the caller gave an exported regex or not.
There was a problem hiding this comment.
OK, this was what I originally had in mind in our previous discussion.
I agree this would be more convenient and there shouldn't be a real downside.
My only point of concern is that the import name doesn't hint at this behavior, but I don't have a better idea in mind (ensure_imported? meh) and it's probably fine.
|
@sabiwara looks good to me! In theory, because our regexes have the source and compile opts, they could be recompiled too. This would be useful in cases where you wrote the full regex (without export) somewhere, and now you need to make it work again. Therefore, we could have this:
On the other hand, With all that said, I think the current action plan is good:
Anyway, I thought I would write my thoughts down for future discussions. |
|
Done in 5efccba. I wonder if we should add the symmetrical for completion, although I'm not sure what the use-case would be. Sending over a node a dynamic regex? |
2 participants
Complements #14907