Make linewrapping plugin recompute breaks before scrolling #1190
Conversation
| local old_scroll_to_make_visible = DocView.scroll_to_make_visible | ||
| function DocView:scroll_to_make_visible(line, col) | ||
| LineWrapping.update_docview_breaks(self) |
There was a problem hiding this comment.
Seems like overriding DocView:scroll_to_line is enough to fix the issue with the projectsearch plugin. Any reason to also call LineWrapping.update_docview_breaks on DocView:scroll_to_make_visible? I'm worried that calling LineWrapping.reconstruct_breaks (which is called by update_docview_breaks) will hurt performance more since it is already been called on each update().
Otherwise the fix is good.
There was a problem hiding this comment.
I am under the impression that update_docview_breaks will only call reconstruct_breaks if necessary, so only once. It'll check the width a few extra times, but that's relatively harmless, IMO.
I added it to the other scroll function as well in case any other plugins or modifications use it, to make sure everything is as robust as possible. One function call with an if statement is an acceptable overhead, I think.
There was a problem hiding this comment.
I did some core.log testing directly on reconstruct_breaks and it was executing its code on every call, it doesn't seems to implement any state tracking for now. And adding the update breaks to scroll visible just increases the amount of times it gets called when manually scrolling the docview
There was a problem hiding this comment.
my bad, that isn't correct... it is only been triggered now on scroll hover with the call to LineWrapping.update_docview_breaks(self) on DocView:scroll_to_make_visible
Edit: Actually I'm wrong again, should get some sleep xD
|
Ok merging this which is good after some play with linewrapping and its code :) |
This ensures that having linewrapping enabled does not break projectsearch. I have also opened an issue for this, but in the plugins repo: lite-xl/lite-xl-plugins#148