-
Notifications
You must be signed in to change notification settings - Fork 318
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 Fallback to Prim_Text_Helper.compile_regex; accept Regex in Text.parse_to_table #7297
Conversation
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.
A couple of minor changes but looks good.
distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table_Conversions.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Parse_To_Table.enso
Outdated
Show resolved
Hide resolved
case_insensitive = case_sensitivity.is_case_insensitive_in_memory | ||
pattern = Regex.compile pattern_string case_insensitive=case_insensitive | ||
pattern = case regex_or_pattern_string of | ||
_ : Regex -> regex_or_pattern_string |
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.
_ : Regex -> regex_or_pattern_string | |
_ : Regex -> regex_or_pattern_string.recompile case_sensitivity |
Will need to be rebased on my one for this to work.
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.
Done.
…rsions.enso Co-authored-by: James Dunkerley <jdunkerley@users.noreply.github.com>
…o_Table.enso Co-authored-by: James Dunkerley <jdunkerley@users.noreply.github.com>
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.
Fallback in RegexCompileNode
seems OK.
p = Regex.compile "[a-z]" | ||
Test.expect_panic_with (Regex.compile p) Type_Error |
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.
Given this behaviour (that I think is good), do we still need the parseRegexPattern
?
I assume it should not be allowed to pass an already-compiled regex to compile_regex
, we should use James's recompile
instead - e.g. to update case sensitivity.
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.
My understanding is that parseRegexPattern is for caching compiled regexes, so if you compile the same one multiple times, it uses the cache. This test is for passing a non-string to Regex.compile.
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.
Ah sorry! I read that wrong apparently.
Pull Request Description
This PR does three related things:
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.