Skip to content

Commit

Permalink
Remove --force for :bind and config.bind(...)
Browse files Browse the repository at this point in the history
Turns out --force is just in the way for most people, and at least for default
bindings it's easy to reset them.

Also, it makes :config-source fail when config.py contains keybindings.

Closes #3049
  • Loading branch information
The-Compiler committed Oct 3, 2017
1 parent 727580d commit 22088d9
Show file tree
Hide file tree
Showing 12 changed files with 22 additions and 72 deletions.
3 changes: 1 addition & 2 deletions doc/help/commands.asciidoc
Expand Up @@ -120,7 +120,7 @@ How many pages to go back.


[[bind]] [[bind]]
=== bind === bind
Syntax: +:bind [*--mode* 'mode'] [*--force*] 'key' ['command']+ Syntax: +:bind [*--mode* 'mode'] 'key' ['command']+


Bind a key to a command. Bind a key to a command.


Expand All @@ -133,7 +133,6 @@ Bind a key to a command.
* +*-m*+, +*--mode*+: A comma-separated list of modes to bind the key in (default: `normal`). See `:help bindings.commands` for the * +*-m*+, +*--mode*+: A comma-separated list of modes to bind the key in (default: `normal`). See `:help bindings.commands` for the
available modes. available modes.


* +*-f*+, +*--force*+: Rebind the key if it is already bound.


==== note ==== note
* This command does not split arguments after the last argument and handles quotes literally. * This command does not split arguments after the last argument and handles quotes literally.
Expand Down
15 changes: 3 additions & 12 deletions doc/help/configuring.asciidoc
Expand Up @@ -61,8 +61,6 @@ link:commands.html#unbind[`:unbind`] commands:
- Binding the key chain `,v` to the `:spawn mpv {url}` command: - Binding the key chain `,v` to the `:spawn mpv {url}` command:
`:bind ,v spawn mpv {url}` `:bind ,v spawn mpv {url}`
- Unbinding the same key chain: `:unbind ,v` - Unbinding the same key chain: `:unbind ,v`
- Changing an existing binding: `bind --force ,v message-info foo`. Without
`--force`, qutebrowser will show an error because `,v` is already bound.
Key chains starting with a comma are ideal for custom bindings, as the comma key Key chains starting with a comma are ideal for custom bindings, as the comma key
will never be used in a default keybinding. will never be used in a default keybinding.
Expand Down Expand Up @@ -179,13 +177,6 @@ To bind a key in a mode other than `'normal'`, add a `mode` argument:
config.bind('<Ctrl-y>', 'prompt-yes', mode='prompt') config.bind('<Ctrl-y>', 'prompt-yes', mode='prompt')
---- ----


If the key is already bound, `force=True` needs to be given to rebind it:

[source,python]
----
config.bind('<Ctrl-v>', 'message-info foo', force=True)
----

To unbind a key (either a key which has been bound before, or a default binding): To unbind a key (either a key which has been bound before, or a default binding):


[source,python] [source,python]
Expand Down Expand Up @@ -339,10 +330,10 @@ to do so:


[source,python] [source,python]
---- ----
def bind_chained(key, *commands, force=False): def bind_chained(key, *commands):
config.bind(key, ' ;; '.join(commands), force=force) config.bind(key, ' ;; '.join(commands))
bind_chained('<Escape>', 'clear-keychain', 'search', force=True) bind_chained('<Escape>', 'clear-keychain', 'search')
---- ----


Avoiding flake8 errors Avoiding flake8 errors
Expand Down
4 changes: 1 addition & 3 deletions qutebrowser/config/config.py
Expand Up @@ -168,13 +168,11 @@ def get_command(self, key, mode):
bindings = self.get_bindings_for(mode) bindings = self.get_bindings_for(mode)
return bindings.get(key, None) return bindings.get(key, None)


def bind(self, key, command, *, mode, force=False, save_yaml=False): def bind(self, key, command, *, mode, save_yaml=False):
"""Add a new binding from key to command.""" """Add a new binding from key to command."""
key = self._prepare(key, mode) key = self._prepare(key, mode)
log.keyboard.vdebug("Adding binding {} -> {} in mode {}.".format( log.keyboard.vdebug("Adding binding {} -> {} in mode {}.".format(
key, command, mode)) key, command, mode))
if key in self.get_bindings_for(mode) and not force:
raise configexc.DuplicateKeyError(key)


bindings = self._config.get_obj('bindings.commands') bindings = self._config.get_obj('bindings.commands')
if mode not in bindings: if mode not in bindings:
Expand Down
9 changes: 2 additions & 7 deletions qutebrowser/config/configcommands.py
Expand Up @@ -92,7 +92,7 @@ def set(self, win_id, option=None, value=None, temp=False, print_=False):
@cmdutils.register(instance='config-commands', maxsplit=1, @cmdutils.register(instance='config-commands', maxsplit=1,
no_cmd_split=True, no_replace_variables=True) no_cmd_split=True, no_replace_variables=True)
@cmdutils.argument('command', completion=configmodel.bind) @cmdutils.argument('command', completion=configmodel.bind)
def bind(self, key, command=None, *, mode='normal', force=False): def bind(self, key, command=None, *, mode='normal'):
"""Bind a key to a command. """Bind a key to a command.
Args: Args:
Expand All @@ -102,7 +102,6 @@ def bind(self, key, command=None, *, mode='normal', force=False):
mode: A comma-separated list of modes to bind the key in mode: A comma-separated list of modes to bind the key in
(default: `normal`). See `:help bindings.commands` for the (default: `normal`). See `:help bindings.commands` for the
available modes. available modes.
force: Rebind the key if it is already bound.
""" """
if command is None: if command is None:
if utils.is_special_key(key): if utils.is_special_key(key):
Expand All @@ -118,11 +117,7 @@ def bind(self, key, command=None, *, mode='normal', force=False):
return return


try: try:
self._keyconfig.bind(key, command, mode=mode, force=force, self._keyconfig.bind(key, command, mode=mode, save_yaml=True)
save_yaml=True)
except configexc.DuplicateKeyError as e:
raise cmdexc.CommandError("bind: {} - use --force to override!"
.format(e))
except configexc.KeybindingError as e: except configexc.KeybindingError as e:
raise cmdexc.CommandError("bind: {}".format(e)) raise cmdexc.CommandError("bind: {}".format(e))


Expand Down
8 changes: 0 additions & 8 deletions qutebrowser/config/configexc.py
Expand Up @@ -59,14 +59,6 @@ class KeybindingError(Error):
"""Raised for issues with keybindings.""" """Raised for issues with keybindings."""




class DuplicateKeyError(KeybindingError):

"""Raised when there was a duplicate key."""

def __init__(self, key):
super().__init__("Duplicate key {}".format(key))


class NoOptionError(Error): class NoOptionError(Error):


"""Raised when an option was not found.""" """Raised when an option was not found."""
Expand Down
8 changes: 2 additions & 6 deletions qutebrowser/config/configfiles.py
Expand Up @@ -236,13 +236,9 @@ def set(self, name, value):
with self._handle_error('setting', name): with self._handle_error('setting', name):
self._config.set_obj(name, value) self._config.set_obj(name, value)


def bind(self, key, command, mode='normal', *, force=False): def bind(self, key, command, mode='normal'):
with self._handle_error('binding', key): with self._handle_error('binding', key):
try: self._keyconfig.bind(key, command, mode=mode)
self._keyconfig.bind(key, command, mode=mode, force=force)
except configexc.DuplicateKeyError as e:
raise configexc.KeybindingError('{} - use force=True to '
'override!'.format(e))


