Match counter: fix lints, implement suggestions

10 files changed, 113 insertions, 150 deletions

diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py
index 60e43fbed..87dd27d8e 100644
--- a/qutebrowser/browser/browsertab.py
+++ b/qutebrowser/browser/browsertab.py
@@ -293,14 +293,23 @@ class AbstractSearch(QObject):
         text: The last thing this view was searched for.
         search_displayed: Whether we're currently displaying search results in
                           this view.
+        current_match: The currently active search match on the page.
+                       0 if no search is active or the feature isn't available.
+        total_match_count: The total number of search matches on the page.
+                           0 if no search is active or the feature isn't available.
         _flags: The flags of the last search (needs to be set by subclasses).
         _widget: The underlying WebView widget.
+
+    Signals:
+        finished: A search has finished. True if the text was found, false otherwise.
+        search_match_changed: The currently active search match has changed.
+                              Emits (0, 0) if no search is active.
+                              Will not be emitted if search matches are not available.
+        cleared: An existing search was cleared.
     """
 
-    #: Signal emitted when a search was finished
-    #: (True if the text was found, False otherwise)
     finished = pyqtSignal(bool)
-    #: Signal emitted when an existing search was cleared.
+    search_match_changed = pyqtSignal(int, int)
     cleared = pyqtSignal()
 
     _Callback = Callable[[bool], None]
@@ -311,6 +320,8 @@ class AbstractSearch(QObject):
         self._widget = cast(QWidget, None)
         self.text: Optional[str] = None
         self.search_displayed = False
+        self.current_match = 0
+        self.total_match_count = 0
 
     def _is_case_sensitive(self, ignore_case: usertypes.IgnoreCase) -> bool:
         """Check if case-sensitivity should be used.
@@ -364,22 +375,6 @@ class AbstractSearch(QObject):
         """
         raise NotImplementedError
 
-    def get_current_match(self) -> int:
-        """Get the 1-based index of the currently highlighted match on the page.
-
-        Returns 0 if no successful search has been performed or this feature is unavailable
-        (QtWebKit and QtWebEngine < 5.14)
-        """
-        raise NotImplementedError
-
-    def get_total_match_count(self) -> int:
-        """Get the total number of search matches on the page.
-
-        Returns 0 if no successful search has been performed or this feature is unavailable
-        (QtWebKit and QtWebEngine < 5.14)
-        """
-        raise NotImplementedError
-
 
 class AbstractZoom(QObject):
 
@@ -926,8 +921,6 @@ class AbstractTab(QWidget):
     icon_changed = pyqtSignal(QIcon)
     #: Signal emitted when a page's title changed (new title as str)
     title_changed = pyqtSignal(str)
-    #: Signal emitted when a page's currently active search match changed (match as current/total)
-    search_match_changed = pyqtSignal(int, int)
     #: Signal emitted when this tab was pinned/unpinned (new pinned state as bool)
     pinned_changed = pyqtSignal(bool)
     #: Signal emitted when a new tab should be opened (url as QUrl)
@@ -1215,9 +1208,6 @@ class AbstractTab(QWidget):
     def icon(self) -> None:
         raise NotImplementedError
 
-    def current_search_match(self) -> (int, int):
-        raise NotImplementedError
-
     def set_html(self, html: str, base_url: QUrl = QUrl()) -> None:
         raise NotImplementedError
 
diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index cefebf4f6..da959b444 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -1534,7 +1534,8 @@ class CommandDispatcher:
         Args:
             found: Whether the text was found.
             tab: The AbstractTab in which the search was made.
-            old_current_match: The previously active match before the search was performed.
+            old_current_match: The previously active match before the search
+                               was performed.
             options: The options (dict) the search was made with.
             text: The text searched for.
             prev: Whether we're searching backwards (i.e. :search-prev)
@@ -1549,7 +1550,7 @@ class CommandDispatcher:
             if not config.val.search.wrap_messages:
                 return
 
-            new_current_match = tab.search.get_current_match()
+            new_current_match = tab.search.current_match
             # Check if the match count change is opposite to the search direction
             if old_current_match > 0:
                 if not going_up:
@@ -1627,7 +1628,7 @@ class CommandDispatcher:
             return
 
         cb = functools.partial(self._search_cb, tab=tab,
-                               old_current_match=tab.search.get_current_match(),
+                               old_current_match=tab.search.current_match,
                                options=window_options, text=window_text,
                                prev=False)
 
@@ -1661,7 +1662,7 @@ class CommandDispatcher:
             return
 
         cb = functools.partial(self._search_cb, tab=tab,
-                               old_current_match=tab.search.get_current_match(),
+                               old_current_match=tab.search.current_match,
                                options=window_options, text=window_text,
                                prev=True)
 
diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py
index cfa5d03ce..ac08476d3 100644
--- a/qutebrowser/browser/webengine/webenginetab.py
+++ b/qutebrowser/browser/webengine/webenginetab.py
@@ -101,79 +101,33 @@ class _WebEngineSearchWrapHandler:
     Attributes:
         flag_wrap: An additional flag indicating whether the last search
                    used wrapping.
-        _active_match: The 1-based index of the currently active match
-                       on the page.
-        _total_matches: The total number of search matches on the page.
-        _nowrap_available: Whether the functionality to prevent wrapping
-                           is available.
+        nowrap_available: Whether the functionality to prevent wrapping
+                          is available.
     """
 
     def __init__(self):
