From c7c5a985062c1adc0e301ad16c0cf72b75ca501c Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Thu, 4 Aug 2016 02:43:02 +0200 Subject: [PATCH 001/160] Allow {url} and {url:pretty} as parts of arguments This makes commands like `:open web.archive.org/web/{url}` possible. This commit also adds a no_replace_variables command register argument that stops the replacement from happening, which is important for commands like `:bind` and `:spawn` that take a command as an argument. --- qutebrowser/browser/commands.py | 2 +- qutebrowser/commands/command.py | 4 +++- qutebrowser/commands/runners.py | 17 ++++++++++------- qutebrowser/config/parsers/keyconf.py | 3 ++- qutebrowser/mainwindow/statusbar/command.py | 3 +-- qutebrowser/misc/utilcmds.py | 4 ++-- 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 77811e0e4..9c031f116 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -992,7 +992,7 @@ class CommandDispatcher: self._tabbed_browser.setUpdatesEnabled(True) @cmdutils.register(instance='command-dispatcher', scope='window', - maxsplit=0) + maxsplit=0, no_replace_variables=True) def spawn(self, cmdline, userscript=False, verbose=False, detach=False): """Spawn a command in a shell. diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index aa75e2748..c06afe646 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -82,6 +82,7 @@ class Command: no_cmd_split: If true, ';;' to split sub-commands is ignored. backend: Which backend the command works with (or None if it works with both) + no_replace_variables: Whether or not to replace variables like {url} _qute_args: The saved data from @cmdutils.argument _needs_js: Whether the command needs javascript enabled _modes: The modes the command can be executed in. @@ -95,7 +96,7 @@ class Command: hide=False, modes=None, not_modes=None, needs_js=False, debug=False, ignore_args=False, deprecated=False, no_cmd_split=False, star_args_optional=False, scope='global', - backend=None): + backend=None, no_replace_variables=False): # I really don't know how to solve this in a better way, I tried. # pylint: disable=too-many-locals if modes is not None and not_modes is not None: @@ -127,6 +128,7 @@ class Command: self.handler = handler self.no_cmd_split = no_cmd_split self.backend = backend + self.no_replace_variables = no_replace_variables self.docparser = docutils.DocstringParser(handler) self.parser = argparser.ArgumentParser( diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index d1f15a415..7d2ecdd7a 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -52,16 +52,16 @@ def replace_variables(win_id, arglist): args = [] tabbed_browser = objreg.get('tabbed-browser', scope='window', window=win_id) - if '{url}' in arglist: + if any('{url}' in arg for arg in arglist): url = _current_url(tabbed_browser).toString(QUrl.FullyEncoded | QUrl.RemovePassword) - if '{url:pretty}' in arglist: + if any('{url:pretty}' in arg for arg in arglist): pretty_url = _current_url(tabbed_browser).toString(QUrl.RemovePassword) for arg in arglist: - if arg == '{url}': - args.append(url) - elif arg == '{url:pretty}': - args.append(pretty_url) + if '{url}' in arg: + args.append(arg.replace('{url}', url)) + elif '{url:pretty}' in arg: + args.append(arg.replace('{url:pretty}', pretty_url)) else: args.append(arg) return args @@ -279,7 +279,10 @@ class CommandRunner(QObject): window=self._win_id) cur_mode = mode_manager.mode - args = replace_variables(self._win_id, result.args) + if result.cmd.no_replace_variables: + args = result.args + else: + args = replace_variables(self._win_id, result.args) if count is not None: if result.count is not None: raise cmdexc.CommandMetaError("Got count via command and " diff --git a/qutebrowser/config/parsers/keyconf.py b/qutebrowser/config/parsers/keyconf.py index b2596fb62..d1c2c7ad1 100644 --- a/qutebrowser/config/parsers/keyconf.py +++ b/qutebrowser/config/parsers/keyconf.py @@ -150,7 +150,8 @@ class KeyConfigParser(QObject): data = str(self) f.write(data) - @cmdutils.register(instance='key-config', maxsplit=1, no_cmd_split=True) + @cmdutils.register(instance='key-config', maxsplit=1, no_cmd_split=True, + no_replace_variables=True) @cmdutils.argument('win_id', win_id=True) @cmdutils.argument('key', completion=usertypes.Completion.empty) @cmdutils.argument('command', completion=usertypes.Completion.command) diff --git a/qutebrowser/mainwindow/statusbar/command.py b/qutebrowser/mainwindow/statusbar/command.py index 68eb615df..781df81ad 100644 --- a/qutebrowser/mainwindow/statusbar/command.py +++ b/qutebrowser/mainwindow/statusbar/command.py @@ -23,7 +23,7 @@ from PyQt5.QtCore import pyqtSignal, pyqtSlot, Qt, QSize from PyQt5.QtWidgets import QSizePolicy from qutebrowser.keyinput import modeman, modeparsers -from qutebrowser.commands import cmdexc, cmdutils, runners +from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.misc import cmdhistory, split from qutebrowser.misc import miscwidgets as misc from qutebrowser.utils import usertypes, log, objreg @@ -109,7 +109,6 @@ class Command(misc.MinimalLineEditMixin, misc.CommandLineEdit): append: If given, the text is appended to the current text. """ args = split.simple_split(text) - args = runners.replace_variables(self._win_id, args) text = ' '.join(args) if space: diff --git a/qutebrowser/misc/utilcmds.py b/qutebrowser/misc/utilcmds.py index e1d7c6a61..05d3fa7c8 100644 --- a/qutebrowser/misc/utilcmds.py +++ b/qutebrowser/misc/utilcmds.py @@ -39,7 +39,7 @@ from PyQt5.QtCore import QUrl from PyQt5.QtWidgets import QApplication # pylint: disable=unused-import -@cmdutils.register(maxsplit=1, no_cmd_split=True) +@cmdutils.register(maxsplit=1, no_cmd_split=True, no_replace_variables=True) @cmdutils.argument('win_id', win_id=True) def later(ms: int, command, win_id): """Execute a command after some time. @@ -69,7 +69,7 @@ def later(ms: int, command, win_id): raise -@cmdutils.register(maxsplit=1, no_cmd_split=True) +@cmdutils.register(maxsplit=1, no_cmd_split=True, no_replace_variables=True) @cmdutils.argument('win_id', win_id=True) def repeat(times: int, command, win_id): """Repeat a given command. From 827de1743d328b50778ea9b52bc5401a5c44419a Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Thu, 4 Aug 2016 13:21:19 +0200 Subject: [PATCH 002/160] Document no_replace_variables, misc fixes Add no_replace_variables to the asciidoc, improve its description in the decorator, remove now unnecessary argument parsing in set-cmd-text --- doc/help/commands.asciidoc | 4 ++++ qutebrowser/commands/command.py | 2 +- qutebrowser/mainwindow/statusbar/command.py | 3 --- scripts/dev/src2asciidoc.py | 4 +++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 9835f1a3f..cbb81235d 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -111,6 +111,7 @@ Bind a key to a command. ==== note * This command does not split arguments after the last argument and handles quotes literally. * With this command, +;;+ is interpreted literally instead of splitting off a second command. +* This command does not replace variables like +\{url\}+. [[bookmark-add]] === bookmark-add @@ -412,6 +413,7 @@ Execute a command after some time. ==== note * This command does not split arguments after the last argument and handles quotes literally. * With this command, +;;+ is interpreted literally instead of splitting off a second command. +* This command does not replace variables like +\{url\}+. [[messages]] === messages @@ -579,6 +581,7 @@ Repeat a given command. ==== note * This command does not split arguments after the last argument and handles quotes literally. * With this command, +;;+ is interpreted literally instead of splitting off a second command. +* This command does not replace variables like +\{url\}+. [[report]] === report @@ -714,6 +717,7 @@ Note the `{url}` and `{url:pretty}` variables might be useful here. `{url}` gets ==== note * This command does not split arguments after the last argument and handles quotes literally. +* This command does not replace variables like +\{url\}+. [[stop]] === stop diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index c06afe646..e49c497af 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -82,7 +82,7 @@ class Command: no_cmd_split: If true, ';;' to split sub-commands is ignored. backend: Which backend the command works with (or None if it works with both) - no_replace_variables: Whether or not to replace variables like {url} + no_replace_variables: Don't replace variables like {url} _qute_args: The saved data from @cmdutils.argument _needs_js: Whether the command needs javascript enabled _modes: The modes the command can be executed in. diff --git a/qutebrowser/mainwindow/statusbar/command.py b/qutebrowser/mainwindow/statusbar/command.py index 781df81ad..62a01ef32 100644 --- a/qutebrowser/mainwindow/statusbar/command.py +++ b/qutebrowser/mainwindow/statusbar/command.py @@ -108,9 +108,6 @@ class Command(misc.MinimalLineEditMixin, misc.CommandLineEdit): space: If given, a space is added to the end. append: If given, the text is appended to the current text. """ - args = split.simple_split(text) - text = ' '.join(args) - if space: text += ' ' if append: diff --git a/scripts/dev/src2asciidoc.py b/scripts/dev/src2asciidoc.py index af1ba0936..5d7819ea0 100755 --- a/scripts/dev/src2asciidoc.py +++ b/scripts/dev/src2asciidoc.py @@ -239,7 +239,7 @@ def _get_command_doc_notes(cmd): Yield: Strings which should be added to the docs. """ - if cmd.maxsplit is not None or cmd.no_cmd_split: + if cmd.maxsplit is not None or cmd.no_cmd_split or cmd.no_replace_variables: yield "" yield "==== note" if cmd.maxsplit is not None: @@ -248,6 +248,8 @@ def _get_command_doc_notes(cmd): if cmd.no_cmd_split: yield ("* With this command, +;;+ is interpreted literally " "instead of splitting off a second command.") + if cmd.no_replace_variables: + yield "* This command does not replace variables like +\{url\}+." def _get_action_metavar(action, nargs=1): From 8b9f37cc8408feea595203c662e859e272e0d51c Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Thu, 4 Aug 2016 13:45:46 +0200 Subject: [PATCH 003/160] Use raw string for asciidoc backslashes --- scripts/dev/src2asciidoc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/dev/src2asciidoc.py b/scripts/dev/src2asciidoc.py index 5d7819ea0..b680a5ae8 100755 --- a/scripts/dev/src2asciidoc.py +++ b/scripts/dev/src2asciidoc.py @@ -249,7 +249,7 @@ def _get_command_doc_notes(cmd): yield ("* With this command, +;;+ is interpreted literally " "instead of splitting off a second command.") if cmd.no_replace_variables: - yield "* This command does not replace variables like +\{url\}+." + yield r"* This command does not replace variables like +\{url\}+." def _get_action_metavar(action, nargs=1): From 7999c493ac7a2da9c79be9493600ede0b9e41a2c Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Thu, 4 Aug 2016 15:16:35 +0200 Subject: [PATCH 004/160] Remove unnecessary import, split long line --- qutebrowser/mainwindow/statusbar/command.py | 2 +- scripts/dev/src2asciidoc.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/qutebrowser/mainwindow/statusbar/command.py b/qutebrowser/mainwindow/statusbar/command.py index 62a01ef32..df419c0dd 100644 --- a/qutebrowser/mainwindow/statusbar/command.py +++ b/qutebrowser/mainwindow/statusbar/command.py @@ -24,7 +24,7 @@ from PyQt5.QtWidgets import QSizePolicy from qutebrowser.keyinput import modeman, modeparsers from qutebrowser.commands import cmdexc, cmdutils -from qutebrowser.misc import cmdhistory, split +from qutebrowser.misc import cmdhistory from qutebrowser.misc import miscwidgets as misc from qutebrowser.utils import usertypes, log, objreg diff --git a/scripts/dev/src2asciidoc.py b/scripts/dev/src2asciidoc.py index b680a5ae8..2589d0fc5 100755 --- a/scripts/dev/src2asciidoc.py +++ b/scripts/dev/src2asciidoc.py @@ -239,7 +239,8 @@ def _get_command_doc_notes(cmd): Yield: Strings which should be added to the docs. """ - if cmd.maxsplit is not None or cmd.no_cmd_split or cmd.no_replace_variables: + if (cmd.maxsplit is not None or cmd.no_cmd_split or + cmd.no_replace_variables): yield "" yield "==== note" if cmd.maxsplit is not None: From b13153aa77ad0cde3eac5d5091ee2a5a759b232c Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Fri, 29 Jul 2016 07:23:54 -0400 Subject: [PATCH 005/160] Test CompletionFilterModel.first/last. --- tests/unit/completion/test_sortfilter.py | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/unit/completion/test_sortfilter.py b/tests/unit/completion/test_sortfilter.py index 86a132e1d..cdc05cb6c 100644 --- a/tests/unit/completion/test_sortfilter.py +++ b/tests/unit/completion/test_sortfilter.py @@ -46,3 +46,34 @@ def test_filter_accepts_row(pattern, data, expected): row_count = filter_model.rowCount(idx) assert row_count == (1 if expected else 0) + +@pytest.mark.parametrize('tree, first, last', [ + ([['Aa']], 'Aa', 'Aa'), + ([['Aa'], ['Ba']], 'Aa', 'Ba'), + ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 'Aa', 'Ca'), + ([[], ['Ba']], 'Ba', 'Ba'), + ([[], [], ['Ca']], 'Ca', 'Ca'), + ([[], [], ['Ca', 'Cb']], 'Ca', 'Cb'), + ([['Aa'], []], 'Aa', 'Aa'), + ([['Aa'], []], 'Aa', 'Aa'), + ([['Aa'], [], []], 'Aa', 'Aa'), + ([['Aa'], [], ['Ca']], 'Aa', 'Ca'), + ([[], []], None, None), +]) +def test_first_last_item(tree, first, last): + """Test that first() and last() return indexes to the first and last items. + + Args: + tree: Each list represents a completion category, with each string + being an item under that category. + first: text of the first item + last: text of the last item + """ + model = base.BaseCompletionModel() + for catdata in tree: + cat = model.new_category('') + for name in catdata: + model.new_item(cat, name, '') + filter_model = sortfilter.CompletionFilterModel(model) + assert filter_model.data(filter_model.first_item()) == first + assert filter_model.data(filter_model.last_item()) == last From f19b818b581969d2f16e301cf138b759139ebfc4 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Fri, 29 Jul 2016 07:32:43 -0400 Subject: [PATCH 006/160] Test CompletionFilterModel.set_source_model --- tests/unit/completion/test_sortfilter.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/completion/test_sortfilter.py b/tests/unit/completion/test_sortfilter.py index cdc05cb6c..0d00fcf64 100644 --- a/tests/unit/completion/test_sortfilter.py +++ b/tests/unit/completion/test_sortfilter.py @@ -77,3 +77,19 @@ def test_first_last_item(tree, first, last): filter_model = sortfilter.CompletionFilterModel(model) assert filter_model.data(filter_model.first_item()) == first assert filter_model.data(filter_model.last_item()) == last + + +def test_set_source_model(): + """Ensure setSourceModel sets source_model and clears the pattern.""" + model1 = base.BaseCompletionModel() + model2 = base.BaseCompletionModel() + filter_model = sortfilter.CompletionFilterModel(model1) + filter_model.set_pattern('foo') + # sourceModel() is cached as srcmodel, so make sure both match + assert filter_model.srcmodel is model1 + assert filter_model.sourceModel() is model1 + assert filter_model.pattern == 'foo' + filter_model.setSourceModel(model2) + assert filter_model.srcmodel is model2 + assert filter_model.sourceModel() is model2 + assert not filter_model.pattern From 8e7002db7a02b4b24a549d1e890f89e7e92c0df1 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Fri, 29 Jul 2016 07:35:29 -0400 Subject: [PATCH 007/160] Test CompletionFilterModel.count --- tests/unit/completion/test_sortfilter.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/completion/test_sortfilter.py b/tests/unit/completion/test_sortfilter.py index 0d00fcf64..50e88130d 100644 --- a/tests/unit/completion/test_sortfilter.py +++ b/tests/unit/completion/test_sortfilter.py @@ -93,3 +93,24 @@ def test_set_source_model(): assert filter_model.srcmodel is model2 assert filter_model.sourceModel() is model2 assert not filter_model.pattern + +@pytest.mark.parametrize('tree, expected', [ + ([['Aa']], 1), + ([['Aa'], ['Ba']], 2), + ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 6), + ([[], ['Ba']], 1), + ([[], [], ['Ca']], 1), + ([[], [], ['Ca', 'Cb']], 2), + ([['Aa'], []], 1), + ([['Aa'], []], 1), + ([['Aa'], [], []], 1), + ([['Aa'], [], ['Ca']], 2), +]) +def test_count(tree, expected): + model = base.BaseCompletionModel() + for catdata in tree: + cat = model.new_category('') + for name in catdata: + model.new_item(cat, name, '') + filter_model = sortfilter.CompletionFilterModel(model) + assert filter_model.count() == expected From 83b621b0e65408b01abfa2448d85038af3ce9374 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Fri, 29 Jul 2016 12:49:32 -0400 Subject: [PATCH 008/160] Test CompletionFilterModel.set_pattern. This is a more rigorous test than filterAcceptsRow as it tests behavior with multiple columns and different sort settings. In addition, it tests intelligentLessThan which is not tested in the filterAcceptsRow test (as lessThan is never called if there is only 1 item to filter). --- tests/unit/completion/test_sortfilter.py | 65 ++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/unit/completion/test_sortfilter.py b/tests/unit/completion/test_sortfilter.py index 50e88130d..e2c6ca97d 100644 --- a/tests/unit/completion/test_sortfilter.py +++ b/tests/unit/completion/test_sortfilter.py @@ -47,6 +47,7 @@ def test_filter_accepts_row(pattern, data, expected): row_count = filter_model.rowCount(idx) assert row_count == (1 if expected else 0) + @pytest.mark.parametrize('tree, first, last', [ ([['Aa']], 'Aa', 'Aa'), ([['Aa'], ['Ba']], 'Aa', 'Ba'), @@ -94,6 +95,7 @@ def test_set_source_model(): assert filter_model.sourceModel() is model2 assert not filter_model.pattern + @pytest.mark.parametrize('tree, expected', [ ([['Aa']], 1), ([['Aa'], ['Ba']], 2), @@ -114,3 +116,66 @@ def test_count(tree, expected): model.new_item(cat, name, '') filter_model = sortfilter.CompletionFilterModel(model) assert filter_model.count() == expected + + +@pytest.mark.parametrize('pattern, dumb_sort, filter_cols, before, after', [ + ('foo', None, [0], + [[('foo', '', ''), ('bar', '', '')]], + [[('foo', '', '')]]), + + ('foo', None, [0], + [[('foob', '', ''), ('fooc', '', ''), ('fooa', '', '')]], + [[('fooa', '', ''), ('foob', '', ''), ('fooc', '', '')]]), + + ('foo', None, [0], + [[('foo', '', '')], [('bar', '', '')]], + [[('foo', '', '')], []]), + + # prefer foobar as it starts with the pattern + ('foo', None, [0], + [[('barfoo', '', ''), ('foobar', '', '')]], + [[('foobar', '', ''), ('barfoo', '', '')]]), + + # however, don't rearrange categories + ('foo', None, [0], + [[('barfoo', '', '')], [('foobar', '', '')]], + [[('barfoo', '', '')], [('foobar', '', '')]]), + + ('foo', None, [1], + [[('foo', 'bar', ''), ('bar', 'foo', '')]], + [[('bar', 'foo', '')]]), + + ('foo', None, [0, 1], + [[('foo', 'bar', ''), ('bar', 'foo', ''), ('bar', 'bar', '')]], + [[('foo', 'bar', ''), ('bar', 'foo', '')]]), + + ('foo', None, [0, 1, 2], + [[('foo', '', ''), ('bar', '')]], + [[('foo', '', '')]]), + + # TODO + #('foo', Qt.DescendingOrder, + # [['foob', 'fooc', 'fooa']], + # [['fooc', 'foob', 'fooa']]), +]) +def test_set_pattern(pattern, dumb_sort, filter_cols, before, after): + """Validate the filtering and sorting results of set_pattern.""" + model = base.BaseCompletionModel() + model.DUMB_SORT = dumb_sort + model.columns_to_filter = filter_cols + for catdata in before: + cat = model.new_category('') + for data in catdata: + model.new_item(cat, *data) + filter_model = sortfilter.CompletionFilterModel(model) + filter_model.set_pattern(pattern) + actual = [] + for i in range(0, filter_model.rowCount()): + cat_idx = filter_model.index(i, 0) + entries = [] + for j in range(0, filter_model.rowCount(cat_idx)): + entries.append((filter_model.data(cat_idx.child(j, 0)), + filter_model.data(cat_idx.child(j, 1)), + filter_model.data(cat_idx.child(j, 2)))) + actual.append(entries) + assert actual == after From ed69ef86abe6781db72e3dfaa49c45a0bbc87e06 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Thu, 4 Aug 2016 15:37:19 -0400 Subject: [PATCH 009/160] Unit test CompletionFilterModel.sort. --- tests/unit/completion/test_sortfilter.py | 56 ++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/tests/unit/completion/test_sortfilter.py b/tests/unit/completion/test_sortfilter.py index e2c6ca97d..6b9488cce 100644 --- a/tests/unit/completion/test_sortfilter.py +++ b/tests/unit/completion/test_sortfilter.py @@ -21,6 +21,8 @@ import pytest +from PyQt5.QtCore import Qt + from qutebrowser.completion.models import base, sortfilter @@ -153,10 +155,18 @@ def test_count(tree, expected): [[('foo', '', ''), ('bar', '')]], [[('foo', '', '')]]), - # TODO - #('foo', Qt.DescendingOrder, - # [['foob', 'fooc', 'fooa']], - # [['fooc', 'foob', 'fooa']]), + # the fourth column is the sort role, which overrides data-based sorting + ('', None, [0], + [[('two', '', '', 2), ('one', '', '', 1), ('three', '', '', 3)]], + [[('one', '', ''), ('two', '', ''), ('three', '', '')]]), + + ('', Qt.AscendingOrder, [0], + [[('two', '', '', 2), ('one', '', '', 1), ('three', '', '', 3)]], + [[('one', '', ''), ('two', '', ''), ('three', '', '')]]), + + ('', Qt.DescendingOrder, [0], + [[('two', '', '', 2), ('one', '', '', 1), ('three', '', '', 3)]], + [[('three', '', ''), ('two', '', ''), ('one', '', '')]]), ]) def test_set_pattern(pattern, dumb_sort, filter_cols, before, after): """Validate the filtering and sorting results of set_pattern.""" @@ -179,3 +189,41 @@ def test_set_pattern(pattern, dumb_sort, filter_cols, before, after): filter_model.data(cat_idx.child(j, 2)))) actual.append(entries) assert actual == after + + +def test_sort(): + """Ensure that a sort argument passed to sort overrides DUMB_SORT. + + While test_set_pattern above covers most of the sorting logic, this + particular case is easier to test separately. + """ + source_model = base.BaseCompletionModel() + cat = source_model.new_category('test') + source_model.new_item(cat, 'B', '', '', sort = 1) + source_model.new_item(cat, 'C', '', '', sort = 2) + source_model.new_item(cat, 'A', '', '', sort = 0) + filter_model = sortfilter.CompletionFilterModel(source_model) + + filter_model.sort(0, Qt.AscendingOrder) + actual = [] + for i in range(0, filter_model.rowCount()): + cat_idx = filter_model.index(i, 0) + entries = [] + for j in range(0, filter_model.rowCount(cat_idx)): + entries.append((filter_model.data(cat_idx.child(j, 0)), + filter_model.data(cat_idx.child(j, 1)), + filter_model.data(cat_idx.child(j, 2)))) + actual.append(entries) + assert actual == [[('A', '', ''), ('B', '', ''), ('C', '', '')]] + + filter_model.sort(0, Qt.DescendingOrder) + actual = [] + for i in range(0, filter_model.rowCount()): + cat_idx = filter_model.index(i, 0) + entries = [] + for j in range(0, filter_model.rowCount(cat_idx)): + entries.append((filter_model.data(cat_idx.child(j, 0)), + filter_model.data(cat_idx.child(j, 1)), + filter_model.data(cat_idx.child(j, 2)))) + actual.append(entries) + assert actual == [[('C', '', ''), ('B', '', ''), ('A', '', '')]] From 3e1409b1f5f33b66ca871dcc8b9e33d72533b980 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Thu, 4 Aug 2016 18:56:56 -0400 Subject: [PATCH 010/160] Dedupe code in test_sortfilter. Add helper functions to create and parse completion models, as this was repeated in many tests. --- tests/unit/completion/test_sortfilter.py | 132 +++++++++++------------ 1 file changed, 64 insertions(+), 68 deletions(-) diff --git a/tests/unit/completion/test_sortfilter.py b/tests/unit/completion/test_sortfilter.py index 6b9488cce..c63d08ed5 100644 --- a/tests/unit/completion/test_sortfilter.py +++ b/tests/unit/completion/test_sortfilter.py @@ -26,6 +26,39 @@ from PyQt5.QtCore import Qt from qutebrowser.completion.models import base, sortfilter +def _create_model(data): + """Create a completion model populated with the given data. + + data: A list of lists, where each sub-list represents a category, each + tuple in the sub-list represents an item, and each value in the + tuple represents the item data for that column + """ + model = base.BaseCompletionModel() + for catdata in data: + cat = model.new_category('') + for itemdata in catdata: + model.new_item(cat, *itemdata) + return model + + +def _extract_model_data(model): + """Express a model's data as a list for easier comparison. + + Return: A list of lists, where each sub-list represents a category, each + tuple in the sub-list represents an item, and each value in the + tuple represents the item data for that column + """ + data = [] + for i in range(0, model.rowCount()): + cat_idx = model.index(i, 0) + row = [] + for j in range(0, model.rowCount(cat_idx)): + row.append((model.data(cat_idx.child(j, 0)), + model.data(cat_idx.child(j, 1)), + model.data(cat_idx.child(j, 2)))) + data.append(row) + return data + @pytest.mark.parametrize('pattern, data, expected', [ ('foo', 'barfoobar', True), ('foo', 'barFOObar', True), @@ -51,16 +84,17 @@ def test_filter_accepts_row(pattern, data, expected): @pytest.mark.parametrize('tree, first, last', [ - ([['Aa']], 'Aa', 'Aa'), - ([['Aa'], ['Ba']], 'Aa', 'Ba'), - ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 'Aa', 'Ca'), - ([[], ['Ba']], 'Ba', 'Ba'), - ([[], [], ['Ca']], 'Ca', 'Ca'), - ([[], [], ['Ca', 'Cb']], 'Ca', 'Cb'), - ([['Aa'], []], 'Aa', 'Aa'), - ([['Aa'], []], 'Aa', 'Aa'), - ([['Aa'], [], []], 'Aa', 'Aa'), - ([['Aa'], [], ['Ca']], 'Aa', 'Ca'), + ([[('Aa',)]], 'Aa', 'Aa'), + ([[('Aa',)], [('Ba',)]], 'Aa', 'Ba'), + ([[('Aa',), ('Ab',), ('Ac',)], [('Ba',), ('Bb',)], [('Ca',)]], + 'Aa', 'Ca'), + ([[], [('Ba',)]], 'Ba', 'Ba'), + ([[], [], [('Ca',)]], 'Ca', 'Ca'), + ([[], [], [('Ca',), ('Cb',)]], 'Ca', 'Cb'), + ([[('Aa',)], []], 'Aa', 'Aa'), + ([[('Aa',)], []], 'Aa', 'Aa'), + ([[('Aa',)], [], []], 'Aa', 'Aa'), + ([[('Aa',)], [], [('Ca',)]], 'Aa', 'Ca'), ([[], []], None, None), ]) def test_first_last_item(tree, first, last): @@ -72,11 +106,7 @@ def test_first_last_item(tree, first, last): first: text of the first item last: text of the last item """ - model = base.BaseCompletionModel() - for catdata in tree: - cat = model.new_category('') - for name in catdata: - model.new_item(cat, name, '') + model = _create_model(tree) filter_model = sortfilter.CompletionFilterModel(model) assert filter_model.data(filter_model.first_item()) == first assert filter_model.data(filter_model.last_item()) == last @@ -99,23 +129,19 @@ def test_set_source_model(): @pytest.mark.parametrize('tree, expected', [ - ([['Aa']], 1), - ([['Aa'], ['Ba']], 2), - ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 6), - ([[], ['Ba']], 1), - ([[], [], ['Ca']], 1), - ([[], [], ['Ca', 'Cb']], 2), - ([['Aa'], []], 1), - ([['Aa'], []], 1), - ([['Aa'], [], []], 1), - ([['Aa'], [], ['Ca']], 2), + ([[('Aa',)]], 1), + ([[('Aa',)], [('Ba',)]], 2), + ([[('Aa',), ('Ab',), ('Ac',)], [('Ba',), ('Bb',)], [('Ca',)]], 6), + ([[], [('Ba',)]], 1), + ([[], [], [('Ca',)]], 1), + ([[], [], [('Ca',), ('Cb',)]], 2), + ([[('Aa',)], []], 1), + ([[('Aa',)], []], 1), + ([[('Aa',)], [], []], 1), + ([[('Aa',)], [], [('Ca',)]], 2), ]) def test_count(tree, expected): - model = base.BaseCompletionModel() - for catdata in tree: - cat = model.new_category('') - for name in catdata: - model.new_item(cat, name, '') + model = _create_model(tree) filter_model = sortfilter.CompletionFilterModel(model) assert filter_model.count() == expected @@ -170,24 +196,12 @@ def test_count(tree, expected): ]) def test_set_pattern(pattern, dumb_sort, filter_cols, before, after): """Validate the filtering and sorting results of set_pattern.""" - model = base.BaseCompletionModel() + model = _create_model(before) model.DUMB_SORT = dumb_sort model.columns_to_filter = filter_cols - for catdata in before: - cat = model.new_category('') - for data in catdata: - model.new_item(cat, *data) filter_model = sortfilter.CompletionFilterModel(model) filter_model.set_pattern(pattern) - actual = [] - for i in range(0, filter_model.rowCount()): - cat_idx = filter_model.index(i, 0) - entries = [] - for j in range(0, filter_model.rowCount(cat_idx)): - entries.append((filter_model.data(cat_idx.child(j, 0)), - filter_model.data(cat_idx.child(j, 1)), - filter_model.data(cat_idx.child(j, 2)))) - actual.append(entries) + actual = _extract_model_data(filter_model) assert actual == after @@ -197,33 +211,15 @@ def test_sort(): While test_set_pattern above covers most of the sorting logic, this particular case is easier to test separately. """ - source_model = base.BaseCompletionModel() - cat = source_model.new_category('test') - source_model.new_item(cat, 'B', '', '', sort = 1) - source_model.new_item(cat, 'C', '', '', sort = 2) - source_model.new_item(cat, 'A', '', '', sort = 0) - filter_model = sortfilter.CompletionFilterModel(source_model) + model = _create_model([[('B', '', '', 1), + ('C', '', '', 2), + ('A', '', '', 0)]]) + filter_model = sortfilter.CompletionFilterModel(model) filter_model.sort(0, Qt.AscendingOrder) - actual = [] - for i in range(0, filter_model.rowCount()): - cat_idx = filter_model.index(i, 0) - entries = [] - for j in range(0, filter_model.rowCount(cat_idx)): - entries.append((filter_model.data(cat_idx.child(j, 0)), - filter_model.data(cat_idx.child(j, 1)), - filter_model.data(cat_idx.child(j, 2)))) - actual.append(entries) + actual = _extract_model_data(filter_model) assert actual == [[('A', '', ''), ('B', '', ''), ('C', '', '')]] filter_model.sort(0, Qt.DescendingOrder) - actual = [] - for i in range(0, filter_model.rowCount()): - cat_idx = filter_model.index(i, 0) - entries = [] - for j in range(0, filter_model.rowCount(cat_idx)): - entries.append((filter_model.data(cat_idx.child(j, 0)), - filter_model.data(cat_idx.child(j, 1)), - filter_model.data(cat_idx.child(j, 2)))) - actual.append(entries) + actual = _extract_model_data(filter_model) assert actual == [[('C', '', ''), ('B', '', ''), ('A', '', '')]] From 8a527b5faf6c3e80662461806c8ba4859e7b3d7c Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 5 Aug 2016 16:52:47 +0200 Subject: [PATCH 011/160] Make :spawn parse normally without maxsplit --- doc/help/commands.asciidoc | 6 +----- qutebrowser/browser/commands.py | 14 +++----------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index cbb81235d..92d4cf02b 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -696,7 +696,7 @@ You can use the `{url}` and `{url:pretty}` variables here which will get replace [[spawn]] === spawn -Syntax: +:spawn [*--userscript*] [*--verbose*] [*--detach*] 'cmdline'+ +Syntax: +:spawn [*--userscript*] [*--verbose*] [*--detach*] 'cmdline' ['cmdline' ...]+ Spawn a command in a shell. @@ -715,10 +715,6 @@ Note the `{url}` and `{url:pretty}` variables might be useful here. `{url}` gets * +*-v*+, +*--verbose*+: Show notifications when the command started/exited. * +*-d*+, +*--detach*+: Whether the command should be detached from qutebrowser. -==== note -* This command does not split arguments after the last argument and handles quotes literally. -* This command does not replace variables like +\{url\}+. - [[stop]] === stop Stop loading in the current/[count]th tab. diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 9c031f116..82062357f 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -991,9 +991,8 @@ class CommandDispatcher: finally: self._tabbed_browser.setUpdatesEnabled(True) - @cmdutils.register(instance='command-dispatcher', scope='window', - maxsplit=0, no_replace_variables=True) - def spawn(self, cmdline, userscript=False, verbose=False, detach=False): + @cmdutils.register(instance='command-dispatcher', scope='window') + def spawn(self, *cmdline, userscript=False, verbose=False, detach=False): """Spawn a command in a shell. Note the `{url}` and `{url:pretty}` variables might be useful here. @@ -1012,14 +1011,7 @@ class CommandDispatcher: detach: Whether the command should be detached from qutebrowser. cmdline: The commandline to execute. """ - try: - cmd, *args = shlex.split(cmdline) - except ValueError as e: - raise cmdexc.CommandError("Error while splitting command: " - "{}".format(e)) - - args = runners.replace_variables(self._win_id, args) - + cmd, *args = cmdline log.procs.debug("Executing {} with args {}, userscript={}".format( cmd, args, userscript)) if userscript: From 1e97247c63e321e28aaf947f07f8c8eb95584f7e Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 5 Aug 2016 18:04:14 +0200 Subject: [PATCH 012/160] Revert spawn's splitting, blacklist from doc Blacklist spawn from getting the no_replace_variables doc note. --- doc/help/commands.asciidoc | 5 ++++- qutebrowser/browser/commands.py | 14 +++++++++++--- scripts/dev/src2asciidoc.py | 4 ++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 92d4cf02b..c0bb43f6c 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -696,7 +696,7 @@ You can use the `{url}` and `{url:pretty}` variables here which will get replace [[spawn]] === spawn -Syntax: +:spawn [*--userscript*] [*--verbose*] [*--detach*] 'cmdline' ['cmdline' ...]+ +Syntax: +:spawn [*--userscript*] [*--verbose*] [*--detach*] 'cmdline'+ Spawn a command in a shell. @@ -715,6 +715,9 @@ Note the `{url}` and `{url:pretty}` variables might be useful here. `{url}` gets * +*-v*+, +*--verbose*+: Show notifications when the command started/exited. * +*-d*+, +*--detach*+: Whether the command should be detached from qutebrowser. +==== note +* This command does not split arguments after the last argument and handles quotes literally. + [[stop]] === stop Stop loading in the current/[count]th tab. diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 82062357f..9c031f116 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -991,8 +991,9 @@ class CommandDispatcher: finally: self._tabbed_browser.setUpdatesEnabled(True) - @cmdutils.register(instance='command-dispatcher', scope='window') - def spawn(self, *cmdline, userscript=False, verbose=False, detach=False): + @cmdutils.register(instance='command-dispatcher', scope='window', + maxsplit=0, no_replace_variables=True) + def spawn(self, cmdline, userscript=False, verbose=False, detach=False): """Spawn a command in a shell. Note the `{url}` and `{url:pretty}` variables might be useful here. @@ -1011,7 +1012,14 @@ class CommandDispatcher: detach: Whether the command should be detached from qutebrowser. cmdline: The commandline to execute. """ - cmd, *args = cmdline + try: + cmd, *args = shlex.split(cmdline) + except ValueError as e: + raise cmdexc.CommandError("Error while splitting command: " + "{}".format(e)) + + args = runners.replace_variables(self._win_id, args) + log.procs.debug("Executing {} with args {}, userscript={}".format( cmd, args, userscript)) if userscript: diff --git a/scripts/dev/src2asciidoc.py b/scripts/dev/src2asciidoc.py index 2589d0fc5..84b774a03 100755 --- a/scripts/dev/src2asciidoc.py +++ b/scripts/dev/src2asciidoc.py @@ -240,7 +240,7 @@ def _get_command_doc_notes(cmd): Strings which should be added to the docs. """ if (cmd.maxsplit is not None or cmd.no_cmd_split or - cmd.no_replace_variables): + cmd.no_replace_variables and cmd.name != "spawn"): yield "" yield "==== note" if cmd.maxsplit is not None: @@ -249,7 +249,7 @@ def _get_command_doc_notes(cmd): if cmd.no_cmd_split: yield ("* With this command, +;;+ is interpreted literally " "instead of splitting off a second command.") - if cmd.no_replace_variables: + if cmd.no_replace_variables and cmd.name != "spawn": yield r"* This command does not replace variables like +\{url\}+." From 54ba27cf7790d5863db8dd77a9e0c70c63035575 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Fri, 5 Aug 2016 21:55:51 -0400 Subject: [PATCH 013/160] Merge :yank-selected into :yank (fixes #820) Changes :yank's flag arguments to a positional "what" argument specifying the object to be yanked. Including "selection" as a possibility allows for the replacement of :yank-selected with :yank selection. --- qutebrowser/browser/commands.py | 65 ++++++++----------- qutebrowser/config/configdata.py | 18 +++--- tests/end2end/features/caret.feature | 80 ++++++++++++------------ tests/end2end/features/search.feature | 36 +++++------ tests/end2end/features/yankpaste.feature | 6 +- tests/end2end/test_insert_mode.py | 2 +- 6 files changed, 98 insertions(+), 109 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 3728ebac4..ff1867389 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -620,30 +620,35 @@ class CommandDispatcher: "representation.") @cmdutils.register(instance='command-dispatcher', scope='window') - def yank(self, title=False, sel=False, domain=False, pretty=False): - """Yank the current URL/title to the clipboard or primary selection. + @cmdutils.argument('what', choices=['selection', 'url', 'pretty-url', + 'title', 'domain']) + def yank(self, what='url', sel=False, keep=False): + """Yank something to the clipboard or primary selection. Args: + what: What to yank. sel: Use the primary selection instead of the clipboard. - title: Yank the title instead of the URL. - domain: Yank only the scheme, domain, and port number. - pretty: Yank the URL in pretty decoded form. + keep: If given, stay in visual mode after yanking. """ - if title: + if what == 'title': s = self._tabbed_browser.page_title(self._current_index()) - what = 'title' - elif domain: + elif what == 'domain': port = self._current_url().port() s = '{}://{}{}'.format(self._current_url().scheme(), self._current_url().host(), ':' + str(port) if port > -1 else '') - what = 'domain' - else: + elif what in ['url', 'pretty-url']: flags = QUrl.RemovePassword - if not pretty: + if what == 'url': # Not pretty flags |= QUrl.FullyEncoded s = self._current_url().toString(flags) - what = 'URL' + what = 'URL' # For printing + elif what == 'selection': + caret = self._current_widget().caret + s = caret.selection() + if not caret.has_selection() or len(s) == 0: + message.info(self._win_id, "Nothing to yank") + return if sel and utils.supports_selection(): target = "primary selection" @@ -652,8 +657,15 @@ class CommandDispatcher: target = "clipboard" utils.set_clipboard(s, selection=sel) - message.info(self._win_id, "Yanked {} to {}: {}".format( - what, target, s)) + if what != 'selection': + message.info(self._win_id, "Yanked {} to {}: {}".format( + what, target, s)) + else: + message.info(self._win_id, "{} {} yanked to {}".format( + len(s), "char" if len(s) == 1 else "chars", target)) + if not keep: + modeman.maybe_leave(self._win_id, KeyMode.caret, + "yank selected") @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('count', count=True) @@ -1729,31 +1741,6 @@ class CommandDispatcher: """Move the cursor or selection to the end of the document.""" self._current_widget().caret.move_to_end_of_document() - @cmdutils.register(instance='command-dispatcher', scope='window') - def yank_selected(self, sel=False, keep=False): - """Yank the selected text to the clipboard or primary selection. - - Args: - sel: Use the primary selection instead of the clipboard. - keep: If given, stay in visual mode after yanking. - """ - caret = self._current_widget().caret - s = caret.selection() - if not caret.has_selection() or len(s) == 0: - message.info(self._win_id, "Nothing to yank") - return - - if sel and utils.supports_selection(): - target = "primary selection" - else: - sel = False - target = "clipboard" - utils.set_clipboard(s, sel) - message.info(self._win_id, "{} {} yanked to {}".format( - len(s), "char" if len(s) == 1 else "chars", target)) - if not keep: - modeman.maybe_leave(self._win_id, KeyMode.caret, "yank selected") - @cmdutils.register(instance='command-dispatcher', hide=True, modes=[KeyMode.caret], scope='window') def toggle_selection(self): diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 8cbfe3938..5c087752e 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1506,12 +1506,12 @@ KEY_DATA = collections.OrderedDict([ ('enter-mode jump_mark', ["'"]), ('yank', ['yy']), ('yank -s', ['yY']), - ('yank -t', ['yt']), - ('yank -ts', ['yT']), - ('yank -d', ['yd']), - ('yank -ds', ['yD']), - ('yank -p', ['yp']), - ('yank -ps', ['yP']), + ('yank title', ['yt']), + ('yank title -s', ['yT']), + ('yank domain', ['yd']), + ('yank domain -s', ['yD']), + ('yank pretty-url', ['yp']), + ('yank pretty-url -s', ['yP']), ('paste', ['pp']), ('paste -s', ['pP']), ('paste -t', ['Pp']), @@ -1638,8 +1638,8 @@ KEY_DATA = collections.OrderedDict([ ('move-to-end-of-line', ['$']), ('move-to-start-of-document', ['gg']), ('move-to-end-of-document', ['G']), - ('yank-selected -p', ['Y']), - ('yank-selected', ['y'] + RETURN_KEYS), + ('yank selection -p', ['Y']), + ('yank selection', ['y'] + RETURN_KEYS), ('scroll left', ['H']), ('scroll down', ['J']), ('scroll up', ['K']), @@ -1677,4 +1677,6 @@ CHANGED_KEY_COMMANDS = [ (re.compile(r'^download-remove --all$'), r'download-clear'), (re.compile(r'^hint links fill "([^"]*)"$'), r'hint links fill \1'), + + (re.compile(r'^yank-selected'), r'yank selection'), ] diff --git a/tests/end2end/features/caret.feature b/tests/end2end/features/caret.feature index 45c6cef9f..fdfb7f6d8 100644 --- a/tests/end2end/features/caret.feature +++ b/tests/end2end/features/caret.feature @@ -10,7 +10,7 @@ Feature: Caret mode Scenario: Selecting the entire document When I run :toggle-selection And I run :move-to-end-of-document - And I run :yank-selected + And I run :yank selection Then the clipboard should contain: one two three eins zwei drei @@ -23,14 +23,14 @@ Feature: Caret mode And I run :move-to-start-of-document And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one" Scenario: Moving to end and to start of document (with selection) When I run :move-to-end-of-document And I run :toggle-selection And I run :move-to-start-of-document - And I run :yank-selected + And I run :yank selection Then the clipboard should contain: one two three eins zwei drei @@ -43,7 +43,7 @@ Feature: Caret mode Scenario: Selecting a block When I run :toggle-selection And I run :move-to-end-of-next-block - And I run :yank-selected + And I run :yank selection Then the clipboard should contain: one two three eins zwei drei @@ -53,7 +53,7 @@ Feature: Caret mode And I run :toggle-selection And I run :move-to-end-of-prev-block And I run :move-to-prev-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain: drei @@ -64,14 +64,14 @@ Feature: Caret mode And I run :move-to-end-of-prev-block And I run :toggle-selection And I run :move-to-prev-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "drei" Scenario: Moving back to the start of previous block (with selection) When I run :move-to-end-of-next-block with count 2 And I run :toggle-selection And I run :move-to-start-of-prev-block - And I run :yank-selected + And I run :yank selection Then the clipboard should contain: eins zwei drei @@ -82,20 +82,20 @@ Feature: Caret mode And I run :move-to-start-of-prev-block And I run :toggle-selection And I run :move-to-next-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "eins " Scenario: Moving to the start of next block (with selection) When I run :toggle-selection And I run :move-to-start-of-next-block - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one two three\n" Scenario: Moving to the start of next block When I run :move-to-start-of-next-block And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "eins" # line @@ -103,20 +103,20 @@ Feature: Caret mode Scenario: Selecting a line When I run :toggle-selection And I run :move-to-end-of-line - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one two three" Scenario: Moving and selecting a line When I run :move-to-next-line And I run :toggle-selection And I run :move-to-end-of-line - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "eins zwei drei" Scenario: Selecting next line When I run :toggle-selection And I run :move-to-next-line - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one two three\n" Scenario: Moving to end and to start of line @@ -124,21 +124,21 @@ Feature: Caret mode And I run :move-to-start-of-line And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one" Scenario: Selecting a line (backwards) When I run :move-to-end-of-line And I run :toggle-selection When I run :move-to-start-of-line - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one two three" Scenario: Selecting previous line When I run :move-to-next-line And I run :toggle-selection When I run :move-to-prev-line - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one two three\n" Scenario: Moving to previous line @@ -146,7 +146,7 @@ Feature: Caret mode When I run :move-to-prev-line And I run :toggle-selection When I run :move-to-next-line - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one two three\n" # word @@ -154,35 +154,35 @@ Feature: Caret mode Scenario: Selecting a word When I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one" Scenario: Moving to end and selecting a word When I run :move-to-end-of-word And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain " two" Scenario: Moving to next word and selecting a word When I run :move-to-next-word And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "two" Scenario: Moving to next word and selecting until next word When I run :move-to-next-word And I run :toggle-selection And I run :move-to-next-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "two " Scenario: Moving to previous word and selecting a word When I run :move-to-end-of-word And I run :toggle-selection And I run :move-to-prev-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one" Scenario: Moving to previous word @@ -190,7 +190,7 @@ Feature: Caret mode And I run :move-to-prev-word And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "one" # char @@ -198,21 +198,21 @@ Feature: Caret mode Scenario: Selecting a char When I run :toggle-selection And I run :move-to-next-char - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "o" Scenario: Moving and selecting a char When I run :move-to-next-char And I run :toggle-selection And I run :move-to-next-char - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "n" Scenario: Selecting previous char When I run :move-to-end-of-word And I run :toggle-selection And I run :move-to-prev-char - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "e" Scenario: Moving to previous char @@ -220,41 +220,41 @@ Feature: Caret mode And I run :move-to-prev-char And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "e" - # :yank-selected + # :yank selection - Scenario: :yank-selected without selection - When I run :yank-selected + Scenario: :yank selection without selection + When I run :yank selection Then the message "Nothing to yank" should be shown. - Scenario: :yank-selected message + Scenario: :yank selection message When I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected + And I run :yank selection Then the message "3 chars yanked to clipboard" should be shown. - Scenario: :yank-selected message with one char + Scenario: :yank selection message with one char When I run :toggle-selection And I run :move-to-next-char - And I run :yank-selected + And I run :yank selection Then the message "1 char yanked to clipboard" should be shown. - Scenario: :yank-selected with primary selection + Scenario: :yank selection with primary selection When selection is supported And I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected --sel + And I run :yank selection --sel Then the message "3 chars yanked to primary selection" should be shown. And the primary selection should contain "one" - Scenario: :yank-selected with --keep + Scenario: :yank selection with --keep When I run :toggle-selection And I run :move-to-end-of-word - And I run :yank-selected --keep + And I run :yank selection --keep And I run :move-to-end-of-word - And I run :yank-selected --keep + And I run :yank selection --keep Then the message "3 chars yanked to clipboard" should be shown. And the message "7 chars yanked to clipboard" should be shown. And the clipboard should contain "one two" @@ -265,7 +265,7 @@ Feature: Caret mode When I run :toggle-selection And I run :move-to-end-of-word And I run :drop-selection - And I run :yank-selected + And I run :yank selection Then the message "Nothing to yank" should be shown. # :follow-selected diff --git a/tests/end2end/features/search.feature b/tests/end2end/features/search.feature index fe54cc14a..87e51fc2c 100644 --- a/tests/end2end/features/search.feature +++ b/tests/end2end/features/search.feature @@ -9,19 +9,19 @@ Feature: Searching on a page Scenario: Searching text When I run :search foo - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "foo" Scenario: Searching twice When I run :search foo And I run :search bar - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Bar" Scenario: Searching with --reverse When I set general -> ignore-case to true And I run :search -r foo - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Foo" Scenario: Searching without matches @@ -32,13 +32,13 @@ Feature: Searching on a page Scenario: Searching with / and spaces at the end (issue 874) When I run :set-cmd-text -s /space And I run :command-accept - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "space " Scenario: Searching with / and slash in search term (issue 507) When I run :set-cmd-text -s //slash And I run :command-accept - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "/slash" # This doesn't work because this is QtWebKit behavior. @@ -52,25 +52,25 @@ Feature: Searching on a page Scenario: Searching text with ignore-case = true When I set general -> ignore-case to true And I run :search bar - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Bar" Scenario: Searching text with ignore-case = false When I set general -> ignore-case to false And I run :search bar - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "bar" Scenario: Searching text with ignore-case = smart (lower-case) When I set general -> ignore-case to smart And I run :search bar - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Bar" Scenario: Searching text with ignore-case = smart (upper-case) When I set general -> ignore-case to smart And I run :search Foo - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Foo" # even though foo was first ## :search-next @@ -79,21 +79,21 @@ Feature: Searching on a page When I set general -> ignore-case to true And I run :search foo And I run :search-next - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Foo" Scenario: Jumping to next match with count When I set general -> ignore-case to true And I run :search baz And I run :search-next with count 2 - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "BAZ" Scenario: Jumping to next match with --reverse When I set general -> ignore-case to true And I run :search --reverse foo And I run :search-next - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "foo" Scenario: Jumping to next match without search @@ -107,7 +107,7 @@ Feature: Searching on a page And I run :search foo And I run :tab-prev And I run :search-next - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "foo" ## :search-prev @@ -117,7 +117,7 @@ Feature: Searching on a page And I run :search foo And I run :search-next And I run :search-prev - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "foo" Scenario: Jumping to previous match with count @@ -126,7 +126,7 @@ Feature: Searching on a page And I run :search-next And I run :search-next And I run :search-prev with count 2 - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "baz" Scenario: Jumping to previous match with --reverse @@ -134,7 +134,7 @@ Feature: Searching on a page And I run :search --reverse foo And I run :search-next And I run :search-prev - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Foo" Scenario: Jumping to previous match without search @@ -149,14 +149,14 @@ Feature: Searching on a page When I run :search foo And I run :search-next And I run :search-next - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "foo" Scenario: Wrapping around page with --reverse When I run :search --reverse foo And I run :search-next And I run :search-next - And I run :yank-selected + And I run :yank selection Then the clipboard should contain "Foo" # TODO: wrapping message with scrolling diff --git a/tests/end2end/features/yankpaste.feature b/tests/end2end/features/yankpaste.feature index db39d2380..a8aae1b90 100644 --- a/tests/end2end/features/yankpaste.feature +++ b/tests/end2end/features/yankpaste.feature @@ -23,13 +23,13 @@ Feature: Yanking and pasting. Scenario: Yanking title to clipboard When I open data/title.html And I wait for regex "Changing title for idx \d to 'Test title'" in the log - And I run :yank --title + And I run :yank title Then the message "Yanked title to clipboard: Test title" should be shown And the clipboard should contain "Test title" Scenario: Yanking domain to clipboard When I open data/title.html - And I run :yank --domain + And I run :yank domain Then the message "Yanked domain to clipboard: http://localhost:(port)" should be shown And the clipboard should contain "http://localhost:(port)" @@ -41,7 +41,7 @@ Feature: Yanking and pasting. Scenario: Yanking pretty decoded URL When I open data/title with spaces.html - And I run :yank --pretty + And I run :yank pretty-url Then the message "Yanked URL to clipboard: http://localhost:(port)/data/title with spaces.html" should be shown And the clipboard should contain "http://localhost:(port)/data/title with spaces.html" diff --git a/tests/end2end/test_insert_mode.py b/tests/end2end/test_insert_mode.py index 23a66b110..c8a7f32de 100644 --- a/tests/end2end/test_insert_mode.py +++ b/tests/end2end/test_insert_mode.py @@ -54,7 +54,7 @@ def test_insert_mode(file_name, source, input_text, auto_insert, quteproc): quteproc.send_cmd(':enter-mode caret') quteproc.send_cmd(':toggle-selection') quteproc.send_cmd(':move-to-prev-word') - quteproc.send_cmd(':yank-selected') + quteproc.send_cmd(':yank selection') expected_message = '{} chars yanked to clipboard'.format(len(input_text)) quteproc.mark_expected(category='message', From c792ffda67c34fd3c5585dffcb8b68b547c1962e Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Tue, 7 Jun 2016 20:20:56 -0400 Subject: [PATCH 014/160] Remove Command.completion. Command completion types are now identified by ArgInfo, so just use that directly and cut out the middle-man. This shouldn't change any completion behavior. Adds a test for get_pos_arg_info to test_cmdutils. Modifies test_completer to test the use of get_pos_arg_info. Instead of using FakeCommand, real Command objects are used, to validate that the Completer works with the real Command interface. This also cleans out some test cases that were testing things already covered by other cases. --- qutebrowser/commands/command.py | 14 +++--- qutebrowser/completion/completer.py | 17 +++---- tests/unit/commands/test_cmdutils.py | 15 +++++++ tests/unit/completion/test_completer.py | 60 +++++++++++++++++-------- 4 files changed, 67 insertions(+), 39 deletions(-) diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index aa75e2748..2858a9f02 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -75,7 +75,6 @@ class Command: deprecated: False, or a string to describe why a command is deprecated. desc: The description of the command. handler: The handler function to call. - completion: Completions to use for arguments, as a list of strings. debug: Whether this is a debugging command (only shown with --debug). parser: The ArgumentParser to use to parse this command. flags_with_args: A list of flags which take an argument. @@ -148,13 +147,7 @@ class Command: self._qute_args = getattr(self.handler, 'qute_args', {}) self.handler.qute_args = None - args = self._inspect_func() - - self.completion = [] - for arg in args: - arg_completion = self.get_arg_info(arg).completion - if arg_completion is not None: - self.completion.append(arg_completion) + self._inspect_func() def _check_prerequisites(self, win_id): """Check if the command is permitted to run currently. @@ -208,6 +201,11 @@ class Command: """Get an ArgInfo tuple for the given inspect.Parameter.""" return self._qute_args.get(param.name, ArgInfo()) + def get_pos_arg_info(self, pos): + """Get an ArgInfo tuple for the given positional parameter.""" + name = self.pos_args[pos][0] + return self._qute_args.get(name, ArgInfo()) + def _inspect_special_param(self, param): """Check if the given parameter is a special one. diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 71202d131..569e105be 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -204,25 +204,18 @@ class Completer(QObject): return sortfilter.CompletionFilterModel(source=model, parent=self) # delegate completion to command try: - completions = cmdutils.cmd_dict[parts[0]].completion + cmd = cmdutils.cmd_dict[parts[0]] except KeyError: # entering an unknown command return None - if completions is None: - # command without any available completions - return None - dbg_completions = [c.name for c in completions] try: idx = cursor_part - 1 - completion = completions[idx] + completion = cmd.get_pos_arg_info(idx).completion except IndexError: - # More arguments than completions - log.completion.debug("completions: {}".format( - ', '.join(dbg_completions))) + # user provided more positional arguments than the command takes + return None + if completion is None: return None - dbg_completions[idx] = '*' + dbg_completions[idx] + '*' - log.completion.debug("completions: {}".format( - ', '.join(dbg_completions))) model = self._get_completion_model(completion, parts, cursor_part) return model diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py index b01411c3d..1ecdbaf3a 100644 --- a/tests/unit/commands/test_cmdutils.py +++ b/tests/unit/commands/test_cmdutils.py @@ -291,6 +291,21 @@ class TestRegister: else: assert cmd._get_call_args(win_id=0) == ([expected], {}) + def test_pos_arg_info(self): + @cmdutils.register() + @cmdutils.argument('foo', choices=('a', 'b')) + @cmdutils.argument('bar', choices=('x', 'y')) + @cmdutils.argument('opt') + def fun(foo, bar, opt=False): + """Blah.""" + pass + + cmd = cmdutils.cmd_dict['fun'] + assert cmd.get_pos_arg_info(0) == command.ArgInfo(choices=('a', 'b')) + assert cmd.get_pos_arg_info(1) == command.ArgInfo(choices=('x', 'y')) + with pytest.raises(IndexError): + cmd.get_pos_arg_info(2) + class TestArgument: diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 29196ea3b..8e9273a90 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -27,6 +27,7 @@ from PyQt5.QtGui import QStandardItemModel from qutebrowser.completion import completer from qutebrowser.utils import usertypes +from qutebrowser.commands import command, cmdutils class FakeCompletionModel(QStandardItemModel): @@ -91,24 +92,49 @@ def instances(monkeypatch): @pytest.fixture(autouse=True) def cmdutils_patch(monkeypatch, stubs): """Patch the cmdutils module to provide fake commands.""" + @cmdutils.argument('section_', completion=usertypes.Completion.section) + @cmdutils.argument('option', completion=usertypes.Completion.option) + @cmdutils.argument('value', completion=usertypes.Completion.value) + def set_command(section_=None, option=None, value=None): + """docstring!""" + pass + + @cmdutils.argument('topic', completion=usertypes.Completion.helptopic) + def show_help(tab=False, bg=False, window=False, topic=None): + """docstring!""" + pass + + @cmdutils.argument('url', completion=usertypes.Completion.url) + @cmdutils.argument('count', count=True) + def openurl(url=None, implicit=False, bg=False, tab=False, window=False, + count=None): + """docstring!""" + pass + + @cmdutils.argument('win_id', win_id=True) + @cmdutils.argument('key', completion=usertypes.Completion.empty) + @cmdutils.argument('command', completion=usertypes.Completion.command) + def bind(key, win_id, command=None, *, mode='normal', force=False): + """docstring!""" + # pylint: disable=unused-variable + pass + + def tab_detach(): + """docstring!""" + pass + cmds = { - 'set': [usertypes.Completion.section, usertypes.Completion.option, - usertypes.Completion.value], - 'help': [usertypes.Completion.helptopic], - 'quickmark-load': [usertypes.Completion.quickmark_by_name], - 'bookmark-load': [usertypes.Completion.bookmark_by_url], - 'open': [usertypes.Completion.url], - 'buffer': [usertypes.Completion.tab], - 'session-load': [usertypes.Completion.sessions], - 'bind': [usertypes.Completion.empty, usertypes.Completion.command], - 'tab-detach': None, + 'set': set_command, + 'help': show_help, + 'open': openurl, + 'bind': bind, + 'tab-detach': tab_detach, } cmd_utils = stubs.FakeCmdUtils({ - name: stubs.FakeCommand(completion=compl) - for name, compl in cmds.items() + name: command.Command(name=name, handler=fn) + for name, fn in cmds.items() }) - monkeypatch.setattr('qutebrowser.completion.completer.cmdutils', - cmd_utils) + monkeypatch.setattr('qutebrowser.completion.completer.cmdutils', cmd_utils) def _set_cmd_prompt(cmd, txt): @@ -143,11 +169,8 @@ def _validate_cmd_prompt(cmd, txt): (':set general ignore-case |', usertypes.Completion.value), (':set general huh |', None), (':help |', usertypes.Completion.helptopic), - (':quickmark-load |', usertypes.Completion.quickmark_by_name), - (':bookmark-load |', usertypes.Completion.bookmark_by_url), + (':help |', usertypes.Completion.helptopic), (':open |', usertypes.Completion.url), - (':buffer |', usertypes.Completion.tab), - (':session-load |', usertypes.Completion.sessions), (':bind |', usertypes.Completion.empty), (':bind |', usertypes.Completion.command), (':bind foo|', usertypes.Completion.command), @@ -157,7 +180,6 @@ def _validate_cmd_prompt(cmd, txt): (':set gene|ral ignore-case', usertypes.Completion.section), (':|', usertypes.Completion.command), (': |', usertypes.Completion.command), - (':bookmark-load |', usertypes.Completion.bookmark_by_url), ('/|', None), (':open -t|', None), (':open --tab|', None), From 66358e5b0f496f6c9d0af81cc3650481936a792f Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sat, 6 Aug 2016 11:38:08 -0400 Subject: [PATCH 015/160] Remove usertypes.Completion.empty. Completion.empty existed to fill a slot in the old Command.completions interface if the first positional arg had no completions but the second did, as is the case for the `bind` command. Now that `Command.completions` is replaced by `Command.get_pos_arg_info`, this is no longer needed. --- qutebrowser/completion/models/instances.py | 11 +---------- qutebrowser/config/parsers/keyconf.py | 1 - qutebrowser/utils/usertypes.py | 3 +-- tests/unit/completion/test_completer.py | 5 ++--- 4 files changed, 4 insertions(+), 16 deletions(-) diff --git a/qutebrowser/completion/models/instances.py b/qutebrowser/completion/models/instances.py index c92225539..568607b79 100644 --- a/qutebrowser/completion/models/instances.py +++ b/qutebrowser/completion/models/instances.py @@ -27,8 +27,7 @@ Module attributes: import functools -from qutebrowser.completion.models import (miscmodels, urlmodel, configmodel, - base) +from qutebrowser.completion.models import miscmodels, urlmodel, configmodel from qutebrowser.utils import objreg, usertypes, log, debug from qutebrowser.config import configdata @@ -115,13 +114,6 @@ def init_session_completion(): _instances[usertypes.Completion.sessions] = model -def _init_empty_completion(): - """Initialize empty completion model.""" - log.completion.debug("Initializing empty completion.") - if usertypes.Completion.empty not in _instances: - _instances[usertypes.Completion.empty] = base.BaseCompletionModel() - - INITIALIZERS = { usertypes.Completion.command: _init_command_completion, usertypes.Completion.helptopic: _init_helptopic_completion, @@ -133,7 +125,6 @@ INITIALIZERS = { usertypes.Completion.quickmark_by_name: init_quickmark_completions, usertypes.Completion.bookmark_by_url: init_bookmark_completions, usertypes.Completion.sessions: init_session_completion, - usertypes.Completion.empty: _init_empty_completion, } diff --git a/qutebrowser/config/parsers/keyconf.py b/qutebrowser/config/parsers/keyconf.py index b2596fb62..e8e7cc6dc 100644 --- a/qutebrowser/config/parsers/keyconf.py +++ b/qutebrowser/config/parsers/keyconf.py @@ -152,7 +152,6 @@ class KeyConfigParser(QObject): @cmdutils.register(instance='key-config', maxsplit=1, no_cmd_split=True) @cmdutils.argument('win_id', win_id=True) - @cmdutils.argument('key', completion=usertypes.Completion.empty) @cmdutils.argument('command', completion=usertypes.Completion.command) def bind(self, key, win_id, command=None, *, mode='normal', force=False): """Bind a key to a command. diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index 65c3c31e2..1cd1f0385 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -238,8 +238,7 @@ KeyMode = enum('KeyMode', ['normal', 'hint', 'command', 'yesno', 'prompt', # Available command completions Completion = enum('Completion', ['command', 'section', 'option', 'value', 'helptopic', 'quickmark_by_name', - 'bookmark_by_url', 'url', 'tab', 'sessions', - 'empty']) + 'bookmark_by_url', 'url', 'tab', 'sessions']) # Exit statuses for errors. Needs to be an int for sys.exit. diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 8e9273a90..f3567674e 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -112,7 +112,6 @@ def cmdutils_patch(monkeypatch, stubs): pass @cmdutils.argument('win_id', win_id=True) - @cmdutils.argument('key', completion=usertypes.Completion.empty) @cmdutils.argument('command', completion=usertypes.Completion.command) def bind(key, win_id, command=None, *, mode='normal', force=False): """docstring!""" @@ -171,10 +170,10 @@ def _validate_cmd_prompt(cmd, txt): (':help |', usertypes.Completion.helptopic), (':help |', usertypes.Completion.helptopic), (':open |', usertypes.Completion.url), - (':bind |', usertypes.Completion.empty), + (':bind |', None), (':bind |', usertypes.Completion.command), (':bind foo|', usertypes.Completion.command), - (':bind | foo', usertypes.Completion.empty), + (':bind | foo', None), (':set| general ', usertypes.Completion.command), (':|set general ', usertypes.Completion.command), (':set gene|ral ignore-case', usertypes.Completion.section), From f23a28079cd019012ec121e2a34db2e784421e60 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Sat, 6 Aug 2016 19:59:56 +0200 Subject: [PATCH 016/160] editor: fix external editor on windows On windows, only one process can open a file in write mode at once. We didn't close the handle we got (self._oshandle) before _cleanup, which means that we had the file open the whole time, which means that the external editor couldn't write back the changes. This patch closes the file while the external editor is running and only opens it once the editor is closed. We re-opened the file anyway, so this shouldn't be a huge change. Additionally, tempfile.NamedTemporaryFile is used instead of mkstemp, as we don't have to deal with os-level file handles that way. --- qutebrowser/misc/editor.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index bcc07129a..fb4cd8676 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -35,8 +35,8 @@ class ExternalEditor(QObject): Attributes: _text: The current text before the editor is opened. - _oshandle: The OS level handle to the tmpfile. - _filehandle: The file handle to the tmpfile. + _file: The file handle as tempfile.NamedTemporaryFile. Note that this + handle will be closed after the initial file has been created. _proc: The GUIProcess of the editor. _win_id: The window ID the ExternalEditor is associated with. """ @@ -46,20 +46,18 @@ class ExternalEditor(QObject): def __init__(self, win_id, parent=None): super().__init__(parent) self._text = None - self._oshandle = None - self._filename = None + self._file = None self._proc = None self._win_id = win_id def _cleanup(self): """Clean up temporary files after the editor closed.""" - if self._oshandle is None or self._filename is None: + if self._file is None: # Could not create initial file. return try: - os.close(self._oshandle) if self._proc.exit_status() != QProcess.CrashExit: - os.remove(self._filename) + os.unlink(self._file.name) except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's # executed async. @@ -82,7 +80,7 @@ class ExternalEditor(QObject): return encoding = config.get('general', 'editor-encoding') try: - with open(self._filename, 'r', encoding=encoding) as f: + with open(self._file.name, 'r', encoding=encoding) as f: text = f.read() except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's @@ -108,13 +106,18 @@ class ExternalEditor(QObject): if self._text is not None: raise ValueError("Already editing a file!") self._text = text + encoding = config.get('general', 'editor-encoding') try: - self._oshandle, self._filename = tempfile.mkstemp( - text=True, prefix='qutebrowser-editor-') - if text: - encoding = config.get('general', 'editor-encoding') - with open(self._filename, 'w', encoding=encoding) as f: - f.write(text) + # Close while the external process is running, as otherwise systems + # with exclusive write access (e.g. Windows) may fail to update + # the file from the external editor, see + # https://github.com/The-Compiler/qutebrowser/issues/1767 + with tempfile.NamedTemporaryFile( + mode='w', prefix='qutebrowser-editor-', encoding=encoding, + delete=False) as fobj: + if text: + fobj.write(text) + self._file = fobj except OSError as e: message.error(self._win_id, "Failed to create initial file: " "{}".format(e)) @@ -125,6 +128,6 @@ class ExternalEditor(QObject): self._proc.error.connect(self.on_proc_error) editor = config.get('general', 'editor') executable = editor[0] - args = [arg.replace('{}', self._filename) for arg in editor[1:]] + args = [arg.replace('{}', self._file.name) for arg in editor[1:]] log.procs.debug("Calling \"{}\" with args {}".format(executable, args)) self._proc.start(executable, args) From 0177dafbd004b88f8ec0a72be3d90407a0c04ae7 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sat, 6 Aug 2016 22:03:50 +0200 Subject: [PATCH 017/160] Add {clipboard} and {primary} to replace :paste :paste is deprecated and replaced by equivalents using :open and the new variables, and :open supports opening newline-separated lists of URLs. --- doc/help/commands.asciidoc | 17 +---- qutebrowser/browser/commands.py | 79 +++++++++++++++--------- qutebrowser/commands/runners.py | 10 ++- qutebrowser/config/configdata.py | 12 ++-- qutebrowser/utils/utils.py | 6 ++ tests/end2end/features/yankpaste.feature | 38 ++++++------ 6 files changed, 93 insertions(+), 69 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index f43be5822..ebe703a44 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -34,7 +34,6 @@ |<>|Show a log of past messages. |<>|Open typical prev/next links or navigate using the URL path. |<>|Open a URL in the current/[count]th tab. -|<>|Open a page from the clipboard. |<>|Print the current/[count]th tab. |<>|Add a new quickmark. |<>|Delete a quickmark. @@ -473,6 +472,8 @@ Syntax: +:open [*--implicit*] [*--bg*] [*--tab*] [*--window*] ['url']+ Open a URL in the current/[count]th tab. +If the URL contains newlines, each line gets opened in its own tab. + ==== positional arguments * +'url'+: The URL to open. @@ -489,20 +490,6 @@ The tab index to open the URL in. ==== note * This command does not split arguments after the last argument and handles quotes literally. -[[paste]] -=== paste -Syntax: +:paste [*--sel*] [*--tab*] [*--bg*] [*--window*]+ - -Open a page from the clipboard. - -If the pasted text contains newlines, each line gets opened in its own tab. - -==== optional arguments -* +*-s*+, +*--sel*+: Use the primary selection instead of the clipboard. -* +*-t*+, +*--tab*+: Open in a new tab. -* +*-b*+, +*--bg*+: Open in a background tab. -* +*-w*+, +*--window*+: Open in new window. - [[print]] === print Syntax: +:print [*--preview*] [*--pdf* 'file']+ diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 3728ebac4..3bfb09fd6 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -236,6 +236,8 @@ class CommandDispatcher: bg=False, tab=False, window=False, count=None): """Open a URL in the current/[count]th tab. + If the URL contains newlines, each line gets opened in its own tab. + Args: url: The URL to open. bg: Open in a new background tab. @@ -245,37 +247,60 @@ class CommandDispatcher: clicking on a link). count: The tab index to open the URL in, or None. """ + force_search = False if url is None: if tab or bg or window: - url = config.get('general', 'default-page') + urls = [config.get('general', 'default-page')] else: raise cmdexc.CommandError("No URL given, but -t/-b/-w is not " "set!") else: - try: - url = objreg.get('quickmark-manager').get(url) - except urlmarks.Error: - try: - url = urlutils.fuzzy_url(url) - except urlutils.InvalidUrlError as e: - # We don't use cmdexc.CommandError here as this can be - # called async from edit_url - message.error(self._win_id, str(e)) - return - if tab or bg or window: - self._open(url, tab, bg, window, not implicit) - else: - curtab = self._cntwidget(count) - if curtab is None: - if count is None: - # We want to open a URL in the current tab, but none exists - # yet. - self._tabbed_browser.tabopen(url) - else: - # Explicit count with a tab that doesn't exist. - return + urllist = [u for u in url.split('\n') if u.strip()] + if (len(urllist) > 1 and + any(not urlutils.is_url(u) and + urlutils.get_path_if_valid(u, check_exists=True) + is None for u in urllist)): + urllist = [url] + force_search = True + urls = [x for x in [self._parse_url(u, force_search=force_search) + for u in urllist] if x is not None] + for i, cur_url in enumerate(urls): + if not window and i > 0: + tab = False + bg = True + if tab or bg or window: + self._open(cur_url, tab, bg, window, not implicit) else: - curtab.openurl(url) + curtab = self._cntwidget(count) + if curtab is None: + if count is None: + # We want to open a URL in the current tab, but none + # exists yet. + self._tabbed_browser.tabopen(cur_url) + else: + curtab.openurl(cur_url) + + def _parse_url(self, url, force_search=False): + """Parse a URL or quickmark or search query. + + Args: + url: The URL to parse. + force_search: Whether to force a search even if the content can be + interpreted as a URL or a path. + + Return: + A URL that can be opened.""" + try: + return objreg.get('quickmark-manager').get(url) + except urlmarks.Error: + try: + return urlutils.fuzzy_url(url, force_search=force_search) + except urlutils.InvalidUrlError as e: + # We don't use cmdexc.CommandError here as this can be + # called async from edit_url + message.error(self._win_id, str(e)) + return + @cmdutils.register(instance='command-dispatcher', name='reload', scope='window') @@ -776,7 +801,8 @@ class CommandDispatcher: else: raise cmdexc.CommandError("Last tab") - @cmdutils.register(instance='command-dispatcher', scope='window') + @cmdutils.register(instance='command-dispatcher', scope='window', + deprecated="Use :open {clipboard}") def paste(self, sel=False, tab=False, bg=False, window=False): """Open a page from the clipboard. @@ -796,9 +822,6 @@ class CommandDispatcher: sel = False target = "Clipboard" text = utils.get_clipboard(selection=sel) - if not text.strip(): - raise cmdexc.CommandError("{} is empty.".format(target)) - log.misc.debug("{} contained: {!r}".format(target, text)) text_urls = [u for u in text.split('\n') if u.strip()] if (len(text_urls) > 1 and not urlutils.is_url(text_urls[0]) and urlutils.get_path_if_valid( diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index f020b9199..91c8bf3c9 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -26,7 +26,7 @@ from PyQt5.QtCore import pyqtSlot, QUrl, QObject from qutebrowser.config import config, configexc from qutebrowser.commands import cmdexc, cmdutils -from qutebrowser.utils import message, objreg, qtutils +from qutebrowser.utils import message, objreg, qtutils, utils from qutebrowser.misc import split @@ -57,11 +57,19 @@ def replace_variables(win_id, arglist): QUrl.RemovePassword) if '{url:pretty}' in arglist: pretty_url = _current_url(tabbed_browser).toString(QUrl.RemovePassword) + if '{clipboard}' in arglist: + clipboard = utils.get_clipboard() + if '{primary}' in arglist: + primary = utils.get_clipboard(selection=True) for arg in arglist: if arg == '{url}': args.append(url) elif arg == '{url:pretty}': args.append(pretty_url) + elif arg == '{clipboard}': + args.append(clipboard) + elif arg == '{primary}': + args.append(primary) else: args.append(arg) return args diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 8cbfe3938..9cc275502 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1512,12 +1512,12 @@ KEY_DATA = collections.OrderedDict([ ('yank -ds', ['yD']), ('yank -p', ['yp']), ('yank -ps', ['yP']), - ('paste', ['pp']), - ('paste -s', ['pP']), - ('paste -t', ['Pp']), - ('paste -ts', ['PP']), - ('paste -w', ['wp']), - ('paste -ws', ['wP']), + ('open {clipboard}', ['pp']), + ('open {primary}', ['pP']), + ('open -t {clipboard}', ['Pp']), + ('open -t {primary}', ['PP']), + ('open -w {clipboard}', ['wp']), + ('open -w {primary}', ['wP']), ('quickmark-save', ['m']), ('set-cmd-text -s :quickmark-load', ['b']), ('set-cmd-text -s :quickmark-load -t', ['B']), diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index fd8419d12..983827d46 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -37,6 +37,7 @@ import pkg_resources import qutebrowser from qutebrowser.utils import qtutils, log +from qutebrowser.commands import cmdexc fake_clipboard = None @@ -810,6 +811,11 @@ def get_clipboard(selection=False): mode = QClipboard.Selection if selection else QClipboard.Clipboard data = QApplication.clipboard().text(mode=mode) + target = "Primary selection" if selection else "Clipboard" + if not data.strip(): + raise cmdexc.CommandError("{} is empty.".format(target)) + log.misc.debug("{} contained: {!r}".format(target, data)) + return data diff --git a/tests/end2end/features/yankpaste.feature b/tests/end2end/features/yankpaste.feature index db39d2380..abf9cf9c3 100644 --- a/tests/end2end/features/yankpaste.feature +++ b/tests/end2end/features/yankpaste.feature @@ -1,6 +1,6 @@ Feature: Yanking and pasting. - :yank and :paste can be used to copy/paste the URL or title from/to the - clipboard and primary selection. + :yank, {clipboard} and {primary} can be used to copy/paste the URL or title + from/to the clipboard and primary selection. Background: Given I run :tab-only @@ -45,11 +45,11 @@ Feature: Yanking and pasting. Then the message "Yanked URL to clipboard: http://localhost:(port)/data/title with spaces.html" should be shown And the clipboard should contain "http://localhost:(port)/data/title with spaces.html" - #### :paste + #### {clipboard} and {primary} Scenario: Pasting a URL When I put "http://localhost:(port)/data/hello.txt" into the clipboard - And I run :paste + And I run :open {clipboard} And I wait until data/hello.txt is loaded Then the requests should be: data/hello.txt @@ -57,32 +57,32 @@ Feature: Yanking and pasting. Scenario: Pasting a URL from primary selection When selection is supported And I put "http://localhost:(port)/data/hello2.txt" into the primary selection - And I run :paste --sel + And I run :open {primary} And I wait until data/hello2.txt is loaded Then the requests should be: data/hello2.txt Scenario: Pasting with empty clipboard When I put "" into the clipboard - And I run :paste + And I run :open {clipboard} (invalid command) Then the error "Clipboard is empty." should be shown Scenario: Pasting with empty selection When selection is supported And I put "" into the primary selection - And I run :paste --sel + And I run :open {primary} (invalid command) Then the error "Primary selection is empty." should be shown Scenario: Pasting with a space in clipboard When I put " " into the clipboard - And I run :paste + And I run :open {clipboard} (invalid command) Then the error "Clipboard is empty." should be shown Scenario: Pasting in a new tab Given I open about:blank When I run :tab-only And I put "http://localhost:(port)/data/hello.txt" into the clipboard - And I run :paste -t + And I run :open -t {clipboard} And I wait until data/hello.txt is loaded Then the following tabs should be open: - about:blank @@ -92,7 +92,7 @@ Feature: Yanking and pasting. Given I open about:blank When I run :tab-only And I put "http://localhost:(port)/data/hello.txt" into the clipboard - And I run :paste -b + And I run :open -b {clipboard} And I wait until data/hello.txt is loaded Then the following tabs should be open: - about:blank (active) @@ -101,7 +101,7 @@ Feature: Yanking and pasting. Scenario: Pasting in a new window Given I have a fresh instance When I put "http://localhost:(port)/data/hello.txt" into the clipboard - And I run :paste -w + And I run :open -w {clipboard} And I wait until data/hello.txt is loaded Then the session should look like: windows: @@ -119,7 +119,7 @@ Feature: Yanking and pasting. Scenario: Pasting an invalid URL When I set general -> auto-search to false And I put "foo bar" into the clipboard - And I run :paste + And I run :open {clipboard} Then the error "Invalid URL" should be shown Scenario: Pasting multiple urls in a new tab @@ -128,7 +128,7 @@ Feature: Yanking and pasting. http://localhost:(port)/data/hello.txt http://localhost:(port)/data/hello2.txt http://localhost:(port)/data/hello3.txt - And I run :paste -t + And I run :open -t {clipboard} And I wait until data/hello.txt is loaded And I wait until data/hello2.txt is loaded And I wait until data/hello3.txt is loaded @@ -145,7 +145,7 @@ Feature: Yanking and pasting. this url: http://qutebrowser.org should not open - And I run :paste -t + And I run :open -t {clipboard} And I wait until data/hello.txt?q=this%20url%3A%0Ahttp%3A//qutebrowser.org%0Ashould%20not%20open is loaded Then the following tabs should be open: - about:blank @@ -159,7 +159,7 @@ Feature: Yanking and pasting. text: should open as search - And I run :paste -t + And I run :open -t {clipboard} And I wait until data/hello.txt?q=text%3A%0Ashould%20open%0Aas%20search is loaded Then the following tabs should be open: - about:blank @@ -172,7 +172,7 @@ Feature: Yanking and pasting. http://localhost:(port)/data/hello.txt http://localhost:(port)/data/hello2.txt http://localhost:(port)/data/hello3.txt - And I run :paste -b + And I run :open -b {clipboard} And I wait until data/hello.txt is loaded And I wait until data/hello2.txt is loaded And I wait until data/hello3.txt is loaded @@ -188,7 +188,7 @@ Feature: Yanking and pasting. http://localhost:(port)/data/hello.txt http://localhost:(port)/data/hello2.txt http://localhost:(port)/data/hello3.txt - And I run :paste -w + And I run :open -w {clipboard} And I wait until data/hello.txt is loaded And I wait until data/hello2.txt is loaded And I wait until data/hello3.txt is loaded @@ -218,13 +218,13 @@ Feature: Yanking and pasting. Scenario: Pasting multiple urls with an empty one When I open about:blank And I put "http://localhost:(port)/data/hello.txt\n\nhttp://localhost:(port)/data/hello2.txt" into the clipboard - And I run :paste -t + And I run :open -t {clipboard} Then no crash should happen Scenario: Pasting multiple urls with an almost empty one When I open about:blank And I put "http://localhost:(port)/data/hello.txt\n \nhttp://localhost:(port)/data/hello2.txt" into the clipboard - And I run :paste -t + And I run :open -t {clipboard} Then no crash should happen #### :paste-primary From 96146c55afbbf8603d800cfddfca8b7035cec39a Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sat, 6 Aug 2016 22:25:08 +0200 Subject: [PATCH 018/160] Remove useless clipboard target information --- qutebrowser/browser/commands.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 3bfb09fd6..f0ed6d67a 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -816,11 +816,8 @@ class CommandDispatcher: window: Open in new window. """ force_search = False - if sel and utils.supports_selection(): - target = "Primary selection" - else: + if not utils.supports_selection(): sel = False - target = "Clipboard" text = utils.get_clipboard(selection=sel) text_urls = [u for u in text.split('\n') if u.strip()] if (len(text_urls) > 1 and not urlutils.is_url(text_urls[0]) and From 9a4655443f62f3e8041a9623f74fdb888b25e539 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Sun, 7 Aug 2016 00:11:58 +0200 Subject: [PATCH 019/160] os.unlink -> os.remove The functions do the same, but remove sounds better. --- qutebrowser/misc/editor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index fb4cd8676..c72841f30 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -57,7 +57,7 @@ class ExternalEditor(QObject): return try: if self._proc.exit_status() != QProcess.CrashExit: - os.unlink(self._file.name) + os.remove(self._file.name) except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's # executed async. From 4d00b8fce96be3411bb687c340e88cdf03e809b6 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Sun, 7 Aug 2016 00:18:22 +0200 Subject: [PATCH 020/160] tests: fix tests for misc.editor Due to the change to NamedTemporaryFile, the _filename attribute was removed. We now have to use _file.name. --- tests/unit/misc/test_editor.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index f9a5d4370..26828ee12 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -69,14 +69,14 @@ class TestArg: config_stub.data['general']['editor'] = ['bin', 'foo', '{}', 'bar'] editor.edit("") editor._proc._proc.start.assert_called_with( - "bin", ["foo", editor._filename, "bar"]) + "bin", ["foo", editor._file.name, "bar"]) def test_placeholder_inline(self, config_stub, editor): """Test starting editor with placeholder arg inside of another arg.""" config_stub.data['general']['editor'] = ['bin', 'foo{}', 'bar'] editor.edit("") editor._proc._proc.start.assert_called_with( - "bin", ["foo" + editor._filename, "bar"]) + "bin", ["foo" + editor._file.name, "bar"]) class TestFileHandling: @@ -86,7 +86,7 @@ class TestFileHandling: def test_ok(self, editor): """Test file handling when closing with an exit status == 0.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) assert os.path.basename(filename).startswith('qutebrowser-editor-') editor._proc.finished.emit(0, QProcess.NormalExit) @@ -95,7 +95,7 @@ class TestFileHandling: def test_error(self, editor): """Test file handling when closing with an exit status != 0.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) editor._proc._proc.exitStatus = mock.Mock( @@ -109,7 +109,7 @@ class TestFileHandling: def test_crash(self, editor): """Test file handling when closing with a crash.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) editor._proc._proc.exitStatus = mock.Mock( @@ -125,7 +125,7 @@ class TestFileHandling: def test_unreadable(self, message_mock, editor): """Test file handling when closing with an unreadable file.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) os.chmod(filename, 0o077) editor._proc.finished.emit(0, QProcess.NormalExit) @@ -160,10 +160,10 @@ def test_modify(editor, initial_text, edited_text): """Test if inputs get modified correctly.""" editor.edit(initial_text) - with open(editor._filename, 'r', encoding='utf-8') as f: + with open(editor._file.name, 'r', encoding='utf-8') as f: assert f.read() == initial_text - with open(editor._filename, 'w', encoding='utf-8') as f: + with open(editor._file.name, 'w', encoding='utf-8') as f: f.write(edited_text) editor._proc.finished.emit(0, QProcess.NormalExit) From 38508274e063c12fc8caf49c746795c0abd57b94 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sun, 7 Aug 2016 00:46:23 +0200 Subject: [PATCH 021/160] Improve clipboard exceptions, migrate bindings --- qutebrowser/browser/commands.py | 14 ++++++++++---- qutebrowser/commands/runners.py | 11 +++++++---- qutebrowser/config/configdata.py | 5 +++++ qutebrowser/misc/miscwidgets.py | 3 ++- qutebrowser/utils/utils.py | 8 ++++++-- tests/unit/config/test_config.py | 4 ++++ 6 files changed, 34 insertions(+), 11 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index f0ed6d67a..6dde5d40f 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -818,7 +818,10 @@ class CommandDispatcher: force_search = False if not utils.supports_selection(): sel = False - text = utils.get_clipboard(selection=sel) + try: + text = utils.get_clipboard(selection=sel) + except utils.ClipboardEmptyError as e: + raise cmdexc.CommandError(e) text_urls = [u for u in text.split('\n') if u.strip()] if (len(text_urls) > 1 and not urlutils.is_url(text_urls[0]) and urlutils.get_path_if_valid( @@ -1462,9 +1465,12 @@ class CommandDispatcher: raise cmdexc.CommandError("Focused element is not editable!") try: - sel = utils.get_clipboard(selection=True) - except utils.SelectionUnsupportedError: - sel = utils.get_clipboard() + try: + sel = utils.get_clipboard(selection=True) + except utils.SelectionUnsupportedError: + sel = utils.get_clipboard() + except utils.ClipboardEmptyError: + return log.misc.debug("Pasting primary selection into element {}".format( elem.debug_text())) diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index 91c8bf3c9..201fc4bbe 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -57,10 +57,13 @@ def replace_variables(win_id, arglist): QUrl.RemovePassword) if '{url:pretty}' in arglist: pretty_url = _current_url(tabbed_browser).toString(QUrl.RemovePassword) - if '{clipboard}' in arglist: - clipboard = utils.get_clipboard() - if '{primary}' in arglist: - primary = utils.get_clipboard(selection=True) + try: + if '{clipboard}' in arglist: + clipboard = utils.get_clipboard() + if '{primary}' in arglist: + primary = utils.get_clipboard(selection=True) + except utils.ClipboardEmptyError as e: + raise cmdexc.CommandError(e) for arg in arglist: if arg == '{url}': args.append(url) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 9cc275502..f770f5c05 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1677,4 +1677,9 @@ CHANGED_KEY_COMMANDS = [ (re.compile(r'^download-remove --all$'), r'download-clear'), (re.compile(r'^hint links fill "([^"]*)"$'), r'hint links fill \1'), + + (re.compile(r'^paste$'), r'open {clipboard}'), + (re.compile(r'^paste -([twb])$'), r'open -\1 {clipboard}'), + (re.compile(r'^paste -([twb])s$'), r'open -\1 {primary}'), + (re.compile(r'^paste -s([twb])$'), r'open -\1 {primary}'), ] diff --git a/qutebrowser/misc/miscwidgets.py b/qutebrowser/misc/miscwidgets.py index 63a5718c6..7e84300d2 100644 --- a/qutebrowser/misc/miscwidgets.py +++ b/qutebrowser/misc/miscwidgets.py @@ -47,7 +47,8 @@ class MinimalLineEditMixin: if e.key() == Qt.Key_Insert and e.modifiers() == Qt.ShiftModifier: try: text = utils.get_clipboard(selection=True) - except utils.SelectionUnsupportedError: + except (utils.SelectionUnsupportedError, + utils.ClipboardEmptyError): pass else: e.accept() diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index 983827d46..32696aacf 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -37,7 +37,6 @@ import pkg_resources import qutebrowser from qutebrowser.utils import qtutils, log -from qutebrowser.commands import cmdexc fake_clipboard = None @@ -49,6 +48,11 @@ class SelectionUnsupportedError(Exception): """Raised if [gs]et_clipboard is used and selection=True is unsupported.""" +class ClipboardEmptyError(Exception): + + """Raised if get_clipboard is used and the clipboard is empty.""" + + def elide(text, length): """Elide text so it uses a maximum of length chars.""" if length < 1: @@ -813,7 +817,7 @@ def get_clipboard(selection=False): target = "Primary selection" if selection else "Clipboard" if not data.strip(): - raise cmdexc.CommandError("{} is empty.".format(target)) + raise ClipboardEmptyError("{} is empty.".format(target)) log.misc.debug("{} contained: {!r}".format(target, data)) return data diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index a6ad3d119..53664b718 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -286,6 +286,10 @@ class TestKeyConfigParser: 'hint links fill :open {hint-url}'), ('hint links fill ":open -t {hint-url}"', 'hint links fill :open -t {hint-url}'), + + ('paste', 'open {clipboard}'), + ('paste -t', 'open -t {clipboard}'), + ('paste -ws', 'open -w {primary}'), ] ) def test_migrations(self, old, new_expected): From a90662ef83289351768e3d288ba04b3efc7b35ec Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 6 Aug 2016 20:08:04 -0400 Subject: [PATCH 022/160] Fix combined yank documentation --- qutebrowser/browser/commands.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index ff1867389..914121310 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -627,8 +627,15 @@ class CommandDispatcher: Args: what: What to yank. + + - `url`: The current URL. + - `pretty-url`: The URL in pretty decoded form. + - `title`: The current page's title. + - `domain`: The current scheme, domain, and port number. + - `selection`: The selection under the cursor. + sel: Use the primary selection instead of the clipboard. - keep: If given, stay in visual mode after yanking. + keep: Stay in visual mode after yanking the selection. """ if what == 'title': s = self._tabbed_browser.page_title(self._current_index()) @@ -639,10 +646,10 @@ class CommandDispatcher: ':' + str(port) if port > -1 else '') elif what in ['url', 'pretty-url']: flags = QUrl.RemovePassword - if what == 'url': # Not pretty + if what == 'url': # Not pretty flags |= QUrl.FullyEncoded s = self._current_url().toString(flags) - what = 'URL' # For printing + what = 'URL' # For printing elif what == 'selection': caret = self._current_widget().caret s = caret.selection() From 47969362e193c2bb7f60d427db6c73ca67079911 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 6 Aug 2016 20:15:51 -0400 Subject: [PATCH 023/160] Fix minor conditional issues in yank --- qutebrowser/browser/commands.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 914121310..3147122b8 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -653,9 +653,11 @@ class CommandDispatcher: elif what == 'selection': caret = self._current_widget().caret s = caret.selection() - if not caret.has_selection() or len(s) == 0: + if not caret.has_selection() or not s: message.info(self._win_id, "Nothing to yank") return + else: # pragma: no cover + raise ValueError("Invalid value {!r} for `what'.".format(what)) if sel and utils.supports_selection(): target = "primary selection" From 28485b0731d45a39fc6f1d7cf8cd706a4cd5a0c5 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 6 Aug 2016 20:22:56 -0400 Subject: [PATCH 024/160] Migrate -p to -s for :yank selection keybinding --- qutebrowser/config/configdata.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 5c087752e..6f6a36710 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1638,7 +1638,7 @@ KEY_DATA = collections.OrderedDict([ ('move-to-end-of-line', ['$']), ('move-to-start-of-document', ['gg']), ('move-to-end-of-document', ['G']), - ('yank selection -p', ['Y']), + ('yank selection -s', ['Y']), ('yank selection', ['y'] + RETURN_KEYS), ('scroll left', ['H']), ('scroll down', ['J']), @@ -1678,5 +1678,6 @@ CHANGED_KEY_COMMANDS = [ (re.compile(r'^hint links fill "([^"]*)"$'), r'hint links fill \1'), + (re.compile(r'^yank-selected -p'), r'yank selection -s'), (re.compile(r'^yank-selected'), r'yank selection'), ] From 006a934913eb48d0563ee4cae1605bad9450370e Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 6 Aug 2016 20:35:49 -0400 Subject: [PATCH 025/160] Migrate yank keybindings --- qutebrowser/config/configdata.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 6f6a36710..785518262 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1678,6 +1678,12 @@ CHANGED_KEY_COMMANDS = [ (re.compile(r'^hint links fill "([^"]*)"$'), r'hint links fill \1'), + (re.compile(r'^yank -t(\S+)'), r'yank title -\1'), + (re.compile(r'^yank -t'), r'yank title'), + (re.compile(r'^yank -d(\S+)'), r'yank domain -\1'), + (re.compile(r'^yank -d'), r'yank domain'), + (re.compile(r'^yank -p(\S+)'), r'yank pretty-url -\1'), + (re.compile(r'^yank -p'), r'yank pretty-url'), (re.compile(r'^yank-selected -p'), r'yank selection -s'), (re.compile(r'^yank-selected'), r'yank selection'), ] From 7bfa46fb501ec5bdded9355d2eab762337394962 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 6 Aug 2016 20:36:13 -0400 Subject: [PATCH 026/160] Add tests for yank migrations --- tests/unit/config/test_config.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index a6ad3d119..f2a6a79f1 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -286,6 +286,17 @@ class TestKeyConfigParser: 'hint links fill :open {hint-url}'), ('hint links fill ":open -t {hint-url}"', 'hint links fill :open -t {hint-url}'), + + ('yank-selected', 'yank selection'), + ('yank-selected --sel', 'yank selection --sel'), + ('yank-selected -p', 'yank selection -s'), + + ('yank -t', 'yank title'), + ('yank -ts', 'yank title -s'), + ('yank -d', 'yank domain'), + ('yank -ds', 'yank domain -s'), + ('yank -p', 'yank pretty-url'), + ('yank -ps', 'yank pretty-url -s'), ] ) def test_migrations(self, old, new_expected): From adc428e5252af98a8364ffb76100e3633b354271 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sat, 6 Aug 2016 20:48:21 -0400 Subject: [PATCH 027/160] Perfect coverage for sortfilter. Mark an area of sortfilter with `#pragma no coverage` and add it to perfect_files. --- qutebrowser/completion/models/sortfilter.py | 4 ++-- scripts/dev/check_coverage.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/qutebrowser/completion/models/sortfilter.py b/qutebrowser/completion/models/sortfilter.py index ffa44b82e..56905bbec 100644 --- a/qutebrowser/completion/models/sortfilter.py +++ b/qutebrowser/completion/models/sortfilter.py @@ -135,8 +135,8 @@ class CompletionFilterModel(QSortFilterProxyModel): for col in self.srcmodel.columns_to_filter: idx = self.srcmodel.index(row, col, parent) - if not idx.isValid(): - # No entries in parent model + if not idx.isValid(): # pragma: no cover + # this is a sanity check not hit by any test case continue data = self.srcmodel.data(idx) if not data: diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index 457f472ba..6d42e76cf 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -150,6 +150,8 @@ PERFECT_FILES = [ ('tests/unit/completion/test_models.py', 'qutebrowser/completion/models/base.py'), + ('tests/unit/completion/test_sortfilter.py', + 'qutebrowser/completion/models/sortfilter.py'), ] From 126e3fd01b93525d80975d586c373ba0ed2ddadf Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 10:42:37 +0200 Subject: [PATCH 028/160] tests: Add webkitinspector.py to WHITELISTED_FILES When running with --no-xvfb, this file has 100% coverage due to the inspector tests in misc.feature. See #1771. --- scripts/dev/check_coverage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index 457f472ba..c8db7e03e 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -155,7 +155,7 @@ PERFECT_FILES = [ # 100% coverage because of end2end tests, but no perfect unit tests yet. -WHITELISTED_FILES = [] +WHITELISTED_FILES = ['qutebrowser/browser/webkit/webkitinspector.py'] class Skipped(Exception): From 88d3e862592c30989ad560d0db20d450c24ac75e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:02:50 +0200 Subject: [PATCH 029/160] Use "what != 'url-pretty'" --- qutebrowser/browser/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 3147122b8..a6d78f7ee 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -646,7 +646,7 @@ class CommandDispatcher: ':' + str(port) if port > -1 else '') elif what in ['url', 'pretty-url']: flags = QUrl.RemovePassword - if what == 'url': # Not pretty + if what != 'url-pretty': flags |= QUrl.FullyEncoded s = self._current_url().toString(flags) what = 'URL' # For printing From ef0b2e1488e74f3675ee0782756c87bef3e4feb3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:09:16 +0200 Subject: [PATCH 030/160] Update docs --- CHANGELOG.asciidoc | 8 ++++++++ README.asciidoc | 2 +- doc/help/commands.asciidoc | 27 +++++++++++++-------------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index d83ada63e..d7bda59b5 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -44,6 +44,14 @@ Changed (i.e. to open it at the position it would be opened if it was a clicked link) - `:download-open` and `:prompt-open-download` now have an optional `cmdline` argument to pass a commandline to open the download with. +- `:yank` now has a position argument to select what to yank instead of using + flags. + +Removed +~~~~~~~ + +- The `:yank-selected` command got merged into `:yank` as `:yank selection` + and thus removed. v0.8.3 (unreleased) ------------------- diff --git a/README.asciidoc b/README.asciidoc index 882301381..6813dc8c8 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -146,10 +146,10 @@ Contributors, sorted by the number of commits in descending order: * Lamar Pavel * Bruno Oliveira * Alexander Cogneau +* Marshall Lochbaum * Felix Van der Jeugt * Jakub Klinkovský * Martin Tournoij -* Marshall Lochbaum * Raphael Pierzina * Joel Torstensson * Jan Verbeek diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index f43be5822..829d5f5fe 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -66,8 +66,7 @@ |<>|Re-open a closed tab (optionally skipping [count] closed tabs). |<>|Show the source of the current page. |<>|Save open pages and quit. -|<>|Yank the current URL/title to the clipboard or primary selection. -|<>|Yank the selected text to the clipboard or primary selection. +|<>|Yank something to the clipboard or primary selection. |<>|Set the zoom level for the current tab. |<>|Increase the zoom level for the current tab. |<>|Decrease the zoom level for the current tab. @@ -853,25 +852,25 @@ Save open pages and quit. [[yank]] === yank -Syntax: +:yank [*--title*] [*--sel*] [*--domain*] [*--pretty*]+ +Syntax: +:yank [*--sel*] [*--keep*] ['what']+ -Yank the current URL/title to the clipboard or primary selection. +Yank something to the clipboard or primary selection. -==== optional arguments -* +*-t*+, +*--title*+: Yank the title instead of the URL. -* +*-s*+, +*--sel*+: Use the primary selection instead of the clipboard. -* +*-d*+, +*--domain*+: Yank only the scheme, domain, and port number. -* +*-p*+, +*--pretty*+: Yank the URL in pretty decoded form. +==== positional arguments +* +'what'+: What to yank. + + - `url`: The current URL. + - `pretty-url`: The URL in pretty decoded form. + - `title`: The current page's title. + - `domain`: The current scheme, domain, and port number. + - `selection`: The selection under the cursor. + -[[yank-selected]] -=== yank-selected -Syntax: +:yank-selected [*--sel*] [*--keep*]+ -Yank the selected text to the clipboard or primary selection. ==== optional arguments * +*-s*+, +*--sel*+: Use the primary selection instead of the clipboard. -* +*-k*+, +*--keep*+: If given, stay in visual mode after yanking. +* +*-k*+, +*--keep*+: Stay in visual mode after yanking the selection. [[zoom]] === zoom From 1b3664d9c8a858115a484b765cc37c3734b7f614 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:10:46 +0200 Subject: [PATCH 031/160] keyconfig: Validate commands *after* transforming Otherwise we'd still get an error when e.g. transforming :yank-selected to :yank selection as :yank-selected got removed. --- qutebrowser/config/parsers/keyconf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/config/parsers/keyconf.py b/qutebrowser/config/parsers/keyconf.py index b2596fb62..68450d54a 100644 --- a/qutebrowser/config/parsers/keyconf.py +++ b/qutebrowser/config/parsers/keyconf.py @@ -361,12 +361,12 @@ class KeyConfigParser(QObject): raise KeyConfigError("Got command '{}' without getting a " "section!".format(line)) else: - self._validate_command(line) for rgx, repl in configdata.CHANGED_KEY_COMMANDS: if rgx.match(line): line = rgx.sub(repl, line) self._mark_config_dirty() break + self._validate_command(line) self._cur_command = line def _read_keybinding(self, line): From d1eef5d1dc0c109625dcb1b4247f7ebb0554eaa1 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:19:03 +0200 Subject: [PATCH 032/160] Update changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index d7bda59b5..a23f283b3 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -60,6 +60,7 @@ Fixed ~~~~~ - Fixed crash when doing `:`, another corner-case introduced in v0.8.0 +- Fixed `:open-editor` (``) on Windows v0.8.2 ------ From 5a2285252161c107827521004a514febca246d28 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:34:52 +0200 Subject: [PATCH 033/160] Update docs --- CHANGELOG.asciidoc | 3 +++ README.asciidoc | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index a23f283b3..e34f92e4b 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -46,6 +46,9 @@ Changed argument to pass a commandline to open the download with. - `:yank` now has a position argument to select what to yank instead of using flags. +- Replacements like `{url}` can now also be used in the middle of an argument. + Consequently, commands taking another command (`:later`, `:repeat` and + `:bind`) now don't immediately evaluate variables. Removed ~~~~~~~ diff --git a/README.asciidoc b/README.asciidoc index 6813dc8c8..f0899b6c1 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -150,9 +150,9 @@ Contributors, sorted by the number of commits in descending order: * Felix Van der Jeugt * Jakub Klinkovský * Martin Tournoij +* Jan Verbeek * Raphael Pierzina * Joel Torstensson -* Jan Verbeek * Patric Schmitz * Tarcisio Fedrizzi * Claude From 8206efc0c2dda792535911fdaf7c0a6b99d2d014 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:37:19 +0200 Subject: [PATCH 034/160] requirements: Update setuptools to 25.1.6 --- misc/requirements/requirements-pip.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/requirements/requirements-pip.txt b/misc/requirements/requirements-pip.txt index 4b1f280c7..647c6d169 100644 --- a/misc/requirements/requirements-pip.txt +++ b/misc/requirements/requirements-pip.txt @@ -1,2 +1,2 @@ pip==8.1.2 -setuptools==25.1.4 +setuptools==25.1.6 From a18104529aeef335689e7779599c2cd243615155 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:38:22 +0200 Subject: [PATCH 035/160] tox requirements: Update virtualenv to 15.0.3 --- misc/requirements/requirements-tox.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/requirements/requirements-tox.txt b/misc/requirements/requirements-tox.txt index b47b75bd3..3f2792189 100644 --- a/misc/requirements/requirements-tox.txt +++ b/misc/requirements/requirements-tox.txt @@ -3,4 +3,4 @@ pluggy==0.3.1 py==1.4.31 tox==2.3.1 -virtualenv==15.0.2 +virtualenv==15.0.3 From 96087bd554e1b688632c8a7ea2963febabb57ac2 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 7 Aug 2016 11:40:26 +0200 Subject: [PATCH 036/160] Fix up pretty URL yanking --- qutebrowser/browser/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 16afed74e..33e778924 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -646,7 +646,7 @@ class CommandDispatcher: ':' + str(port) if port > -1 else '') elif what in ['url', 'pretty-url']: flags = QUrl.RemovePassword - if what != 'url-pretty': + if what != 'pretty-url': flags |= QUrl.FullyEncoded s = self._current_url().toString(flags) what = 'URL' # For printing From 6bbde59de7c2ee91cf7c96e925a60f2ee514f312 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sun, 7 Aug 2016 16:15:24 +0200 Subject: [PATCH 037/160] Rebuild buffer completion when tab title changes --- qutebrowser/completion/models/miscmodels.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py index 3daa03323..bbbaa8a9d 100644 --- a/qutebrowser/completion/models/miscmodels.py +++ b/qutebrowser/completion/models/miscmodels.py @@ -174,6 +174,7 @@ class TabCompletionModel(base.BaseCompletionModel): for i in range(tabbed_browser.count()): tab = tabbed_browser.widget(i) tab.url_changed.connect(self.rebuild) + tab.title_changed.connect(self.rebuild) tab.shutting_down.connect(self.delayed_rebuild) tabbed_browser.new_tab.connect(self.on_new_tab) objreg.get("app").new_window.connect(self.on_new_window) @@ -187,6 +188,7 @@ class TabCompletionModel(base.BaseCompletionModel): def on_new_tab(self, tab): """Add hooks to new tabs.""" tab.url_changed.connect(self.rebuild) + tab.title_changed.connect(self.rebuild) tab.shutting_down.connect(self.delayed_rebuild) self.rebuild() From 627f743c26f72792293126e12118d3e53f25d161 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 08:04:55 +0200 Subject: [PATCH 038/160] Handle float in _convert_js_arg This fixes 'gg' with QtWebEngine. --- qutebrowser/utils/javascript.py | 2 +- tests/unit/utils/test_javascript.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/utils/javascript.py b/qutebrowser/utils/javascript.py index b7519596d..a40fa7075 100644 --- a/qutebrowser/utils/javascript.py +++ b/qutebrowser/utils/javascript.py @@ -55,7 +55,7 @@ def _convert_js_arg(arg): return 'undefined' elif isinstance(arg, str): return '"{}"'.format(string_escape(arg)) - elif isinstance(arg, int): + elif isinstance(arg, (int, float)): return str(arg) else: raise TypeError("Don't know how to handle {!r} of type {}!".format( diff --git a/tests/unit/utils/test_javascript.py b/tests/unit/utils/test_javascript.py index 1c86d859d..d11ab0074 100644 --- a/tests/unit/utils/test_javascript.py +++ b/tests/unit/utils/test_javascript.py @@ -126,6 +126,7 @@ class TestStringEscape: ('foobar', '"foobar"'), ('foo\\bar', r'"foo\\bar"'), (42, '42'), + (23.42, '23.42'), (None, 'undefined'), (object(), TypeError), ]) From 64566fff35d32397c52a0bd39a2bb466f4aa4e4b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 08:47:21 +0200 Subject: [PATCH 039/160] Set maximum value for auto-save-interval correctly --- CHANGELOG.asciidoc | 1 + qutebrowser/config/configdata.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index e34f92e4b..4e9c69db8 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -64,6 +64,7 @@ Fixed - Fixed crash when doing `:`, another corner-case introduced in v0.8.0 - Fixed `:open-editor` (``) on Windows +- Fixed crash when setting `general -> auto-save-interval` to a too big value. v0.8.2 ------ diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 785518262..7f2d9049a 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -154,7 +154,7 @@ def data(readonly=False): "Whether to save the config automatically on quit."), ('auto-save-interval', - SettingValue(typ.Int(minval=0), '15000'), + SettingValue(typ.Int(minval=0, maxval=MAXVALS['int']), '15000'), "How often (in milliseconds) to auto-save config/cookies/etc."), ('editor', From 4505d74ae32c8727b02e871dd856ec320a1f29a2 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 09:42:08 +0200 Subject: [PATCH 040/160] Add tests/manual/completion/changing_title.html --- tests/manual/completion/changing_title.html | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 tests/manual/completion/changing_title.html diff --git a/tests/manual/completion/changing_title.html b/tests/manual/completion/changing_title.html new file mode 100644 index 000000000..19e0e6c05 --- /dev/null +++ b/tests/manual/completion/changing_title.html @@ -0,0 +1,11 @@ + + Old title + + + + +

