Speedup matching

[mheinzler]

Currently live matching will always search through the whole file which can be very slow for large files and more complex regex patterns.

With this change it will only search through the currently visible region of text.

[rchl]

I've tried to run with this change for a day and I'm seeing a lot of errors:

Traceback (most recent call last):
  File "/Applications/Sublime Text.app/Contents/MacOS/Lib/python33/sublime_plugin.py", line 1310, in run_
    self.view.end_edit(edit)
  File "/Applications/Sublime Text.app/Contents/MacOS/Lib/python33/sublime.py", line 961, in end_edit
    sublime_api.view_end_edit(self.view_id, edit.edit_token)
  File "/Applications/Sublime Text.app/Contents/MacOS/Lib/python33/sublime_plugin.py", line 775, in on_selection_modified
    run_view_callbacks('on_selection_modified', view_id)
  File "/Applications/Sublime Text.app/Contents/MacOS/Lib/python33/sublime_plugin.py", line 616, in run_view_callbacks
    callback(v, *args)
  File "/Applications/Sublime Text.app/Contents/MacOS/Lib/python33/sublime_plugin.py", line 129, in profiler
    return event_handler(*args)
  File "/Users/dme/Library/Application Support/Sublime Text/Installed Packages/TrailingSpaces.sublime-package/trailing_spaces.py", line 461, in on_selection_modified
    match_trailing_spaces(view)
  File "/Users/dme/Library/Application Support/Sublime Text/Installed Packages/TrailingSpaces.sublime-package/trailing_spaces.py", line 178, in match_trailing_spaces
    (matched, highlightable) = find_trailing_spaces(view)
  File "/Users/dme/Library/Application Support/Sublime Text/Installed Packages/TrailingSpaces.sublime-package/trailing_spaces.py", line 125, in find_trailing_spaces
    sel = view.sel()[0]
  File "/Applications/Sublime Text.app/Contents/MacOS/Lib/python33/sublime.py", line 748, in __getitem__
    raise IndexError()
IndexError

So I wouldn't recommend it in its current state.

[mheinzler]

Sorry, missed to include the fix for that in this branch. It should be working now.

[FichteFoll]

Because this PR was included in #92, I ended up reviewing and commenting there instead of in this PR. Quoting myself:

My issue with this implementation is that you will accumulate watchers for every view ever opened in the current session until their views are closed, while in reality you'd only need to watch the active view. Please make such adjustments that on_deactivated removes views from the watching pipeline.

Also, switching to async hooks would be appropriate.

[mheinzler]

The reason they are not removed in on_deactivated is that you could have multiple groups of views and I want a view to update when I scroll in it even if it is not activated. What do you think of this new solution that removes views when they become hidden?

I also rebased on current master.

[FichteFoll]

Hm, good point about scrolling views when they are not active. I didn't think of that.

I'll leave this to @rchl.

[rchl]

So before this change, matching trailing spaces would be done on moving selection or editing file while with this change matching is done every 250ms and for every view that was activated before.

Granted, matching performance is improved but still, it feels a bit inefficient to trigger (potentially) so many timeouts and do matching from them, even if the user is just looking at the code. I'm not saying it's terrible or anything but it would really help if sublime would provide scroll listener...

I'm still not quite sure what to do with it and if it can be optimized even more somehow but at least, as @FichteFoll said, you could switch to async versions of on_modified, on_selection_modified, on_activated and set_timeout (I don't think there is anything to consider when doing that in this particular case).

[mheinzler]

Well the matching is only done when the visible region changed, not on every timeout, but I agree that it is not an ideal solution.

With the newest commits the timeouts are async and views that are no longer visible will be removed on the next cycle. All in all I think it is better to check a few times per second if a small number of views need to match their visible region again compared to matching the whole of possibly very large views but only when necessary.
I hope this sentence makes sense 😄 to you.

I don't see a reason to switch the other events to async (at least for this PR). They mostly only call match_trailing_spaces same as before, which should now block even less.

[rchl]

With the newest commits the timeouts are async and views that are no longer visible will be removed on the next cycle.

Nice. I've missed that as I was looking at slightly older changes.
I'm happy with this change then.
If you could just rebase and maybe address the minor style thing I've pointed out.

[rchl]

Hmm, I've just realized that this would break "Remove trailing spaces" command since it relies on the whole document being scanned for trailing spaces.

(The reason this package "duplicates" ST functionality is stated here, in case someone was wondering as I did)

I'm gonna have to have another look at that at a later time...

[mheinzler]

I have never really used that command, but it looks like it would be enough to reduce find_regions_to_delete to always calling find_trailing_spaces and adding some flag to make it scan the whole view?

[rchl]