-        self._active_match = 0
-        self._total_matches = 0
         self.flag_wrap = True
-        self._nowrap_available = False
+        self.nowrap_available = False
 
-    def connect_signal(self, page):
-        """Connect to the findTextFinished signal of the page.
-
-        Args:
-            page: The QtWebEnginePage to connect to this handler.
-        """
-        page.findTextFinished.connect(self._store_match_data)
-        self._nowrap_available = True
-
-    def _store_match_data(self, result):
-        """Store information on the last match.
-
-        The information will be checked against when wrapping is turned off.
-
-        Args:
-            result: A FindTextResult passed by the findTextFinished signal.
-        """
-        self._active_match = result.activeMatch()
-        self._total_matches = result.numberOfMatches()
-        log.webview.debug("Active search match: {}/{}"
-                          .format(self._active_match, self._total_matches))
-
-    def reset_match_data(self):
-        """Reset match information.
-
-        Stale information could lead to next_result or prev_result misbehaving.
-        """
-        self._active_match = 0
-        self._total_matches = 0
-
-    def get_active_match(self):
-        """Get the active search match as tracked by this handler.
-
-        Returns 0 if no search is active or this feature is unavailable
-        (QtWebKit or QtWebEngine < 5.14)
-        """
-        return self._active_match
-
-    def get_total_match_count(self):
-        """Get the total search match count as tracked by this handler.
-
-        Returns 0 if no search is active or this feature is unavailable
-        (QtWebKit or QtWebEngine < 5.14)
-        """
-        return self._total_matches
-
-    def prevent_wrapping(self, *, going_up):
+    def prevent_wrapping(self, current_match, total_match_count, *, going_up):
         """Prevent wrapping if possible and required.
 
         Returns True if a wrap was prevented and False if not.
 
         Args:
+            current_match: The currently active search match on the page.
+            total_match_count: The total number of search matches on the page.
             going_up: Whether the search would scroll the page up or down.
         """
-        if (not self._nowrap_available or
-                self.flag_wrap or self._total_matches == 0):
-            return False
-        elif going_up and self._active_match == 1:
-            return True
-        return not going_up and self._active_match == self._total_matches
+        return (
+            self.nowrap_available and
+            not self.flag_wrap and
+            total_match_count != 0 and
+            (
+                going_up and current_match == 1 or
+                not going_up and current_match == total_match_count
+            )
+        )
 
 
 class WebEngineSearch(browsertab.AbstractSearch):
@@ -185,13 +139,8 @@ class WebEngineSearch(browsertab.AbstractSearch):
         _pending_searches: How many searches have been started but not called
                            back yet.
 
