Fix handling of empty bindings without breaking :unbind

1899e313fd4f0a35cb71390053ebbcb8df451537 as a fix for #3631 broke :unbind, as
the config system treats None and '' equally.

Instead, allow None/'' again, but just handle it as "no binding".

5 files changed, 17 insertions, 3 deletions

diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc
index 0d6adc5b6..26294d7ca 100644
--- a/doc/changelog.asciidoc
+++ b/doc/changelog.asciidoc
@@ -134,7 +134,7 @@ Fixed
 - HTML/JS resource files are now read into RAM on start to avoid crashes when
   changing qutebrowser versions while it's open.
 - Setting `bindings.key_mappings` to an empty value is now allowed.
-- Binding to an empty command now shows an error rather than crashing.
+- Bindings to an empty commands are now ignored rather than crashing.
 
 Removed
 ~~~~~~~
diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py
index de8cc7042..7204fb8f3 100644
--- a/qutebrowser/config/config.py
+++ b/qutebrowser/config/config.py
@@ -147,7 +147,7 @@ class KeyConfig:
         """Get the combined bindings for the given mode."""
         bindings = dict(val.bindings.default[mode])
         for key, binding in val.bindings.commands[mode].items():
-            if binding is None:
+            if not binding:
                 bindings.pop(key, None)
             else:
                 bindings[key] = binding
diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml
index a778d1d35..a6a2d5317 100644
--- a/qutebrowser/config/configdata.yml
+++ b/qutebrowser/config/configdata.yml
@@ -2485,7 +2485,9 @@ bindings.commands:
       name: Dict
       none_ok: true
       keytype: Key
-      valtype: Command
+      valtype:
+        name: Command
+        none_ok: true  # needed for :unbind
   desc: >-
     Keybindings mapping keys to commands in different modes.
 
diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py
index d51d79370..9104c0f53 100644
--- a/tests/unit/config/test_config.py
+++ b/tests/unit/config/test_config.py
@@ -161,6 +161,12 @@ class TestKeyConfig:
         for key, command in expected.items():
             assert key_config_stub.get_command(key, 'normal') == command
 
+    def test_get_bindings_for_empty_command(self, key_config_stub,
+                                            config_stub):
+        config_stub.val.bindings.commands = {'normal': {',x': ''}}
+        bindings = key_config_stub.get_bindings_for('normal')
+        assert keyseq(',x') not in bindings
+
     def test_get_command_unbound(self, key_config_stub, config_stub,
                                  no_bindings):
         config_stub.val.bindings.default = no_bindings
diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py
index b8747580b..6465db875 100644
--- a/tests/unit/keyinput/test_basekeyparser.py
+++ b/tests/unit/keyinput/test_basekeyparser.py
@@ -115,6 +115,12 @@ class TestReadConfig:
         assert keyseq('foo') in keyparser.bindings
         assert keyseq('<ctrl+x>') in keyparser.bindings
 
+    def test_read_config_empty_binding(self, keyparser, config_stub):
+        """Make sure setting an empty binding doesn't crash."""
+        keyparser._read_config('normal')
+        config_stub.val.bindings.commands = {'normal': {'co': ''}}
+        # The config is re-read automatically
+
     def test_read_config_modename_none(self, keyparser):
         assert keyparser._modename is None