-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Avoid copying fragments #3136
Avoid copying fragments #3136
Conversation
f071e5b
to
34e5d60
Compare
In most cases we do want to use a |
It's more of an opinion, but I think it's always better to not hide computational complexity in APIs. A lot of use cases want to iterate over the fragment, which can be done without copying the contents. Ideally we could also avoid the places where we turn the |
The For cases where we're iterating over raw rope chunks, there's
Yeah this is a known issue, we can't really solve this until the next version of regex-automata is finished (rust-lang/regex#425) or by implementing our own NFA. Current implementation uses |
Ok I'll change this to add a The regex functions are actually the only ones that need |
b1b4b93
to
02b5163
Compare
3e11e4a
to
a4781e2
Compare
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.
Thanks for working on this! 🎉
* Avoid copying fragments * Add slice / slices method * Better documentation for fragment and slice methods
Might fix #3127.This changes the
Range::fragment
andSelection::fragments
to returnRopeSlice
. This way we can avoid copying the ropes' contents in a few cases, specificallyshell_impl
switch_to_*case
sort
androtate_selection_contents
Also, it will make the computational overhead of copying the fragments more apparent in places where
Range::fragment
is used (until now it almost looked like there was no copy, since aCow
was returned).(I also rewrite the mouse event handling because (I think, without optimizations) until now it copied the entire file on each mouse up event)