-    Signals:
-        search_match_changed: The currently active search match has changed.
-                              Emits (0, 0) if no search is active.
     """
 
-    search_match_changed = pyqtSignal(int, int)
-
     def __init__(self, tab, parent=None):
         super().__init__(tab, parent)
         self._flags = self._empty_flags()
@@ -202,6 +151,7 @@ class WebEngineSearch(browsertab.AbstractSearch):
         return QWebEnginePage.FindFlags(0)  # type: ignore[call-overload]
 
     def connect_signals(self):
+        """Connect the signals necessary for this class to function."""
         # The API necessary to stop wrapping was added in this version
         if not qtutils.version_check("5.14"):
             return
@@ -218,7 +168,7 @@ class WebEngineSearch(browsertab.AbstractSearch):
                                 "to rebuild PyQtWebEngine.")
             return
 
-        self._wrap_handler.connect_signal(self._widget.page())
+        self._wrap_handler.nowrap_available = True
         self._widget.page().findTextFinished.connect(self._on_find_finished)
 
     def _find(self, text, flags, callback, caller):
@@ -260,9 +210,12 @@ class WebEngineSearch(browsertab.AbstractSearch):
         self._widget.page().findText(text, flags, wrapped_callback)
 
     def _on_find_finished(self, find_text_result):
-        """Unwrap the QWebEngineFindTextResult and pass it along."""
-        self.search_match_changed.emit(find_text_result.activeMatch(),
-                                       find_text_result.numberOfMatches())
+        """Unwrap the result, store it, and pass it along."""
+        self.current_match = find_text_result.activeMatch()
+        self.total_match_count = find_text_result.numberOfMatches()
+        log.webview.debug("Active search match: {}/{}"
+                          .format(self.current_match, self.total_match_count))
+        self.search_match_changed.emit(self.current_match, self.total_match_count)
 
     def search(self, text, *, ignore_case=usertypes.IgnoreCase.never,
                reverse=False, wrap=True, result_cb=None):
@@ -274,7 +227,7 @@ class WebEngineSearch(browsertab.AbstractSearch):
 
         self.text = text
         self._flags = self._empty_flags()
-        self._wrap_handler.reset_match_data()
+        self._reset_match_data()
         self._wrap_handler.flag_wrap = wrap
         if self._is_case_sensitive(ignore_case):
             self._flags |= QWebEnginePage.FindCaseSensitively
@@ -288,7 +241,7 @@ class WebEngineSearch(browsertab.AbstractSearch):
             self.cleared.emit()
             self.search_match_changed.emit(0, 0)
         self.search_displayed = False
-        self._wrap_handler.reset_match_data()
+        self._reset_match_data()
         self._widget.page().findText('')
 
     def prev_result(self, *, result_cb=None):
@@ -296,12 +249,14 @@ class WebEngineSearch(browsertab.AbstractSearch):
         flags = QWebEnginePage.FindFlags(
             int(self._flags))  # type: ignore[call-overload]
         if flags & QWebEnginePage.FindBackward:
-            if self._wrap_handler.prevent_wrapping(going_up=False):
+            if self._wrap_handler.prevent_wrapping(self.current_match,
+                    self.total_match_count, going_up=False):
                 result_cb(True)
                 return
             flags &= ~QWebEnginePage.FindBackward
         else:
-            if self._wrap_handler.prevent_wrapping(going_up=True):
+            if self._wrap_handler.prevent_wrapping(self.current_match,
+                    self.total_match_count, going_up=True):
                 result_cb(True)
                 return
             flags |= QWebEnginePage.FindBackward
@@ -309,16 +264,19 @@ class WebEngineSearch(browsertab.AbstractSearch):
 
     def next_result(self, *, result_cb=None):
         going_up = self._flags & QWebEnginePage.FindBackward
-        if self._wrap_handler.prevent_wrapping(going_up=going_up):
+        if self._wrap_handler.prevent_wrapping(self.current_match,
+                self.total_match_count, going_up=going_up):
             result_cb(True)
             return
         self._find(self.text, self._flags, result_cb, 'next_result')
 
-    def get_current_match(self):
-        return self._wrap_handler.get_active_match()
+    def _reset_match_data(self):
+        """Reset match counter information.
 