def unbind(self, key, mode='normal'): def unbind(self, key, mode='normal'):
with self._handle_error('unbinding', key): with self._handle_error('unbinding', key):
Expand Down
4 changes: 2 additions & 2 deletions tests/end2end/features/hints.feature
Expand Up @@ -246,7 +246,7 @@ Feature: Using hints
Scenario: Ignoring key presses after auto-following hints Scenario: Ignoring key presses after auto-following hints
When I set hints.auto_follow_timeout to 1000 When I set hints.auto_follow_timeout to 1000
And I set hints.mode to number And I set hints.mode to number
And I run :bind --force , message-error "This error message was triggered via a keybinding which should have been inhibited" And I run :bind , message-error "This error message was triggered via a keybinding which should have been inhibited"
And I open data/hints/html/simple.html And I open data/hints/html/simple.html
And I hint with args "all" And I hint with args "all"
And I press the key "f" And I press the key "f"
Expand All @@ -259,7 +259,7 @@ Feature: Using hints
Scenario: Turning off auto_follow_timeout Scenario: Turning off auto_follow_timeout
When I set hints.auto_follow_timeout to 0 When I set hints.auto_follow_timeout to 0
And I set hints.mode to number And I set hints.mode to number
And I run :bind --force , message-info "Keypress worked!" And I run :bind , message-info "Keypress worked!"
And I open data/hints/html/simple.html And I open data/hints/html/simple.html
And I hint with args "all" And I hint with args "all"
And I press the key "f" And I press the key "f"
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/completion/test_completer.py
Expand Up @@ -120,7 +120,7 @@ def openurl(url=None, related=False, bg=False, tab=False, window=False,


@cmdutils.argument('win_id', win_id=True) @cmdutils.argument('win_id', win_id=True)
@cmdutils.argument('command', completion=miscmodels_patch.command) @cmdutils.argument('command', completion=miscmodels_patch.command)
def bind(key, win_id, command=None, *, mode='normal', force=False): def bind(key, win_id, command=None, *, mode='normal'):
"""docstring.""" """docstring."""
pass pass


Expand Down
12 changes: 3 additions & 9 deletions tests/unit/config/test_config.py
Expand Up @@ -172,19 +172,13 @@ def test_get_reverse_bindings_for(self, keyconf, config_stub, no_bindings,
config_stub.val.bindings.commands = {'normal': bindings} config_stub.val.bindings.commands = {'normal': bindings}
assert keyconf.get_reverse_bindings_for('normal') == expected assert keyconf.get_reverse_bindings_for('normal') == expected


@pytest.mark.parametrize('force', [True, False])
@pytest.mark.parametrize('key', ['a', '<Ctrl-X>', 'b']) @pytest.mark.parametrize('key', ['a', '<Ctrl-X>', 'b'])
def test_bind_duplicate(self, keyconf, config_stub, force, key): def test_bind_duplicate(self, keyconf, config_stub, key):
config_stub.val.bindings.default = {'normal': {'a': 'nop', config_stub.val.bindings.default = {'normal': {'a': 'nop',
'<Ctrl+x>': 'nop'}} '<Ctrl+x>': 'nop'}}
config_stub.val.bindings.commands = {'normal': {'b': 'nop'}} config_stub.val.bindings.commands = {'normal': {'b': 'nop'}}
if force: keyconf.bind(key, 'message-info foo', mode='normal')
keyconf.bind(key, 'message-info foo', mode='normal', force=True) assert keyconf.get_command(key, 'normal') == 'message-info foo'
assert keyconf.get_command(key, 'normal') == 'message-info foo'
else:
with pytest.raises(configexc.DuplicateKeyError):
keyconf.bind(key, 'message-info foo', mode='normal')
assert keyconf.get_command(key, 'normal') == 'nop'


@pytest.mark.parametrize('mode', ['normal', 'caret']) @pytest.mark.parametrize('mode', ['normal', 'caret'])
@pytest.mark.parametrize('command', [ @pytest.mark.parametrize('command', [
Expand Down
14 changes: 3 additions & 11 deletions tests/unit/config/test_configcommands.py
Expand Up @@ -418,9 +418,8 @@ def test_bind_invalid_mode(self, commands):
match='bind: Invalid mode wrongmode!'): match='bind: Invalid mode wrongmode!'):
commands.bind('a', 'nop', mode='wrongmode') commands.bind('a', 'nop', mode='wrongmode')


@pytest.mark.parametrize('force', [True, False])
@pytest.mark.parametrize('key', ['a', 'b', '<Ctrl-X>']) @pytest.mark.parametrize('key', ['a', 'b', '<Ctrl-X>'])
def test_bind_duplicate(self, commands, config_stub, keyconf, force, key): def test_bind_duplicate(self, commands, config_stub, keyconf, key):
"""Run ':bind' with a key which already has been bound.'. """Run ':bind' with a key which already has been bound.'.
Also tests for https://github.com/qutebrowser/qutebrowser/issues/1544 Also tests for https://github.com/qutebrowser/qutebrowser/issues/1544
Expand All @@ -432,15 +431,8 @@ def test_bind_duplicate(self, commands, config_stub, keyconf, force, key):
'normal': {'b': 'nop'}, 'normal': {'b': 'nop'},
} }


if force: commands.bind(key, 'message-info foo', mode='normal')
commands.bind(key, 'message-info foo', mode='normal', force=True) assert keyconf.get_command(key, 'normal') == 'message-info foo'
assert keyconf.get_command(key, 'normal') == 'message-info foo'
else:
with pytest.raises(cmdexc.CommandError,
match="bind: Duplicate key .* - use --force to "
"override"):
commands.bind(key, 'message-info foo', mode='normal')
assert keyconf.get_command(key, 'normal') == 'nop'


def test_bind_none(self, commands, config_stub): def test_bind_none(self, commands, config_stub):
config_stub.val.bindings.commands = None config_stub.val.bindings.commands = None
Expand Down
6 changes: 0 additions & 6 deletions tests/unit/config/test_configexc.py
Expand Up @@ -43,12 +43,6 @@ def test_backend_error():
assert str(e) == "This setting is not available with the QtWebKit backend!" assert str(e) == "This setting is not available with the QtWebKit backend!"




def test_duplicate_key_error():
e = configexc.DuplicateKeyError('asdf')
assert isinstance(e, configexc.KeybindingError)
assert str(e) == "Duplicate key asdf"


def test_desc_with_text(): def test_desc_with_text():
"""Test ConfigErrorDesc.with_text.""" """Test ConfigErrorDesc.with_text."""
old = configexc.ConfigErrorDesc("Error text", Exception("Exception text")) old = configexc.ConfigErrorDesc("Error text", Exception("Exception text"))
Expand Down
9 changes: 4 additions & 5 deletions tests/unit/config/test_configfiles.py
Expand Up @@ -403,12 +403,11 @@ def test_bind_freshly_defined_alias(self, confpy):
confpy.read() confpy.read()


def test_bind_duplicate_key(self, confpy): def test_bind_duplicate_key(self, confpy):
"""Make sure we get a nice error message on duplicate key bindings.""" """Make sure overriding a keybinding works."""
confpy.write("config.bind('H', 'message-info back')") confpy.write("config.bind('H', 'message-info back')")
error = confpy.read(error=True) confpy.read()

expected = {'normal': {'H': 'message-info back'}}
expected = "Duplicate key H - use force=True to override!" assert config.instance._values['bindings.commands'] == expected
assert str(error.exception) == expected


def test_bind_none(self, confpy): def test_bind_none(self, confpy):
confpy.write("c.bindings.commands = None", confpy.write("c.bindings.commands = None",
Expand Down

0 comments on commit 22088d9

Please sign in to comment.