From 2c4bb064e69d7e49a70090fd3df84c66277a1405 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Thu, 17 Dec 2020 17:25:00 -0500 Subject: Show set-cmd-text command bindings in completion. Fixes #5942. For bindings like `o -> :set-cmd-text -s :open`, the user conceptually expects that `o` maps to `open`, yet they do not show in the command completion UI. With this patch, a binding to a command of form `set-cmd-text [flags...] : [cmdargs]` will be treated as if it were bound to ` [cmdargs]` instead for the purpose of completion. Bindings to `set-cmd-text --append` are ignored. --- qutebrowser/config/config.py | 31 +++++++++++++++++++++++++++++-- tests/unit/config/test_config.py | 20 ++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index d8e8d612e..21616cb56 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -162,13 +162,40 @@ class KeyConfig: bindings[key] = binding return bindings + def _strip_leading_flags(self, cmdline: str) -> List[str]: + """Split cmdline at whitespace until the first non-flag.""" + first, _, rest = cmdline.partition(" ") + if first.startswith("-"): + return [first] + self._strip_leading_flags(rest) + return [cmdline] + + def _implied_cmd(self, cmdline: str) -> str: + """Return cmdline, or the implied cmd if cmdline is a set-cmd-text.""" + if not cmdline.startswith("set-cmd-text "): + return cmdline + cmdline = cmdline[len("set-cmd-text "):] + *flags, cmd = self._strip_leading_flags(cmdline) + if "-a" in flags or "--append" in flags or not cmd.startswith(":"): + return "" # doesn't look like this sets a command + return cmd.lstrip(":") + def get_reverse_bindings_for(self, mode: str) -> '_ReverseBindings': - """Get a dict of commands to a list of bindings for the mode.""" + """Get a dict of commands to a list of bindings for the mode. + + This is intented for user-facing display of keybindings. + As such, bindings for 'set-cmd-text [flags] : ...' are translated + to ' ...', as from the user's perspective these keys behave like + bindings for '' (that allow for further input before running). + + See #5942. + """ cmd_to_keys: KeyConfig._ReverseBindings = {} bindings = self.get_bindings_for(mode) for seq, full_cmd in sorted(bindings.items()): for cmd in full_cmd.split(';;'): - cmd = cmd.strip() + cmd = self._implied_cmd(cmd.strip()) + if not cmd: + continue cmd_to_keys.setdefault(cmd, []) # Put bindings involving modifiers last if any(info.modifiers for info in seq): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index b30ab4bee..5e97cf714 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -198,6 +198,26 @@ class TestKeyConfig: # Chained command ({'a': 'message-info foo ;; message-info bar'}, {'message-info foo': ['a'], 'message-info bar': ['a']}), + # Command using set-cmd-text (#5942) + ( + { + "o": "set-cmd-text -s :open", + "O": "set-cmd-text -s :open -t", + "go": "set-cmd-text :open {url:pretty}", + # all of these should be ignored + "/": "set-cmd-text /", + "?": "set-cmd-text ?", + ":": "set-cmd-text :", + "a": "set-cmd-text no_leading_colon", + "b": "set-cmd-text -s -a :skip_cuz_append", + "c": "set-cmd-text --append :skip_cuz_append", + }, + { + "open": ["o"], + "open -t": ["O"], + "open {url:pretty}": ["go"], + } + ), ]) def test_get_reverse_bindings_for(self, key_config_stub, config_stub, no_bindings, bindings, expected): -- cgit v1.2.3-54-g00ecf From 4cf6d910498f8a4cf7636a0d278c8bd8458f058b Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 27 Dec 2020 12:08:40 -0500 Subject: Add CommandParser tests. All the existing tests just verify whether parsing suceeds or not, but don't verify the result. These tests make it easier to see what the intended behavior of CommandParser is. --- tests/unit/commands/test_runners.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit/commands/test_runners.py b/tests/unit/commands/test_runners.py index 497a99663..0d7f27fa7 100644 --- a/tests/unit/commands/test_runners.py +++ b/tests/unit/commands/test_runners.py @@ -67,6 +67,30 @@ class TestCommandParser: with pytest.raises(cmdexc.NoSuchCommandError): parser.parse_all(command) + @pytest.mark.parametrize('command, name, args', [ + ("set-cmd-text -s :open", "set-cmd-text", ["-s", ":open"]), + ("set-cmd-text :open {url:pretty}", "set-cmd-text", + [":open {url:pretty}"]), + ("set-cmd-text -s :open -t", "set-cmd-text", ["-s", ":open -t"]), + ("set-cmd-text :open -t -r {url:pretty}", "set-cmd-text", + [":open -t -r {url:pretty}"]), + ("set-cmd-text -s :open -b", "set-cmd-text", ["-s", ":open -b"]), + ("set-cmd-text :open -b -r {url:pretty}", "set-cmd-text", + [":open -b -r {url:pretty}"]), + ("set-cmd-text -s :open -w", "set-cmd-text", + ["-s", ":open -w"]), + ("set-cmd-text :open -w {url:pretty}", "set-cmd-text", + [":open -w {url:pretty}"]), + ("set-cmd-text /", "set-cmd-text", ["/"]), + ("set-cmd-text ?", "set-cmd-text", ["?"]), + ("set-cmd-text :", "set-cmd-text", [":"]), + ]) + def test_parse_result(self, command, name, args): + parser = runners.CommandParser() + result = parser.parse_all(command, aliases=False)[0] + assert result.cmd.name == name + assert result.args == args + class TestCompletions: -- cgit v1.2.3-54-g00ecf From 3b7d459e0df635fa513472a8a00e84dc67c48aa1 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 27 Dec 2020 13:11:44 -0500 Subject: Use CommandParser for binding completion. Reduce the amount of custom parsing logic in config.py. Turns out circular import isn't a problem here. Some of the test cases in `test_get_reverse_bindings_for` had to be changed from `message-info` to `open`, as CommandParser expects commands to actually exist, and `message-info` isn't registered during those tests. --- qutebrowser/config/config.py | 20 ++++++++++---------- tests/unit/config/test_config.py | 16 ++++++++-------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 21616cb56..b2ba1a6a5 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -27,6 +27,7 @@ from typing import (TYPE_CHECKING, Any, Callable, Dict, Iterator, List, Mapping, from PyQt5.QtCore import pyqtSignal, QObject, QUrl +from qutebrowser.commands import cmdexc, runners from qutebrowser.config import configdata, configexc, configutils from qutebrowser.utils import utils, log, urlmatch from qutebrowser.misc import objects @@ -162,19 +163,18 @@ class KeyConfig: bindings[key] = binding return bindings - def _strip_leading_flags(self, cmdline: str) -> List[str]: - """Split cmdline at whitespace until the first non-flag.""" - first, _, rest = cmdline.partition(" ") - if first.startswith("-"): - return [first] + self._strip_leading_flags(rest) - return [cmdline] - def _implied_cmd(self, cmdline: str) -> str: """Return cmdline, or the implied cmd if cmdline is a set-cmd-text.""" - if not cmdline.startswith("set-cmd-text "): + try: + results = runners.CommandParser().parse_all(cmdline, aliases=False) + if len(results) == 0: + return "" + result = results[0] + except cmdexc.NoSuchCommandError: + return "" + if result.cmd.name != "set-cmd-text": return cmdline - cmdline = cmdline[len("set-cmd-text "):] - *flags, cmd = self._strip_leading_flags(cmdline) + *flags, cmd = result.args if "-a" in flags or "--append" in flags or not cmd.startswith(":"): return "" # doesn't look like this sets a command return cmd.lstrip(":") diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 5e97cf714..b0c32c6fb 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -187,17 +187,17 @@ class TestKeyConfig: @pytest.mark.parametrize('bindings, expected', [ # Simple - ({'a': 'message-info foo', 'b': 'message-info bar'}, - {'message-info foo': ['a'], 'message-info bar': ['b']}), + ({'a': 'open foo', 'b': 'open bar'}, + {'open foo': ['a'], 'open bar': ['b']}), # Multiple bindings - ({'a': 'message-info foo', 'b': 'message-info foo'}, - {'message-info foo': ['b', 'a']}), + ({'a': 'open foo', 'b': 'open foo'}, + {'open foo': ['b', 'a']}), # With modifier keys (should be listed last and normalized) - ({'a': 'message-info foo', '': 'message-info foo'}, - {'message-info foo': ['a', '']}), + ({'a': 'open foo', '': 'open foo'}, + {'open foo': ['a', '']}), # Chained command - ({'a': 'message-info foo ;; message-info bar'}, - {'message-info foo': ['a'], 'message-info bar': ['a']}), + ({'a': 'open foo ;; open bar'}, + {'open foo': ['a'], 'open bar': ['a']}), # Command using set-cmd-text (#5942) ( { -- cgit v1.2.3-54-g00ecf From e16e67baeb59024cad52a33844cccb81045865cf Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 3 Jan 2021 13:41:11 -0500 Subject: Fix PR issues for set-cmd-text completions. - Prefer None to "" - Prefer implicit bool checking - Try to fix mypy warnings --- qutebrowser/config/config.py | 13 +++++++------ qutebrowser/keyinput/modeparsers.py | 4 ++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index b2ba1a6a5..9af764253 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -163,20 +163,21 @@ class KeyConfig: bindings[key] = binding return bindings - def _implied_cmd(self, cmdline: str) -> str: + def _implied_cmd(self, cmdline: str) -> Optional[str]: """Return cmdline, or the implied cmd if cmdline is a set-cmd-text.""" try: results = runners.CommandParser().parse_all(cmdline, aliases=False) - if len(results) == 0: - return "" - result = results[0] except cmdexc.NoSuchCommandError: - return "" + return None + + if not results: + return None + result = results[0] if result.cmd.name != "set-cmd-text": return cmdline *flags, cmd = result.args if "-a" in flags or "--append" in flags or not cmd.startswith(":"): - return "" # doesn't look like this sets a command + return None # doesn't look like this sets a command return cmd.lstrip(":") def get_reverse_bindings_for(self, mode: str) -> '_ReverseBindings': diff --git a/qutebrowser/keyinput/modeparsers.py b/qutebrowser/keyinput/modeparsers.py index 48f3594a5..fe03a9db6 100644 --- a/qutebrowser/keyinput/modeparsers.py +++ b/qutebrowser/keyinput/modeparsers.py @@ -86,6 +86,8 @@ class NormalKeyParser(CommandKeyParser): _partial_timer: Timer to clear partial keypresses. """ + _sequence: keyutils.KeySequence + def __init__(self, *, win_id: int, commandrunner: 'runners.CommandRunner', parent: QObject = None) -> None: @@ -154,6 +156,8 @@ class HintKeyParser(basekeyparser.BaseKeyParser): _last_press: The nature of the last keypress, a LastPress member. """ + _sequence: keyutils.KeySequence + def __init__(self, *, win_id: int, commandrunner: 'runners.CommandRunner', hintmanager: hints.HintManager, -- cgit v1.2.3-54-g00ecf From 8d1c5b690ee421144b499d14d62be644310bc29d Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Wed, 6 Jan 2021 09:15:13 -0500 Subject: Eliminate parser dependency on config. commands.runners depends on config.config to get aliases and the best_match setting, while config.config depends on commands.runners to split commands. Eliminate this circular dependency by: - Moving CommandParser from commands.runners into its a new commands.parser module - Pass aliases/config settings into CommandParser as arguments --- qutebrowser/commands/parser.py | 211 +++++++++++++++++++++++++++ qutebrowser/commands/runners.py | 194 +----------------------- qutebrowser/completion/completer.py | 5 +- qutebrowser/completion/models/configmodel.py | 8 +- qutebrowser/config/config.py | 9 +- tests/unit/commands/test_parser.py | 137 +++++++++++++++++ tests/unit/commands/test_runners.py | 137 ----------------- 7 files changed, 362 insertions(+), 339 deletions(-) create mode 100644 qutebrowser/commands/parser.py create mode 100644 tests/unit/commands/test_parser.py delete mode 100644 tests/unit/commands/test_runners.py diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py new file mode 100644 index 000000000..562b48922 --- /dev/null +++ b/qutebrowser/commands/parser.py @@ -0,0 +1,211 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2021 Ryan Roden-Corrent (rcorre) +# +# 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 . + +"""Module for parsing commands entered into the browser.""" + +import attr +from PyQt5.QtCore import pyqtSlot, QUrl, QObject + +from qutebrowser.commands import cmdexc +from qutebrowser.misc import split, objects + + +@attr.s +class ParseResult: + + """The result of parsing a commandline.""" + + cmd = attr.ib() + args = attr.ib() + cmdline = attr.ib() + + +class CommandParser: + + """Parse qutebrowser commandline commands. + + Attributes: + _partial_match: Whether to allow partial command matches. + """ + + def __init__(self, partial_match=False): + self._partial_match = partial_match + + def _get_alias(self, text, aliases, default=None): + """Get an alias from the config. + + Args: + text: The text to parse. + aliases: A map of aliases to commands. + default : Default value to return when alias was not found. + + Return: + The new command string if an alias was found. Default value + otherwise. + """ + parts = text.strip().split(maxsplit=1) + if parts[0] not in aliases: + return default + alias = aliases[parts[0]] + + try: + new_cmd = '{} {}'.format(alias, parts[1]) + except IndexError: + new_cmd = alias + if text.endswith(' '): + new_cmd += ' ' + return new_cmd + + def _parse_all_gen(self, text, *args, aliases=None, **kwargs): + """Split a command on ;; and parse all parts. + + If the first command in the commandline is a non-split one, it only + returns that. + + Args: + text: Text to parse. + aliases: A map of aliases to commands. + *args/**kwargs: Passed to parse(). + + Yields: + ParseResult tuples. + """ + text = text.strip().lstrip(':').strip() + if not text: + raise cmdexc.NoSuchCommandError("No command given") + + if aliases: + text = self._get_alias(text, aliases, text) + + if ';;' in text: + # Get the first command and check if it doesn't want to have ;; + # split. + first = text.split(';;')[0] + result = self.parse(first, *args, **kwargs) + if result.cmd.no_cmd_split: + sub_texts = [text] + else: + sub_texts = [e.strip() for e in text.split(';;')] + else: + sub_texts = [text] + for sub in sub_texts: + yield self.parse(sub, *args, **kwargs) + + def parse_all(self, *args, **kwargs): + """Wrapper over _parse_all_gen.""" + return list(self._parse_all_gen(*args, **kwargs)) + + def parse(self, text, *, fallback=False, keep=False, best_match=False): + """Split the commandline text into command and arguments. + + Args: + text: Text to parse. + fallback: Whether to do a fallback splitting when the command was + unknown. + keep: Whether to keep special chars and whitespace + + Return: + A ParseResult tuple. + """ + cmdstr, sep, argstr = text.partition(' ') + + if not cmdstr and not fallback: + raise cmdexc.NoSuchCommandError("No command given") + + if self._partial_match: + cmdstr = self._completion_match(cmdstr, best_match) + + try: + cmd = objects.commands[cmdstr] + except KeyError: + if not fallback: + raise cmdexc.NoSuchCommandError( + '{}: no such command'.format(cmdstr)) + cmdline = split.split(text, keep=keep) + return ParseResult(cmd=None, args=None, cmdline=cmdline) + + args = self._split_args(cmd, argstr, keep) + if keep and args: + cmdline = [cmdstr, sep + args[0]] + args[1:] + elif keep: + cmdline = [cmdstr, sep] + else: + cmdline = [cmdstr] + args[:] + + return ParseResult(cmd=cmd, args=args, cmdline=cmdline) + + def _completion_match(self, cmdstr, best): + """Replace cmdstr with a matching completion if there's only one match. + + Args: + cmdstr: The string representing the entered command so far + + Return: + cmdstr modified to the matching completion or unmodified + """ + matches = [cmd for cmd in sorted(objects.commands, key=len) + if cmdstr in cmd] + if len(matches) == 1: + cmdstr = matches[0] + elif len(matches) > 1 and best: + cmdstr = matches[0] + return cmdstr + + def _split_args(self, cmd, argstr, keep): + """Split the arguments from an arg string. + + Args: + cmd: The command we're currently handling. + argstr: An argument string. + keep: Whether to keep special chars and whitespace + + Return: + A list containing the split strings. + """ + if not argstr: + return [] + elif cmd.maxsplit is None: + return split.split(argstr, keep=keep) + else: + # If split=False, we still want to split the flags, but not + # everything after that. + # We first split the arg string and check the index of the first + # non-flag args, then we re-split again properly. + # example: + # + # input: "--foo -v bar baz" + # first split: ['--foo', '-v', 'bar', 'baz'] + # 0 1 2 3 + # second split: ['--foo', '-v', 'bar baz'] + # (maxsplit=2) + split_args = split.simple_split(argstr, keep=keep) + flag_arg_count = 0 + for i, arg in enumerate(split_args): + arg = arg.strip() + if arg.startswith('-'): + if arg in cmd.flags_with_args: + flag_arg_count += 1 + else: + maxsplit = i + cmd.maxsplit + flag_arg_count + return split.simple_split(argstr, keep=keep, + maxsplit=maxsplit) + + # If there are only flags, we got it right on the first try + # already. + return split_args diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index c195a8be9..7d32c7f50 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -24,14 +24,12 @@ import re import contextlib from typing import TYPE_CHECKING, Callable, Dict, Iterator, Mapping, MutableMapping -import attr from PyQt5.QtCore import pyqtSlot, QUrl, QObject from qutebrowser.api import cmdutils from qutebrowser.config import config -from qutebrowser.commands import cmdexc +from qutebrowser.commands import cmdexc, parser from qutebrowser.utils import message, objreg, qtutils, usertypes, utils -from qutebrowser.misc import split, objects from qutebrowser.keyinput import macros, modeman if TYPE_CHECKING: @@ -42,16 +40,6 @@ _ReplacementFunction = Callable[['tabbedbrowser.TabbedBrowser'], str] last_command = {} -@attr.s -class ParseResult: - - """The result of parsing a commandline.""" - - cmd = attr.ib() - args = attr.ib() - cmdline = attr.ib() - - def _url(tabbed_browser): """Convenience method to get the current url.""" try: @@ -129,181 +117,6 @@ def replace_variables(win_id, arglist): return args -class CommandParser: - - """Parse qutebrowser commandline commands. - - Attributes: - _partial_match: Whether to allow partial command matches. - """ - - def __init__(self, partial_match=False): - self._partial_match = partial_match - - def _get_alias(self, text, default=None): - """Get an alias from the config. - - Args: - text: The text to parse. - default : Default value to return when alias was not found. - - Return: - The new command string if an alias was found. Default value - otherwise. - """ - parts = text.strip().split(maxsplit=1) - aliases = config.cache['aliases'] - if parts[0] not in aliases: - return default - alias = aliases[parts[0]] - - try: - new_cmd = '{} {}'.format(alias, parts[1]) - except IndexError: - new_cmd = alias - if text.endswith(' '): - new_cmd += ' ' - return new_cmd - - def _parse_all_gen(self, text, *args, aliases=True, **kwargs): - """Split a command on ;; and parse all parts. - - If the first command in the commandline is a non-split one, it only - returns that. - - Args: - text: Text to parse. - aliases: Whether to handle aliases. - *args/**kwargs: Passed to parse(). - - Yields: - ParseResult tuples. - """ - text = text.strip().lstrip(':').strip() - if not text: - raise cmdexc.NoSuchCommandError("No command given") - - if aliases: - text = self._get_alias(text, text) - - if ';;' in text: - # Get the first command and check if it doesn't want to have ;; - # split. - first = text.split(';;')[0] - result = self.parse(first, *args, **kwargs) - if result.cmd.no_cmd_split: - sub_texts = [text] - else: - sub_texts = [e.strip() for e in text.split(';;')] - else: - sub_texts = [text] - for sub in sub_texts: - yield self.parse(sub, *args, **kwargs) - - def parse_all(self, *args, **kwargs): - """Wrapper over _parse_all_gen.""" - return list(self._parse_all_gen(*args, **kwargs)) - - def parse(self, text, *, fallback=False, keep=False): - """Split the commandline text into command and arguments. - - Args: - text: Text to parse. - fallback: Whether to do a fallback splitting when the command was - unknown. - keep: Whether to keep special chars and whitespace - - Return: - A ParseResult tuple. - """ - cmdstr, sep, argstr = text.partition(' ') - - if not cmdstr and not fallback: - raise cmdexc.NoSuchCommandError("No command given") - - if self._partial_match: - cmdstr = self._completion_match(cmdstr) - - try: - cmd = objects.commands[cmdstr] - except KeyError: - if not fallback: - raise cmdexc.NoSuchCommandError( - '{}: no such command'.format(cmdstr)) - cmdline = split.split(text, keep=keep) - return ParseResult(cmd=None, args=None, cmdline=cmdline) - - args = self._split_args(cmd, argstr, keep) - if keep and args: - cmdline = [cmdstr, sep + args[0]] + args[1:] - elif keep: - cmdline = [cmdstr, sep] - else: - cmdline = [cmdstr] + args[:] - - return ParseResult(cmd=cmd, args=args, cmdline=cmdline) - - def _completion_match(self, cmdstr): - """Replace cmdstr with a matching completion if there's only one match. - - Args: - cmdstr: The string representing the entered command so far - - Return: - cmdstr modified to the matching completion or unmodified - """ - matches = [cmd for cmd in sorted(objects.commands, key=len) - if cmdstr in cmd] - if len(matches) == 1: - cmdstr = matches[0] - elif len(matches) > 1 and config.val.completion.use_best_match: - cmdstr = matches[0] - return cmdstr - - def _split_args(self, cmd, argstr, keep): - """Split the arguments from an arg string. - - Args: - cmd: The command we're currently handling. - argstr: An argument string. - keep: Whether to keep special chars and whitespace - - Return: - A list containing the split strings. - """ - if not argstr: - return [] - elif cmd.maxsplit is None: - return split.split(argstr, keep=keep) - else: - # If split=False, we still want to split the flags, but not - # everything after that. - # We first split the arg string and check the index of the first - # non-flag args, then we re-split again properly. - # example: - # - # input: "--foo -v bar baz" - # first split: ['--foo', '-v', 'bar', 'baz'] - # 0 1 2 3 - # second split: ['--foo', '-v', 'bar baz'] - # (maxsplit=2) - split_args = split.simple_split(argstr, keep=keep) - flag_arg_count = 0 - for i, arg in enumerate(split_args): - arg = arg.strip() - if arg.startswith('-'): - if arg in cmd.flags_with_args: - flag_arg_count += 1 - else: - maxsplit = i + cmd.maxsplit + flag_arg_count - return split.simple_split(argstr, keep=keep, - maxsplit=maxsplit) - - # If there are only flags, we got it right on the first try - # already. - return split_args - - class AbstractCommandRunner(QObject): """Abstract base class for CommandRunner.""" @@ -328,7 +141,7 @@ class CommandRunner(AbstractCommandRunner): def __init__(self, win_id, partial_match=False, parent=None): super().__init__(parent) - self._parser = CommandParser(partial_match=partial_match) + self._parser = parser.CommandParser(partial_match=partial_match) self._win_id = win_id @contextlib.contextmanager @@ -358,7 +171,8 @@ class CommandRunner(AbstractCommandRunner): parsed = None with self._handle_error(safely): - parsed = self._parser.parse_all(text) + parsed = self._parser.parse_all( + text, best_match=config.val.completion.use_best_match) if parsed is None: return diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index b06611bc0..1b8e44bc1 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -23,7 +23,7 @@ import attr from PyQt5.QtCore import pyqtSlot, QObject, QTimer from qutebrowser.config import config -from qutebrowser.commands import runners +from qutebrowser.commands import parser from qutebrowser.misc import objects from qutebrowser.utils import log, utils, debug, objreg from qutebrowser.completion.models import miscmodels @@ -135,8 +135,7 @@ class Completer(QObject): if not text or not text.strip(): # Only ":", empty part under the cursor with nothing before/after return [], '', [] - parser = runners.CommandParser() - result = parser.parse(text, fallback=True, keep=True) + result = parser.CommandParser().parse(text, fallback=True, keep=True) parts = [x for x in result.cmdline if x] pos = self._cmd.cursorPosition() - len(self._cmd.prefix()) pos = min(pos, len(text)) # Qt treats 2-byte UTF-16 chars as 2 chars diff --git a/qutebrowser/completion/models/configmodel.py b/qutebrowser/completion/models/configmodel.py index b6337bdbd..2fbdae2ad 100644 --- a/qutebrowser/completion/models/configmodel.py +++ b/qutebrowser/completion/models/configmodel.py @@ -21,7 +21,7 @@ from qutebrowser.config import configdata, configexc from qutebrowser.completion.models import completionmodel, listcategory, util -from qutebrowser.commands import runners, cmdexc +from qutebrowser.commands import parser, cmdexc from qutebrowser.keyinput import keyutils @@ -117,9 +117,8 @@ def _bind_current_default(key, info): cmd_text = info.keyconf.get_command(seq, 'normal') if cmd_text: - parser = runners.CommandParser() try: - cmd = parser.parse(cmd_text).cmd + cmd = parser.CommandParser().parse(cmd_text).cmd except cmdexc.NoSuchCommandError: data.append((cmd_text, '(Current) Invalid command!', key)) else: @@ -127,8 +126,7 @@ def _bind_current_default(key, info): cmd_text = info.keyconf.get_command(seq, 'normal', default=True) if cmd_text: - parser = runners.CommandParser() - cmd = parser.parse(cmd_text).cmd + cmd = parser.CommandParser().parse(cmd_text).cmd data.append((cmd_text, '(Default) {}'.format(cmd.desc), key)) return data diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 9af764253..fee3fec47 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -27,7 +27,7 @@ from typing import (TYPE_CHECKING, Any, Callable, Dict, Iterator, List, Mapping, from PyQt5.QtCore import pyqtSignal, QObject, QUrl -from qutebrowser.commands import cmdexc, runners +from qutebrowser.commands import cmdexc, parser from qutebrowser.config import configdata, configexc, configutils from qutebrowser.utils import utils, log, urlmatch from qutebrowser.misc import objects @@ -166,7 +166,8 @@ class KeyConfig: def _implied_cmd(self, cmdline: str) -> Optional[str]: """Return cmdline, or the implied cmd if cmdline is a set-cmd-text.""" try: - results = runners.CommandParser().parse_all(cmdline, aliases=False) + results = parser.CommandParser().parse_all( + cmdline, aliases=cache['aliases']) except cmdexc.NoSuchCommandError: return None @@ -193,8 +194,8 @@ class KeyConfig: cmd_to_keys: KeyConfig._ReverseBindings = {} bindings = self.get_bindings_for(mode) for seq, full_cmd in sorted(bindings.items()): - for cmd in full_cmd.split(';;'): - cmd = self._implied_cmd(cmd.strip()) + for cmdtext in full_cmd.split(';;'): + cmd = self._implied_cmd(cmdtext.strip()) if not cmd: continue cmd_to_keys.setdefault(cmd, []) diff --git a/tests/unit/commands/test_parser.py b/tests/unit/commands/test_parser.py new file mode 100644 index 000000000..011c1ca02 --- /dev/null +++ b/tests/unit/commands/test_parser.py @@ -0,0 +1,137 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2015-2020 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 . + +"""Tests for qutebrowser.commands.parser.""" + +import pytest + +from qutebrowser.misc import objects +from qutebrowser.commands import parser, cmdexc + + +class TestCommandParser: + + def test_parse_all(self, cmdline_test): + """Test parsing of commands. + + See https://github.com/qutebrowser/qutebrowser/issues/615 + + Args: + cmdline_test: A pytest fixture which provides testcases. + """ + p = parser.CommandParser() + if cmdline_test.valid: + p.parse_all(cmdline_test.cmd) + else: + with pytest.raises(cmdexc.NoSuchCommandError): + p.parse_all(cmdline_test.cmd) + + def test_parse_all_with_alias(self, cmdline_test, monkeypatch, + config_stub): + if not cmdline_test.cmd: + pytest.skip("Empty command") + + aliases = {'alias_name': cmdline_test.cmd} + + p = parser.CommandParser() + if cmdline_test.valid: + assert len(p.parse_all("alias_name", aliases=aliases)) > 0 + else: + with pytest.raises(cmdexc.NoSuchCommandError): + p.parse_all("alias_name", aliases=aliases) + + @pytest.mark.parametrize('command', ['', ' ']) + def test_parse_empty_with_alias(self, command): + """An empty command should not crash. + + See https://github.com/qutebrowser/qutebrowser/issues/1690 + and https://github.com/qutebrowser/qutebrowser/issues/1773 + """ + p = parser.CommandParser() + with pytest.raises(cmdexc.NoSuchCommandError): + p.parse_all(command, aliases={"foo": "bar"}) + + @pytest.mark.parametrize('command, name, args', [ + ("set-cmd-text -s :open", "set-cmd-text", ["-s", ":open"]), + ("set-cmd-text :open {url:pretty}", "set-cmd-text", + [":open {url:pretty}"]), + ("set-cmd-text -s :open -t", "set-cmd-text", ["-s", ":open -t"]), + ("set-cmd-text :open -t -r {url:pretty}", "set-cmd-text", + [":open -t -r {url:pretty}"]), + ("set-cmd-text -s :open -b", "set-cmd-text", ["-s", ":open -b"]), + ("set-cmd-text :open -b -r {url:pretty}", "set-cmd-text", + [":open -b -r {url:pretty}"]), + ("set-cmd-text -s :open -w", "set-cmd-text", + ["-s", ":open -w"]), + ("set-cmd-text :open -w {url:pretty}", "set-cmd-text", + [":open -w {url:pretty}"]), + ("set-cmd-text /", "set-cmd-text", ["/"]), + ("set-cmd-text ?", "set-cmd-text", ["?"]), + ("set-cmd-text :", "set-cmd-text", [":"]), + ]) + def test_parse_result(self, command, name, args): + p = parser.CommandParser() + result = p.parse_all(command)[0] + assert result.cmd.name == name + assert result.args == args + + +class TestCompletions: + + """Tests for completions.use_best_match.""" + + @pytest.fixture(autouse=True) + def cmdutils_stub(self, monkeypatch, stubs): + """Patch the cmdutils module to provide fake commands.""" + monkeypatch.setattr(objects, 'commands', { + 'one': stubs.FakeCommand(name='one'), + 'two': stubs.FakeCommand(name='two'), + 'two-foo': stubs.FakeCommand(name='two-foo'), + }) + + def test_partial_parsing(self, config_stub): + """Test partial parsing with a runner where it's enabled. + + The same with it being disabled is tested by test_parse_all. + """ + p = parser.CommandParser(partial_match=True) + result = p.parse('on') + assert result.cmd.name == 'one' + + def test_dont_use_best_match(self, config_stub): + """Test multiple completion options with use_best_match set to false. + + Should raise NoSuchCommandError + """ + config_stub.val.completion.use_best_match = False + p = parser.CommandParser(partial_match=True) + + with pytest.raises(cmdexc.NoSuchCommandError): + p.parse('tw') + + def test_use_best_match(self, config_stub): + """Test multiple completion options with use_best_match set to true. + + The resulting command should be the best match + """ + config_stub.val.completion.use_best_match = True + p = parser.CommandParser(partial_match=True) + + result = p.parse('tw', best_match=True) + assert result.cmd.name == 'two' diff --git a/tests/unit/commands/test_runners.py b/tests/unit/commands/test_runners.py deleted file mode 100644 index 0d7f27fa7..000000000 --- a/tests/unit/commands/test_runners.py +++ /dev/null @@ -1,137 +0,0 @@ -# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: - -# Copyright 2015-2020 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 . - -"""Tests for qutebrowser.commands.runners.""" - -import pytest - -from qutebrowser.misc import objects -from qutebrowser.commands import runners, cmdexc - - -class TestCommandParser: - - def test_parse_all(self, cmdline_test): - """Test parsing of commands. - - See https://github.com/qutebrowser/qutebrowser/issues/615 - - Args: - cmdline_test: A pytest fixture which provides testcases. - """ - parser = runners.CommandParser() - if cmdline_test.valid: - parser.parse_all(cmdline_test.cmd, aliases=False) - else: - with pytest.raises(cmdexc.NoSuchCommandError): - parser.parse_all(cmdline_test.cmd, aliases=False) - - def test_parse_all_with_alias(self, cmdline_test, monkeypatch, - config_stub): - if not cmdline_test.cmd: - pytest.skip("Empty command") - - config_stub.val.aliases = {'alias_name': cmdline_test.cmd} - - parser = runners.CommandParser() - if cmdline_test.valid: - assert len(parser.parse_all("alias_name")) > 0 - else: - with pytest.raises(cmdexc.NoSuchCommandError): - parser.parse_all("alias_name") - - @pytest.mark.parametrize('command', ['', ' ']) - def test_parse_empty_with_alias(self, command): - """An empty command should not crash. - - See https://github.com/qutebrowser/qutebrowser/issues/1690 - and https://github.com/qutebrowser/qutebrowser/issues/1773 - """ - parser = runners.CommandParser() - with pytest.raises(cmdexc.NoSuchCommandError): - parser.parse_all(command) - - @pytest.mark.parametrize('command, name, args', [ - ("set-cmd-text -s :open", "set-cmd-text", ["-s", ":open"]), - ("set-cmd-text :open {url:pretty}", "set-cmd-text", - [":open {url:pretty}"]), - ("set-cmd-text -s :open -t", "set-cmd-text", ["-s", ":open -t"]), - ("set-cmd-text :open -t -r {url:pretty}", "set-cmd-text", - [":open -t -r {url:pretty}"]), - ("set-cmd-text -s :open -b", "set-cmd-text", ["-s", ":open -b"]), - ("set-cmd-text :open -b -r {url:pretty}", "set-cmd-text", - [":open -b -r {url:pretty}"]), - ("set-cmd-text -s :open -w", "set-cmd-text", - ["-s", ":open -w"]), - ("set-cmd-text :open -w {url:pretty}", "set-cmd-text", - [":open -w {url:pretty}"]), - ("set-cmd-text /", "set-cmd-text", ["/"]), - ("set-cmd-text ?", "set-cmd-text", ["?"]), - ("set-cmd-text :", "set-cmd-text", [":"]), - ]) - def test_parse_result(self, command, name, args): - parser = runners.CommandParser() - result = parser.parse_all(command, aliases=False)[0] - assert result.cmd.name == name - assert result.args == args - - -class TestCompletions: - - """Tests for completions.use_best_match.""" - - @pytest.fixture(autouse=True) - def cmdutils_stub(self, monkeypatch, stubs): - """Patch the cmdutils module to provide fake commands.""" - monkeypatch.setattr(objects, 'commands', { - 'one': stubs.FakeCommand(name='one'), - 'two': stubs.FakeCommand(name='two'), - 'two-foo': stubs.FakeCommand(name='two-foo'), - }) - - def test_partial_parsing(self, config_stub): - """Test partial parsing with a runner where it's enabled. - - The same with it being disabled is tested by test_parse_all. - """ - parser = runners.CommandParser(partial_match=True) - result = parser.parse('on') - assert result.cmd.name == 'one' - - def test_dont_use_best_match(self, config_stub): - """Test multiple completion options with use_best_match set to false. - - Should raise NoSuchCommandError - """ - config_stub.val.completion.use_best_match = False - parser = runners.CommandParser(partial_match=True) - - with pytest.raises(cmdexc.NoSuchCommandError): - parser.parse('tw') - - def test_use_best_match(self, config_stub): - """Test multiple completion options with use_best_match set to true. - - The resulting command should be the best match - """ - config_stub.val.completion.use_best_match = True - parser = runners.CommandParser(partial_match=True) - - result = parser.parse('tw') - assert result.cmd.name == 'two' -- cgit v1.2.3-54-g00ecf From 0b979666b6584db2ec28c0977097af60ab3a63af Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Wed, 6 Jan 2021 09:47:17 -0500 Subject: Remove unused imports in commands.parser --- qutebrowser/commands/parser.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index 562b48922..1f0fb01b2 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -20,7 +20,6 @@ """Module for parsing commands entered into the browser.""" import attr -from PyQt5.QtCore import pyqtSlot, QUrl, QObject from qutebrowser.commands import cmdexc from qutebrowser.misc import split, objects -- cgit v1.2.3-54-g00ecf From a7178298efe371be292d85ab0a2f1cf7943365e0 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Thu, 7 Jan 2021 08:44:15 -0500 Subject: Restore coverage of config.py to 100%. Test an empty/unknown command, and remove the check for an empty parse_all result (as parse_all will either throw or return a result. --- qutebrowser/config/config.py | 2 -- tests/unit/config/test_config.py | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index fee3fec47..1c1de0f82 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -171,8 +171,6 @@ class KeyConfig: except cmdexc.NoSuchCommandError: return None - if not results: - return None result = results[0] if result.cmd.name != "set-cmd-text": return cmdline diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index b0c32c6fb..3e81833e3 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -218,6 +218,8 @@ class TestKeyConfig: "open {url:pretty}": ["go"], } ), + # Empty/unknown commands + ({"a": "", "b": "notreal"}, {}), ]) def test_get_reverse_bindings_for(self, key_config_stub, config_stub, no_bindings, bindings, expected): -- cgit v1.2.3-54-g00ecf From cf30285b40fbac965e458efb51f376ad2812a8b4 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Mar 2021 18:20:34 +0100 Subject: Adjust imports --- qutebrowser/commands/parser.py | 2 +- qutebrowser/commands/runners.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index 0927d1940..e8efbfb42 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -19,7 +19,7 @@ """Module for parsing commands entered into the browser.""" -import attr +import dataclasses from qutebrowser.commands import cmdexc, command from qutebrowser.misc import split, objects diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index 105887b0b..80822d625 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -22,7 +22,6 @@ import traceback import re import contextlib -import dataclasses from typing import (TYPE_CHECKING, Callable, Dict, Iterator, Mapping, MutableMapping, List, Optional) -- cgit v1.2.3-54-g00ecf From f72caf745185e661b08890692c3b6a03c932612a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Mar 2021 18:24:53 +0100 Subject: Document and rename use_best_match --- qutebrowser/commands/parser.py | 12 +++++++----- qutebrowser/commands/runners.py | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index e8efbfb42..85e974965 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -110,7 +110,7 @@ class CommandParser: """Wrapper over _parse_all_gen.""" return list(self._parse_all_gen(*args, **kwargs)) - def parse(self, text, *, fallback=False, keep=False, best_match=False): + def parse(self, text, *, fallback=False, keep=False, use_best_match=False): """Split the commandline text into command and arguments. Args: @@ -118,6 +118,7 @@ class CommandParser: fallback: Whether to do a fallback splitting when the command was unknown. keep: Whether to keep special chars and whitespace + use_best_match: Whether to use the best match even if incomplete. Return: A ParseResult tuple. @@ -128,7 +129,7 @@ class CommandParser: raise cmdexc.NoSuchCommandError("No command given") if self._partial_match: - cmdstr = self._completion_match(cmdstr, best_match) + cmdstr = self._completion_match(cmdstr, use_best_match) try: cmd = objects.commands[cmdstr] @@ -149,11 +150,12 @@ class CommandParser: return ParseResult(cmd=cmd, args=args, cmdline=cmdline) - def _completion_match(self, cmdstr, best): + def _completion_match(self, cmdstr, use_best_match): """Replace cmdstr with a matching completion if there's only one match. Args: - cmdstr: The string representing the entered command so far + cmdstr: The string representing the entered command so far. + use_best_match: Whether to use the best match even if incomplete. Return: cmdstr modified to the matching completion or unmodified @@ -162,7 +164,7 @@ class CommandParser: if cmdstr in cmd] if len(matches) == 1: cmdstr = matches[0] - elif len(matches) > 1 and best: + elif len(matches) > 1 and use_best_match: cmdstr = matches[0] return cmdstr diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index 80822d625..6d4ccdfc2 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -173,7 +173,7 @@ class CommandRunner(AbstractCommandRunner): parsed = None with self._handle_error(safely): parsed = self._parser.parse_all( - text, best_match=config.val.completion.use_best_match) + text, use_best_match=config.val.completion.use_best_match) if parsed is None: return -- cgit v1.2.3-54-g00ecf From ea70b091753c7b836a314d7b8a1e3ac2390c8f16 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Mar 2021 18:25:46 +0100 Subject: Fix imports --- qutebrowser/commands/parser.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index 85e974965..bcc075608 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -20,6 +20,7 @@ """Module for parsing commands entered into the browser.""" import dataclasses +from typing import Optional, List from qutebrowser.commands import cmdexc, command from qutebrowser.misc import split, objects -- cgit v1.2.3-54-g00ecf From 570e5fac671f89e1107ec500f3e0cf740bcd5226 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Mar 2021 18:42:13 +0100 Subject: WIP: Add type annotations for parser --- qutebrowser/commands/parser.py | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index bcc075608..ceebad57e 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -20,7 +20,7 @@ """Module for parsing commands entered into the browser.""" import dataclasses -from typing import Optional, List +from typing import Optional, List, Mapping, Iterator, List, Union from qutebrowser.commands import cmdexc, command from qutebrowser.misc import split, objects @@ -44,10 +44,10 @@ class CommandParser: _partial_match: Whether to allow partial command matches. """ - def __init__(self, partial_match=False): + def __init__(self, partial_match: bool = False) -> None: self._partial_match = partial_match - def _get_alias(self, text, aliases, default=None): + def _get_alias(self, text: str, aliases: Mapping[str, str], *, default: str) -> str: """Get an alias from the config. Args: @@ -72,7 +72,12 @@ class CommandParser: new_cmd += ' ' return new_cmd - def _parse_all_gen(self, text, *args, aliases=None, **kwargs): + def _parse_all_gen( + self, + text: str, + aliases: Mapping[str, str] = None, + **kwargs: bool, + ) -> Iterator[ParseResult]: """Split a command on ;; and parse all parts. If the first command in the commandline is a non-split one, it only @@ -81,7 +86,7 @@ class CommandParser: Args: text: Text to parse. aliases: A map of aliases to commands. - *args/**kwargs: Passed to parse(). + **kwargs: Passed to parse(). Yields: ParseResult tuples. @@ -91,13 +96,13 @@ class CommandParser: raise cmdexc.NoSuchCommandError("No command given") if aliases: - text = self._get_alias(text, aliases, text) + text = self._get_alias(text, aliases, default=text) if ';;' in text: # Get the first command and check if it doesn't want to have ;; # split. first = text.split(';;')[0] - result = self.parse(first, *args, **kwargs) + result = self.parse(first, **kwargs) if result.cmd.no_cmd_split: sub_texts = [text] else: @@ -105,13 +110,20 @@ class CommandParser: else: sub_texts = [text] for sub in sub_texts: - yield self.parse(sub, *args, **kwargs) + yield self.parse(first, **kwargs) - def parse_all(self, *args, **kwargs): + def parse_all(self, text: str, **kwargs: bool) -> List[ParseResult]: """Wrapper over _parse_all_gen.""" - return list(self._parse_all_gen(*args, **kwargs)) - - def parse(self, text, *, fallback=False, keep=False, use_best_match=False): + return list(self._parse_all_gen(text, **kwargs)) + + def parse( + self, + text: str, + *, + fallback: bool = False, + keep: bool = False, + use_best_match: bool = False, + ) -> ParseResult: """Split the commandline text into command and arguments. Args: @@ -151,7 +163,7 @@ class CommandParser: return ParseResult(cmd=cmd, args=args, cmdline=cmdline) - def _completion_match(self, cmdstr, use_best_match): + def _completion_match(self, cmdstr: str, use_best_match: bool) -> str: """Replace cmdstr with a matching completion if there's only one match. Args: @@ -169,7 +181,7 @@ class CommandParser: cmdstr = matches[0] return cmdstr - def _split_args(self, cmd, argstr, keep): + def _split_args(self, cmd: command.Command, argstr: str, keep: bool) -> List[str]: """Split the arguments from an arg string. Args: -- cgit v1.2.3-54-g00ecf From 2279d8fc230515a768dfe525bb366cfddb2713b4 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 18 Mar 2021 14:49:45 +0100 Subject: Restore config access in commands.parser See https://github.com/qutebrowser/qutebrowser/pull/5967#issuecomment-791373157 but no issues with circular imports here, from what I can see... --- qutebrowser/commands/parser.py | 19 +++++++++---------- qutebrowser/commands/runners.py | 3 +-- qutebrowser/config/config.py | 3 +-- tests/unit/commands/test_parser.py | 16 ++++++++-------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index ceebad57e..710c31717 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -24,6 +24,7 @@ from typing import Optional, List, Mapping, Iterator, List, Union from qutebrowser.commands import cmdexc, command from qutebrowser.misc import split, objects +from qutebrowser.config import config @dataclasses.dataclass @@ -47,7 +48,7 @@ class CommandParser: def __init__(self, partial_match: bool = False) -> None: self._partial_match = partial_match - def _get_alias(self, text: str, aliases: Mapping[str, str], *, default: str) -> str: + def _get_alias(self, text: str, *, default: str) -> str: """Get an alias from the config. Args: @@ -60,6 +61,7 @@ class CommandParser: otherwise. """ parts = text.strip().split(maxsplit=1) + aliases = config.cache['aliases'] if parts[0] not in aliases: return default alias = aliases[parts[0]] @@ -75,7 +77,7 @@ class CommandParser: def _parse_all_gen( self, text: str, - aliases: Mapping[str, str] = None, + aliases: bool = True, **kwargs: bool, ) -> Iterator[ParseResult]: """Split a command on ;; and parse all parts. @@ -85,7 +87,7 @@ class CommandParser: Args: text: Text to parse. - aliases: A map of aliases to commands. + aliases: Whether to handle aliases. **kwargs: Passed to parse(). Yields: @@ -96,7 +98,7 @@ class CommandParser: raise cmdexc.NoSuchCommandError("No command given") if aliases: - text = self._get_alias(text, aliases, default=text) + text = self._get_alias(text, default=text) if ';;' in text: # Get the first command and check if it doesn't want to have ;; @@ -122,7 +124,6 @@ class CommandParser: *, fallback: bool = False, keep: bool = False, - use_best_match: bool = False, ) -> ParseResult: """Split the commandline text into command and arguments. @@ -131,7 +132,6 @@ class CommandParser: fallback: Whether to do a fallback splitting when the command was unknown. keep: Whether to keep special chars and whitespace - use_best_match: Whether to use the best match even if incomplete. Return: A ParseResult tuple. @@ -142,7 +142,7 @@ class CommandParser: raise cmdexc.NoSuchCommandError("No command given") if self._partial_match: - cmdstr = self._completion_match(cmdstr, use_best_match) + cmdstr = self._completion_match(cmdstr) try: cmd = objects.commands[cmdstr] @@ -163,12 +163,11 @@ class CommandParser: return ParseResult(cmd=cmd, args=args, cmdline=cmdline) - def _completion_match(self, cmdstr: str, use_best_match: bool) -> str: + def _completion_match(self, cmdstr: str) -> str: """Replace cmdstr with a matching completion if there's only one match. Args: cmdstr: The string representing the entered command so far. - use_best_match: Whether to use the best match even if incomplete. Return: cmdstr modified to the matching completion or unmodified @@ -177,7 +176,7 @@ class CommandParser: if cmdstr in cmd] if len(matches) == 1: cmdstr = matches[0] - elif len(matches) > 1 and use_best_match: + elif len(matches) > 1 and config.val.completion.use_best_match: cmdstr = matches[0] return cmdstr diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index 6d4ccdfc2..7386d7d3b 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -172,8 +172,7 @@ class CommandRunner(AbstractCommandRunner): parsed = None with self._handle_error(safely): - parsed = self._parser.parse_all( - text, use_best_match=config.val.completion.use_best_match) + parsed = self._parser.parse_all(text) if parsed is None: return diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 719721cb7..07d16ea92 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -166,8 +166,7 @@ class KeyConfig: def _implied_cmd(self, cmdline: str) -> Optional[str]: """Return cmdline, or the implied cmd if cmdline is a set-cmd-text.""" try: - results = parser.CommandParser().parse_all( - cmdline, aliases=cache['aliases']) + results = parser.CommandParser().parse_all(cmdline) except cmdexc.NoSuchCommandError: return None diff --git a/tests/unit/commands/test_parser.py b/tests/unit/commands/test_parser.py index 3a0c5f314..b851ad3b0 100644 --- a/tests/unit/commands/test_parser.py +++ b/tests/unit/commands/test_parser.py @@ -37,24 +37,24 @@ class TestCommandParser: """ p = parser.CommandParser() if cmdline_test.valid: - p.parse_all(cmdline_test.cmd) + p.parse_all(cmdline_test.cmd, aliases=False) else: with pytest.raises(cmdexc.NoSuchCommandError): - p.parse_all(cmdline_test.cmd) + p.parse_all(cmdline_test.cmd, aliases=False) def test_parse_all_with_alias(self, cmdline_test, monkeypatch, config_stub): if not cmdline_test.cmd: pytest.skip("Empty command") - aliases = {'alias_name': cmdline_test.cmd} + config_stub.val.aliases = {'alias_name': cmdline_test.cmd} p = parser.CommandParser() if cmdline_test.valid: - assert len(p.parse_all("alias_name", aliases=aliases)) > 0 + assert len(p.parse_all("alias_name")) > 0 else: with pytest.raises(cmdexc.NoSuchCommandError): - p.parse_all("alias_name", aliases=aliases) + p.parse_all("alias_name") @pytest.mark.parametrize('command', ['', ' ']) def test_parse_empty_with_alias(self, command): @@ -65,7 +65,7 @@ class TestCommandParser: """ p = parser.CommandParser() with pytest.raises(cmdexc.NoSuchCommandError): - p.parse_all(command, aliases={"foo": "bar"}) + p.parse_all(command) @pytest.mark.parametrize('command, name, args', [ ("set-cmd-text -s :open", "set-cmd-text", ["-s", ":open"]), @@ -85,7 +85,7 @@ class TestCommandParser: ("set-cmd-text ?", "set-cmd-text", ["?"]), ("set-cmd-text :", "set-cmd-text", [":"]), ]) - def test_parse_result(self, command, name, args): + def test_parse_result(self, config_stub, command, name, args): p = parser.CommandParser() result = p.parse_all(command)[0] assert result.cmd.name == name @@ -133,5 +133,5 @@ class TestCompletions: config_stub.val.completion.use_best_match = True p = parser.CommandParser(partial_match=True) - result = p.parse('tw', best_match=True) + result = p.parse('tw') assert result.cmd.name == 'two' -- cgit v1.2.3-54-g00ecf From f3e604d7fbf2ceb8f15a92c5ba7c177c83ee4db1 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 18 Mar 2021 14:54:55 +0100 Subject: Fix copy-paste issue --- qutebrowser/commands/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index 710c31717..547641281 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -112,7 +112,7 @@ class CommandParser: else: sub_texts = [text] for sub in sub_texts: - yield self.parse(first, **kwargs) + yield self.parse(sub, **kwargs) def parse_all(self, text: str, **kwargs: bool) -> List[ParseResult]: """Wrapper over _parse_all_gen.""" -- cgit v1.2.3-54-g00ecf From 3d1f975391ef61a43490e93b4ce0c54fdd5c22ab Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 18 Mar 2021 15:15:22 +0100 Subject: Move completion fallback handling out of CommandParser This makes CompletionParser.parse simpler and makes ParseResult.cmd and .args non-Optional. Them being Optional would mean we would've to either resort to more complex typing with Literal, or to check whether they are really non-None everywhere. Since fallback=True is only used at one point, let's just handle this at the calling site instead. In theory, this changes the behavior when the cmdstr is empty and self._partial_match is set, because we now raise early and self._completion_match isn't called anymore. In practice, I think this shouldn't make a difference anywhere, and tests seem to agree. If cmdstr is empty and self._partial_match is False, the behavior should be the same, because objects.commands[''] will raise KeyError. --- qutebrowser/commands/parser.py | 29 +++++++---------------------- qutebrowser/completion/completer.py | 23 ++++++++++++++--------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index 547641281..5adbc003c 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -20,7 +20,7 @@ """Module for parsing commands entered into the browser.""" import dataclasses -from typing import Optional, List, Mapping, Iterator, List, Union +from typing import Optional, List, Mapping, Iterator, Union from qutebrowser.commands import cmdexc, command from qutebrowser.misc import split, objects @@ -32,8 +32,8 @@ class ParseResult: """The result of parsing a commandline.""" - cmd: Optional[command.Command] - args: Optional[List[str]] + cmd: command.Command + args: List[str] cmdline: List[str] @@ -118,27 +118,16 @@ class CommandParser: """Wrapper over _parse_all_gen.""" return list(self._parse_all_gen(text, **kwargs)) - def parse( - self, - text: str, - *, - fallback: bool = False, - keep: bool = False, - ) -> ParseResult: + def parse(self, text: str, *, keep: bool = False) -> ParseResult: """Split the commandline text into command and arguments. Args: text: Text to parse. - fallback: Whether to do a fallback splitting when the command was - unknown. - keep: Whether to keep special chars and whitespace - - Return: - A ParseResult tuple. + keep: Whether to keep special chars and whitespace. """ cmdstr, sep, argstr = text.partition(' ') - if not cmdstr and not fallback: + if not cmdstr: raise cmdexc.NoSuchCommandError("No command given") if self._partial_match: @@ -147,11 +136,7 @@ class CommandParser: try: cmd = objects.commands[cmdstr] except KeyError: - if not fallback: - raise cmdexc.NoSuchCommandError( - '{}: no such command'.format(cmdstr)) - cmdline = split.split(text, keep=keep) - return ParseResult(cmd=None, args=None, cmdline=cmdline) + raise cmdexc.NoSuchCommandError(f'{cmdstr}: no such command') args = self._split_args(cmd, argstr, keep) if keep and args: diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 548ace2c7..778333854 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -25,8 +25,8 @@ from typing import TYPE_CHECKING from PyQt5.QtCore import pyqtSlot, QObject, QTimer from qutebrowser.config import config -from qutebrowser.commands import parser -from qutebrowser.misc import objects +from qutebrowser.commands import parser, cmdexc +from qutebrowser.misc import objects, split from qutebrowser.utils import log, utils, debug, objreg from qutebrowser.completion.models import miscmodels if TYPE_CHECKING: @@ -139,12 +139,18 @@ class Completer(QObject): if not text or not text.strip(): # Only ":", empty part under the cursor with nothing before/after return [], '', [] - result = parser.CommandParser().parse(text, fallback=True, keep=True) - parts = [x for x in result.cmdline if x] + + try: + parse_result = parser.CommandParser().parse(text, keep=True) + except cmdexc.NoSuchCommandError: + cmdline = split.split(text, keep=True) + else: + cmdline = parse_result.cmdline + + parts = [x for x in cmdline if x] pos = self._cmd.cursorPosition() - len(self._cmd.prefix()) pos = min(pos, len(text)) # Qt treats 2-byte UTF-16 chars as 2 chars - log.completion.debug('partitioning {} around position {}'.format(parts, - pos)) + log.completion.debug(f'partitioning {parts} around position {pos}') for i, part in enumerate(parts): pos -= len(part) if pos <= 0: @@ -155,11 +161,10 @@ class Completer(QObject): center = parts[i].strip() # strip trailing whitespace included as a separate token postfix = [x.strip() for x in parts[i+1:] if not x.isspace()] - log.completion.debug( - "partitioned: {} '{}' {}".format(prefix, center, postfix)) + log.completion.debug(f"partitioned: {prefix} '{center}' {postfix}") return prefix, center, postfix - raise utils.Unreachable("Not all parts consumed: {}".format(parts)) + raise utils.Unreachable(f"Not all parts consumed: {parts}") @pyqtSlot(str) def on_selection_changed(self, text): -- cgit v1.2.3-54-g00ecf From c158cafe3395e130946cb192247e544251da8601 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 18 Mar 2021 15:24:58 +0100 Subject: Fix lint As for the mypy unreachable warning, see: See https://github.com/python/mypy/issues/7214 and https://github.com/python/mypy/issues/8766 Includes cherry-pick of 27ad47825279a39141efd11ec9cc54ff2a872517 --- qutebrowser/commands/parser.py | 4 ++-- qutebrowser/commands/runners.py | 6 ++---- qutebrowser/misc/earlyinit.py | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/qutebrowser/commands/parser.py b/qutebrowser/commands/parser.py index 5adbc003c..06a20cdf6 100644 --- a/qutebrowser/commands/parser.py +++ b/qutebrowser/commands/parser.py @@ -15,12 +15,12 @@ # 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 . +# along with qutebrowser. If not, see . """Module for parsing commands entered into the browser.""" import dataclasses -from typing import Optional, List, Mapping, Iterator, Union +from typing import List, Iterator from qutebrowser.commands import cmdexc, command from qutebrowser.misc import split, objects diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index 7386d7d3b..5fb054455 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -22,13 +22,11 @@ import traceback import re import contextlib -from typing import (TYPE_CHECKING, Callable, Dict, Iterator, Mapping, MutableMapping, - List, Optional) +from typing import TYPE_CHECKING, Callable, Dict, Iterator, Mapping, MutableMapping from PyQt5.QtCore import pyqtSlot, QUrl, QObject from qutebrowser.api import cmdutils -from qutebrowser.config import config from qutebrowser.commands import cmdexc, parser from qutebrowser.utils import message, objreg, qtutils, usertypes, utils from qutebrowser.keyinput import macros, modeman @@ -175,7 +173,7 @@ class CommandRunner(AbstractCommandRunner): parsed = self._parser.parse_all(text) if parsed is None: - return + return # type: ignore[unreachable] for result in parsed: with self._handle_error(safely): diff --git a/qutebrowser/misc/earlyinit.py b/qutebrowser/misc/earlyinit.py index ecd3fac1a..ca8f9e8fe 100644 --- a/qutebrowser/misc/earlyinit.py +++ b/qutebrowser/misc/earlyinit.py @@ -289,7 +289,7 @@ def webengine_early_import(): error messages in backendproblem.py are accurate. """ try: - from PyQt5 import QtWebEngineWidgets + from PyQt5 import QtWebEngineWidgets # pylint: disable=unused-import except ImportError: pass -- cgit v1.2.3-54-g00ecf