rnhmjoj/qutebrowser
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Do not validate commands in the config and with :bind

Browse Source 
There are just way too many gotchas related to valid modes, aliases, and
circular dependencies when validating aliases/bindings in the config.

Let's just remove this and let invalid commands fail late, when they're actually
used.
This commit is contained in:
Florian Bruhin 2017-09-21 22:28:51 +02:00
parent f97f427100
commit 64b783d9c0
5 changed files with 41 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
qutebrowser/config/config.py
View File

@ -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:

36
qutebrowser/config/configtypes.py
View File

@ -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):

46
tests/unit/config/test_config.py
View File

@ -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', '<Ctrl-X>', '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', '<Ctrl-X>'])

9
tests/unit/config/test_configfiles.py
View File

@ -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'),

27
tests/unit/config/test_configtypes.py
View File

@ -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()