OK, I have merged your changes now with slight adjustments.
I will fix the mentioned problem before releasing the new version.

Thanks for working on this, for being patient (waiting 3 years for someone to merge is something) and dealing with my picky comments. :)

For reference, I did those minor style/lint changes to your code before merging:

diff --git a/trailing_spaces.py b/trailing_spaces.py
index b4ef110..d76a82a 100644
--- a/trailing_spaces.py
+++ b/trailing_spaces.py
@@ -78,6 +78,7 @@ def plugin_unloaded():
     # clear all active views to kill all timeouts
     active_views.clear()
 
+
 # Private: Updates user's settings with in-memory values.
 #
 # Allows for persistent settings from the menu.
@@ -100,8 +101,7 @@ def view_find_all_in_region(view, region, regex):
     matches = re.finditer(regex, text, re.MULTILINE)
 
     # return the found positions translated to the region's starting position
-    results = [sublime.Region(m.start() + region.begin(), m.end() + region.begin()) for m in matches]
-    return results
+    return [sublime.Region(m.start() + region.begin(), m.end() + region.begin()) for m in matches]
:...skipping...                                                                                                                                                                    
diff --git a/trailing_spaces.py b/trailing_spaces.py
index b4ef110..d76a82a 100644
--- a/trailing_spaces.py
+++ b/trailing_spaces.py
@@ -78,6 +78,7 @@ def plugin_unloaded():
     # clear all active views to kill all timeouts
     active_views.clear()
 
+
 # Private: Updates user's settings with in-memory values.
 #
 # Allows for persistent settings from the menu.
@@ -100,8 +101,7 @@ def view_find_all_in_region(view, region, regex):
     matches = re.finditer(regex, text, re.MULTILINE)
 
     # return the found positions translated to the region's starting position
-    results = [sublime.Region(m.start() + region.begin(), m.end() + region.begin()) for m in matches]
-    return results
+    return [sublime.Region(m.start() + region.begin(), m.end() + region.begin()) for m in matches]
 
 
 # Private: Get the regions matching trailing spaces.
@@ -122,11 +122,10 @@ def find_trailing_spaces(view):
     if not include_empty_lines:
         regexp = "(?<=\\S)%s$" % regexp
 
-    # find all matches in the currently visible region plus a little before and
-    # after
+    # find all matches in the currently visible region plus a little before and after
     searched_region = view.visible_region()
-    searched_region.a = max(searched_region.a - trailing_spaces_non_visible_highlighting, 0);
-    searched_region.b = min(searched_region.b + trailing_spaces_non_visible_highlighting, view.size());
+    searched_region.a = max(searched_region.a - trailing_spaces_non_visible_highlighting, 0)
+    searched_region.b = min(searched_region.b + trailing_spaces_non_visible_highlighting, view.size())
 
     searched_region = view.line(searched_region)  # align to line start and end
     offending_lines = view_find_all_in_region(view, searched_region, regexp)
@@ -471,8 +470,7 @@ class TrailingSpacesListener(sublime_plugin.EventListener):
             if not view.id() in active_views:
                 # track
                 active_views[view.id()] = view.visible_region()
-
-                self.update_view_on_scroll(view)                                                                                                                                  
+                self.update_on_region_change(view)
 
     def on_pre_save(self, view):
         global trim_modified_lines_only
@@ -486,13 +484,7 @@ class TrailingSpacesListener(sublime_plugin.EventListener):
         # untrack
         active_views.pop(view.id(), None)
 
-    def update_view_on_scroll(self, view):
-        # panel views don't trigger on_close but are also not valid anymore
-        # after being hidden, so try to detect these cases here
-        if view.size() == 0 and not view.file_name():
-            active_views.pop(view.id(), None)
-            return
-
+    def update_on_region_change(self, view):
         # remove views not currently visible
         if not self.is_view_visible(view):
             active_views.pop(view.id(), None)
@@ -506,7 +498,7 @@ class TrailingSpacesListener(sublime_plugin.EventListener):
 
         # continue only if the view is still active
         if trailing_spaces_live_matching and view.id() in active_views:
-            sublime.set_timeout_async(lambda: self.update_view_on_scroll(view),
+            sublime.set_timeout_async(lambda: self.update_on_region_change(view),
                                       trailing_spaces_update_interval)
 
     # Toggling messes with what is red from the disk, and it breaks the diff
@@ -529,6 +521,11 @@ class TrailingSpacesListener(sublime_plugin.EventListener):
         if not window:
             return False
 
+        # panel views don't trigger on_close but are also not valid anymore
+        # after being hidden, so try to detect these cases here
+        if view.size() == 0 and not view.file_name():
+            return False
+
         # see if this view is visible in its group
         group = window.get_view_index(view)[0]
         if group != -1: