From 6fdd007dbb8da0cf95dffcce2c0c16a708e464d9 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Wed, 21 Sep 2016 21:34:05 -0400 Subject: [PATCH 1/3] Simplify mode-checking in command. Rather than maintaining separate _modes and _not_modes lists, just build a single _modes list in the constructor. --- qutebrowser/commands/command.py | 33 +++++++++++++++------------- tests/end2end/features/hints.feature | 2 +- tests/end2end/features/misc.feature | 2 +- tests/end2end/test_insert_mode.py | 2 +- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index ff4b2768a..16d9037d3 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -82,7 +82,6 @@ class Command: no_replace_variables: Don't replace variables like {url} _qute_args: The saved data from @cmdutils.argument _modes: The modes the command can be executed in. - _not_modes: The modes the command can not be executed in. _count: The count set for the command. _instance: The object to bind 'self' to. _scope: The scope to get _instance for in the object registry. @@ -101,10 +100,14 @@ class Command: for m in modes: if not isinstance(m, usertypes.KeyMode): raise TypeError("Mode {} is no KeyMode member!".format(m)) - if not_modes is not None: + self._modes = modes + elif not_modes is not None: for m in not_modes: if not isinstance(m, usertypes.KeyMode): raise TypeError("Mode {} is no KeyMode member!".format(m)) + self._modes = [m for m in usertypes.KeyMode if m not in not_modes] + else: + self._modes = list(usertypes.KeyMode) if scope != 'global' and instance is None: raise ValueError("Setting scope without setting instance makes " "no sense!") @@ -114,8 +117,6 @@ class Command: self.hide = hide self.deprecated = deprecated self._instance = instance - self._modes = modes - self._not_modes = not_modes self._scope = scope self._star_args_optional = star_args_optional self.debug = debug @@ -155,17 +156,7 @@ class Command: """ mode_manager = objreg.get('mode-manager', scope='window', window=win_id) - curmode = mode_manager.mode - if self._modes is not None and curmode not in self._modes: - mode_names = '/'.join(mode.name for mode in self._modes) - raise cmdexc.PrerequisitesError( - "{}: This command is only allowed in {} mode.".format( - self.name, mode_names)) - elif self._not_modes is not None and curmode in self._not_modes: - mode_names = '/'.join(mode.name for mode in self._not_modes) - raise cmdexc.PrerequisitesError( - "{}: This command is not allowed in {} mode.".format( - self.name, mode_names)) + self.validate_mode(mode_manager.mode) used_backend = usertypes.arg2backend[objreg.get('args').backend] if self.backend is not None and used_backend != self.backend: @@ -525,3 +516,15 @@ class Command: log.commands.debug('Calling {}'.format( debug_utils.format_call(self.handler, posargs, kwargs))) self.handler(*posargs, **kwargs) + + def validate_mode(self, mode): + """Raise cmdexc.PrerequisitesError unless allowed in the given mode. + + Args: + mode: The usertypes.KeyMode to check. + """ + if mode not in self._modes: + mode_names = '/'.join(m.name for m in self._modes) + raise cmdexc.PrerequisitesError( + "{}: This command is only allowed in {} mode, not {}.".format( + self.name, mode_names, mode.name)) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 7da850b50..08e6d718a 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -2,7 +2,7 @@ Feature: Using hints Scenario: Using :follow-hint outside of hint mode (issue 1105) When I run :follow-hint - Then the error "follow-hint: This command is only allowed in hint mode." should be shown + Then the error "follow-hint: This command is only allowed in hint mode, not normal." should be shown Scenario: Using :follow-hint with an invalid index. When I open data/hints/html/simple.html diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index f06fae098..8a4c3f09e 100644 --- a/tests/end2end/features/misc.feature +++ b/tests/end2end/features/misc.feature @@ -589,7 +589,7 @@ Feature: Various utility commands. And I run :repeat-command with count 2 And I wait until the scroll position changed to 0/0 Then the page should not be scrolled - And the error "prompt-accept: This command is only allowed in prompt/yesno mode." should be shown + And the error "prompt-accept: This command is only allowed in prompt/yesno mode, not normal." should be shown @qtwebengine_createWindow Scenario: :repeat-command with mode-switching command diff --git a/tests/end2end/test_insert_mode.py b/tests/end2end/test_insert_mode.py index d08f1b161..cd57968ca 100644 --- a/tests/end2end/test_insert_mode.py +++ b/tests/end2end/test_insert_mode.py @@ -93,7 +93,7 @@ def test_auto_leave_insert_mode(quteproc): quteproc.send_cmd(':paste-primary') expected_message = ('paste-primary: This command is only allowed in ' - 'insert mode.') + 'insert mode, not caret.') quteproc.mark_expected(category='message', loglevel=logging.ERROR, message=expected_message) From 14f8ec8754c0f8bf1718b2ec3258229a3435eac2 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Thu, 22 Sep 2016 12:17:25 -0400 Subject: [PATCH 2/3] Error on mode/command mismatch with :bind. Resolves #1964 (:bind should error for mode/command mismatch) --- qutebrowser/config/parsers/keyconf.py | 27 ++++++++++++++++++------- tests/end2end/features/keyinput.feature | 4 ++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/qutebrowser/config/parsers/keyconf.py b/qutebrowser/config/parsers/keyconf.py index 7952c32e5..aa1600ae1 100644 --- a/qutebrowser/config/parsers/keyconf.py +++ b/qutebrowser/config/parsers/keyconf.py @@ -177,12 +177,13 @@ class KeyConfigParser(QObject): mode)) return - mode = self._normalize_sectname(mode) - for m in mode.split(','): + modenames = self._normalize_sectname(mode).split(',') + for m in modenames: if m not in configdata.KEY_DATA: raise cmdexc.CommandError("Invalid mode {}!".format(m)) try: - self._validate_command(command) + modes = [usertypes.KeyMode[m] for m in modenames] + self._validate_command(command, modes) except KeyConfigError as e: raise cmdexc.CommandError(str(e)) try: @@ -192,7 +193,7 @@ class KeyConfigParser(QObject): "override!".format(str(e.keychain))) except KeyConfigError as e: raise cmdexc.CommandError(e) - for m in mode.split(','): + for m in modenames: self.changed.emit(m) self._mark_config_dirty() @@ -331,8 +332,13 @@ class KeyConfigParser(QObject): self.is_dirty = True self.config_dirty.emit() - def _validate_command(self, line): - """Check if a given command is valid.""" + def _validate_command(self, line, modes=None): + """Check if a given command is valid. + + Args: + line: The commandline to validate. + modes: A list of modes to validate the commands for, or None. + """ from qutebrowser.config import config if line == self.UNBOUND_COMMAND: return @@ -352,8 +358,15 @@ class KeyConfigParser(QObject): commands = [c.split(maxsplit=1)[0].strip() for c in commands] for cmd in commands: aliases = config.section('aliases') - if cmd not in cmdutils.cmd_dict and cmd not in aliases: + if cmd in cmdutils.cmd_dict: + cmdname = cmd + elif cmd in aliases: + cmdname = aliases[cmd].split(maxsplit=1)[0].strip() + else: raise KeyConfigError("Invalid command '{}'!".format(cmd)) + cmd_obj = cmdutils.cmd_dict[cmdname] + for m in modes or []: + cmd_obj.validate_mode(m) def _read_command(self, line): """Read a command from a line.""" diff --git a/tests/end2end/features/keyinput.feature b/tests/end2end/features/keyinput.feature index 3ae32368b..e02b33d71 100644 --- a/tests/end2end/features/keyinput.feature +++ b/tests/end2end/features/keyinput.feature @@ -73,6 +73,10 @@ Feature: Keyboard input And I run :bind Then the message " is bound to 'mib' in normal mode" should be shown + Scenario: Binding with an unsupported mode + When I run :bind --mode=caret test27 rl-unix-filename-rubout + Then the error "rl-unix-filename-rubout: This command is only allowed in command/prompt mode, not caret." should be shown + # :unbind Scenario: Binding and unbinding a keychain From 6aaa1386198a34028e1288c2b6b3cec849a76db4 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 26 Sep 2016 07:50:57 -0400 Subject: [PATCH 3/3] Use a set instead of a list for Command._modes. --- qutebrowser/commands/command.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index 16d9037d3..d899a543f 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -100,14 +100,14 @@ class Command: for m in modes: if not isinstance(m, usertypes.KeyMode): raise TypeError("Mode {} is no KeyMode member!".format(m)) - self._modes = modes + self._modes = set(modes) elif not_modes is not None: for m in not_modes: if not isinstance(m, usertypes.KeyMode): raise TypeError("Mode {} is no KeyMode member!".format(m)) - self._modes = [m for m in usertypes.KeyMode if m not in not_modes] + self._modes = set(usertypes.KeyMode).difference(not_modes) else: - self._modes = list(usertypes.KeyMode) + self._modes = set(usertypes.KeyMode) if scope != 'global' and instance is None: raise ValueError("Setting scope without setting instance makes " "no sense!") @@ -524,7 +524,7 @@ class Command: mode: The usertypes.KeyMode to check. """ if mode not in self._modes: - mode_names = '/'.join(m.name for m in self._modes) + mode_names = '/'.join(sorted(m.name for m in self._modes)) raise cmdexc.PrerequisitesError( "{}: This command is only allowed in {} mode, not {}.".format( self.name, mode_names, mode.name))