-    def get_total_match_count(self):
-        return self._wrap_handler.get_total_match_count()
+        Stale information could lead to next_result or prev_result misbehaving.
+        """
+        self.current_match = 0
+        self.total_match_count = 0
 
 
 class WebEngineCaret(browsertab.AbstractCaret):
@@ -1402,9 +1360,6 @@ class WebEngineTab(browsertab.AbstractTab):
     def icon(self):
         return self._widget.icon()
 
-    def current_search_match(self) -> (int, int):
-        return (self.search.get_current_match(), self.search.get_total_match_count())
-
     def set_html(self, html, base_url=QUrl()):
         # FIXME:qtwebengine
         # check this and raise an exception if too big:
@@ -1649,11 +1604,6 @@ class WebEngineTab(browsertab.AbstractTab):
                 not qtutils.version_check('5.14')):
             self.settings.update_for_url(url)
 
-    @pyqtSlot(int, int)
-    def _on_search_match_changed(self, current: int, total: int):
-        """Pass the signal from the WebEngineSearch along."""
-        self.search_match_changed.emit(current, total)
-
     @pyqtSlot(usertypes.NavigationRequest)
     def _on_navigation_request(self, navigation):
         super()._on_navigation_request(navigation)
@@ -1730,8 +1680,6 @@ class WebEngineTab(browsertab.AbstractTab):
         page.loadFinished.connect(self._restore_zoom)
         page.loadFinished.connect(self._on_load_finished)
 
-        self.search.search_match_changed.connect(self._on_search_match_changed)
-
         try:
             page.renderProcessPidChanged.connect(self._on_renderer_process_pid_changed)
         except AttributeError:
diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py
index 58e753c3f..df3491ec2 100644
--- a/qutebrowser/browser/webkit/webkittab.py
+++ b/qutebrowser/browser/webkit/webkittab.py
@@ -189,13 +189,6 @@ class WebKitSearch(browsertab.AbstractSearch):
         found = self._widget.findText(self.text, flags)
         self._call_cb(result_cb, found, self.text, flags, 'prev_result')
 
-    # These aren't supported when using the QtWebKit backend.
-    def get_current_match(self):
-        return 0
-
-    def get_total_match_count(self):
-        return 0
-
 
 class WebKitCaret(browsertab.AbstractCaret):
 
@@ -885,10 +878,6 @@ class WebKitTab(browsertab.AbstractTab):
     def icon(self):
         return self._widget.icon()
 
-    def current_search_match(self):
-        """QtWebKit doesn't support match counts."""
-        return (0, 0)
-
     def reload(self, *, force=False):
         if force:
             action = QWebPage.ReloadAndBypassCache
diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py
index 011e4ddcb..c84a29430 100644
--- a/qutebrowser/mainwindow/mainwindow.py
+++ b/qutebrowser/mainwindow/mainwindow.py
@@ -537,7 +537,7 @@ class MainWindow(QWidget):
         self.tabbed_browser.cur_load_status_changed.connect(
             self.status.url.on_load_status_changed)
 
