From b416e998fa0eb450c78d9f4ee49e8d1080914888 Mon Sep 17 00:00:00 2001 From: phesch Date: Wed, 4 Aug 2021 20:49:56 +0200 Subject: Move search messages into callback Also add the necessary functions and arguments to facilitate this. --- qutebrowser/browser/browsertab.py | 16 ++++++++++++ qutebrowser/browser/commands.py | 36 +++++++++++++++++++-------- qutebrowser/browser/webengine/webenginetab.py | 34 +++++++++++++++++++------ qutebrowser/browser/webkit/webkittab.py | 7 ++++++ 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index b1827dbf4..c26591189 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -364,6 +364,22 @@ 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): diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 8cd73ae4f..982f74bd4 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1528,13 +1528,13 @@ class CommandDispatcher: message.error(str(e)) ed.backup() - def _search_cb(self, found, *, tab, old_scroll_pos, options, text, prev): + def _search_cb(self, found, *, tab, old_current_match, options, text, prev): """Callback called from search/search_next/search_prev. Args: found: Whether the text was found. tab: The AbstractTab in which the search was made. - old_scroll_pos: The scroll position (QPoint) before the search. + 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) @@ -1546,11 +1546,25 @@ class CommandDispatcher: going_up = options['reverse'] ^ prev if found: - # Check if the scroll position got smaller and show info. - if not going_up and tab.scroller.pos_px().y() < old_scroll_pos.y(): - message.info("Search hit BOTTOM, continuing at TOP") - elif going_up and tab.scroller.pos_px().y() > old_scroll_pos.y(): - message.info("Search hit TOP, continuing at BOTTOM") + new_current_match = tab.search.get_current_match() + total_match_count = tab.search.get_total_match_count() + if total_match_count > 0: + message.info("Match {}/{}".format(new_current_match, total_match_count), + replace="search-match-count") + # Check if the match count change is opposite to the search direction + if old_current_match > 0: + if not going_up: + if old_current_match > new_current_match: + message.info("Search hit BOTTOM, continuing at TOP", + replace="search-hit-msg") + elif old_current_match == new_current_match: + message.info("Search hit BOTTOM", replace="search-hit-msg") + elif going_up: + if old_current_match < new_current_match: + message.info("Search hit TOP, continuing at BOTTOM", + replace="search-hit-msg") + elif old_current_match == new_current_match: + message.info("Search hit TOP", replace="search-hit-msg") else: message.warning(f"Text '{text}' not found on page!", replace='find-in-page') @@ -1581,8 +1595,8 @@ class CommandDispatcher: self._tabbed_browser.search_options = dict(options) cb = functools.partial(self._search_cb, tab=tab, - old_scroll_pos=tab.scroller.pos_px(), - options=options, text=text, prev=False) + old_current_match=0, options=options, + text=text, prev=False) options['result_cb'] = cb tab.scroller.before_jump_requested.emit() @@ -1614,7 +1628,7 @@ class CommandDispatcher: return cb = functools.partial(self._search_cb, tab=tab, - old_scroll_pos=tab.scroller.pos_px(), + old_current_match=tab.search.get_current_match(), options=window_options, text=window_text, prev=False) @@ -1648,7 +1662,7 @@ class CommandDispatcher: return cb = functools.partial(self._search_cb, tab=tab, - old_scroll_pos=tab.scroller.pos_px(), + old_current_match=tab.search.get_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 ace23d14a..3d1fc637b 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -120,6 +120,7 @@ class _WebEngineSearchWrapHandler: Args: page: The QtWebEnginePage to connect to this handler. """ + # The API necessary to stop wrapping was added in this version if not qtutils.version_check("5.14"): return @@ -159,6 +160,22 @@ class _WebEngineSearchWrapHandler: 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): """Prevent wrapping if possible and required. @@ -171,13 +188,8 @@ class _WebEngineSearchWrapHandler: self.flag_wrap or self._total_matches == 0): return False elif going_up and self._active_match == 1: - message.info("Search hit TOP") return True - elif not going_up and self._active_match == self._total_matches: - message.info("Search hit BOTTOM") - return True - else: - return False + return not going_up and self._active_match == self._total_matches class WebEngineSearch(browsertab.AbstractSearch): @@ -194,7 +206,6 @@ class WebEngineSearch(browsertab.AbstractSearch): super().__init__(tab, parent) self._flags = self._empty_flags() self._pending_searches = 0 - # The API necessary to stop wrapping was added in this version self._wrap_handler = _WebEngineSearchWrapHandler() def _empty_flags(self): @@ -273,10 +284,12 @@ class WebEngineSearch(browsertab.AbstractSearch): int(self._flags)) # type: ignore[call-overload] if flags & QWebEnginePage.FindBackward: if self._wrap_handler.prevent_wrapping(going_up=False): + result_cb(True) return flags &= ~QWebEnginePage.FindBackward else: if self._wrap_handler.prevent_wrapping(going_up=True): + result_cb(True) return flags |= QWebEnginePage.FindBackward self._find(self.text, flags, result_cb, 'prev_result') @@ -284,9 +297,16 @@ 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): + 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 get_total_match_count(self): + return self._wrap_handler.get_total_match_count() + class WebEngineCaret(browsertab.AbstractCaret): diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index df3491ec2..b12652c35 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -189,6 +189,13 @@ 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): -- cgit v1.2.3-54-g00ecf From 183156b5c2af589582b914274428ad58c500547a Mon Sep 17 00:00:00 2001 From: phesch Date: Thu, 2 Sep 2021 20:08:20 +0200 Subject: Add statusbar search_match widget --- qutebrowser/browser/browsertab.py | 5 ++ qutebrowser/browser/commands.py | 3 -- qutebrowser/browser/webengine/webenginetab.py | 49 +++++++++++++------- qutebrowser/browser/webkit/webkittab.py | 3 ++ qutebrowser/config/configdata.yml | 3 +- qutebrowser/mainwindow/mainwindow.py | 3 ++ qutebrowser/mainwindow/statusbar/bar.py | 9 +++- qutebrowser/mainwindow/statusbar/searchmatch.py | 61 +++++++++++++++++++++++++ qutebrowser/mainwindow/tabbedbrowser.py | 8 ++++ 9 files changed, 122 insertions(+), 22 deletions(-) create mode 100644 qutebrowser/mainwindow/statusbar/searchmatch.py diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index c26591189..7b5553576 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -926,6 +926,8 @@ class AbstractTab(QWidget): icon_changed = pyqtSignal(QIcon) #: Signal emitted when a page's title changed (new title as str) title_changed = pyqtSignal(str) + + 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) @@ -1213,6 +1215,9 @@ 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 982f74bd4..2346c2470 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1548,9 +1548,6 @@ class CommandDispatcher: if found: new_current_match = tab.search.get_current_match() total_match_count = tab.search.get_total_match_count() - if total_match_count > 0: - message.info("Match {}/{}".format(new_current_match, total_match_count), - replace="search-match-count") # Check if the match count change is opposite to the search direction if old_current_match > 0: if not going_up: diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 3d1fc637b..9734cccde 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -120,22 +120,6 @@ class _WebEngineSearchWrapHandler: Args: page: The QtWebEnginePage to connect to this handler. """ - # The API necessary to stop wrapping was added in this version - if not qtutils.version_check("5.14"): - return - - try: - # pylint: disable=unused-import - from PyQt5.QtWebEngineCore import QWebEngineFindTextResult - except ImportError: - # WORKAROUND for some odd PyQt/packaging bug where the - # findTextResult signal is available, but QWebEngineFindTextResult - # is not. Seems to happen on e.g. Gentoo. - log.webview.warning("Could not import QWebEngineFindTextResult " - "despite running on Qt 5.14. You might need " - "to rebuild PyQtWebEngine.") - return - page.findTextFinished.connect(self._store_match_data) self._nowrap_available = True @@ -202,6 +186,8 @@ class WebEngineSearch(browsertab.AbstractSearch): back yet. """ + search_match_changed = pyqtSignal(int, int) + def __init__(self, tab, parent=None): super().__init__(tab, parent) self._flags = self._empty_flags() @@ -212,7 +198,24 @@ class WebEngineSearch(browsertab.AbstractSearch): return QWebEnginePage.FindFlags(0) # type: ignore[call-overload] def connect_signals(self): + # The API necessary to stop wrapping was added in this version + if not qtutils.version_check("5.14"): + return + + try: + # pylint: disable=unused-import + from PyQt5.QtWebEngineCore import QWebEngineFindTextResult + except ImportError: + # WORKAROUND for some odd PyQt/packaging bug where the + # findTextResult signal is available, but QWebEngineFindTextResult + # is not. Seems to happen on e.g. Gentoo. + log.webview.warning("Could not import QWebEngineFindTextResult " + "despite running on Qt 5.14. You might need " + "to rebuild PyQtWebEngine.") + return + self._wrap_handler.connect_signal(self._widget.page()) + self._widget.page().findTextFinished.connect(self._on_find_finished) def _find(self, text, flags, callback, caller): """Call findText on the widget.""" @@ -252,6 +255,10 @@ class WebEngineSearch(browsertab.AbstractSearch): self._widget.page().findText(text, flags, wrapped_callback) + def _on_find_finished(self, findTextResult): + self.search_match_changed.emit(findTextResult.activeMatch(), + findTextResult.numberOfMatches()) + def search(self, text, *, ignore_case=usertypes.IgnoreCase.never, reverse=False, wrap=True, result_cb=None): # Don't go to next entry on duplicate search @@ -274,6 +281,7 @@ class WebEngineSearch(browsertab.AbstractSearch): def clear(self): if self.search_displayed: self.cleared.emit() + self.search_match_changed.emit(0, 0) self.search_displayed = False self._wrap_handler.reset_match_data() self._widget.page().findText('') @@ -1389,6 +1397,9 @@ 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: @@ -1633,6 +1644,10 @@ 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): + self.search_match_changed.emit(current, total) + @pyqtSlot(usertypes.NavigationRequest) def _on_navigation_request(self, navigation): super()._on_navigation_request(navigation) @@ -1709,6 +1724,8 @@ 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 b12652c35..66467f18a 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -885,6 +885,9 @@ class WebKitTab(browsertab.AbstractTab): def icon(self): return self._widget.icon() + def current_search_match(self): + return (0, 0) + def reload(self, *, force=False): if force: action = QWebPage.ReloadAndBypassCache diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index 5f5a71b06..349a42f23 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -1963,12 +1963,13 @@ statusbar.widgets: - scroll_raw: "Raw percentage of the current page position like `10`." - history: "Display an arrow when possible to go back/forward in history." + - search_match: "A match count when searching, e.g. `Match [2/10]`." - tabs: "Current active tab, e.g. `2`." - keypress: "Display pressed keys when composing a vi command." - progress: "Progress bar for the current page loading." - 'text:foo': "Display the static text after the colon, `foo` in the example." none_ok: true - default: ['keypress', 'url', 'scroll', 'history', 'tabs', 'progress'] + default: ['keypress', 'search_match', 'url', 'scroll', 'history', 'tabs', 'progress'] desc: "List of widgets displayed in the statusbar." ## tabs diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index d0723742a..011e4ddcb 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -537,6 +537,9 @@ 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.status.search_match.set_match_index) + self.tabbed_browser.cur_caret_selection_toggled.connect( self.status.on_caret_selection_toggled) diff --git a/qutebrowser/mainwindow/statusbar/bar.py b/qutebrowser/mainwindow/statusbar/bar.py index 8bad290be..6daf22752 100644 --- a/qutebrowser/mainwindow/statusbar/bar.py +++ b/qutebrowser/mainwindow/statusbar/bar.py @@ -32,7 +32,7 @@ from qutebrowser.keyinput import modeman from qutebrowser.utils import usertypes, log, objreg, utils from qutebrowser.mainwindow.statusbar import (backforward, command, progress, keystring, percentage, url, - tabindex, textbase) + tabindex, textbase, searchmatch) @dataclasses.dataclass @@ -144,6 +144,7 @@ class StatusBar(QWidget): url: The UrlText widget in the statusbar. prog: The Progress widget in the statusbar. cmd: The Command widget in the statusbar. + search_match: The SearchMatch widget in the statusbar. _hbox: The main QHBoxLayout. _stack: The QStackedLayout with cmd/txt widgets. _win_id: The window ID the statusbar is associated with. @@ -194,6 +195,8 @@ class StatusBar(QWidget): self.cmd.hide_cmd.connect(self._hide_cmd_widget) self._hide_cmd_widget() + self.search_match = searchmatch.SearchMatch() + self.url = url.UrlText() self.percentage = percentage.Percentage() self.backforward = backforward.Backforward() @@ -226,7 +229,9 @@ class StatusBar(QWidget): # Read the list and set widgets accordingly for segment in config.val.statusbar.widgets: - if segment == 'url': + if segment == 'search_match': + self._hbox.addWidget(self.search_match) + elif segment == 'url': self._hbox.addWidget(self.url) self.url.show() elif segment == 'scroll': diff --git a/qutebrowser/mainwindow/statusbar/searchmatch.py b/qutebrowser/mainwindow/statusbar/searchmatch.py new file mode 100644 index 000000000..ebcd8b033 --- /dev/null +++ b/qutebrowser/mainwindow/statusbar/searchmatch.py @@ -0,0 +1,61 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2014-2021 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + +"""The search match indicator in the statusbar.""" + + +from PyQt5.QtCore import pyqtSlot + +from qutebrowser.utils import log +from qutebrowser.config import config +from qutebrowser.mainwindow.statusbar import textbase + + +class SearchMatch(textbase.TextBase): + + """The commandline part of the statusbar. + + Attributes: + _win_id: The window ID this widget is associated with. + + Signals: + got_cmd: Emitted when a command is triggered by the user. + arg: The command string and also potentially the count. + got_search: Emitted when a search should happen. + clear_completion_selection: Emitted before the completion widget is + hidden. + hide_completion: Emitted when the completion widget should be hidden. + update_completion: Emitted when the completion should be shown/updated. + show_cmd: Emitted when command input should be shown. + hide_cmd: Emitted when command input can be hidden. + """ + + @pyqtSlot(int, int) + def set_match_index(self, current: int, total: int) -> None: + """Preset the statusbar to some text. + + Args: + text: The text to set as string. + """ + if current <= 0 and total <= 0: + self.setText('') + 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)) diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py index e081284ee..4bf18cd30 100644 --- a/qutebrowser/mainwindow/tabbedbrowser.py +++ b/qutebrowser/mainwindow/tabbedbrowser.py @@ -196,6 +196,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_fullscreen_requested = pyqtSignal(bool) cur_caret_selection_toggled = pyqtSignal(browsertab.SelectionState) close_window = pyqtSignal() @@ -346,6 +347,8 @@ 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( @@ -793,6 +796,10 @@ 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): + 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.""" @@ -851,6 +858,7 @@ 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()) QTimer.singleShot(0, self._update_window_title) self._tab_insert_idx_left = self.widget.currentIndex() self._tab_insert_idx_right = self.widget.currentIndex() + 1 -- cgit v1.2.3-54-g00ecf From dab1a76b05f9e94ddfa06e7e7538a75bbf9892f4 Mon Sep 17 00:00:00 2001 From: phesch Date: Thu, 2 Sep 2021 21:07:19 +0200 Subject: Add documentation for statusbar match counter --- qutebrowser/browser/browsertab.py | 2 +- qutebrowser/browser/webengine/webenginetab.py | 6 ++++++ qutebrowser/browser/webkit/webkittab.py | 1 + qutebrowser/mainwindow/statusbar/searchmatch.py | 23 +++++------------------ qutebrowser/mainwindow/tabbedbrowser.py | 6 ++++++ 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 7b5553576..60e43fbed 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -926,7 +926,7 @@ 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) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 9734cccde..46fd8bf29 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -184,6 +184,10 @@ class WebEngineSearch(browsertab.AbstractSearch): _flags: The QWebEnginePage.FindFlags of the last search. _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) @@ -256,6 +260,7 @@ class WebEngineSearch(browsertab.AbstractSearch): self._widget.page().findText(text, flags, wrapped_callback) def _on_find_finished(self, findTextResult): + """Unwrap the QWebEngineFindTextResult and pass it along.""" self.search_match_changed.emit(findTextResult.activeMatch(), findTextResult.numberOfMatches()) @@ -1646,6 +1651,7 @@ class WebEngineTab(browsertab.AbstractTab): @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) diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 66467f18a..58e753c3f 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -886,6 +886,7 @@ class WebKitTab(browsertab.AbstractTab): return self._widget.icon() def current_search_match(self): + """QtWebKit doesn't support match counts.""" return (0, 0) def reload(self, *, force=False): diff --git a/qutebrowser/mainwindow/statusbar/searchmatch.py b/qutebrowser/mainwindow/statusbar/searchmatch.py index ebcd8b033..d2650d6b7 100644 --- a/qutebrowser/mainwindow/statusbar/searchmatch.py +++ b/qutebrowser/mainwindow/statusbar/searchmatch.py @@ -29,29 +29,16 @@ from qutebrowser.mainwindow.statusbar import textbase class SearchMatch(textbase.TextBase): - """The commandline part of the statusbar. - - Attributes: - _win_id: The window ID this widget is associated with. - - Signals: - got_cmd: Emitted when a command is triggered by the user. - arg: The command string and also potentially the count. - got_search: Emitted when a search should happen. - clear_completion_selection: Emitted before the completion widget is - hidden. - hide_completion: Emitted when the completion widget should be hidden. - update_completion: Emitted when the completion should be shown/updated. - show_cmd: Emitted when command input should be shown. - hide_cmd: Emitted when command input can be hidden. - """ + """The part of the statusbar that displays the search match counter.""" @pyqtSlot(int, int) def set_match_index(self, current: int, total: int) -> None: - """Preset the statusbar to some text. + """Set the match counts in the statusbar. + Passing (0, 0) hides the match counter. Args: - text: The text to set as string. + current: The currently active search match. + total: The total number of search matches on the page. """ if current <= 0 and total <= 0: self.setText('') diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py index 4bf18cd30..0f3c61885 100644 --- a/qutebrowser/mainwindow/tabbedbrowser.py +++ b/qutebrowser/mainwindow/tabbedbrowser.py @@ -798,6 +798,12 @@ class TabbedBrowser(QWidget): @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) -- cgit v1.2.3-54-g00ecf From 9cea6ade95355616d7e2e71b4aa2d45c4e724b90 Mon Sep 17 00:00:00 2001 From: phesch Date: Fri, 3 Sep 2021 10:06:30 +0200 Subject: Add setting to disable wrap messages, fix lints --- qutebrowser/browser/commands.py | 4 +++- qutebrowser/browser/webengine/webenginetab.py | 6 +++--- qutebrowser/config/configdata.yml | 7 +++++++ qutebrowser/mainwindow/statusbar/searchmatch.py | 1 + 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 2346c2470..cefebf4f6 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1546,8 +1546,10 @@ class CommandDispatcher: going_up = options['reverse'] ^ prev if found: + if not config.val.search.wrap_messages: + return + new_current_match = tab.search.get_current_match() - total_match_count = tab.search.get_total_match_count() # Check if the match count change is opposite to the search direction if old_current_match > 0: if not going_up: diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 46fd8bf29..cfa5d03ce 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -259,10 +259,10 @@ class WebEngineSearch(browsertab.AbstractSearch): self._widget.page().findText(text, flags, wrapped_callback) - def _on_find_finished(self, findTextResult): + def _on_find_finished(self, find_text_result): """Unwrap the QWebEngineFindTextResult and pass it along.""" - self.search_match_changed.emit(findTextResult.activeMatch(), - findTextResult.numberOfMatches()) + self.search_match_changed.emit(find_text_result.activeMatch(), + find_text_result.numberOfMatches()) def search(self, text, *, ignore_case=usertypes.IgnoreCase.never, reverse=False, wrap=True, result_cb=None): diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index 349a42f23..c41ce083b 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -72,6 +72,13 @@ search.wrap: Wrap around at the top and bottom of the page when advancing through text matches using `:search-next` and `:search-prev`. +search.wrap_messages: + type: Bool + default: true + desc: >- + Display messages when advancing through text matches at the top and bottom + of the page, e.g. `Search hit TOP`. + new_instance_open_target: type: name: String diff --git a/qutebrowser/mainwindow/statusbar/searchmatch.py b/qutebrowser/mainwindow/statusbar/searchmatch.py index d2650d6b7..9c4e67c32 100644 --- a/qutebrowser/mainwindow/statusbar/searchmatch.py +++ b/qutebrowser/mainwindow/statusbar/searchmatch.py @@ -34,6 +34,7 @@ class SearchMatch(textbase.TextBase): @pyqtSlot(int, int) def set_match_index(self, current: int, total: int) -> None: """Set the match counts in the statusbar. + Passing (0, 0) hides the match counter. Args: -- cgit v1.2.3-54-g00ecf From 57f0155fa0d9972f22298d1189ea54e9efa15433 Mon Sep 17 00:00:00 2001 From: phesch Date: Wed, 8 Sep 2021 21:03:07 +0200 Subject: Match counter: fix lints, implement suggestions --- qutebrowser/browser/browsertab.py | 38 +++---- qutebrowser/browser/commands.py | 9 +- qutebrowser/browser/webengine/webenginetab.py | 126 +++++++----------------- qutebrowser/browser/webkit/webkittab.py | 11 --- qutebrowser/mainwindow/mainwindow.py | 2 +- qutebrowser/mainwindow/statusbar/bar.py | 2 +- qutebrowser/mainwindow/statusbar/searchmatch.py | 4 +- qutebrowser/mainwindow/tabbedbrowser.py | 20 ++-- scripts/dev/check_coverage.py | 2 + tests/end2end/features/search.feature | 49 ++++++++- 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 -- cgit v1.2.3-54-g00ecf From 265b018c172f8c1f6d9e7f8850256363f0629f82 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 13 Jun 2022 16:18:42 +0200 Subject: Add a SearchMatch helper class --- qutebrowser/browser/browsertab.py | 53 ++++++++++++++++++++----- qutebrowser/browser/commands.py | 24 +++++------ qutebrowser/browser/webengine/webenginetab.py | 52 +++++++++--------------- qutebrowser/mainwindow/mainwindow.py | 2 +- qutebrowser/mainwindow/statusbar/searchmatch.py | 17 ++++---- qutebrowser/mainwindow/tabbedbrowser.py | 7 ++-- 6 files changed, 85 insertions(+), 70 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index f8ac3d24c..7745cff4b 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -287,6 +287,43 @@ class AbstractPrinting: diag.open(do_print) +@dataclasses.dataclass +class SearchMatch: + + """The currently highlighted search match. + + Attributes: + current: The currently active search match on the page. + 0 if no search is active or the feature isn't available. + total: The total number of search matches on the page. + 0 if no search is active or the feature isn't available. + """ + + current: int = 0 + total: int = 0 + + def reset(self) -> None: + """Reset match counter information. + + Stale information could lead to next_result or prev_result misbehaving. + """ + self.current = 0 + self.total = 0 + + def at_limit(self, going_up: bool) -> bool: + """Whether the SearchMatch is currently at the first/last result.""" + return ( + self.total != 0 and + ( + going_up and self.current == 1 or + not going_up and self.current == self.total + ) + ) + + def __str__(self) -> str: + return f"{self.current}/{self.total}" + + class AbstractSearch(QObject): """Attribute ``search`` of AbstractTab for doing searches. @@ -295,23 +332,20 @@ 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. + match: The currently active search match. _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. + match_changed: The currently active search match has changed. + Emits SearchMatch(0, 0) if no search is active. + Will not be emitted if search matches are not available. cleared: An existing search was cleared. """ finished = pyqtSignal(bool) - search_match_changed = pyqtSignal(int, int) + match_changed = pyqtSignal(SearchMatch) cleared = pyqtSignal() _Callback = Callable[[bool], None] @@ -322,8 +356,7 @@ class AbstractSearch(QObject): self._widget = cast(_WidgetType, None) self.text: Optional[str] = None self.search_displayed = False - self.current_match = 0 - self.total_match_count = 0 + self.match = SearchMatch() def _is_case_sensitive(self, ignore_case: usertypes.IgnoreCase) -> bool: """Check if case-sensitivity should be used. diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 939f02108..50dae13ea 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1538,14 +1538,14 @@ class CommandDispatcher: message.error(str(e)) ed.backup() - def _search_cb(self, found, *, tab, old_current_match, options, text, prev): + def _search_cb(self, found, *, tab, old_match, options, text, prev): """Callback called from search/search_next/search_prev. 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_match: The previously active search 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) @@ -1560,20 +1560,19 @@ class CommandDispatcher: if not config.val.search.wrap_messages: return - 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 old_match.current > 0: if not going_up: - if old_current_match > new_current_match: + if old_match.current > tab.search.match.current: message.info("Search hit BOTTOM, continuing at TOP", replace="search-hit-msg") - elif old_current_match == new_current_match: + elif old_match.current == tab.search.match.current: message.info("Search hit BOTTOM", replace="search-hit-msg") elif going_up: - if old_current_match < new_current_match: + if old_match.current < tab.search.match.current: message.info("Search hit TOP, continuing at BOTTOM", replace="search-hit-msg") - elif old_current_match == new_current_match: + elif old_match.current == tab.search.match.current: message.info("Search hit TOP", replace="search-hit-msg") else: message.warning(f"Text '{text}' not found on page!", @@ -1605,7 +1604,8 @@ class CommandDispatcher: self._tabbed_browser.search_options = dict(options) cb = functools.partial(self._search_cb, tab=tab, - old_current_match=0, options=options, + old_match=browsertab.SearchMatch(), + options=options, text=text, prev=False) options['result_cb'] = cb @@ -1638,7 +1638,7 @@ class CommandDispatcher: return cb = functools.partial(self._search_cb, tab=tab, - old_current_match=tab.search.current_match, + old_match=tab.search.match, options=window_options, text=window_text, prev=False) @@ -1672,7 +1672,7 @@ class CommandDispatcher: return cb = functools.partial(self._search_cb, tab=tab, - old_current_match=tab.search.current_match, + old_match=tab.search.match, options=window_options, text=window_text, prev=True) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 1d02f7e99..e513e4e24 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -112,25 +112,21 @@ class _WebEngineSearchWrapHandler: self.flag_wrap = True self.nowrap_available = False - def prevent_wrapping(self, current_match, total_match_count, *, going_up): + def prevent_wrapping( + self, + match: browsertab.SearchMatch, + *, + going_up: bool, + ) -> None: """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. + match: The currently active search match on the page. going_up: Whether the search would scroll the page up or down. """ - 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 - ) - ) + return self.nowrap_available and not self.flag_wrap and match.at_limit(going_up) class WebEngineSearch(browsertab.AbstractSearch): @@ -224,11 +220,10 @@ class WebEngineSearch(browsertab.AbstractSearch): def _on_find_finished(self, find_text_result): """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) + self.match.current = find_text_result.activeMatch() + self.match.total = find_text_result.numberOfMatches() + log.webview.debug(f"Active search match: {self.match}") + self.match_changed.emit(self.match) def search(self, text, *, ignore_case=usertypes.IgnoreCase.never, reverse=False, wrap=True, result_cb=None): @@ -241,7 +236,7 @@ class WebEngineSearch(browsertab.AbstractSearch): self.text = text self._flags = self._args_to_flags(reverse, ignore_case) - self._reset_match_data() + self.match.reset() self._wrap_handler.flag_wrap = wrap self._find(text, self._flags, result_cb, 'search') @@ -249,23 +244,21 @@ class WebEngineSearch(browsertab.AbstractSearch): def clear(self): if self.search_displayed: self.cleared.emit() - self.search_match_changed.emit(0, 0) + self.match_changed.emit(browsertab.SearchMatch()) self.search_displayed = False - self._reset_match_data() + self.match.reset() self._widget.page().findText('') def prev_result(self, *, result_cb=None): # The int() here makes sure we get a copy of the flags. flags = QWebEnginePage.FindFlags(int(self._flags)) if flags & QWebEnginePage.FindBackward: - if self._wrap_handler.prevent_wrapping(self.current_match, - self.total_match_count, going_up=False): + if self._wrap_handler.prevent_wrapping(self.match, going_up=False): result_cb(True) return flags &= ~QWebEnginePage.FindBackward else: - if self._wrap_handler.prevent_wrapping(self.current_match, - self.total_match_count, going_up=True): + if self._wrap_handler.prevent_wrapping(self.match, going_up=True): result_cb(True) return flags |= QWebEnginePage.FindBackward @@ -273,20 +266,11 @@ class WebEngineSearch(browsertab.AbstractSearch): def next_result(self, *, result_cb=None): going_up = self._flags & QWebEnginePage.FindBackward - if self._wrap_handler.prevent_wrapping(self.current_match, - self.total_match_count, going_up=going_up): + if self._wrap_handler.prevent_wrapping(self.match, going_up=going_up): result_cb(True) return self._find(self.text, self._flags, result_cb, 'next_result') - def _reset_match_data(self): - """Reset match counter information. - - Stale information could lead to next_result or prev_result misbehaving. - """ - self.current_match = 0 - self.total_match_count = 0 - class WebEngineCaret(browsertab.AbstractCaret): diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index c9744cfc1..22245d8c1 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -538,7 +538,7 @@ class MainWindow(QWidget): self.status.url.on_load_status_changed) self.tabbed_browser.cur_search_match_changed.connect( - self.status.search_match.set_match_index) + self.status.search_match.set_match) self.tabbed_browser.cur_caret_selection_toggled.connect( self.status.on_caret_selection_toggled) diff --git a/qutebrowser/mainwindow/statusbar/searchmatch.py b/qutebrowser/mainwindow/statusbar/searchmatch.py index 6e9a435de..da337b19a 100644 --- a/qutebrowser/mainwindow/statusbar/searchmatch.py +++ b/qutebrowser/mainwindow/statusbar/searchmatch.py @@ -23,6 +23,7 @@ from PyQt5.QtCore import pyqtSlot from qutebrowser.utils import log +from qutebrowser.browser import browsertab from qutebrowser.mainwindow.statusbar import textbase @@ -30,20 +31,18 @@ class SearchMatch(textbase.TextBase): """The part of the statusbar that displays the search match counter.""" - @pyqtSlot(int, int) - def set_match_index(self, current: int, total: int) -> None: + @pyqtSlot(browsertab.SearchMatch) + def set_match(self, match: browsertab.SearchMatch) -> None: """Set the match counts in the statusbar. - Passing (0, 0) hides the match counter. + Passing SearchMatch(0, 0) hides the match counter. Args: - current: The currently active search match. - total: The total number of search matches on the page. + match: The currently active search match. """ - if current <= 0 and total <= 0: + if match.current <= 0 and match.total <= 0: self.setText('') 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)) + self.setText(f'Match [{match}]') + log.statusbar.debug(f'Setting search match text to {match}') diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py index 9a98b3d70..c623ce809 100644 --- a/qutebrowser/mainwindow/tabbedbrowser.py +++ b/qutebrowser/mainwindow/tabbedbrowser.py @@ -202,7 +202,7 @@ class TabbedBrowser(QWidget): cur_link_hovered = pyqtSignal(str) cur_scroll_perc_changed = pyqtSignal(int, int) cur_load_status_changed = pyqtSignal(usertypes.LoadStatus) - cur_search_match_changed = pyqtSignal(int, int) + cur_search_match_changed = pyqtSignal(browsertab.SearchMatch) cur_fullscreen_requested = pyqtSignal(bool) cur_caret_selection_toggled = pyqtSignal(browsertab.SelectionState) close_window = pyqtSignal() @@ -349,7 +349,7 @@ 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( + tab.search.match_changed.connect( self._filter.create(self.cur_search_match_changed, tab)) # misc tab.scroller.perc_changed.connect(self._on_scroll_pos_changed) @@ -905,8 +905,7 @@ class TabbedBrowser(QWidget): .format(current_mode.name, mode_on_change)) self._now_focused = tab self.current_tab_changed.emit(tab) - self.cur_search_match_changed.emit(tab.search.current_match, - tab.search.total_match_count) + self.cur_search_match_changed.emit(tab.search.match) QTimer.singleShot(0, self._update_window_title) self._tab_insert_idx_left = self.widget.currentIndex() self._tab_insert_idx_right = self.widget.currentIndex() + 1 -- cgit v1.2.3-54-g00ecf From f326e0d96973aace648aad0976721ee2b2dbfa3c Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 13 Jun 2022 16:28:17 +0200 Subject: Get rid of _WebEngineSearchWrapHandler --- qutebrowser/browser/webengine/webenginetab.py | 58 +++++++++------------------ 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index e513e4e24..3ad1d99a7 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -97,44 +97,16 @@ class WebEnginePrinting(browsertab.AbstractPrinting): self._widget.page().print(printer, callback) -class _WebEngineSearchWrapHandler: - - """QtWebEngine implementations related to wrapping when searching. - - Attributes: - flag_wrap: An additional flag indicating whether the last search - used wrapping. - nowrap_available: Whether the functionality to prevent wrapping - is available. - """ - - def __init__(self): - self.flag_wrap = True - self.nowrap_available = False - - def prevent_wrapping( - self, - match: browsertab.SearchMatch, - *, - going_up: bool, - ) -> None: - """Prevent wrapping if possible and required. - - Returns True if a wrap was prevented and False if not. - - Args: - match: The currently active search match on the page. - going_up: Whether the search would scroll the page up or down. - """ - return self.nowrap_available and not self.flag_wrap and match.at_limit(going_up) - - class WebEngineSearch(browsertab.AbstractSearch): """QtWebEngine implementations related to searching on the page. Attributes: _flags: The QWebEnginePage.FindFlags of the last search. + _flag_wrap: An additional flag indicating whether the last search + used wrapping. + _nowrap_available: Whether the functionality to prevent wrapping + is available. _pending_searches: How many searches have been started but not called back yet. @@ -146,7 +118,8 @@ class WebEngineSearch(browsertab.AbstractSearch): super().__init__(tab, parent) self._flags = self._empty_flags() self._pending_searches = 0 - self._wrap_handler = _WebEngineSearchWrapHandler() + self._flag_wrap = None + self._nowrap_available = False # gets set in connect_signals def _empty_flags(self): return QWebEnginePage.FindFlags(0) @@ -177,7 +150,7 @@ class WebEngineSearch(browsertab.AbstractSearch): "to rebuild PyQtWebEngine.") return - self._wrap_handler.nowrap_available = True + self._nowrap_available = True self._widget.page().findTextFinished.connect(self._on_find_finished) def _find(self, text, flags, callback, caller): @@ -237,7 +210,7 @@ class WebEngineSearch(browsertab.AbstractSearch): self.text = text self._flags = self._args_to_flags(reverse, ignore_case) self.match.reset() - self._wrap_handler.flag_wrap = wrap + self._flag_wrap = wrap self._find(text, self._flags, result_cb, 'search') @@ -249,16 +222,25 @@ class WebEngineSearch(browsertab.AbstractSearch): self.match.reset() self._widget.page().findText('') + def _prevent_wrapping(self, going_up: bool) -> bool: + """Whether wrapping should be prevented.""" + assert self._flag_wrap is not None + return ( + self._nowrap_available and + not self._flag_wrap and + self.match.at_limit(going_up) + ) + def prev_result(self, *, result_cb=None): # The int() here makes sure we get a copy of the flags. flags = QWebEnginePage.FindFlags(int(self._flags)) if flags & QWebEnginePage.FindBackward: - if self._wrap_handler.prevent_wrapping(self.match, going_up=False): + if self._prevent_wrapping(going_up=False): result_cb(True) return flags &= ~QWebEnginePage.FindBackward else: - if self._wrap_handler.prevent_wrapping(self.match, going_up=True): + if self._prevent_wrapping(going_up=True): result_cb(True) return flags |= QWebEnginePage.FindBackward @@ -266,7 +248,7 @@ class WebEngineSearch(browsertab.AbstractSearch): def next_result(self, *, result_cb=None): going_up = self._flags & QWebEnginePage.FindBackward - if self._wrap_handler.prevent_wrapping(self.match, going_up=going_up): + if self._prevent_wrapping(going_up=going_up): result_cb(True) return self._find(self.text, self._flags, result_cb, 'next_result') -- cgit v1.2.3-54-g00ecf From 8ed08eb213f6218f779f5e79884aba6f3a594397 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 13 Jun 2022 16:55:32 +0200 Subject: search: Move wrap argument to next/prev_result The fact that we need to specify this while searching rather than when "zapping" through the results make no sense. It makes both the API as well as our own code more complex. --- qutebrowser/browser/browsertab.py | 8 +++---- qutebrowser/browser/commands.py | 11 +++++----- qutebrowser/browser/webengine/webenginetab.py | 31 +++++++-------------------- qutebrowser/browser/webkit/webkittab.py | 26 +++++++++++++++------- 4 files changed, 36 insertions(+), 40 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 7745cff4b..f2ed3f084 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -377,7 +377,6 @@ class AbstractSearch(QObject): def search(self, text: str, *, ignore_case: usertypes.IgnoreCase = usertypes.IgnoreCase.never, reverse: bool = False, - wrap: bool = True, result_cb: _Callback = None) -> None: """Find the given text on the page. @@ -385,7 +384,6 @@ class AbstractSearch(QObject): text: The text to search for. ignore_case: Search case-insensitively. reverse: Reverse search direction. - wrap: Allow wrapping at the top or bottom of the page. result_cb: Called with a bool indicating whether a match was found. """ raise NotImplementedError @@ -394,18 +392,20 @@ class AbstractSearch(QObject): """Clear the current search.""" raise NotImplementedError - def prev_result(self, *, result_cb: _Callback = None) -> None: + def prev_result(self, *, wrap: bool = False, result_cb: _Callback = None) -> None: """Go to the previous result of the current search. Args: + wrap: Allow wrapping at the top or bottom of the page. result_cb: Called with a bool indicating whether a match was found. """ raise NotImplementedError - def next_result(self, *, result_cb: _Callback = None) -> None: + def next_result(self, *, wrap: bool = False, result_cb: _Callback = None) -> None: """Go to the next result of the current search. Args: + wrap: Allow wrapping at the top or bottom of the page. result_cb: Called with a bool indicating whether a match was found. """ raise NotImplementedError diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 50dae13ea..073fe1c49 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1597,7 +1597,6 @@ class CommandDispatcher: options = { 'ignore_case': config.val.search.ignore_case, 'reverse': reverse, - 'wrap': config.val.search.wrap, } self._tabbed_browser.search_text = text @@ -1641,10 +1640,11 @@ class CommandDispatcher: old_match=tab.search.match, options=window_options, text=window_text, prev=False) + wrap = config.val.search.wrap for _ in range(count - 1): - tab.search.next_result() - tab.search.next_result(result_cb=cb) + tab.search.next_result(wrap=wrap) + tab.search.next_result(result_cb=cb, wrap=wrap) @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('count', value=cmdutils.Value.count) @@ -1675,10 +1675,11 @@ class CommandDispatcher: old_match=tab.search.match, options=window_options, text=window_text, prev=True) + wrap = config.val.search.wrap for _ in range(count - 1): - tab.search.prev_result() - tab.search.prev_result(result_cb=cb) + tab.search.prev_result(wrap=wrap) + tab.search.prev_result(result_cb=cb, wrap=wrap) def _jseval_cb(self, out): """Show the data returned from JS.""" diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 3ad1d99a7..f1d1a0cbb 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -103,10 +103,6 @@ class WebEngineSearch(browsertab.AbstractSearch): Attributes: _flags: The QWebEnginePage.FindFlags of the last search. - _flag_wrap: An additional flag indicating whether the last search - used wrapping. - _nowrap_available: Whether the functionality to prevent wrapping - is available. _pending_searches: How many searches have been started but not called back yet. @@ -118,8 +114,6 @@ class WebEngineSearch(browsertab.AbstractSearch): super().__init__(tab, parent) self._flags = self._empty_flags() self._pending_searches = 0 - self._flag_wrap = None - self._nowrap_available = False # gets set in connect_signals def _empty_flags(self): return QWebEnginePage.FindFlags(0) @@ -150,7 +144,6 @@ class WebEngineSearch(browsertab.AbstractSearch): "to rebuild PyQtWebEngine.") return - self._nowrap_available = True self._widget.page().findTextFinished.connect(self._on_find_finished) def _find(self, text, flags, callback, caller): @@ -199,7 +192,7 @@ class WebEngineSearch(browsertab.AbstractSearch): self.match_changed.emit(self.match) def search(self, text, *, ignore_case=usertypes.IgnoreCase.never, - reverse=False, wrap=True, result_cb=None): + reverse=False, result_cb=None): # Don't go to next entry on duplicate search if self.text == text and self.search_displayed: log.webview.debug("Ignoring duplicate search request" @@ -210,7 +203,6 @@ class WebEngineSearch(browsertab.AbstractSearch): self.text = text self._flags = self._args_to_flags(reverse, ignore_case) self.match.reset() - self._flag_wrap = wrap self._find(text, self._flags, result_cb, 'search') @@ -222,33 +214,26 @@ class WebEngineSearch(browsertab.AbstractSearch): self.match.reset() self._widget.page().findText('') - def _prevent_wrapping(self, going_up: bool) -> bool: - """Whether wrapping should be prevented.""" - assert self._flag_wrap is not None - return ( - self._nowrap_available and - not self._flag_wrap and - self.match.at_limit(going_up) - ) - - def prev_result(self, *, result_cb=None): + def prev_result(self, *, wrap=False, result_cb=None): # The int() here makes sure we get a copy of the flags. flags = QWebEnginePage.FindFlags(int(self._flags)) + if flags & QWebEnginePage.FindBackward: - if self._prevent_wrapping(going_up=False): + if self.match.at_limit(going_up=False) and not wrap: result_cb(True) return flags &= ~QWebEnginePage.FindBackward else: - if self._prevent_wrapping(going_up=True): + if self.match.at_limit(going_up=True) and not wrap: result_cb(True) return flags |= QWebEnginePage.FindBackward + self._find(self.text, flags, result_cb, 'prev_result') - def next_result(self, *, result_cb=None): + def next_result(self, *, wrap=False, result_cb=None): going_up = self._flags & QWebEnginePage.FindBackward - if self._prevent_wrapping(going_up=going_up): + if self.match.at_limit(going_up=going_up) and not wrap: result_cb(True) return self._find(self.text, self._flags, result_cb, 'next_result') diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 24d232c9c..05e48b2f6 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -115,14 +115,12 @@ class WebKitSearch(browsertab.AbstractSearch): def _empty_flags(self): return QWebPage.FindFlags(0) # type: ignore[call-overload] - def _args_to_flags(self, reverse, ignore_case, wrap): + def _args_to_flags(self, reverse, ignore_case): flags = self._empty_flags() if self._is_case_sensitive(ignore_case): flags |= QWebPage.FindCaseSensitively if reverse: flags |= QWebPage.FindBackward - if wrap: - flags |= QWebPage.FindWrapsAroundDocument return flags def _call_cb(self, callback, found, text, flags, caller): @@ -164,12 +162,12 @@ class WebKitSearch(browsertab.AbstractSearch): '', QWebPage.HighlightAllOccurrences) # type: ignore[arg-type] def search(self, text, *, ignore_case=usertypes.IgnoreCase.never, - reverse=False, wrap=True, result_cb=None): + reverse=False, result_cb=None): # Don't go to next entry on duplicate search if self.text == text and self.search_displayed: log.webview.debug("Ignoring duplicate search request" " for {}, but resetting flags".format(text)) - self._flags = self._args_to_flags(reverse, ignore_case, wrap) + self._flags = self._args_to_flags(reverse, ignore_case) return # Clear old search results, this is done automatically on QtWebEngine. @@ -177,7 +175,7 @@ class WebKitSearch(browsertab.AbstractSearch): self.text = text self.search_displayed = True - self._flags = self._args_to_flags(reverse, ignore_case, wrap) + self._flags = self._args_to_flags(reverse, ignore_case) # We actually search *twice* - once to highlight everything, then again # to get a mark so we can navigate. found = self._widget.findText(text, self._flags) @@ -185,20 +183,32 @@ class WebKitSearch(browsertab.AbstractSearch): self._flags | QWebPage.HighlightAllOccurrences) self._call_cb(result_cb, found, text, self._flags, 'search') - def next_result(self, *, result_cb=None): + def next_result(self, *, wrap=False, result_cb=None): self.search_displayed = True + # The int() here makes sure we get a copy of the flags. + flags = QWebPage.FindFlags( + int(self._flags)) # type: ignore[call-overload] + + if wrap: + flags |= QWebPage.FindWrapsAroundDocument + found = self._widget.findText(self.text, self._flags) # type: ignore[arg-type] self._call_cb(result_cb, found, self.text, self._flags, 'next_result') - def prev_result(self, *, result_cb=None): + def prev_result(self, *, wrap=False, result_cb=None): self.search_displayed = True # The int() here makes sure we get a copy of the flags. flags = QWebPage.FindFlags( int(self._flags)) # type: ignore[call-overload] + if flags & QWebPage.FindBackward: flags &= ~QWebPage.FindBackward else: flags |= QWebPage.FindBackward + + if wrap: + flags |= QWebPage.FindWrapsAroundDocument + found = self._widget.findText(self.text, flags) # type: ignore[arg-type] self._call_cb(result_cb, found, self.text, flags, 'prev_result') -- cgit v1.2.3-54-g00ecf From e15bda307e42c288b926f578e7bf8c610e4767af Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 13 Jun 2022 17:41:43 +0200 Subject: search: Split navigation/search callbacks This way, we can move more logic (checking wrapping, etc.) into the API, thus making the commands much more simple and stateless. --- qutebrowser/browser/browsertab.py | 23 ++++++-- qutebrowser/browser/commands.py | 83 +++++++++++---------------- qutebrowser/browser/webengine/webenginetab.py | 60 ++++++++++++++----- qutebrowser/browser/webkit/webkittab.py | 24 ++++++-- tests/end2end/features/search.feature | 32 ++--------- tests/unit/browser/test_caret.py | 4 +- 6 files changed, 121 insertions(+), 105 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index f2ed3f084..ca6a9c483 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -324,6 +324,20 @@ class SearchMatch: return f"{self.current}/{self.total}" +class SearchNavigationResult(enum.Enum): + + """The outcome of calling prev_/next_result.""" + + found = enum.auto() + not_found = enum.auto() + + wrapped_bottom = enum.auto() + wrap_prevented_bottom = enum.auto() + + wrapped_top = enum.auto() + wrap_prevented_top = enum.auto() + + class AbstractSearch(QObject): """Attribute ``search`` of AbstractTab for doing searches. @@ -349,6 +363,7 @@ class AbstractSearch(QObject): cleared = pyqtSignal() _Callback = Callable[[bool], None] + _NavCallback = Callable[[SearchNavigationResult], None] def __init__(self, tab: 'AbstractTab', parent: QWidget = None): super().__init__(parent) @@ -392,21 +407,21 @@ class AbstractSearch(QObject): """Clear the current search.""" raise NotImplementedError - def prev_result(self, *, wrap: bool = False, result_cb: _Callback = None) -> None: + def prev_result(self, *, wrap: bool = False, callback: _NavCallback = None) -> None: """Go to the previous result of the current search. Args: wrap: Allow wrapping at the top or bottom of the page. - result_cb: Called with a bool indicating whether a match was found. + callback: Called with a SearchNavigationResult. """ raise NotImplementedError - def next_result(self, *, wrap: bool = False, result_cb: _Callback = None) -> None: + def next_result(self, *, wrap: bool = False, callback: _NavCallback = None) -> None: """Go to the next result of the current search. Args: wrap: Allow wrapping at the top or bottom of the page. - result_cb: Called with a bool indicating whether a match was found. + callback: Called with a SearchNavigationResult. """ raise NotImplementedError diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 073fe1c49..f534b91b3 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1538,46 +1538,39 @@ class CommandDispatcher: message.error(str(e)) ed.backup() - def _search_cb(self, found, *, tab, old_match, options, text, prev): - """Callback called from search/search_next/search_prev. + def _search_cb(self, found, *, text): + """Callback called from :search. Args: found: Whether the text was found. - tab: The AbstractTab in which the search was made. - old_match: The previously active search 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) """ - # :search/:search-next without reverse -> down - # :search/:search-next with reverse -> up - # :search-prev without reverse -> up - # :search-prev with reverse -> down - going_up = options['reverse'] ^ prev - - if found: - if not config.val.search.wrap_messages: - return - - # Check if the match count change is opposite to the search direction - if old_match.current > 0: - if not going_up: - if old_match.current > tab.search.match.current: - message.info("Search hit BOTTOM, continuing at TOP", - replace="search-hit-msg") - elif old_match.current == tab.search.match.current: - message.info("Search hit BOTTOM", replace="search-hit-msg") - elif going_up: - if old_match.current < tab.search.match.current: - message.info("Search hit TOP, continuing at BOTTOM", - replace="search-hit-msg") - elif old_match.current == tab.search.match.current: - message.info("Search hit TOP", replace="search-hit-msg") - else: + if not found: message.warning(f"Text '{text}' not found on page!", replace='find-in-page') + def _search_navigation_cb(self, result): + """Callback called from :search-prev/next.""" + if result == browsertab.SearchNavigationResult.not_found: + # FIXME check if this actually can happen... + message.warning("Search result vanished...") + return + elif result == browsertab.SearchNavigationResult.found: + return + elif not config.val.search.wrap_messages: + return + + messages = { + browsertab.SearchNavigationResult.wrap_prevented_bottom: + "Search hit BOTTOM", + browsertab.SearchNavigationResult.wrap_prevented_top: + "Search hit TOP", + browsertab.SearchNavigationResult.wrapped_bottom: + "Search hit BOTTOM, continuing at TOP", + browsertab.SearchNavigationResult.wrapped_top: + "Search hit TOP, continuing at BOTTOM", + } + message.info(messages[result], replace="search-hit-msg") + @cmdutils.register(instance='command-dispatcher', scope='window', maxsplit=0) def search(self, text="", reverse=False): @@ -1600,16 +1593,12 @@ class CommandDispatcher: } self._tabbed_browser.search_text = text - self._tabbed_browser.search_options = dict(options) - - cb = functools.partial(self._search_cb, tab=tab, - old_match=browsertab.SearchMatch(), - options=options, - text=text, prev=False) - options['result_cb'] = cb + self._tabbed_browser.search_options = options tab.scroller.before_jump_requested.emit() - tab.search.search(text, **options) + + cb = functools.partial(self._search_cb, text=text) + tab.search.search(text, **options, result_cb=cb) @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('count', value=cmdutils.Value.count) @@ -1636,15 +1625,11 @@ class CommandDispatcher: if count == 0: return - cb = functools.partial(self._search_cb, tab=tab, - old_match=tab.search.match, - options=window_options, text=window_text, - prev=False) wrap = config.val.search.wrap for _ in range(count - 1): tab.search.next_result(wrap=wrap) - tab.search.next_result(result_cb=cb, wrap=wrap) + tab.search.next_result(callback=self._search_navigation_cb, wrap=wrap) @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('count', value=cmdutils.Value.count) @@ -1671,15 +1656,11 @@ class CommandDispatcher: if count == 0: return - cb = functools.partial(self._search_cb, tab=tab, - old_match=tab.search.match, - options=window_options, text=window_text, - prev=True) wrap = config.val.search.wrap for _ in range(count - 1): tab.search.prev_result(wrap=wrap) - tab.search.prev_result(result_cb=cb, wrap=wrap) + tab.search.prev_result(callback=self._search_navigation_cb, wrap=wrap) def _jseval_cb(self, out): """Show the data returned from JS.""" diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index f1d1a0cbb..7a7c5a8d4 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -114,6 +114,8 @@ class WebEngineSearch(browsertab.AbstractSearch): super().__init__(tab, parent) self._flags = self._empty_flags() self._pending_searches = 0 + self.match = browsertab.SearchMatch() + self._old_match = browsertab.SearchMatch() def _empty_flags(self): return QWebEnginePage.FindFlags(0) @@ -180,14 +182,18 @@ class WebEngineSearch(browsertab.AbstractSearch): if callback is not None: callback(found) + self.finished.emit(found) self._widget.page().findText(text, flags, wrapped_callback) def _on_find_finished(self, find_text_result): """Unwrap the result, store it, and pass it along.""" - self.match.current = find_text_result.activeMatch() - self.match.total = find_text_result.numberOfMatches() + self._old_match = self.match + self.match = browsertab.SearchMatch( + current=find_text_result.activeMatch(), + total=find_text_result.numberOfMatches(), + ) log.webview.debug(f"Active search match: {self.match}") self.match_changed.emit(self.match) @@ -214,29 +220,55 @@ class WebEngineSearch(browsertab.AbstractSearch): self.match.reset() self._widget.page().findText('') - def prev_result(self, *, wrap=False, result_cb=None): + def _prev_next_cb(self, found, *, going_up, callback): + """Call the prev/next callback based on the search result.""" + if found: + result = browsertab.SearchNavigationResult.found + # Check if the match count change is opposite to the search direction + if self._old_match.current > 0: + if not going_up and self._old_match.current > self.match.current: + result = browsertab.SearchNavigationResult.wrapped_bottom + elif going_up and self._old_match.current < self.match.current: + result = browsertab.SearchNavigationResult.wrapped_top + else: + result = browsertab.SearchNavigationResult.not_found + + callback(result) + + def prev_result(self, *, wrap=False, callback=None): # The int() here makes sure we get a copy of the flags. flags = QWebEnginePage.FindFlags(int(self._flags)) if flags & QWebEnginePage.FindBackward: - if self.match.at_limit(going_up=False) and not wrap: - result_cb(True) - return + going_up = False flags &= ~QWebEnginePage.FindBackward else: - if self.match.at_limit(going_up=True) and not wrap: - result_cb(True) - return + going_up = True flags |= QWebEnginePage.FindBackward - self._find(self.text, flags, result_cb, 'prev_result') + if self.match.at_limit(going_up=going_up) and not wrap: + res = ( + browsertab.SearchNavigationResult.wrap_prevented_top if going_up else + browsertab.SearchNavigationResult.wrap_prevented_bottom + ) + callback(res) + return - def next_result(self, *, wrap=False, result_cb=None): - going_up = self._flags & QWebEnginePage.FindBackward + cb = functools.partial(self._prev_next_cb, going_up=going_up, callback=callback) + self._find(self.text, flags, cb, 'prev_result') + + def next_result(self, *, wrap=False, callback=None): + going_up = bool(self._flags & QWebEnginePage.FindBackward) if self.match.at_limit(going_up=going_up) and not wrap: - result_cb(True) + res = ( + browsertab.SearchNavigationResult.wrap_prevented_top if going_up else + browsertab.SearchNavigationResult.wrap_prevented_bottom + ) + callback(res) return - self._find(self.text, self._flags, result_cb, 'next_result') + + cb = functools.partial(self._prev_next_cb, going_up=going_up, callback=callback) + self._find(self.text, self._flags, cb, 'next_result') class WebEngineCaret(browsertab.AbstractCaret): diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 05e48b2f6..2faf93a32 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -148,7 +148,19 @@ class WebKitSearch(browsertab.AbstractSearch): log.webview.debug(' '.join([caller, found_text, text, flag_text]) .strip()) if callback is not None: - QTimer.singleShot(0, functools.partial(callback, found)) + if caller in ["prev_result", "next_result"]: + if found: + # no wrapping detection + cb_value = browsertab.SearchNavigationResult.found + elif flags & QWebPage.FindBackward: + cb_value = browsertab.SearchNavigationResult.wrap_prevented_top + else: + cb_value = browsertab.SearchNavigationResult.wrap_prevented_bottom + elif caller == "search": + cb_value = found + else: + raise utils.Unreachable(caller) + QTimer.singleShot(0, functools.partial(callback, cb_value)) self.finished.emit(found) @@ -183,7 +195,7 @@ class WebKitSearch(browsertab.AbstractSearch): self._flags | QWebPage.HighlightAllOccurrences) self._call_cb(result_cb, found, text, self._flags, 'search') - def next_result(self, *, wrap=False, result_cb=None): + def next_result(self, *, wrap=False, callback=None): self.search_displayed = True # The int() here makes sure we get a copy of the flags. flags = QWebPage.FindFlags( @@ -192,10 +204,10 @@ class WebKitSearch(browsertab.AbstractSearch): if wrap: flags |= QWebPage.FindWrapsAroundDocument - found = self._widget.findText(self.text, self._flags) # type: ignore[arg-type] - self._call_cb(result_cb, found, self.text, self._flags, 'next_result') + found = self._widget.findText(self.text, flags) # type: ignore[arg-type] + self._call_cb(callback, found, self.text, flags, 'next_result') - def prev_result(self, *, wrap=False, result_cb=None): + def prev_result(self, *, wrap=False, callback=None): self.search_displayed = True # The int() here makes sure we get a copy of the flags. flags = QWebPage.FindFlags( @@ -210,7 +222,7 @@ class WebKitSearch(browsertab.AbstractSearch): flags |= QWebPage.FindWrapsAroundDocument found = self._widget.findText(self.text, flags) # type: ignore[arg-type] - self._call_cb(result_cb, found, self.text, flags, 'prev_result') + self._call_cb(callback, found, self.text, flags, 'prev_result') class WebKitCaret(browsertab.AbstractCaret): diff --git a/tests/end2end/features/search.feature b/tests/end2end/features/search.feature index 86ceab70e..f2d793907 100644 --- a/tests/end2end/features/search.feature +++ b/tests/end2end/features/search.feature @@ -233,8 +233,8 @@ Feature: Searching on a page ## wrapping prevented - @qtwebkit_skip @qt>=5.14 - Scenario: Preventing wrapping at the top of the page with QtWebEngine + @qt>=5.14 + Scenario: Preventing wrapping at the top of the page When I set search.ignore_case to always And I set search.wrap to false And I set search.wrap_messages to true @@ -245,8 +245,8 @@ Feature: Searching on a page And I run :search-next 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 + @qt>=5.14 + Scenario: Preventing wrapping at the bottom of the page When I set search.ignore_case to always And I set search.wrap to false And I run :search foo @@ -256,30 +256,6 @@ Feature: Searching on a page And I run :search-next Then the message "Search hit BOTTOM" should be shown - @qtwebengine_skip - Scenario: Preventing wrapping at the top of the page with QtWebKit - When I set search.ignore_case to always - And I set search.wrap to false - 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 "next_result didn't find foo with flags FindBackward" in the log - Then the warning "Text 'foo' not found on page!" should be shown - - @qtwebengine_skip - Scenario: Preventing wrapping at the bottom of the page with QtWebKit - When I set search.ignore_case to always - And I set search.wrap to false - And I run :search foo - And I wait for "search found foo" in the log - 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 "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 diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index 86014040d..85301e358 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -345,8 +345,8 @@ class TestSearch: callback.assert_called_with(True) with qtbot.wait_callback() as callback: - web_tab.search.next_result(result_cb=callback) - callback.assert_called_with(True) + web_tab.search.next_result(callback=callback) + callback.assert_called_with(browsertab.SearchNavigationResult.found) mode_manager.enter(usertypes.KeyMode.caret) -- cgit v1.2.3-54-g00ecf From bf045f7ec7c27709ea3ef61cf41a24e8fdd2e7da Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 19 May 2022 16:43:06 +0200 Subject: Add a helper dataclass for find flags When e.g. doing: - '?foo' (search with reverse=True -> FindBackwards) - 'N' (prev_result -> no FindBackwards) - 'n' (next_result -> FindBackwards again) we need to take a copy of the flags so that we can temporarily clear FindBackwards when pressing 'N'. Relevant history: - We originally did int(self._flags) in d4502574856c7d539a0890fbb6b093942c4551b6. - In f0da508c218ad57289bdb9268faeba7b7741a233, we used QWebPage.FindFlags(int(self._flags)) instead. - With 97bdcb8e674c8ff27ab92448effef263880ab3aa (picked from c349fbd18084183d44f351af617f6ad5d16efabf) we instead do: flags = QWebEnginePage.FindFlag(self._flags) Using FindFlag instead of FindFlags seemed to work fine with PyQt6 and enum.Flag. With PyQt5, however, later clearing a flag bit ends up with us getting 0 as an integer, thus losing the type information about this being a FindFlag instance, and resulting in a TypeError when calling into Qt. We could use FindFlags() with PyQt 6 but FindFlag() with PyQt 5 to copy the original flags, but that's getting rather cumbersome. Instead, let's have a helper dataclass of bools, do away with the bit-twiddling, and only convert it to a Qt flags when we actually need them. This solves the copying issue nicely, and also makes the code a lot nicer. Finally, this also adds a test case which fails when the flags are mutated in place instead of copied. We could do the same kind of change for QtWebKit as well, but given that it's somewhat dead - and perhaps more importantly, won't run with Qt 6 - let's not bother. To not break the end2end tests with QtWebKit, the output still is the same as before. (cherry picked from commit 96a0cc39512753445bc7a01b218b2f1290819ddd) --- qutebrowser/browser/webengine/webenginetab.py | 74 ++++++++++++++--------- tests/end2end/features/search.feature | 14 +++++ tests/unit/browser/webengine/test_webenginetab.py | 42 +++++++++++++ 3 files changed, 103 insertions(+), 27 deletions(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 7a7c5a8d4..419c3842a 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -97,12 +97,47 @@ class WebEnginePrinting(browsertab.AbstractPrinting): self._widget.page().print(printer, callback) +@dataclasses.dataclass +class _FindFlags: + + case_sensitive: bool = False + backward: bool = False + + def to_qt(self): + """Convert flags into Qt flags.""" + flags = QWebEnginePage.FindFlag(0) + if self.case_sensitive: + flags |= QWebEnginePage.FindFlag.FindCaseSensitively + if self.backward: + flags |= QWebEnginePage.FindFlag.FindBackward + return flags + + def __bool__(self): + """Flags are truthy if any flag is set to True.""" + return any(dataclasses.astuple(self)) + + def __str__(self): + """List all true flags, in Qt enum style. + + This needs to be in the same format as QtWebKit, for tests. + """ + names = { + "case_sensitive": "FindCaseSensitively", + "backward": "FindBackward", + } + d = dataclasses.asdict(self) + truthy = [names[key] for key, value in d.items() if value] + if not truthy: + return "" + return "|".join(truthy) + + class WebEngineSearch(browsertab.AbstractSearch): """QtWebEngine implementations related to searching on the page. Attributes: - _flags: The QWebEnginePage.FindFlags of the last search. + _flags: The FindFlags of the last search. _pending_searches: How many searches have been started but not called back yet. @@ -112,21 +147,14 @@ class WebEngineSearch(browsertab.AbstractSearch): def __init__(self, tab, parent=None): super().__init__(tab, parent) - self._flags = self._empty_flags() + self._flags = _FindFlags() self._pending_searches = 0 self.match = browsertab.SearchMatch() self._old_match = browsertab.SearchMatch() - def _empty_flags(self): - return QWebEnginePage.FindFlags(0) - - def _args_to_flags(self, reverse, ignore_case): - flags = self._empty_flags() - if self._is_case_sensitive(ignore_case): - flags |= QWebEnginePage.FindCaseSensitively - if reverse: - flags |= QWebEnginePage.FindBackward - return flags + def _store_flags(self, reverse, ignore_case): + self._flags.case_sensitive = self._is_case_sensitive(ignore_case) + self._flags.backward = reverse def connect_signals(self): """Connect the signals necessary for this class to function.""" @@ -173,8 +201,7 @@ class WebEngineSearch(browsertab.AbstractSearch): found_text = 'found' if found else "didn't find" if flags: - flag_text = 'with flags {}'.format(debug.qflags_key( - QWebEnginePage, flags, klass=QWebEnginePage.FindFlag)) + flag_text = f'with flags {flags}' else: flag_text = '' log.webview.debug(' '.join([caller, found_text, text, flag_text]) @@ -185,7 +212,7 @@ class WebEngineSearch(browsertab.AbstractSearch): self.finished.emit(found) - self._widget.page().findText(text, flags, wrapped_callback) + self._widget.page().findText(text, flags.to_qt(), wrapped_callback) def _on_find_finished(self, find_text_result): """Unwrap the result, store it, and pass it along.""" @@ -203,11 +230,11 @@ class WebEngineSearch(browsertab.AbstractSearch): if self.text == text and self.search_displayed: log.webview.debug("Ignoring duplicate search request" " for {}, but resetting flags".format(text)) - self._flags = self._args_to_flags(reverse, ignore_case) + self._store_flags(reverse, ignore_case) return self.text = text - self._flags = self._args_to_flags(reverse, ignore_case) + self._store_flags(reverse, ignore_case) self.match.reset() self._find(text, self._flags, result_cb, 'search') @@ -236,15 +263,8 @@ class WebEngineSearch(browsertab.AbstractSearch): callback(result) def prev_result(self, *, wrap=False, callback=None): - # The int() here makes sure we get a copy of the flags. - flags = QWebEnginePage.FindFlags(int(self._flags)) - - if flags & QWebEnginePage.FindBackward: - going_up = False - flags &= ~QWebEnginePage.FindBackward - else: - going_up = True - flags |= QWebEnginePage.FindBackward + going_up = not self._flags.backward + flags = dataclasses.replace(self._flags, backward=going_up) if self.match.at_limit(going_up=going_up) and not wrap: res = ( @@ -258,7 +278,7 @@ class WebEngineSearch(browsertab.AbstractSearch): self._find(self.text, flags, cb, 'prev_result') def next_result(self, *, wrap=False, callback=None): - going_up = bool(self._flags & QWebEnginePage.FindBackward) + going_up = self._flags.backward if self.match.at_limit(going_up=going_up) and not wrap: res = ( browsertab.SearchNavigationResult.wrap_prevented_top if going_up else diff --git a/tests/end2end/features/search.feature b/tests/end2end/features/search.feature index f2d793907..1397bad13 100644 --- a/tests/end2end/features/search.feature +++ b/tests/end2end/features/search.feature @@ -202,6 +202,20 @@ Feature: Searching on a page And I wait for "prev_result found foo" in the log Then "Foo" should be found + # This makes sure we don't mutate the original flags + Scenario: Jumping to previous match with --reverse twice + When I set search.ignore_case to always + And I run :search --reverse baz + # BAZ + And I wait for "search found baz with flags FindBackward" in the log + And I run :search-prev + # Baz + And I wait for "prev_result found baz" in the log + And I run :search-prev + # baz + And I wait for "prev_result found baz" in the log + Then "baz" should be found + Scenario: Jumping to previous match without search # Make sure there was no search in the same window before When I open data/search.html in a new window diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py index 3d8eec663..446dbd9a7 100644 --- a/tests/unit/browser/webengine/test_webenginetab.py +++ b/tests/unit/browser/webengine/test_webenginetab.py @@ -214,3 +214,45 @@ def test_notification_permission_workaround(): permissions = webenginetab._WebEnginePermissions assert permissions._options[notifications] == 'content.notifications.enabled' assert permissions._messages[notifications] == 'show notifications' + + +class TestFindFlags: + + @pytest.mark.parametrize("case_sensitive, backward, expected", [ + (True, True, QWebEnginePage.FindFlag.FindCaseSensitively | QWebEnginePage.FindFlag.FindBackward), + (True, False, QWebEnginePage.FindFlag.FindCaseSensitively), + (False, True, QWebEnginePage.FindFlag.FindBackward), + (False, False, QWebEnginePage.FindFlag(0)), + ]) + def test_to_qt(self, case_sensitive, backward, expected): + flags = webenginetab._FindFlags( + case_sensitive=case_sensitive, + backward=backward, + ) + assert flags.to_qt() == expected + + @pytest.mark.parametrize("case_sensitive, backward, expected", [ + (True, True, True), + (True, False, True), + (False, True, True), + (False, False, False), + ]) + def test_bool(self, case_sensitive, backward, expected): + flags = webenginetab._FindFlags( + case_sensitive=case_sensitive, + backward=backward, + ) + assert bool(flags) == expected + + @pytest.mark.parametrize("case_sensitive, backward, expected", [ + (True, True, "FindCaseSensitively|FindBackward"), + (True, False, "FindCaseSensitively"), + (False, True, "FindBackward"), + (False, False, ""), + ]) + def test_str(self, case_sensitive, backward, expected): + flags = webenginetab._FindFlags( + case_sensitive=case_sensitive, + backward=backward, + ) + assert str(flags) == expected -- cgit v1.2.3-54-g00ecf From f81bc8a53f7791093938965676ecdbd4e62beb33 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 13 Jun 2022 18:38:07 +0200 Subject: Fix prev_result/next_result with None callbacks --- qutebrowser/browser/webengine/webenginetab.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 419c3842a..0d0edabe3 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -271,7 +271,8 @@ class WebEngineSearch(browsertab.AbstractSearch): browsertab.SearchNavigationResult.wrap_prevented_top if going_up else browsertab.SearchNavigationResult.wrap_prevented_bottom ) - callback(res) + if callback is not None: + callback(res) return cb = functools.partial(self._prev_next_cb, going_up=going_up, callback=callback) @@ -284,7 +285,8 @@ class WebEngineSearch(browsertab.AbstractSearch): browsertab.SearchNavigationResult.wrap_prevented_top if going_up else browsertab.SearchNavigationResult.wrap_prevented_bottom ) - callback(res) + if callback is not None: + callback(res) return cb = functools.partial(self._prev_next_cb, going_up=going_up, callback=callback) -- cgit v1.2.3-54-g00ecf From cd1be710de29eb29cf16e3b22185afd093c2ff19 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 13 Jun 2022 19:37:05 +0200 Subject: Fix lint with FindFlags change --- qutebrowser/browser/webengine/webenginetab.py | 6 ++++-- tests/unit/browser/webengine/test_webenginetab.py | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 0d0edabe3..afb41c262 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -105,11 +105,13 @@ class _FindFlags: def to_qt(self): """Convert flags into Qt flags.""" + # FIXME:mypy Those should be correct, reevaluate with PyQt6-stubs flags = QWebEnginePage.FindFlag(0) if self.case_sensitive: - flags |= QWebEnginePage.FindFlag.FindCaseSensitively + flags |= ( # type: ignore[assignment] + QWebEnginePage.FindFlag.FindCaseSensitively) if self.backward: - flags |= QWebEnginePage.FindFlag.FindBackward + flags |= QWebEnginePage.FindFlag.FindBackward # type: ignore[assignment] return flags def __bool__(self): diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py index 446dbd9a7..30807bb4e 100644 --- a/tests/unit/browser/webengine/test_webenginetab.py +++ b/tests/unit/browser/webengine/test_webenginetab.py @@ -219,7 +219,8 @@ def test_notification_permission_workaround(): class TestFindFlags: @pytest.mark.parametrize("case_sensitive, backward, expected", [ - (True, True, QWebEnginePage.FindFlag.FindCaseSensitively | QWebEnginePage.FindFlag.FindBackward), + (True, True, (QWebEnginePage.FindFlag.FindCaseSensitively | + QWebEnginePage.FindFlag.FindBackward)), (True, False, QWebEnginePage.FindFlag.FindCaseSensitively), (False, True, QWebEnginePage.FindFlag.FindBackward), (False, False, QWebEnginePage.FindFlag(0)), -- cgit v1.2.3-54-g00ecf From 2cb20dbf5b836183698c719772c32142fd53f16f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Apr 2022 14:04:15 +0200 Subject: Skip test broken on QtWebKit (cherry picked from commit d01dd9be07bd1591b1ecda298a2bf84b41ea280c) --- tests/end2end/features/search.feature | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/end2end/features/search.feature b/tests/end2end/features/search.feature index 1397bad13..9446c36ac 100644 --- a/tests/end2end/features/search.feature +++ b/tests/end2end/features/search.feature @@ -203,6 +203,8 @@ Feature: Searching on a page Then "Foo" should be found # This makes sure we don't mutate the original flags + # Seems to be broken with QtWebKit, wontfix + @qtwebkit_skip Scenario: Jumping to previous match with --reverse twice When I set search.ignore_case to always And I run :search --reverse baz -- cgit v1.2.3-54-g00ecf From 4a2001dd2ce70fcd64a86627b6be50b38ba97ee4 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 14 Jun 2022 10:01:23 +0200 Subject: Remove now-unneeded lint ignore --- qutebrowser/mainwindow/statusbar/bar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/mainwindow/statusbar/bar.py b/qutebrowser/mainwindow/statusbar/bar.py index eaf60db3e..0d5f096bf 100644 --- a/qutebrowser/mainwindow/statusbar/bar.py +++ b/qutebrowser/mainwindow/statusbar/bar.py @@ -248,7 +248,7 @@ class StatusBar(QWidget): elif option == 'statusbar.widgets': self._draw_widgets() - def _draw_widgets(self): # noqa: C901 + def _draw_widgets(self): """Draw statusbar widgets.""" self._clear_widgets() -- cgit v1.2.3-54-g00ecf From 77269f789addcc2f73b9eb2d53eb429090a9cd7d Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 14 Jun 2022 10:03:06 +0200 Subject: Fix lint --- qutebrowser/browser/webengine/webenginetab.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index afb41c262..916164ba7 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -142,7 +142,6 @@ class WebEngineSearch(browsertab.AbstractSearch): _flags: The FindFlags of the last search. _pending_searches: How many searches have been started but not called back yet. - """ _widget: webview.WebEngineView -- cgit v1.2.3-54-g00ecf From 8137f2a6d90e30643ad3bcf0c2c182c9a48012eb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 14 Jun 2022 10:05:14 +0200 Subject: Add a SearchMatch.is_null() --- qutebrowser/browser/browsertab.py | 4 ++++ qutebrowser/mainwindow/statusbar/searchmatch.py | 13 ++++--------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index ca6a9c483..e87ac806e 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -310,6 +310,10 @@ class SearchMatch: self.current = 0 self.total = 0 + def is_null(self) -> bool: + """Whether the SearchMatch is set to zero.""" + return self.current == 0 and self.total == 0 + def at_limit(self, going_up: bool) -> bool: """Whether the SearchMatch is currently at the first/last result.""" return ( diff --git a/qutebrowser/mainwindow/statusbar/searchmatch.py b/qutebrowser/mainwindow/statusbar/searchmatch.py index da337b19a..6d0b86007 100644 --- a/qutebrowser/mainwindow/statusbar/searchmatch.py +++ b/qutebrowser/mainwindow/statusbar/searchmatch.py @@ -22,7 +22,6 @@ from PyQt5.QtCore import pyqtSlot -from qutebrowser.utils import log from qutebrowser.browser import browsertab from qutebrowser.mainwindow.statusbar import textbase @@ -32,17 +31,13 @@ class SearchMatch(textbase.TextBase): """The part of the statusbar that displays the search match counter.""" @pyqtSlot(browsertab.SearchMatch) - def set_match(self, match: browsertab.SearchMatch) -> None: + def set_match(self, search_match: browsertab.SearchMatch) -> None: """Set the match counts in the statusbar. Passing SearchMatch(0, 0) hides the match counter. Args: - match: The currently active search match. + search_match: The currently active search match. """ - if match.current <= 0 and match.total <= 0: - self.setText('') - log.statusbar.debug('Clearing search match text.') - else: - self.setText(f'Match [{match}]') - log.statusbar.debug(f'Setting search match text to {match}') + text = '' if search_match.is_null() else f'Match [{search_match}]' + self.setText(text) -- cgit v1.2.3-54-g00ecf From f2ee683a0877c7351d63687f04bd01f9ddbdfd25 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 14 Jun 2022 10:12:21 +0200 Subject: Update docs --- doc/changelog.asciidoc | 10 ++++++++-- doc/help/settings.asciidoc | 11 +++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index 62835e412..36ff49e51 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -67,14 +67,20 @@ Changed currently focused element (`focused`). - The `:click-element` command now can select the first found element via `--select-first`. -- The `statusbar.widgets` setting now supports a `clock` option to show the - current time. +- New `search.wrap_messages` setting, making it possible to disable search + wrapping messages. +- New widgets for `statusbar.widgets`: + * `clock`, showing the current time + * `search_match`, showing the current match and total count when finding text + on a page Fixed ~~~~~ - When the devtools are clicked but `input.insert_mode.auto_enter` is set to `false`, insert mode now isn't entered anymore. +- The search wrapping messages are now correctly displayed in (hopefully) all + cases with QtWebEngine. [[v2.5.2]] v2.5.2 (unreleased) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 344879b4e..b16fe2a06 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -303,6 +303,7 @@ |<>|When to find text on a page case-insensitively. |<>|Find text on a page incrementally, renewing the search for each typed character. |<>|Wrap around at the top and bottom of the page when advancing through text matches using `:search-next` and `:search-prev`. +|<>|Display messages when advancing through text matches at the top and bottom of the page, e.g. `Search hit TOP`. |<>|Name of the session to save by default. |<>|Load a restored tab as soon as it takes focus. |<>|Languages to use for spell checking. @@ -4002,6 +4003,14 @@ Type: <> Default: +pass:[true]+ +[[search.wrap_messages]] +=== search.wrap_messages +Display messages when advancing through text matches at the top and bottom of the page, e.g. `Search hit TOP`. + +Type: <> + +Default: +pass:[true]+ + [[session.default_name]] === session.default_name Name of the session to save by default. @@ -4128,6 +4137,7 @@ Valid values: * +scroll+: Percentage of the current page position like `10%`. * +scroll_raw+: Raw percentage of the current page position like `10`. * +history+: Display an arrow when possible to go back/forward in history. + * +search_match+: A match count when searching, e.g. `Match [2/10]`. * +tabs+: Current active tab, e.g. `2`. * +keypress+: Display pressed keys when composing a vi command. * +progress+: Progress bar for the current page loading. @@ -4137,6 +4147,7 @@ Valid values: Default: - +pass:[keypress]+ +- +pass:[search_match]+ - +pass:[url]+ - +pass:[scroll]+ - +pass:[history]+ -- cgit v1.2.3-54-g00ecf From ede1981ccb56068767e74caaff79bcb1f4001668 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 14 Jun 2022 10:46:02 +0200 Subject: Unify duplicated prev/next search code --- qutebrowser/browser/commands.py | 48 ++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index f534b91b3..e6d2af822 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1600,15 +1600,8 @@ class CommandDispatcher: cb = functools.partial(self._search_cb, text=text) tab.search.search(text, **options, result_cb=cb) - @cmdutils.register(instance='command-dispatcher', scope='window') - @cmdutils.argument('count', value=cmdutils.Value.count) - def search_next(self, count=1): - """Continue the search to the ([count]th) next term. - - Args: - count: How many elements to ignore. - """ - tab = self._current_widget() + def _search_prev_next(self, count, tab, method): + """Continue the search to the prev/next term.""" window_text = self._tabbed_browser.search_text window_options = self._tabbed_browser.search_options @@ -1628,39 +1621,30 @@ class CommandDispatcher: wrap = config.val.search.wrap for _ in range(count - 1): - tab.search.next_result(wrap=wrap) - tab.search.next_result(callback=self._search_navigation_cb, wrap=wrap) + method(wrap=wrap) + method(callback=self._search_navigation_cb, wrap=wrap) @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('count', value=cmdutils.Value.count) - def search_prev(self, count=1): - """Continue the search to the ([count]th) previous term. + def search_next(self, count=1): + """Continue the search to the ([count]th) next term. Args: count: How many elements to ignore. """ tab = self._current_widget() - window_text = self._tabbed_browser.search_text - window_options = self._tabbed_browser.search_options - - if window_text is None: - raise cmdutils.CommandError("No search done yet.") - - tab.scroller.before_jump_requested.emit() - - if window_text is not None and window_text != tab.search.text: - tab.search.clear() - tab.search.search(window_text, **window_options) - count -= 1 + self._search_prev_next(count, tab, tab.search.next_result) - if count == 0: - return - - wrap = config.val.search.wrap + @cmdutils.register(instance='command-dispatcher', scope='window') + @cmdutils.argument('count', value=cmdutils.Value.count) + def search_prev(self, count=1): + """Continue the search to the ([count]th) previous term. - for _ in range(count - 1): - tab.search.prev_result(wrap=wrap) - tab.search.prev_result(callback=self._search_navigation_cb, wrap=wrap) + Args: + count: How many elements to ignore. + """ + tab = self._current_widget() + self._search_prev_next(count, tab, tab.search.prev_result) def _jseval_cb(self, out): """Show the data returned from JS.""" -- cgit v1.2.3-54-g00ecf