-
Notifications
You must be signed in to change notification settings - Fork 323
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
Data analysts should be able to use Text.replace
to substitute parts of the text
#3393
Conversation
ea9240d
to
27d4f78
Compare
IntArrayBuilder codeunit_start_mapping = new IntArrayBuilder(charSequence.length() + 1); | ||
IntArrayBuilder codeunit_end_mapping = new IntArrayBuilder(charSequence.length() + 1); |
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.
These two additional mappings are used in replace
but not in location_of
.
For simplicity I always compute them - this is just creating 2 additional arrays along with the third one that is always created, so the cost shouldn't be awful. I prefer to keep it this way for simplicity instead of maintaining two pieces of code that do the same thing (which is prone to desynchronization).
Still, if anyone thinks that it's worth cutting the extra cost, I can make it into two separate functions. I just thought it is a premature optimization and decided to go for more readable code - we can always revisit it when we will have performance issues with Text
and approach this correctly with comparative benchmarks instead of my gut instinct of what is important for perf. and what is not.
4458478
to
1ef58cc
Compare
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Text/Regex/Engine/Default.enso
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Text/Regex/Engine/Default.enso
Outdated
Show resolved
Hide resolved
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.
Couple of minor things but other wise LGTM
Pull Request Description
Implements https://www.pivotaltracker.com/story/show/181266274
Important Notes
Checklist
Please include the following checklist in your PR:
./run dist
and./run watch
.