-        self.tabbed_browser.search_match_changed.connect(
+        self.tabbed_browser.cur_search_match_changed.connect(
             self.status.search_match.set_match_index)
 
         self.tabbed_browser.cur_caret_selection_toggled.connect(
diff --git a/qutebrowser/mainwindow/statusbar/bar.py b/qutebrowser/mainwindow/statusbar/bar.py
index 6daf22752..708da23b2 100644
--- a/qutebrowser/mainwindow/statusbar/bar.py
+++ b/qutebrowser/mainwindow/statusbar/bar.py
@@ -221,7 +221,7 @@ class StatusBar(QWidget):
         elif option == 'statusbar.widgets':
             self._draw_widgets()
 
-    def _draw_widgets(self):
+    def _draw_widgets(self):  # noqa: C901
         """Draw statusbar widgets."""
         self._clear_widgets()
 
diff --git a/qutebrowser/mainwindow/statusbar/searchmatch.py b/qutebrowser/mainwindow/statusbar/searchmatch.py
index 9c4e67c32..6e9a435de 100644
--- a/qutebrowser/mainwindow/statusbar/searchmatch.py
+++ b/qutebrowser/mainwindow/statusbar/searchmatch.py
@@ -23,7 +23,6 @@
 from PyQt5.QtCore import pyqtSlot
 
 from qutebrowser.utils import log
-from qutebrowser.config import config
 from qutebrowser.mainwindow.statusbar import textbase
 
 
@@ -46,4 +45,5 @@ class SearchMatch(textbase.TextBase):
             log.statusbar.debug('Clearing search match text.')
         else:
             self.setText('Match [{}/{}]'.format(current, total))
-            log.statusbar.debug('Setting search match text to {}/{}'.format(current, total))
+            log.statusbar.debug('Setting search match text to {}/{}'
+                    .format(current, total))
diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py
index 0f3c61885..26df9813b 100644
--- a/qutebrowser/mainwindow/tabbedbrowser.py
+++ b/qutebrowser/mainwindow/tabbedbrowser.py
@@ -180,6 +180,7 @@ class TabbedBrowser(QWidget):
                                  arg 1: x-position in %.
                                  arg 2: y-position in %.
         cur_load_status_changed: Loading status of current tab changed.
+        cur_search_match_changed: The active search match changed.
         close_window: The last tab was closed, close this window.
         resized: Emitted when the browser window has resized, so the completion
                  widget can adjust its size to it.
@@ -196,7 +197,7 @@ class TabbedBrowser(QWidget):
     cur_link_hovered = pyqtSignal(str)
     cur_scroll_perc_changed = pyqtSignal(int, int)
     cur_load_status_changed = pyqtSignal(usertypes.LoadStatus)
-    search_match_changed = pyqtSignal(int, int)
+    cur_search_match_changed = pyqtSignal(int, int)
     cur_fullscreen_requested = pyqtSignal(bool)
     cur_caret_selection_toggled = pyqtSignal(browsertab.SelectionState)
     close_window = pyqtSignal()
@@ -338,6 +339,8 @@ class TabbedBrowser(QWidget):
             self._filter.create(self.cur_fullscreen_requested, tab))
         tab.caret.selection_toggled.connect(
             self._filter.create(self.cur_caret_selection_toggled, tab))
+        tab.search.search_match_changed.connect(
+            self._filter.create(self.cur_search_match_changed, tab))
         # misc
         tab.scroller.perc_changed.connect(self._on_scroll_pos_changed)
         tab.scroller.before_jump_requested.connect(lambda: self.set_mark("'"))
@@ -347,8 +350,6 @@ class TabbedBrowser(QWidget):
             functools.partial(self._on_title_changed, tab))
         tab.icon_changed.connect(
             functools.partial(self._on_icon_changed, tab))
-        tab.search_match_changed.connect(
-            functools.partial(self._on_search_match_changed, tab))
         tab.pinned_changed.connect(
             functools.partial(self._on_pinned_changed, tab))
         tab.load_progress.connect(
@@ -796,16 +797,6 @@ class TabbedBrowser(QWidget):
             return
         self.widget.update_tab_favicon(tab)
 
-    @pyqtSlot(browsertab.AbstractTab, int, int)
-    def _on_search_match_changed(self, tab, current: int, total: int):
-        """Pass along the signal of a changed active match.
-
-        Args:
-            current: The number of the currently active match.
-            total: The total number of matches on the page.
-        """
-        self.search_match_changed.emit(current, total)
-
     @pyqtSlot(usertypes.KeyMode)
     def on_mode_entered(self, mode):
         """Save input mode when tabs.mode_on_change = restore."""
@@ -864,7 +855,8 @@ class TabbedBrowser(QWidget):
                         .format(current_mode.name, mode_on_change))
         self._now_focused = tab
         self.current_tab_changed.emit(tab)
