diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 1efc96856..e99498947 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -31,7 +31,7 @@ from qutebrowser.config import configdata, configexc, configtypes, configfiles from qutebrowser.utils import (utils, objreg, message, log, usertypes, jinja, qtutils) from qutebrowser.misc import objects, msgbox, earlyinit -from qutebrowser.commands import cmdexc, cmdutils, runners +from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.completion.models import configmodel # An easy way to access the config from other code via config.val.foo @@ -178,19 +178,6 @@ class KeyConfig: def bind(self, key, command, *, mode, force=False, save_yaml=False): """Add a new binding from key to command.""" key = self._prepare(key, mode) - - parser = runners.CommandParser() - try: - results = parser.parse_all(command) - except cmdexc.Error as e: - raise configexc.KeybindingError("Invalid command: {}".format(e)) - - for result in results: # pragma: no branch - try: - result.cmd.validate_mode(usertypes.KeyMode[mode]) - except cmdexc.PrerequisitesError as e: - raise configexc.KeybindingError(str(e)) - log.keyboard.vdebug("Adding binding {} -> {} in mode {}.".format( key, command, mode)) if key in self.get_bindings_for(mode) and not force: diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index c669ff426..aca9fffda 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -59,7 +59,7 @@ from PyQt5.QtCore import QUrl, Qt from PyQt5.QtGui import QColor, QFont from PyQt5.QtWidgets import QTabWidget, QTabBar -from qutebrowser.commands import cmdutils, runners, cmdexc +from qutebrowser.commands import cmdutils from qutebrowser.config import configexc from qutebrowser.utils import standarddir, utils, qtutils, urlutils @@ -773,33 +773,13 @@ class PercOrInt(_Numeric): class Command(BaseType): - """Base class for a command value with arguments.""" + """Base class for a command value with arguments. - # See to_py for details - unvalidated = False + // - def to_py(self, value): - self._basic_py_validation(value, str) - if not value: - return None - - # This requires some trickery, as runners.CommandParser uses - # conf.val.aliases, which in turn map to a command again, - # leading to an endless recursion. - # To fix that, we turn off validating other commands (alias values) - # while validating a command. - if not Command.unvalidated: - Command.unvalidated = True - try: - parser = runners.CommandParser() - try: - parser.parse_all(value) - except cmdexc.Error as e: - raise configexc.ValidationError(value, str(e)) - finally: - Command.unvalidated = False - - return value + Since validation is quite tricky here, we don't do so, and instead let + invalid commands (in bindings/aliases) fail when used. + """ def complete(self): out = [] @@ -807,6 +787,10 @@ class Command(BaseType): out.append((cmdname, obj.desc)) return out + def to_py(self, value): + self._basic_py_validation(value, str) + return value + class ColorSystem(MappingType): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 3f419e8d0..4e8bd684b 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -182,17 +182,6 @@ class TestKeyConfig: config_stub.val.bindings.commands = {'normal': bindings} assert keyconf.get_reverse_bindings_for('normal') == expected - def test_bind_invalid_command(self, keyconf): - with pytest.raises(configexc.KeybindingError, - match='Invalid command: foobar'): - keyconf.bind('a', 'foobar', mode='normal') - - def test_bind_invalid_mode(self, keyconf): - with pytest.raises(configexc.KeybindingError, - match='completion-item-del: This command is only ' - 'allowed in command mode, not normal.'): - keyconf.bind('a', 'completion-item-del', mode='normal') - @pytest.mark.parametrize('force', [True, False]) @pytest.mark.parametrize('key', ['a', '', 'b']) def test_bind_duplicate(self, keyconf, config_stub, force, key): @@ -208,12 +197,15 @@ class TestKeyConfig: assert keyconf.get_command(key, 'normal') == 'nop' @pytest.mark.parametrize('mode', ['normal', 'caret']) - def test_bind(self, keyconf, config_stub, qtbot, no_bindings, mode): + @pytest.mark.parametrize('command', [ + 'message-info foo', + 'nop ;; wq', # https://github.com/qutebrowser/qutebrowser/issues/3002 + ]) + def test_bind(self, keyconf, config_stub, qtbot, no_bindings, + mode, command): config_stub.val.bindings.default = no_bindings config_stub.val.bindings.commands = no_bindings - command = 'message-info foo' - with qtbot.wait_signal(config_stub.changed): keyconf.bind('a', command, mode=mode) @@ -221,6 +213,16 @@ class TestKeyConfig: assert keyconf.get_bindings_for(mode)['a'] == command assert keyconf.get_command('a', mode) == command + def test_bind_mode_changing(self, keyconf, config_stub, no_bindings): + """Make sure we can bind to a command which changes the mode. + + https://github.com/qutebrowser/qutebrowser/issues/2989 + """ + config_stub.val.bindings.default = no_bindings + config_stub.val.bindings.commands = no_bindings + keyconf.bind('a', 'set-cmd-text :nop ;; rl-beginning-of-line', + mode='normal') + @pytest.mark.parametrize('key, normalized', [ ('a', 'a'), # default bindings ('b', 'b'), # custom bindings @@ -504,20 +506,14 @@ class TestBindConfigCommand: msg = message_mock.getmsg(usertypes.MessageLevel.info) assert msg.text == expected - @pytest.mark.parametrize('command, mode, expected', [ - ('foobar', 'normal', "bind: Invalid command: foobar"), - ('completion-item-del', 'normal', - "bind: completion-item-del: This command is only allowed in " - "command mode, not normal."), - ('nop', 'wrongmode', "bind: Invalid mode wrongmode!"), - ]) - def test_bind_invalid(self, commands, command, mode, expected): - """Run ':bind a foobar' / ':bind a completion-item-del'. + def test_bind_invalid_mode(self, commands): + """Run ':bind --mode=wrongmode nop'. Should show an error. """ - with pytest.raises(cmdexc.CommandError, match=expected): - commands.bind('a', command, mode=mode) + with pytest.raises(cmdexc.CommandError, + match='bind: Invalid mode wrongmode!'): + commands.bind('a', 'nop', mode='wrongmode') @pytest.mark.parametrize('force', [True, False]) @pytest.mark.parametrize('key', ['a', 'b', '']) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 0453314bd..975e9c46a 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -279,6 +279,15 @@ class TestConfigPy: expected = {mode: {',a': 'message-info foo'}} assert config.instance._values['bindings.commands'] == expected + def test_bind_freshly_defined_alias(self, confpy): + """Make sure we can bind to a new alias. + + https://github.com/qutebrowser/qutebrowser/issues/3001 + """ + confpy.write("c.aliases['foo'] = 'message-info foo'", + "config.bind(',f', 'foo')") + confpy.read() + @pytest.mark.parametrize('line, key, mode', [ ('config.unbind("o")', 'o', 'normal'), ('config.unbind("y", mode="prompt")', 'y', 'prompt'), diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 122a1c20c..daf785d9c 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1072,37 +1072,10 @@ class TestCommand: monkeypatch.setattr(configtypes, 'cmdutils', cmd_utils) monkeypatch.setattr('qutebrowser.commands.runners.cmdutils', cmd_utils) - @pytest.fixture(autouse=True) - def patch_aliases(self, config_stub): - """Patch the aliases setting.""" - configtypes.Command.unvalidated = True - config_stub.val.aliases = {'alias': 'cmd1'} - configtypes.Command.unvalidated = False - @pytest.fixture def klass(self): return configtypes.Command - @pytest.mark.parametrize('val', ['cmd1', 'cmd2', 'cmd1 foo bar', - 'cmd2 baz fish', 'alias foo']) - def test_to_py_valid(self, patch_cmdutils, klass, val): - expected = None if not val else val - assert klass().to_py(val) == expected - - @pytest.mark.parametrize('val', ['cmd3', 'cmd3 foo bar', ' ']) - def test_to_py_invalid(self, patch_cmdutils, klass, val): - with pytest.raises(configexc.ValidationError): - klass().to_py(val) - - def test_cmdline(self, klass, cmdline_test): - """Test some commandlines from the cmdline_test fixture.""" - typ = klass() - if cmdline_test.valid: - typ.to_py(cmdline_test.cmd) - else: - with pytest.raises(configexc.ValidationError): - typ.to_py(cmdline_test.cmd) - def test_complete(self, patch_cmdutils, klass): """Test completion.""" items = klass().complete()