This page should change its title after 3s.

+

When opening the :buffer completion ("gt"), the title should update while it's open.

+ From 5b1ff84e714e63d977660d5826d0ef19958a7b92 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 09:43:15 +0200 Subject: [PATCH 041/160] Update changelog --- CHANGELOG.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 4e9c69db8..81365d3c1 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -49,6 +49,8 @@ Changed - Replacements like `{url}` can now also be used in the middle of an argument. Consequently, commands taking another command (`:later`, `:repeat` and `:bind`) now don't immediately evaluate variables. +- Tab titles in the `:buffer` completion now update correctly when a page's + title is changed via javascript. Removed ~~~~~~~ From c04adb94b21f476c2d3b708e0f543bf08670204d Mon Sep 17 00:00:00 2001 From: Moez Bouhlel Date: Thu, 4 Aug 2016 20:53:13 +0100 Subject: [PATCH 042/160] DocstringParser - support python with optimizations on --- qutebrowser/misc/earlyinit.py | 6 ++++++ qutebrowser/utils/docutils.py | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/qutebrowser/misc/earlyinit.py b/qutebrowser/misc/earlyinit.py index 284478866..256d80f1c 100644 --- a/qutebrowser/misc/earlyinit.py +++ b/qutebrowser/misc/earlyinit.py @@ -300,6 +300,11 @@ def init_log(args): log.init_log(args) log.init.debug("Log initialized.") +def check_optimize_flag(): + from qutebrowser.utils import log + if sys.flags.optimize >= 2: + log.init.warning("Running on optimize level higher than 1, " + "unexpected behaviors may occur.") def earlyinit(args): """Do all needed early initialization. @@ -327,3 +332,4 @@ def earlyinit(args): remove_inputhook() check_libraries(args) check_ssl_support() + check_optimize_flag() diff --git a/qutebrowser/utils/docutils.py b/qutebrowser/utils/docutils.py index d93e7564d..40cb0cb70 100644 --- a/qutebrowser/utils/docutils.py +++ b/qutebrowser/utils/docutils.py @@ -26,7 +26,7 @@ import os.path import collections import qutebrowser -from qutebrowser.utils import usertypes +from qutebrowser.utils import usertypes, log, utils def is_git_repo(): @@ -98,6 +98,15 @@ class DocstringParser: self.State.arg_inside: self._parse_arg_inside, self.State.misc: self._skip, } + if doc is None: + if sys.flags.optimize < 2: + log.commands.warning( + "Function {}() from {} has no docstring".format( + utils.qualname(func), + inspect.getsourcefile(func))) + self.long_desc = "" + self.short_desc = "" + return for line in doc.splitlines(): handler = handlers[self._state] stop = handler(line) From dfbadaf7c24f3245e387f33f1347c2e53d74b820 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 12:52:04 +0200 Subject: [PATCH 043/160] Split WebElementWrapper into abstract/webkit parts --- qutebrowser/browser/commands.py | 12 +- qutebrowser/browser/hints.py | 17 +- qutebrowser/browser/navigate.py | 2 +- qutebrowser/browser/webelem.py | 370 ++++++++++++++++++ qutebrowser/browser/webkit/mhtml.py | 8 +- .../webkit/{webelem.py => webkitelem.py} | 295 +++----------- qutebrowser/browser/webkit/webkittab.py | 6 +- qutebrowser/browser/webkit/webview.py | 16 +- .../{test_webelem.py => test_webkitelem.py} | 48 +-- 9 files changed, 470 insertions(+), 304 deletions(-) create mode 100644 qutebrowser/browser/webelem.py rename qutebrowser/browser/webkit/{webelem.py => webkitelem.py} (58%) rename tests/unit/browser/webkit/{test_webelem.py => test_webkitelem.py} (96%) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 33e778924..6848531db 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -40,7 +40,7 @@ import pygments.formatters from qutebrowser.commands import userscripts, cmdexc, cmdutils, runners from qutebrowser.config import config, configexc from qutebrowser.browser import urlmarks, browsertab, inspector, navigate -from qutebrowser.browser.webkit import webelem, downloads, mhtml +from qutebrowser.browser.webkit import webkitelem, downloads, mhtml from qutebrowser.keyinput import modeman from qutebrowser.utils import (message, usertypes, log, qtutils, urlutils, objreg, utils, typing, javascript) @@ -1422,8 +1422,8 @@ class CommandDispatcher: tab = self._current_widget() page = tab._widget.page() # pylint: disable=protected-access try: - elem = webelem.focus_elem(page.currentFrame()) - except webelem.IsNullError: + elem = webkitelem.focus_elem(page.currentFrame()) + except webkitelem.IsNullError: raise cmdexc.CommandError("No element focused!") if not elem.is_editable(strict=True): raise cmdexc.CommandError("Focused element is not editable!") @@ -1444,7 +1444,7 @@ class CommandDispatcher: """ try: elem.set_text(text, use_js=True) - except webelem.IsNullError: + except webkitelem.IsNullError: raise cmdexc.CommandError("Element vanished while editing!") @cmdutils.register(instance='command-dispatcher', @@ -1456,8 +1456,8 @@ class CommandDispatcher: tab = self._current_widget() page = tab._widget.page() # pylint: disable=protected-access try: - elem = webelem.focus_elem(page.currentFrame()) - except webelem.IsNullError: + elem = webkitelem.focus_elem(page.currentFrame()) + except webkitelem.IsNullError: raise cmdexc.CommandError("No element focused!") if not elem.is_editable(strict=True): raise cmdexc.CommandError("Focused element is not editable!") diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index a886915b5..146a9af35 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -28,12 +28,11 @@ from string import ascii_lowercase from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QEvent, Qt, QUrl, QTimer) from PyQt5.QtGui import QMouseEvent -from PyQt5.QtWebKit import QWebElement from PyQt5.QtWebKitWidgets import QWebPage from qutebrowser.config import config from qutebrowser.keyinput import modeman, modeparsers -from qutebrowser.browser.webkit import webelem +from qutebrowser.browser import webelem from qutebrowser.commands import userscripts, cmdexc, cmdutils, runners from qutebrowser.utils import usertypes, log, qtutils, message, objreg, utils @@ -374,7 +373,7 @@ class HintManager(QObject): for elem in self._context.all_elems: try: elem.label.remove_from_document() - except webelem.IsNullError: + except webelem.Error: pass text = self._get_text() message_bridge = objreg.get('message-bridge', scope='window', @@ -516,7 +515,7 @@ class HintManager(QObject): def _is_hidden(self, elem): """Check if the element is hidden via display=none.""" - display = elem.style_property('display', QWebElement.InlineStyle) + display = elem.style_property('display', strategy='inline') return display == 'none' def _show_elem(self, elem): @@ -767,7 +766,7 @@ class HintManager(QObject): else: # element doesn't match anymore -> hide it self._hide_elem(elem.label) - except webelem.IsNullError: + except webelem.Error: pass def _filter_number_hints(self): @@ -782,7 +781,7 @@ class HintManager(QObject): try: if not self._is_hidden(e.label): elems.append(e) - except webelem.IsNullError: + except webelem.Error: pass if not elems: # Whoops, filtered all hints @@ -813,7 +812,7 @@ class HintManager(QObject): try: if not self._is_hidden(elem.label): visible[string] = elem - except webelem.IsNullError: + except webelem.Error: pass if not visible: # Whoops, filtered all hints @@ -844,7 +843,7 @@ class HintManager(QObject): else: # element doesn't match anymore -> hide it self._hide_elem(elem.label) - except webelem.IsNullError: + except webelem.Error: pass if config.get('hints', 'mode') == 'number': @@ -961,7 +960,7 @@ class HintManager(QObject): e.label.remove_from_document() continue self._set_style_position(e.elem, e.label) - except webelem.IsNullError: + except webelem.Error: pass @pyqtSlot(usertypes.KeyMode) diff --git a/qutebrowser/browser/navigate.py b/qutebrowser/browser/navigate.py index 9689105f3..04d4c6be9 100644 --- a/qutebrowser/browser/navigate.py +++ b/qutebrowser/browser/navigate.py @@ -21,7 +21,7 @@ import posixpath -from qutebrowser.browser.webkit import webelem +from qutebrowser.browser import webelem from qutebrowser.config import config from qutebrowser.utils import (usertypes, objreg, urlutils, log, message, qtutils) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py new file mode 100644 index 000000000..c1910145a --- /dev/null +++ b/qutebrowser/browser/webelem.py @@ -0,0 +1,370 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2014-2016 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 . + +"""Generic web element related code. + +Module attributes: + Group: Enum for different kinds of groups. + SELECTORS: CSS selectors for different groups of elements. + FILTERS: A dictionary of filter functions for the modes. + The filter for "links" filters javascript:-links and a-tags + without "href". +""" + +import collections.abc + +from PyQt5.QtCore import QUrl + +from qutebrowser.config import config +from qutebrowser.utils import log, usertypes, utils, qtutils + + +Group = usertypes.enum('Group', ['all', 'links', 'images', 'url', 'prevnext', + 'focus', 'inputs']) + + +SELECTORS = { + Group.all: ('a, area, textarea, select, input:not([type=hidden]), button, ' + 'frame, iframe, link, [onclick], [onmousedown], [role=link], ' + '[role=option], [role=button], img'), + Group.links: 'a, area, link, [role=link]', + Group.images: 'img', + Group.url: '[src], [href]', + Group.prevnext: 'a, area, button, link, [role=button]', + Group.focus: '*:focus', + Group.inputs: ('input[type=text], input[type=email], input[type=url], ' + 'input[type=tel], input[type=number], ' + 'input[type=password], input[type=search], ' + 'input:not([type]), textarea'), +} + + +def filter_links(elem): + return 'href' in elem and QUrl(elem['href']).scheme() != 'javascript' + + +FILTERS = { + Group.links: filter_links, + Group.prevnext: filter_links, +} + + +class Error(Exception): + + """Base class for WebElement errors.""" + + pass + + +class AbstractWebElement(collections.abc.MutableMapping): + + """A wrapper around QtWebKit/QtWebEngine web element.""" + + def __eq__(self, other): + raise NotImplementedError + + def __str__(self): + raise NotImplementedError + + def __getitem__(self, key): + raise NotImplementedError + + def __setitem__(self, key, val): + raise NotImplementedError + + def __delitem__(self, key): + raise NotImplementedError + + def __contains__(self, key): + raise NotImplementedError + + def __iter__(self): + raise NotImplementedError + + def __len__(self): + raise NotImplementedError + + def __repr__(self): + try: + html = self.debug_text() + except Error: + html = None + return utils.get_repr(self, html=html) + + def frame(self): + """Get the main frame of this element.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def geometry(self): + """Get the geometry for this element.""" + raise NotImplementedError + + def document_element(self): + """Get the document element of this element.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def create_inside(self, tagname): + """Append the given element inside the current one.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def find_first(self, selector): + """Find the first child based on the given CSS selector.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def style_property(self, name, *, strategy): + """Get the element style resolved with the given strategy.""" + raise NotImplementedError + + def classes(self): + """Get a list of classes assigned to this element.""" + raise NotImplementedError + + def tag_name(self): + """Get the tag name of this element.""" + raise NotImplementedError + + def outer_xml(self): + """Get the full HTML representation of this element.""" + raise NotImplementedError + + def text(self, *, use_js=False): + """Get the plain text content for this element. + + Args: + use_js: Whether to use javascript if the element isn't + content-editable. + """ + # FIXME:qtwebengine what to do about use_js with WebEngine? + raise NotImplementedError + + def set_text(self, text, *, use_js=False): + """Set the given plain text. + + Args: + use_js: Whether to use javascript if the element isn't + content-editable. + """ + # FIXME:qtwebengine what to do about use_js with WebEngine? + raise NotImplementedError + + def set_inner_xml(self, xml): + """Set the given inner XML.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def remove_from_document(self): + """Remove the node from the document.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def set_style_property(self, name, value): + """Set the element style.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def run_js_async(self, code, callback=None): + """Run the given JS snippet async on the element.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def parent(self): + """Get the parent element of this element.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, + no_js=False): + """Get the geometry of the element relative to the webview. + + Uses the getClientRects() JavaScript method to obtain the collection of + rectangles containing the element and returns the first rectangle which + is large enough (larger than 1px times 1px). If all rectangles returned + by getClientRects() are too small, falls back to elem.rect_on_view(). + + Skipping of small rectangles is due to elements containing other + elements with "display:block" style, see + https://github.com/The-Compiler/qutebrowser/issues/1298 + + Args: + elem_geometry: The geometry of the element, or None. + Calling QWebElement::geometry is rather expensive so + we want to avoid doing it twice. + adjust_zoom: Whether to adjust the element position based on the + current zoom level. + no_js: Fall back to the Python implementation + """ + raise NotImplementedError + + def is_visible(self, mainframe): + """Check if the given element is visible in the given frame.""" + # FIXME:qtwebengine get rid of this? + raise NotImplementedError + + def is_writable(self): + """Check whether an element is writable.""" + return not ('disabled' in self or 'readonly' in self) + + def is_content_editable(self): + """Check if an element has a contenteditable attribute. + + Args: + elem: The QWebElement to check. + + Return: + True if the element has a contenteditable attribute, + False otherwise. + """ + try: + return self['contenteditable'].lower() not in ['false', 'inherit'] + except KeyError: + return False + + def _is_editable_object(self): + """Check if an object-element is editable.""" + if 'type' not in self: + log.webview.debug(" without type clicked...") + return False + objtype = self['type'].lower() + if objtype.startswith('application/') or 'classid' in self: + # Let's hope flash/java stuff has an application/* mimetype OR + # at least a classid attribute. Oh, and let's hope images/... + # DON'T have a classid attribute. HTML sucks. + log.webview.debug(" clicked.".format(objtype)) + return config.get('input', 'insert-mode-on-plugins') + else: + # Image/Audio/... + return False + + def _is_editable_input(self): + """Check if an input-element is editable. + + Return: + True if the element is editable, False otherwise. + """ + try: + objtype = self['type'].lower() + except KeyError: + return self.is_writable() + else: + if objtype in ['text', 'email', 'url', 'tel', 'number', 'password', + 'search']: + return self.is_writable() + else: + return False + + def _is_editable_div(self): + """Check if a div-element is editable. + + Return: + True if the element is editable, False otherwise. + """ + # Beginnings of div-classes which are actually some kind of editor. + div_classes = ('CodeMirror', # Javascript editor over a textarea + 'kix-', # Google Docs editor + 'ace_') # http://ace.c9.io/ + for klass in self.classes(): + if any([klass.startswith(e) for e in div_classes]): + return True + return False + + def is_editable(self, strict=False): + """Check whether we should switch to insert mode for this element. + + Args: + strict: Whether to do stricter checking so only fields where we can + get the value match, for use with the :editor command. + + Return: + True if we should switch to insert mode, False otherwise. + """ + roles = ('combobox', 'textbox') + log.misc.debug("Checking if element is editable: {}".format( + repr(self))) + tag = self.tag_name().lower() + if self.is_content_editable() and self.is_writable(): + return True + elif self.get('role', None) in roles and self.is_writable(): + return True + elif tag == 'input': + return self._is_editable_input() + elif tag == 'textarea': + return self.is_writable() + elif tag in ['embed', 'applet']: + # Flash/Java/... + return config.get('input', 'insert-mode-on-plugins') and not strict + elif tag == 'object': + return self._is_editable_object() and not strict + elif tag == 'div': + return self._is_editable_div() and not strict + else: + return False + + def is_text_input(self): + """Check if this element is some kind of text box.""" + roles = ('combobox', 'textbox') + tag = self.tag_name().lower() + return self.get('role', None) in roles or tag in ['input', 'textarea'] + + def remove_blank_target(self): + """Remove target from link.""" + elem = self + for _ in range(5): + if elem is None: + break + tag = elem.tag_name().lower() + if tag == 'a' or tag == 'area': + if elem.get('target', None) == '_blank': + elem['target'] = '_top' + break + elem = elem.parent() + + def debug_text(self): + """Get a text based on an element suitable for debug output.""" + return utils.compact_text(self.outer_xml(), 500) + + def resolve_url(self, baseurl): + """Resolve the URL in the element's src/href attribute. + + Args: + baseurl: The URL to base relative URLs on as QUrl. + + Return: + A QUrl with the absolute URL, or None. + """ + if baseurl.isRelative(): + raise ValueError("Need an absolute base URL!") + + for attr in ['href', 'src']: + if attr in self: + text = self[attr].strip() + break + else: + return None + + url = QUrl(text) + if not url.isValid(): + return None + if url.isRelative(): + url = baseurl.resolved(url) + qtutils.ensure_valid(url) + return url diff --git a/qutebrowser/browser/webkit/mhtml.py b/qutebrowser/browser/webkit/mhtml.py index 8a157fa03..2928c5a3f 100644 --- a/qutebrowser/browser/webkit/mhtml.py +++ b/qutebrowser/browser/webkit/mhtml.py @@ -34,7 +34,7 @@ import email.message from PyQt5.QtCore import QUrl -from qutebrowser.browser.webkit import webelem, downloads +from qutebrowser.browser.webkit import webkitelem, downloads from qutebrowser.utils import log, objreg, message, usertypes, utils, urlutils try: @@ -271,7 +271,7 @@ class _Downloader: elements = web_frame.findAllElements('link, script, img') for element in elements: - element = webelem.WebElementWrapper(element) + element = webkitelem.WebKitElement(element) # Websites are free to set whatever rel=... attribute they want. # We just care about stylesheets and icons. if not _check_rel(element): @@ -288,7 +288,7 @@ class _Downloader: styles = web_frame.findAllElements('style') for style in styles: - style = webelem.WebElementWrapper(style) + style = webkitelem.WebKitElement(style) # The Mozilla Developer Network says: # type: This attribute defines the styling language as a MIME type # (charset should not be specified). This attribute is optional and @@ -301,7 +301,7 @@ class _Downloader: # Search for references in inline styles for element in web_frame.findAllElements('[style]'): - element = webelem.WebElementWrapper(element) + element = webkitelem.WebKitElement(element) style = element['style'] for element_url in _get_css_imports(style, inline=True): self._fetch_url(web_url.resolved(QUrl(element_url))) diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webkitelem.py similarity index 58% rename from qutebrowser/browser/webkit/webelem.py rename to qutebrowser/browser/webkit/webkitelem.py index 2eb0e2e05..3bbb8a1ae 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -17,65 +17,26 @@ # You should have received a copy of the GNU General Public License # along with qutebrowser. If not, see . -"""Utilities related to QWebElements. +"""QtWebKit specific part of the web element API.""" -Module attributes: - Group: Enum for different kinds of groups. - SELECTORS: CSS selectors for different groups of elements. - FILTERS: A dictionary of filter functions for the modes. - The filter for "links" filters javascript:-links and a-tags - without "href". -""" - -import collections.abc - -from PyQt5.QtCore import QRect, QUrl +from PyQt5.QtCore import QRect from PyQt5.QtWebKit import QWebElement from qutebrowser.config import config -from qutebrowser.utils import log, usertypes, utils, javascript, qtutils +from qutebrowser.utils import log, utils, javascript +from qutebrowser.browser import webelem -Group = usertypes.enum('Group', ['all', 'links', 'images', 'url', 'prevnext', - 'focus', 'inputs']) +class IsNullError(webelem.Error): - -SELECTORS = { - Group.all: ('a, area, textarea, select, input:not([type=hidden]), button, ' - 'frame, iframe, link, [onclick], [onmousedown], [role=link], ' - '[role=option], [role=button], img'), - Group.links: 'a, area, link, [role=link]', - Group.images: 'img', - Group.url: '[src], [href]', - Group.prevnext: 'a, area, button, link, [role=button]', - Group.focus: '*:focus', - Group.inputs: ('input[type=text], input[type=email], input[type=url], ' - 'input[type=tel], input[type=number], ' - 'input[type=password], input[type=search], ' - 'input:not([type]), textarea'), -} - - -def filter_links(elem): - return 'href' in elem and QUrl(elem['href']).scheme() != 'javascript' - - -FILTERS = { - Group.links: filter_links, - Group.prevnext: filter_links, -} - - -class IsNullError(Exception): - - """Gets raised by WebElementWrapper if an element is null.""" + """Gets raised by WebKitElement if an element is null.""" pass -class WebElementWrapper(collections.abc.MutableMapping): +class WebKitElement(webelem.AbstractWebElement): - """A wrapper around QWebElement to make it more intelligent.""" + """A wrapper around a QWebElement.""" def __init__(self, elem): if isinstance(elem, self.__class__): @@ -85,7 +46,7 @@ class WebElementWrapper(collections.abc.MutableMapping): self._elem = elem def __eq__(self, other): - if not isinstance(other, WebElementWrapper): + if not isinstance(other, WebKitElement): return NotImplemented return self._elem == other._elem # pylint: disable=protected-access @@ -93,13 +54,6 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return self._elem.toPlainText() - def __repr__(self): - try: - html = self.debug_text() - except IsNullError: - html = None - return utils.get_repr(self, html=html) - def __getitem__(self, key): self._check_vanished() if key not in self: @@ -134,24 +88,19 @@ class WebElementWrapper(collections.abc.MutableMapping): raise IsNullError('Element {} vanished!'.format(self._elem)) def frame(self): - """Get the main frame of this element.""" - # FIXME:qtwebengine how to get rid of this? self._check_vanished() return self._elem.webFrame() def geometry(self): - """Get the geometry for this element.""" self._check_vanished() return self._elem.geometry() def document_element(self): - """Get the document element of this element.""" self._check_vanished() elem = self._elem.webFrame().documentElement() - return WebElementWrapper(elem) + return WebKitElement(elem) def create_inside(self, tagname): - """Append the given element inside the current one.""" # It seems impossible to create an empty QWebElement for which isNull() # is false so we can work with it. # As a workaround, we use appendInside() with markup as argument, and @@ -159,28 +108,40 @@ class WebElementWrapper(collections.abc.MutableMapping): # See: http://stackoverflow.com/q/7364852/2085149 self._check_vanished() self._elem.appendInside('<{}>'.format(tagname, tagname)) - return WebElementWrapper(self._elem.lastChild()) + return WebKitElement(self._elem.lastChild()) def find_first(self, selector): - """Find the first child based on the given CSS selector.""" self._check_vanished() elem = self._elem.findFirst(selector) if elem.isNull(): return None - return WebElementWrapper(elem) + return WebKitElement(elem) - def style_property(self, name, strategy): - """Get the element style resolved with the given strategy.""" + def style_property(self, name, *, strategy): self._check_vanished() - return self._elem.styleProperty(name, strategy) + strategies = { + # FIXME:qtwebengine which ones do we actually need? + 'inline': QWebElement.InlineStyle, + 'computed': QWebElement.ComputedStyle, + } + qt_strategy = strategies[strategy] + return self._elem.styleProperty(name, qt_strategy) + + def classes(self): + self._check_vanished() + return self._elem.classes() + + def tag_name(self): + """Get the tag name for the current element.""" + self._check_vanished() + return self._elem.tagName() + + def outer_xml(self): + """Get the full HTML representation of this element.""" + self._check_vanished() + return self._elem.toOuterXml() def text(self, *, use_js=False): - """Get the plain text content for this element. - - Args: - use_js: Whether to use javascript if the element isn't - content-editable. - """ self._check_vanished() if self.is_content_editable() or not use_js: return self._elem.toPlainText() @@ -188,12 +149,6 @@ class WebElementWrapper(collections.abc.MutableMapping): return self._elem.evaluateJavaScript('this.value') def set_text(self, text, *, use_js=False): - """Set the given plain text. - - Args: - use_js: Whether to use javascript if the element isn't - content-editable. - """ self._check_vanished() if self.is_content_editable() or not use_js: log.misc.debug("Filling element {} via set_text.".format( @@ -206,158 +161,17 @@ class WebElementWrapper(collections.abc.MutableMapping): self._elem.evaluateJavaScript("this.value='{}'".format(text)) def set_inner_xml(self, xml): - """Set the given inner XML.""" self._check_vanished() self._elem.setInnerXml(xml) def remove_from_document(self): - """Remove the node from the document.""" self._check_vanished() self._elem.removeFromDocument() def set_style_property(self, name, value): - """Set the element style.""" self._check_vanished() return self._elem.setStyleProperty(name, value) - def is_writable(self): - """Check whether an element is writable.""" - self._check_vanished() - return not ('disabled' in self or 'readonly' in self) - - def is_content_editable(self): - """Check if an element has a contenteditable attribute. - - Args: - elem: The QWebElement to check. - - Return: - True if the element has a contenteditable attribute, - False otherwise. - """ - self._check_vanished() - try: - return self['contenteditable'].lower() not in ['false', 'inherit'] - except KeyError: - return False - - def _is_editable_object(self): - """Check if an object-element is editable.""" - if 'type' not in self: - log.webview.debug(" without type clicked...") - return False - objtype = self['type'].lower() - if objtype.startswith('application/') or 'classid' in self: - # Let's hope flash/java stuff has an application/* mimetype OR - # at least a classid attribute. Oh, and let's hope images/... - # DON'T have a classid attribute. HTML sucks. - log.webview.debug(" clicked.".format(objtype)) - return config.get('input', 'insert-mode-on-plugins') - else: - # Image/Audio/... - return False - - def _is_editable_input(self): - """Check if an input-element is editable. - - Return: - True if the element is editable, False otherwise. - """ - try: - objtype = self['type'].lower() - except KeyError: - return self.is_writable() - else: - if objtype in ['text', 'email', 'url', 'tel', 'number', 'password', - 'search']: - return self.is_writable() - else: - return False - - def _is_editable_div(self): - """Check if a div-element is editable. - - Return: - True if the element is editable, False otherwise. - """ - # Beginnings of div-classes which are actually some kind of editor. - div_classes = ('CodeMirror', # Javascript editor over a textarea - 'kix-', # Google Docs editor - 'ace_') # http://ace.c9.io/ - for klass in self._elem.classes(): - if any([klass.startswith(e) for e in div_classes]): - return True - return False - - def is_editable(self, strict=False): - """Check whether we should switch to insert mode for this element. - - Args: - strict: Whether to do stricter checking so only fields where we can - get the value match, for use with the :editor command. - - Return: - True if we should switch to insert mode, False otherwise. - """ - self._check_vanished() - roles = ('combobox', 'textbox') - log.misc.debug("Checking if element is editable: {}".format( - repr(self))) - tag = self._elem.tagName().lower() - if self.is_content_editable() and self.is_writable(): - return True - elif self.get('role', None) in roles and self.is_writable(): - return True - elif tag == 'input': - return self._is_editable_input() - elif tag == 'textarea': - return self.is_writable() - elif tag in ['embed', 'applet']: - # Flash/Java/... - return config.get('input', 'insert-mode-on-plugins') and not strict - elif tag == 'object': - return self._is_editable_object() and not strict - elif tag == 'div': - return self._is_editable_div() and not strict - else: - return False - - def is_text_input(self): - """Check if this element is some kind of text box.""" - self._check_vanished() - roles = ('combobox', 'textbox') - tag = self._elem.tagName().lower() - return self.get('role', None) in roles or tag in ['input', 'textarea'] - - def remove_blank_target(self): - """Remove target from link.""" - self._check_vanished() - elem = self._elem - for _ in range(5): - if elem is None: - break - tag = elem.tagName().lower() - if tag == 'a' or tag == 'area': - if elem.attribute('target') == '_blank': - elem.setAttribute('target', '_top') - break - elem = elem.parent() - - def debug_text(self): - """Get a text based on an element suitable for debug output.""" - self._check_vanished() - return utils.compact_text(self._elem.toOuterXml(), 500) - - def outer_xml(self): - """Get the full HTML representation of this element.""" - self._check_vanished() - return self._elem.toOuterXml() - - def tag_name(self): - """Get the tag name for the current element.""" - self._check_vanished() - return self._elem.tagName() - def run_js_async(self, code, callback=None): """Run the given JS snippet async on the element.""" self._check_vanished() @@ -365,8 +179,16 @@ class WebElementWrapper(collections.abc.MutableMapping): if callback is not None: callback(result) + def parent(self): + self._check_vanished() + elem = self._elem.parent() + if elem is None: + return None + return WebKitElement(elem) + def _rect_on_view_js(self, adjust_zoom): """Javascript implementation for rect_on_view.""" + # FIXME:qtwebengine maybe we can reuse this? rects = self._elem.evaluateJavaScript("this.getClientRects()") if rects is None: # pragma: no cover # Depending on unknown circumstances, this might not work with JS @@ -444,6 +266,8 @@ class WebElementWrapper(collections.abc.MutableMapping): current zoom level. no_js: Fall back to the Python implementation """ + # FIXME:qtwebengine can we get rid of this with + # find_all_elements(only_visible=True)? self._check_vanished() # First try getting the element rect via JS, as that's usually more @@ -500,33 +324,6 @@ class WebElementWrapper(collections.abc.MutableMapping): visible_in_frame = visible_on_screen return all([visible_on_screen, visible_in_frame]) - def resolve_url(self, baseurl): - """Resolve the URL in the element's src/href attribute. - - Args: - baseurl: The URL to base relative URLs on as QUrl. - - Return: - A QUrl with the absolute URL, or None. - """ - if baseurl.isRelative(): - raise ValueError("Need an absolute base URL!") - - for attr in ['href', 'src']: - if attr in self: - text = self[attr].strip() - break - else: - return None - - url = QUrl(text) - if not url.isValid(): - return None - if url.isRelative(): - url = baseurl.resolved(url) - qtutils.ensure_valid(url) - return url - def get_child_frames(startframe): """Get all children recursively of a given QWebFrame. @@ -556,5 +353,5 @@ def focus_elem(frame): Args: frame: The QWebFrame to search in. """ - elem = frame.findFirstElement(SELECTORS[Group.focus]) - return WebElementWrapper(elem) + elem = frame.findFirstElement(webelem.SELECTORS[webelem.Group.focus]) + return WebKitElement(elem) diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 5dce30abd..0f479886c 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -31,7 +31,7 @@ from PyQt5.QtWebKit import QWebSettings from PyQt5.QtPrintSupport import QPrinter from qutebrowser.browser import browsertab -from qutebrowser.browser.webkit import webview, tabhistory, webelem +from qutebrowser.browser.webkit import webview, tabhistory, webkitelem from qutebrowser.utils import qtutils, objreg, usertypes, utils @@ -564,10 +564,10 @@ class WebKitTab(browsertab.AbstractTab): raise browsertab.WebTabError("No frame focused!") elems = [] - frames = webelem.get_child_frames(mainframe) + frames = webkitelem.get_child_frames(mainframe) for f in frames: for elem in f.findAllElements(selector): - elems.append(webelem.WebElementWrapper(elem)) + elems.append(webkitelem.WebKitElement(elem)) if only_visible: elems = [e for e in elems if e.is_visible(mainframe)] diff --git a/qutebrowser/browser/webkit/webview.py b/qutebrowser/browser/webkit/webview.py index ee562ceda..e871cdb4f 100644 --- a/qutebrowser/browser/webkit/webview.py +++ b/qutebrowser/browser/webkit/webview.py @@ -31,7 +31,7 @@ from qutebrowser.config import config from qutebrowser.keyinput import modeman from qutebrowser.utils import message, log, usertypes, utils, qtutils, objreg from qutebrowser.browser import hints -from qutebrowser.browser.webkit import webpage, webelem +from qutebrowser.browser.webkit import webpage, webkitelem class WebView(QWebView): @@ -196,13 +196,13 @@ class WebView(QWebView): if hitresult.isNull(): # For some reason, the whole hit result can be null sometimes (e.g. # on doodle menu links). If this is the case, we schedule a check - # later (in mouseReleaseEvent) which uses webelem.focus_elem. + # later (in mouseReleaseEvent) which uses webkitelem.focus_elem. log.mouse.debug("Hitresult is null!") self._check_insertmode = True return try: - elem = webelem.WebElementWrapper(hitresult.element()) - except webelem.IsNullError: + elem = webkitelem.WebKitElement(hitresult.element()) + except webkitelem.IsNullError: # For some reason, the hit result element can be a null element # sometimes (e.g. when clicking the timetable fields on # http://www.sbb.ch/ ). If this is the case, we schedule a check @@ -227,8 +227,8 @@ class WebView(QWebView): return self._check_insertmode = False try: - elem = webelem.focus_elem(self.page().currentFrame()) - except (webelem.IsNullError, RuntimeError): + elem = webkitelem.focus_elem(self.page().currentFrame()) + except (webkitelem.IsNullError, RuntimeError): log.mouse.debug("Element/page vanished!") return if elem.is_editable(): @@ -325,8 +325,8 @@ class WebView(QWebView): return frame = self.page().currentFrame() try: - elem = webelem.WebElementWrapper(frame.findFirstElement(':focus')) - except webelem.IsNullError: + elem = webkitelem.WebKitElement(frame.findFirstElement(':focus')) + except webkitelem.IsNullError: log.webview.debug("Focused element is null!") return log.modes.debug("focus element: {}".format(repr(elem))) diff --git a/tests/unit/browser/webkit/test_webelem.py b/tests/unit/browser/webkit/test_webkitelem.py similarity index 96% rename from tests/unit/browser/webkit/test_webelem.py rename to tests/unit/browser/webkit/test_webkitelem.py index 774bc4911..635c3a1b0 100644 --- a/tests/unit/browser/webkit/test_webelem.py +++ b/tests/unit/browser/webkit/test_webkitelem.py @@ -28,13 +28,14 @@ from PyQt5.QtCore import QRect, QPoint, QUrl from PyQt5.QtWebKit import QWebElement import pytest -from qutebrowser.browser.webkit import webelem +from qutebrowser.browser import webelem +from qutebrowser.browser.webkit import webkitelem def get_webelem(geometry=None, frame=None, *, null=False, style=None, attributes=None, tagname=None, classes=None, parent=None, js_rect_return=None, zoom_text_only=False): - """Factory for WebElementWrapper objects based on a mock. + """Factory for WebKitElement objects based on a mock. Args: geometry: The geometry of the QWebElement as QRect. @@ -117,7 +118,7 @@ def get_webelem(geometry=None, frame=None, *, null=False, style=None, return style_dict[name] elem.styleProperty.side_effect = _style_property - wrapped = webelem.WebElementWrapper(elem) + wrapped = webkitelem.WebKitElement(elem) return wrapped @@ -215,15 +216,14 @@ class TestSelectorsAndFilters: # Make sure setting HTML succeeded and there's a new element assert len(webframe.findAllElements('*')) == 3 elems = webframe.findAllElements(webelem.SELECTORS[group]) - elems = [webelem.WebElementWrapper(e) for e in elems] + elems = [webkitelem.WebKitElement(e) for e in elems] filterfunc = webelem.FILTERS.get(group, lambda e: True) elems = [e for e in elems if filterfunc(e)] assert bool(elems) == matching +class TestWebKitElement: -class TestWebElementWrapper: - - """Generic tests for WebElementWrapper. + """Generic tests for WebKitElement. Note: For some methods, there's a dedicated test class with more involved tests. @@ -235,13 +235,13 @@ class TestWebElementWrapper: def test_nullelem(self): """Test __init__ with a null element.""" - with pytest.raises(webelem.IsNullError): + with pytest.raises(webkitelem.IsNullError): get_webelem(null=True) def test_double_wrap(self, elem): - """Test wrapping a WebElementWrapper.""" + """Test wrapping a WebKitElement.""" with pytest.raises(TypeError) as excinfo: - webelem.WebElementWrapper(elem) + webkitelem.WebKitElement(elem) assert str(excinfo.value) == "Trying to wrap a wrapper!" @pytest.mark.parametrize('code', [ @@ -257,7 +257,7 @@ class TestWebElementWrapper: lambda e: e.document_element(), lambda e: e.create_inside('span'), lambda e: e.find_first('span'), - lambda e: e.style_property('visibility', QWebElement.ComputedStyle), + lambda e: e.style_property('visibility', strategy='computed'), lambda e: e.text(), lambda e: e.set_text('foo'), lambda e: e.set_inner_xml(''), @@ -285,16 +285,16 @@ class TestWebElementWrapper: """Make sure methods check if the element is vanished.""" elem._elem.isNull.return_value = True elem._elem.tagName.return_value = 'span' - with pytest.raises(webelem.IsNullError): + with pytest.raises(webkitelem.IsNullError): code(elem) def test_str(self, elem): assert str(elem) == 'text' @pytest.mark.parametrize('is_null, expected', [ - (False, ""), - (True, ''), ]) def test_repr(self, elem, is_null, expected): @@ -334,7 +334,7 @@ class TestWebElementWrapper: def test_eq(self): one = get_webelem() - two = webelem.WebElementWrapper(one._elem) + two = webkitelem.WebKitElement(one._elem) assert one == two def test_eq_other_type(self): @@ -422,7 +422,7 @@ class TestWebElementWrapper: mock.assert_called_with(*args) def test_style_property(self, elem): - assert elem.style_property('foo', QWebElement.ComputedStyle) == 'bar' + assert elem.style_property('foo', strategy='computed') == 'bar' def test_document_element(self, stubs): doc_elem = get_webelem() @@ -430,14 +430,14 @@ class TestWebElementWrapper: elem = get_webelem(frame=frame) doc_elem_ret = elem.document_element() - assert isinstance(doc_elem_ret, webelem.WebElementWrapper) + assert isinstance(doc_elem_ret, webkitelem.WebKitElement) assert doc_elem_ret == doc_elem def test_find_first(self, elem): result = get_webelem() elem._elem.findFirst.return_value = result._elem find_result = elem.find_first('') - assert isinstance(find_result, webelem.WebElementWrapper) + assert isinstance(find_result, webkitelem.WebKitElement) assert find_result == result def test_create_inside(self, elem): @@ -727,7 +727,7 @@ def test_focus_element(stubs): frame = stubs.FakeWebFrame(QRect(0, 0, 100, 100)) elem = get_webelem() frame.focus_elem = elem._elem - assert webelem.focus_elem(frame)._elem is elem._elem + assert webkitelem.focus_elem(frame)._elem is elem._elem class TestRectOnView: @@ -739,7 +739,7 @@ class TestRectOnView: This is needed for all the tests calling rect_on_view or is_visible. """ config_stub.data = {'ui': {'zoom-text-only': 'true'}} - monkeypatch.setattr('qutebrowser.browser.webkit.webelem.config', + monkeypatch.setattr('qutebrowser.browser.webkit.webkitelem.config', config_stub) return config_stub @@ -821,7 +821,7 @@ class TestGetChildFrames: def test_single_frame(self, stubs): """Test get_child_frames with a single frame without children.""" frame = stubs.FakeChildrenFrame() - children = webelem.get_child_frames(frame) + children = webkitelem.get_child_frames(frame) assert len(children) == 1 assert children[0] is frame frame.childFrames.assert_called_once_with() @@ -836,7 +836,7 @@ class TestGetChildFrames: child1 = stubs.FakeChildrenFrame() child2 = stubs.FakeChildrenFrame() parent = stubs.FakeChildrenFrame([child1, child2]) - children = webelem.get_child_frames(parent) + children = webkitelem.get_child_frames(parent) assert len(children) == 3 assert children[0] is parent assert children[1] is child1 @@ -858,7 +858,7 @@ class TestGetChildFrames: first = [stubs.FakeChildrenFrame(second[0:2]), stubs.FakeChildrenFrame(second[2:4])] root = stubs.FakeChildrenFrame(first) - children = webelem.get_child_frames(root) + children = webkitelem.get_child_frames(root) assert len(children) == 7 assert children[0] is root for frame in [root] + first + second: @@ -873,7 +873,7 @@ class TestIsEditable: def stubbed_config(self, config_stub, monkeypatch): """Fixture to create a config stub with an input section.""" config_stub.data = {'input': {}} - monkeypatch.setattr('qutebrowser.browser.webkit.webelem.config', + monkeypatch.setattr('qutebrowser.browser.webkit.webkitelem.config', config_stub) return config_stub From bac263d8a5210769f9687c95bb4549687de4b006 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 13:26:02 +0200 Subject: [PATCH 044/160] Adjust PERFECT_FILES for webelem split --- scripts/dev/check_coverage.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index 4b1b9378a..f705f7a72 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -60,7 +60,9 @@ PERFECT_FILES = [ ('tests/unit/browser/webkit/http/test_content_disposition.py', 'qutebrowser/browser/webkit/rfc6266.py'), ('tests/unit/browser/webkit/test_webelem.py', - 'qutebrowser/browser/webkit/webelem.py'), + 'qutebrowser/browser/webkit/webkitwebelem.py'), + ('tests/unit/browser/webkit/test_webelem.py', + 'qutebrowser/browser/webelem.py'), ('tests/unit/browser/webkit/network/test_schemehandler.py', 'qutebrowser/browser/webkit/network/schemehandler.py'), ('tests/unit/browser/webkit/network/test_filescheme.py', From 7038db6b17f69ddb5e703b47adb3267134671927 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 8 Aug 2016 07:37:53 -0400 Subject: [PATCH 045/160] Combine completion-item-{prev,next}. - Use a single command completion-item-focus with a next/prev argument. - Add a rule to translate old configs. - Regenerate the docs. See #1095. --- doc/help/commands.asciidoc | 16 +++++------ qutebrowser/completion/completionwidget.py | 28 +++++-------------- qutebrowser/config/configdata.py | 10 ++++--- .../unit/completion/test_completionwidget.py | 17 +++++------ 4 files changed, 28 insertions(+), 43 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 9f301c42f..aa3b47658 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -914,8 +914,7 @@ How many steps to zoom out. |<>|Go forward in the commandline history. |<>|Go back in the commandline history. |<>|Delete the current completion item. -|<>|Select the next completion item. -|<>|Select the previous completion item. +|<>|Shift the focus of the completion menu to another item. |<>|Drop selection and keep selection mode enabled. |<>|Enter a key mode. |<>|Follow a hint. @@ -991,13 +990,14 @@ Go back in the commandline history. === completion-item-del Delete the current completion item. -[[completion-item-next]] -=== completion-item-next -Select the next completion item. +[[completion-item-focus]] +=== completion-item-focus +Syntax: +:completion-item-focus 'which'+ -[[completion-item-prev]] -=== completion-item-prev -Select the previous completion item. +Shift the focus of the completion menu to another item. + +==== positional arguments +* +'which'+: 'next' or 'previous' [[drop-selection]] === drop-selection diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 6e83e6fee..3c608951f 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -181,16 +181,14 @@ class CompletionView(QTreeView): # Item is a real item, not a category header -> success return idx - def _next_prev_item(self, prev): - """Handle a tab press for the CompletionView. - - Select the previous/next item and write the new text to the - statusbar. - - Helper for completion_item_next and completion_item_prev. + @cmdutils.register(instance='completion', hide=True, + modes=[usertypes.KeyMode.command], scope='window') + @cmdutils.argument('which', choices=['next', 'prev']) + def completion_item_focus(self, which): + """Shift the focus of the completion menu to another item. Args: - prev: True for prev item, False for next one. + which: 'next' or 'prev' """ # selmodel can be None if 'show' and 'auto-open' are set to False # https://github.com/The-Compiler/qutebrowser/issues/1731 @@ -198,7 +196,7 @@ class CompletionView(QTreeView): if selmodel is None: return - idx = self._next_idx(prev) + idx = self._next_idx(which == 'prev') if not idx.isValid(): return @@ -278,18 +276,6 @@ class CompletionView(QTreeView): scrollbar.setValue(scrollbar.minimum()) super().showEvent(e) - @cmdutils.register(instance='completion', hide=True, - modes=[usertypes.KeyMode.command], scope='window') - def completion_item_prev(self): - """Select the previous completion item.""" - self._next_prev_item(True) - - @cmdutils.register(instance='completion', hide=True, - modes=[usertypes.KeyMode.command], scope='window') - def completion_item_next(self): - """Select the next completion item.""" - self._next_prev_item(False) - @cmdutils.register(instance='completion', hide=True, modes=[usertypes.KeyMode.command], scope='window') def completion_item_del(self): diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 7f2d9049a..df673a50b 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1415,8 +1415,7 @@ KEY_SECTION_DESC = { "Useful hidden commands to map in this section:\n\n" " * `command-history-prev`: Switch to previous command in history.\n" " * `command-history-next`: Switch to next command in history.\n" - " * `completion-item-prev`: Select previous item in completion.\n" - " * `completion-item-next`: Select next item in completion.\n" + " * `completion-item-focus`: Select another item in completion.\n" " * `command-accept`: Execute the command currently in the " "commandline."), 'prompt': ( @@ -1589,8 +1588,8 @@ KEY_DATA = collections.OrderedDict([ ('command', collections.OrderedDict([ ('command-history-prev', ['']), ('command-history-next', ['']), - ('completion-item-prev', ['', '']), - ('completion-item-next', ['', '']), + ('completion-item-focus prev', ['', '']), + ('completion-item-focus next', ['', '']), ('completion-item-del', ['']), ('command-accept', RETURN_KEYS), ])), @@ -1686,4 +1685,7 @@ CHANGED_KEY_COMMANDS = [ (re.compile(r'^yank -p'), r'yank pretty-url'), (re.compile(r'^yank-selected -p'), r'yank selection -s'), (re.compile(r'^yank-selected'), r'yank selection'), + + (re.compile(r'^completion-item-next'), r'completion-item-focus next'), + (re.compile(r'^completion-item-prev'), r'completion-item-focus prev'), ] diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index badc1fc89..f07bfc96b 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -122,7 +122,7 @@ def test_maybe_resize_completion(completionview, config_stub, qtbot): ([[]], 1, None), ([[]], -1, None), ]) -def test_completion_item_next_prev(tree, count, expected, completionview): +def test_completion_item_focus(tree, count, expected, completionview): """Test that on_next_prev_item moves the selection properly. Args: @@ -140,21 +140,18 @@ def test_completion_item_next_prev(tree, count, expected, completionview): filtermodel = sortfilter.CompletionFilterModel(model, parent=completionview) completionview.set_model(filtermodel) - if count < 0: - for _ in range(-count): - completionview.completion_item_prev() - else: - for _ in range(count): - completionview.completion_item_next() + direction = 'prev' if count < 0 else 'next' + for _ in range(abs(count)): + completionview.completion_item_focus(direction) idx = completionview.selectionModel().currentIndex() assert filtermodel.data(idx) == expected -def test_completion_item_next_prev_no_model(completionview): +def test_completion_item_focus_no_model(completionview): """Test that next/prev won't crash with no model set. This can happen if completion.show and completion.auto-open are False. Regression test for issue #1722. """ - completionview.completion_item_prev() - completionview.completion_item_next() + completionview.completion_item_focus('prev') + completionview.completion_item_focus('next') From 92049155af245e9e369e80871bf7133127539c63 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 13:42:37 +0200 Subject: [PATCH 046/160] Whoops... --- scripts/dev/check_coverage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index f705f7a72..98d069a0a 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -60,7 +60,7 @@ PERFECT_FILES = [ ('tests/unit/browser/webkit/http/test_content_disposition.py', 'qutebrowser/browser/webkit/rfc6266.py'), ('tests/unit/browser/webkit/test_webelem.py', - 'qutebrowser/browser/webkit/webkitwebelem.py'), + 'qutebrowser/browser/webkit/webkitelem.py'), ('tests/unit/browser/webkit/test_webelem.py', 'qutebrowser/browser/webelem.py'), ('tests/unit/browser/webkit/network/test_schemehandler.py', From 27330bd4d1ed67447ce610a0b03784e0b3baaadf Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 13:51:42 +0200 Subject: [PATCH 047/160] Fix :search-prev with QtWebEngine Fixes #1797 --- qutebrowser/browser/webengine/webenginetab.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 37d69b491..39c6ed47e 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -95,7 +95,7 @@ class WebEngineSearch(browsertab.AbstractSearch): flags &= ~QWebEnginePage.FindBackward else: flags |= QWebEnginePage.FindBackward - self._find(self.text, self._flags, result_cb) + self._find(self.text, flags, result_cb) def next_result(self, *, result_cb=None): self._find(self.text, self._flags, result_cb) From 0b16a361204080f1ce365901ee92fe99aea96542 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 14:05:30 +0200 Subject: [PATCH 048/160] Clean up handling of focus element Also fixes #1359. --- qutebrowser/browser/webelem.py | 3 +-- qutebrowser/browser/webkit/webkitelem.py | 2 +- qutebrowser/browser/webkit/webview.py | 2 +- tests/unit/browser/webkit/test_webkitelem.py | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index c1910145a..bd4bb20b4 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -36,7 +36,7 @@ from qutebrowser.utils import log, usertypes, utils, qtutils Group = usertypes.enum('Group', ['all', 'links', 'images', 'url', 'prevnext', - 'focus', 'inputs']) + 'inputs']) SELECTORS = { @@ -47,7 +47,6 @@ SELECTORS = { Group.images: 'img', Group.url: '[src], [href]', Group.prevnext: 'a, area, button, link, [role=button]', - Group.focus: '*:focus', Group.inputs: ('input[type=text], input[type=email], input[type=url], ' 'input[type=tel], input[type=number], ' 'input[type=password], input[type=search], ' diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 3bbb8a1ae..71d0b9650 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -353,5 +353,5 @@ def focus_elem(frame): Args: frame: The QWebFrame to search in. """ - elem = frame.findFirstElement(webelem.SELECTORS[webelem.Group.focus]) + elem = frame.findFirstElement('*:focus') return WebKitElement(elem) diff --git a/qutebrowser/browser/webkit/webview.py b/qutebrowser/browser/webkit/webview.py index e871cdb4f..34246e069 100644 --- a/qutebrowser/browser/webkit/webview.py +++ b/qutebrowser/browser/webkit/webview.py @@ -325,7 +325,7 @@ class WebView(QWebView): return frame = self.page().currentFrame() try: - elem = webkitelem.WebKitElement(frame.findFirstElement(':focus')) + elem = webkitelem.focus_elem(frame) except webkitelem.IsNullError: log.webview.debug("Focused element is null!") return diff --git a/tests/unit/browser/webkit/test_webkitelem.py b/tests/unit/browser/webkit/test_webkitelem.py index 635c3a1b0..2fec4d1dc 100644 --- a/tests/unit/browser/webkit/test_webkitelem.py +++ b/tests/unit/browser/webkit/test_webkitelem.py @@ -188,7 +188,7 @@ class SelectionAndFilterTests: webelem.Group.url]), ] - GROUPS = [e for e in webelem.Group if e != webelem.Group.focus] + GROUPS = list(webelem.Group) COMBINATIONS = list(itertools.product(TESTS, GROUPS)) From 9bcce37ff36f67424f2eb7d975da61588d27536f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 14:09:28 +0200 Subject: [PATCH 049/160] WebEngine: Allow :navigate up/decrement/increment --- qutebrowser/browser/commands.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 6848531db..9ff8564cc 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -439,8 +439,7 @@ class CommandDispatcher: """ self._back_forward(tab, bg, window, count, forward=True) - @cmdutils.register(instance='command-dispatcher', scope='window', - backend=usertypes.Backend.QtWebKit) + @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('where', choices=['prev', 'next', 'up', 'increment', 'decrement']) def navigate(self, where: str, tab=False, bg=False, window=False): From 1c73751fd9958e767f0b28831a25473bceef00f8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 14:11:01 +0200 Subject: [PATCH 050/160] Always lowercase tagName --- qutebrowser/browser/webelem.py | 11 +++++++---- qutebrowser/browser/webkit/webkitelem.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index bd4bb20b4..8bb60d4ea 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -139,7 +139,10 @@ class AbstractWebElement(collections.abc.MutableMapping): raise NotImplementedError def tag_name(self): - """Get the tag name of this element.""" + """Get the tag name of this element. + + The returned name will always be lower-case. + """ raise NotImplementedError def outer_xml(self): @@ -299,7 +302,7 @@ class AbstractWebElement(collections.abc.MutableMapping): roles = ('combobox', 'textbox') log.misc.debug("Checking if element is editable: {}".format( repr(self))) - tag = self.tag_name().lower() + tag = self.tag_name() if self.is_content_editable() and self.is_writable(): return True elif self.get('role', None) in roles and self.is_writable(): @@ -321,7 +324,7 @@ class AbstractWebElement(collections.abc.MutableMapping): def is_text_input(self): """Check if this element is some kind of text box.""" roles = ('combobox', 'textbox') - tag = self.tag_name().lower() + tag = self.tag_name() return self.get('role', None) in roles or tag in ['input', 'textarea'] def remove_blank_target(self): @@ -330,7 +333,7 @@ class AbstractWebElement(collections.abc.MutableMapping): for _ in range(5): if elem is None: break - tag = elem.tag_name().lower() + tag = elem.tag_name() if tag == 'a' or tag == 'area': if elem.get('target', None) == '_blank': elem['target'] = '_top' diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 71d0b9650..46f4fb8e9 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -134,7 +134,7 @@ class WebKitElement(webelem.AbstractWebElement): def tag_name(self): """Get the tag name for the current element.""" self._check_vanished() - return self._elem.tagName() + return self._elem.tagName().lower() def outer_xml(self): """Get the full HTML representation of this element.""" From b8e2d5f8f6a3547fede59e1dbc8e65f5e3c7358f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 15:01:41 +0200 Subject: [PATCH 051/160] Add initial WebEngineElement implementation This allows :navigate prev/next to work correctly via the javascript bridge. --- qutebrowser/browser/navigate.py | 5 - qutebrowser/browser/webelem.py | 5 +- .../browser/webengine/webengineelem.py | 176 ++++++++++++++++++ qutebrowser/browser/webengine/webenginetab.py | 22 ++- qutebrowser/browser/webkit/webkitelem.py | 4 - qutebrowser/javascript/webelem.js | 63 +++++++ 6 files changed, 259 insertions(+), 16 deletions(-) create mode 100644 qutebrowser/browser/webengine/webengineelem.py create mode 100644 qutebrowser/javascript/webelem.js diff --git a/qutebrowser/browser/navigate.py b/qutebrowser/browser/navigate.py index 04d4c6be9..a76bec05d 100644 --- a/qutebrowser/browser/navigate.py +++ b/qutebrowser/browser/navigate.py @@ -109,11 +109,6 @@ def prevnext(*, browsertab, win_id, baseurl, prev=False, background: True to open in a background tab. window: True to open in a new window, False for the current one. """ - # FIXME:qtwebengine have a proper API for this - if browsertab.backend == usertypes.Backend.QtWebEngine: - raise Error(":navigate prev/next is not supported yet with " - "QtWebEngine") - def _prevnext_cb(elems): elem = _find_prevnext(prev, elems) word = 'prev' if prev else 'forward' diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index 8bb60d4ea..749cb151f 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -79,7 +79,7 @@ class AbstractWebElement(collections.abc.MutableMapping): raise NotImplementedError def __str__(self): - raise NotImplementedError + return self.text() def __getitem__(self, key): raise NotImplementedError @@ -90,9 +90,6 @@ class AbstractWebElement(collections.abc.MutableMapping): def __delitem__(self, key): raise NotImplementedError - def __contains__(self, key): - raise NotImplementedError - def __iter__(self): raise NotImplementedError diff --git a/qutebrowser/browser/webengine/webengineelem.py b/qutebrowser/browser/webengine/webengineelem.py new file mode 100644 index 000000000..69de7c906 --- /dev/null +++ b/qutebrowser/browser/webengine/webengineelem.py @@ -0,0 +1,176 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2016 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 . + +# FIXME:qtwebengine remove this once the stubs are gone +# pylint: disable=unused-variable + +"""QtWebEngine specific part of the web element API.""" + +from PyQt5.QtCore import QRect + +from qutebrowser.utils import log +from qutebrowser.browser import webelem + + +class WebEngineElement(webelem.AbstractWebElement): + + """A web element for QtWebEngine, using JS under the hood.""" + + def __init__(self, js_dict): + self._id = js_dict['id'] + self._js_dict = js_dict + + def __eq__(self, other): + if not isinstance(other, WebEngineElement): + return NotImplemented + return self._id == other._id # pylint: disable=protected-access + + def __getitem__(self, key): + attrs = self._js_dict['attributes'] + return attrs[key] + + def __setitem__(self, key, val): + log.stub() + + def __delitem__(self, key): + log.stub() + + def __iter__(self): + return iter(self._js_dict['attributes']) + + def __len__(self): + return len(self._js_dict['attributes']) + + def frame(self): + log.stub() + return None + + def geometry(self): + log.stub() + return QRect() + + def document_element(self): + log.stub() + return None + + def create_inside(self, tagname): + log.stub() + return None + + def find_first(self, selector): + log.stub() + return None + + def style_property(self, name, *, strategy): + log.stub() + return '' + + def classes(self): + """Get a list of classes assigned to this element.""" + log.stub() + return [] + + def tag_name(self): + """Get the tag name of this element. + + The returned name will always be lower-case. + """ + return self._js_dict['tag_name'] + + def outer_xml(self): + """Get the full HTML representation of this element.""" + return self._js_dict['outer_xml'] + + def text(self, *, use_js=False): + """Get the plain text content for this element. + + Args: + use_js: Whether to use javascript if the element isn't + content-editable. + """ + if use_js: + # FIXME:qtwebengine what to do about use_js with WebEngine? + log.stub('with use_js=True') + return self._js_dict.get('text', '') + + def set_text(self, text, *, use_js=False): + """Set the given plain text. + + Args: + use_js: Whether to use javascript if the element isn't + content-editable. + """ + # FIXME:qtwebengine what to do about use_js with WebEngine? + log.stub() + + def set_inner_xml(self, xml): + """Set the given inner XML.""" + # FIXME:qtwebengine get rid of this? + log.stub() + + def remove_from_document(self): + """Remove the node from the document.""" + # FIXME:qtwebengine get rid of this? + log.stub() + + def set_style_property(self, name, value): + """Set the element style.""" + # FIXME:qtwebengine get rid of this? + log.stub() + + def run_js_async(self, code, callback=None): + """Run the given JS snippet async on the element.""" + # FIXME:qtwebengine get rid of this? + log.stub() + + def parent(self): + """Get the parent element of this element.""" + # FIXME:qtwebengine get rid of this? + log.stub() + return None + + def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, + no_js=False): + """Get the geometry of the element relative to the webview. + + Uses the getClientRects() JavaScript method to obtain the collection of + rectangles containing the element and returns the first rectangle which + is large enough (larger than 1px times 1px). If all rectangles returned + by getClientRects() are too small, falls back to elem.rect_on_view(). + + Skipping of small rectangles is due to elements containing other + elements with "display:block" style, see + https://github.com/The-Compiler/qutebrowser/issues/1298 + + Args: + elem_geometry: The geometry of the element, or None. + Calling QWebElement::geometry is rather expensive so + we want to avoid doing it twice. + adjust_zoom: Whether to adjust the element position based on the + current zoom level. + no_js: Fall back to the Python implementation + """ + log.stub() + return QRect() + + def is_visible(self, mainframe): + """Check if the given element is visible in the given frame.""" + # FIXME:qtwebengine get rid of this? + log.stub() + return True diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 39c6ed47e..3ac460a87 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -22,6 +22,8 @@ """Wrapper over a QWebEngineView.""" +import functools + from PyQt5.QtCore import pyqtSlot, Qt, QEvent, QPoint from PyQt5.QtGui import QKeyEvent, QIcon from PyQt5.QtWidgets import QApplication @@ -30,7 +32,7 @@ from PyQt5.QtWebEngineWidgets import QWebEnginePage, QWebEngineScript # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import browsertab -from qutebrowser.browser.webengine import webview +from qutebrowser.browser.webengine import webview, webengineelem from qutebrowser.utils import usertypes, qtutils, log, javascript @@ -411,9 +413,23 @@ class WebEngineTab(browsertab.AbstractTab): def clear_ssl_errors(self): log.stub() + def _find_all_elements_js_cb(self, callback, js_elems): + """Handle found elements coming from JS and call the real callback. + + Args: + callback: The callback originally passed to find_all_elements. + js_elems: The elements serialized from javascript. + """ + elems = [] + for js_elem in js_elems: + elem = webengineelem.WebEngineElement(js_elem) + elems.append(elem) + callback(elems) + def find_all_elements(self, selector, callback, *, only_visible=False): - log.stub() - callback([]) + js_code = javascript.assemble('webelem', 'find_all_elements', selector) + js_cb = functools.partial(self._find_all_elements_js_cb, callback) + self.run_js_async(js_code, js_cb) def _connect_signals(self): view = self._widget diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 46f4fb8e9..895c48e59 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -50,10 +50,6 @@ class WebKitElement(webelem.AbstractWebElement): return NotImplemented return self._elem == other._elem # pylint: disable=protected-access - def __str__(self): - self._check_vanished() - return self._elem.toPlainText() - def __getitem__(self, key): self._check_vanished() if key not in self: diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js new file mode 100644 index 000000000..9aa132e41 --- /dev/null +++ b/qutebrowser/javascript/webelem.js @@ -0,0 +1,63 @@ +/** + * Copyright 2016 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 . + */ + + +document._qutebrowser_elements = []; + + +function _qutebrowser_serialize_elem(elem, id) { + var out = {}; + + var attributes = {}; + for (var i = 0; i < elem.attributes.length; ++i) { + attr = elem.attributes[i]; + attributes[attr.name] = attr.value; + } + out["attributes"] = attributes; + + out["text"] = elem.text; + out["tag_name"] = elem.tagName; + out["outer_xml"] = elem.outerHTML; + out["id"] = id; + + // console.log(JSON.stringify(out)); + + return out; +} + + +function _qutebrowser_find_all_elements(selector) { + var elems = document.querySelectorAll(selector); + var out = []; + var id = document._qutebrowser_elements.length; + + for (var i = 0; i < elems.length; ++i) { + var elem = elems[i]; + out.push(_qutebrowser_serialize_elem(elem, id)); + document._qutebrowser_elements[id] = elem; + id++; + } + + return out; +} + + +function _qutebrowser_get_element(id) { + return document._qutebrowser_elements[id]; +} From 3c573ac187c7c11adab459a91bf7fb8dc6dc53ec Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 15:13:30 +0200 Subject: [PATCH 052/160] Fix upper-case tag names for WordHinter --- qutebrowser/browser/hints.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 146a9af35..278ad7d0d 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -1027,9 +1027,9 @@ class WordHinter: } extractable_attrs = collections.defaultdict(list, { - "IMG": ["alt", "title", "src"], - "A": ["title", "href", "text"], - "INPUT": ["name"] + "img": ["alt", "title", "src"], + "a": ["title", "href", "text"], + "input": ["name"] }) return (attr_extractors[attr](elem) From ded214376134644c23d36acf527a3008f239a6be Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 15:20:04 +0200 Subject: [PATCH 053/160] test requirements: Update pytest-cov to 2.3.1 - Fixed regression causing spurious errors when xdist was used. - Fixed DeprecationWarning about incorrect addoption use. - Fixed deprecated use of funcarg fixture API. --- misc/requirements/requirements-tests.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/requirements/requirements-tests.txt b/misc/requirements/requirements-tests.txt index ecec6b37b..703c8786e 100644 --- a/misc/requirements/requirements-tests.txt +++ b/misc/requirements/requirements-tests.txt @@ -18,7 +18,7 @@ py==1.4.31 pytest==2.9.2 pytest-bdd==2.17.0 pytest-catchlog==1.2.2 -pytest-cov==2.3.0 +pytest-cov==2.3.1 pytest-faulthandler==1.3.0 pytest-instafail==0.3.0 pytest-mock==1.2 From 7c689d83bfa04722f568141256e0fe391e3b3cd3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 15:22:57 +0200 Subject: [PATCH 054/160] Update docs --- CHANGELOG.asciidoc | 2 ++ doc/help/commands.asciidoc | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 81365d3c1..f1a585a18 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -57,6 +57,8 @@ Removed - The `:yank-selected` command got merged into `:yank` as `:yank selection` and thus removed. +- The `:completion-item-prev` and `:completion-item-next` commands got merged + into a new `:completion-focus {prev,next}` command and thus removed. v0.8.3 (unreleased) ------------------- diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index aa3b47658..36c2d9653 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -997,7 +997,7 @@ Syntax: +:completion-item-focus 'which'+ Shift the focus of the completion menu to another item. ==== positional arguments -* +'which'+: 'next' or 'previous' +* +'which'+: 'next' or 'prev' [[drop-selection]] === drop-selection From 58fb41ab9da860bc576b721515783b34a459e5af Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 15:28:11 +0200 Subject: [PATCH 055/160] tests: Fix webkitelem test for tagName --- tests/unit/browser/webkit/test_webkitelem.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/browser/webkit/test_webkitelem.py b/tests/unit/browser/webkit/test_webkitelem.py index 2fec4d1dc..ea613699e 100644 --- a/tests/unit/browser/webkit/test_webkitelem.py +++ b/tests/unit/browser/webkit/test_webkitelem.py @@ -402,7 +402,6 @@ class TestWebKitElement: ('webFrame', lambda e: e.frame()), ('geometry', lambda e: e.geometry()), ('toOuterXml', lambda e: e.outer_xml()), - ('tagName', lambda e: e.tag_name()), ]) def test_simple_getters(self, elem, attribute, code): sentinel = object() @@ -421,6 +420,10 @@ class TestWebKitElement: mock = getattr(elem._elem, method) mock.assert_called_with(*args) + def test_tag_name(self, elem): + elem._elem.tagName.return_value = 'SPAN' + assert elem.tag_name() == 'span' + def test_style_property(self, elem): assert elem.style_property('foo', strategy='computed') == 'bar' From 9a17591fb7761bbe31c853ca01d22f1c82037944 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 16:24:34 +0200 Subject: [PATCH 056/160] Start getting :open-editor to work with WebEngine It doesn't actually work yet (as it claims the field is not editable), but at least does not crash when the backend limitation for the command is removed. --- qutebrowser/browser/browsertab.py | 9 ++++++ qutebrowser/browser/commands.py | 29 ++++++++++--------- qutebrowser/browser/webengine/webenginetab.py | 19 ++++++++++++ qutebrowser/browser/webkit/webkittab.py | 12 ++++++++ qutebrowser/browser/webkit/webview.py | 1 + qutebrowser/javascript/webelem.js | 25 ++++++++++++---- 6 files changed, 76 insertions(+), 19 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index a0362c4c9..662c88b5f 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -613,6 +613,15 @@ class AbstractTab(QWidget): """ raise NotImplementedError + def find_focus_element(self, callback): + """Find the focused element on the page async. + + Args: + callback: The callback to be called when the search finished. + Called with a WebEngineElement or None. + """ + raise NotImplementedError + def __repr__(self): try: url = utils.elide(self.url().toDisplayString(QUrl.EncodeUnicode), diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 9ff8564cc..41256c6f8 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1408,6 +1408,21 @@ class CommandDispatcher: url = QUrl('qute://log?level={}'.format(level)) self._open(url, tab, bg, window) + def _open_editor_cb(self, elem): + """Open editor after the focus elem was found in open_editor.""" + if elem is None: + message.error(self._win_id, "No element focused!") + return + if not elem.is_editable(strict=True): + message.error(self._win_id, "Focused element is not editable!") + return + + text = elem.text(use_js=True) + ed = editor.ExternalEditor(self._win_id, self._tabbed_browser) + ed.editing_finished.connect(functools.partial( + self.on_editing_finished, elem)) + ed.edit(text) + @cmdutils.register(instance='command-dispatcher', modes=[KeyMode.insert], hide=True, scope='window', backend=usertypes.Backend.QtWebKit) @@ -1417,20 +1432,8 @@ class CommandDispatcher: The editor which should be launched can be configured via the `general -> editor` config option. """ - # FIXME:qtwebengine have a proper API for this tab = self._current_widget() - page = tab._widget.page() # pylint: disable=protected-access - try: - elem = webkitelem.focus_elem(page.currentFrame()) - except webkitelem.IsNullError: - raise cmdexc.CommandError("No element focused!") - if not elem.is_editable(strict=True): - raise cmdexc.CommandError("Focused element is not editable!") - text = elem.text(use_js=True) - ed = editor.ExternalEditor(self._win_id, self._tabbed_browser) - ed.editing_finished.connect(functools.partial( - self.on_editing_finished, elem)) - ed.edit(text) + tab.find_focus_element(self._open_editor_cb) def on_editing_finished(self, elem, text): """Write the editor text into the form field and clean up tempfile. diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 3ac460a87..7f43d352a 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -431,6 +431,25 @@ class WebEngineTab(browsertab.AbstractTab): js_cb = functools.partial(self._find_all_elements_js_cb, callback) self.run_js_async(js_code, js_cb) + def _find_focus_element_js_cb(self, callback, js_elem): + """Handle a found focus elem coming from JS and call the real callback. + + Args: + callback: The callback originally passed to find_focus_element. + Called with a WebEngineElement or None. + js_elem: The element serialized from javascript. + """ + log.webview.debug("Got focus element from JS: {!r}".format(js_elem)) + if js_elem is None: + callback(None) + else: + callback(webengineelem.WebEngineElement(js_elem)) + + def find_focus_element(self, callback): + js_code = javascript.assemble('webelem', 'focus_element') + js_cb = functools.partial(self._find_focus_element_js_cb, callback) + self.run_js_async(js_code, js_cb) + def _connect_signals(self): view = self._widget page = view.page() diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 0f479886c..47f866559 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -574,6 +574,18 @@ class WebKitTab(browsertab.AbstractTab): callback(elems) + def find_focus_element(self, callback): + frame = self._widget.page().currentFrame() + if frame is None: + callback(None) + return + + elem = frame.findFirstElement('*:focus') + if elem.isNull(): + callback(None) + else: + callback(webkitelem.WebKitElement(elem)) + @pyqtSlot() def _on_frame_load_finished(self): """Make sure we emit an appropriate status when loading finished. diff --git a/qutebrowser/browser/webkit/webview.py b/qutebrowser/browser/webkit/webview.py index 34246e069..adf2ea925 100644 --- a/qutebrowser/browser/webkit/webview.py +++ b/qutebrowser/browser/webkit/webview.py @@ -223,6 +223,7 @@ class WebView(QWebView): def mouserelease_insertmode(self): """If we have an insertmode check scheduled, handle it.""" + # FIXME:qtwebengine Use tab.find_focus_element here if not self._check_insertmode: return self._check_insertmode = False diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 9aa132e41..9163d6727 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -22,7 +22,12 @@ document._qutebrowser_elements = []; function _qutebrowser_serialize_elem(elem, id) { - var out = {}; + var out = { + "id": id, + "text": elem.text, + "tag_name": elem.tagName, + "outer_xml": elem.outerHTML + }; var attributes = {}; for (var i = 0; i < elem.attributes.length; ++i) { @@ -31,11 +36,6 @@ function _qutebrowser_serialize_elem(elem, id) { } out["attributes"] = attributes; - out["text"] = elem.text; - out["tag_name"] = elem.tagName; - out["outer_xml"] = elem.outerHTML; - out["id"] = id; - // console.log(JSON.stringify(out)); return out; @@ -58,6 +58,19 @@ function _qutebrowser_find_all_elements(selector) { } +function _qutebrowser_focus_element() { + var elem = document.activeElement; + if (!elem || elem === document.body) { + // "When there is no selection, the active element is the page's + // or null." + return null; + } + + var id = document._qutebrowser_elements.length; + return _qutebrowser_serialize_elem(elem, id); +} + + function _qutebrowser_get_element(id) { return document._qutebrowser_elements[id]; } From af8302b678c3b4ab50834071ddff2c01bcf4de44 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 8 Aug 2016 16:32:23 +0200 Subject: [PATCH 057/160] Remove unused import --- qutebrowser/browser/navigate.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/browser/navigate.py b/qutebrowser/browser/navigate.py index a76bec05d..235cb0a7a 100644 --- a/qutebrowser/browser/navigate.py +++ b/qutebrowser/browser/navigate.py @@ -23,8 +23,7 @@ import posixpath from qutebrowser.browser import webelem from qutebrowser.config import config -from qutebrowser.utils import (usertypes, objreg, urlutils, log, message, - qtutils) +from qutebrowser.utils import objreg, urlutils, log, message, qtutils class Error(Exception): From cf26201e86e6e53d0ef4240935be7faffc32bd49 Mon Sep 17 00:00:00 2001 From: Niklas Haas Date: Mon, 8 Aug 2016 17:28:13 +0200 Subject: [PATCH 058/160] Extract hint tags from This is useful for very many input fields, especially prominent on GitHub itself. --- qutebrowser/browser/hints.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 278ad7d0d..045c8456a 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -1021,6 +1021,7 @@ class WordHinter: "alt": lambda elem: elem["alt"], "name": lambda elem: elem["name"], "title": lambda elem: elem["title"], + "placeholder": lambda elem: elem["placeholder"], "src": lambda elem: elem["src"].split('/')[-1], "href": lambda elem: elem["href"].split('/')[-1], "text": str, @@ -1029,7 +1030,7 @@ class WordHinter: extractable_attrs = collections.defaultdict(list, { "img": ["alt", "title", "src"], "a": ["title", "href", "text"], - "input": ["name"] + "input": ["name", "placeholder"] }) return (attr_extractors[attr](elem) From 6e279f1b1e36fdc6013eef27c028773aedbf0548 Mon Sep 17 00:00:00 2001 From: Niklas Haas Date: Mon, 8 Aug 2016 17:49:35 +0200 Subject: [PATCH 059/160] Extract hint tags from