-        self.search_match_changed.emit(*tab.current_search_match())
+        self.cur_search_match_changed.emit(tab.search.current_match,
+                                           tab.search.total_match_count)
         QTimer.singleShot(0, self._update_window_title)
         self._tab_insert_idx_left = self.widget.currentIndex()
         self._tab_insert_idx_right = self.widget.currentIndex() + 1
diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py
index c66cb3e8d..c446e5262 100644
--- a/scripts/dev/check_coverage.py
+++ b/scripts/dev/check_coverage.py
@@ -139,6 +139,8 @@ PERFECT_FILES = [
 
     (None,
      'qutebrowser/mainwindow/statusbar/keystring.py'),
+    (None,
+     'qutebrowser/mainwindow/statusbar/searchmatch.py'),
     ('tests/unit/mainwindow/statusbar/test_percentage.py',
      'qutebrowser/mainwindow/statusbar/percentage.py'),
     ('tests/unit/mainwindow/statusbar/test_progress.py',
diff --git a/tests/end2end/features/search.feature b/tests/end2end/features/search.feature
index 5fafd19f0..aed40f349 100644
--- a/tests/end2end/features/search.feature
+++ b/tests/end2end/features/search.feature
@@ -219,13 +219,13 @@ Feature: Searching on a page
     Scenario: Preventing wrapping at the top of the page with QtWebEngine
         When I set search.ignore_case to always
         And I set search.wrap to false
+        And I set search.wrap_messages to true
         And I run :search --reverse foo
         And I wait for "search found foo with flags FindBackward" in the log
         And I run :search-next
         And I wait for "next_result found foo with flags FindBackward" in the log
         And I run :search-next
-        And I wait for "Search hit TOP" in the log
-        Then "foo" should be found
+        Then the message "Search hit TOP" should be shown
 
     @qtwebkit_skip @qt>=5.14
     Scenario: Preventing wrapping at the bottom of the page with QtWebEngine
@@ -236,8 +236,7 @@ Feature: Searching on a page
         And I run :search-next
         And I wait for "next_result found foo" in the log
         And I run :search-next
-        And I wait for "Search hit BOTTOM" in the log
-        Then "Foo" should be found
+        Then the message "Search hit BOTTOM" should be shown
 
     @qtwebengine_skip
     Scenario: Preventing wrapping at the top of the page with QtWebKit
@@ -263,6 +262,48 @@ Feature: Searching on a page
         And I wait for "next_result didn't find foo" in the log
         Then the warning "Text 'foo' not found on page!" should be shown
 
+    ## search match counter
+
+    @qtwebkit_skip @qt>=5.14
+    Scenario: Setting search match counter on search
+        When I set search.ignore_case to always
+        And I set search.wrap to true
+        And I run :search ba
+        And I wait for "search found ba" in the log
+        Then "Setting search match text to 1/5" should be logged
+
+    @qtwebkit_skip @qt>=5.14
+    Scenario: Updating search match counter on search-next
+        When I set search.ignore_case to always
+        And I set search.wrap to true
+        And I run :search ba
+        And I wait for "search found ba" in the log
+        And I run :search-next
+        And I wait for "next_result found ba" in the log
+        And I run :search-next
+        And I wait for "next_result found ba" in the log
+        Then "Setting search match text to 3/5" should be logged
+
+    @qtwebkit_skip @qt>=5.14
+    Scenario: Updating search match counter on search-prev with wrapping
+        When I set search.ignore_case to always
+        And I set search.wrap to true
+        And I run :search ba
+        And I wait for "search found ba" in the log
+        And I run :search-prev
+        And I wait for the message "Search hit TOP, continuing at BOTTOM"
+        Then "Setting search match text to 5/5" should be logged
+
+    @qtwebkit_skip @qt>=5.14
+    Scenario: Updating search match counter on search-prev without wrapping
+        When I set search.ignore_case to always
+        And I set search.wrap to false
+        And I run :search ba
+        And I wait for "search found ba" in the log
+        And I run :search-prev
+        And I wait for the message "Search hit TOP"
+        Then "Setting search match text to 1/5" should be logged
+
     ## follow searched links
     @skip  # Too flaky
     Scenario: Follow a searched link