config: Properly convert values for list/dict commands

Those commands use the config interface expecting Python objects (via
update_mutables), but we always used the raw user-supplied string as
input.

Thus far, it was very hard to trigger this bug: It would only
trigger with a List or Dict config option, with a value type which does
*not* accept a string type in to_py(). That means:

- List / FlagList / ConfirmQuit / ShellCommand
- Bool / BoolAsk
- Int
- Float
- Dict / Padding

(Notably, Perc, PercOrInt and Regex all *do* accept a string.)

That leaves only a couple of candidates:

- hints.selectors, but that's "no_autoconfig: true"
- bindings.default, ditto
- bindings.commands, but no reason to use :config-dict-* on it,
  especially with fixed keys

Therefore, this got only uncovered now, after adding
content.javascript.log_message.levels, which is a Dict with FlagList
values.

Note that we still don't have any config definition with an affected
List type, thus currently making it impossible to test the changes for
:config-list-{add,remove}.

Also note the adjusted test_{dict,list}_add_invalid_value tests - they
were just plain wrong. The command functions are never going to be
called by a Python object from the user (only with a str).

Finally, test_list_remove_no_value also needed an adjustment because
the value validation now happens *before* the other validation done
by the command.

2 files changed, 33 insertions, 20 deletions

diff --git a/qutebrowser/config/configcommands.py b/qutebrowser/config/configcommands.py
index 407c74214..b85031818 100644
--- a/qutebrowser/config/configcommands.py
+++ b/qutebrowser/config/configcommands.py
@@ -308,7 +308,7 @@ class ConfigCommands:
 
         with self._handle_config_error():
             option_value = self._config.get_mutable_obj(option)
-            option_value.append(value)
+            option_value.append(opt.typ.valtype.from_str(value))
             self._config.update_mutables(save_yaml=not temp)
 
     @cmdutils.register(instance='config-commands')
@@ -327,6 +327,7 @@ class ConfigCommands:
         """
         with self._handle_config_error():
             opt = self._config.get_opt(option)
+
         if not isinstance(opt.typ, configtypes.Dict):
             raise cmdutils.CommandError(":config-dict-add can only be used "
                                         "for dicts")
@@ -339,7 +340,7 @@ class ConfigCommands:
                                             "--replace to overwrite!"
                                             .format(key, option))
 
-            option_value[key] = value
+            option_value[key] = opt.typ.valtype.from_str(value)
             self._config.update_mutables(save_yaml=not temp)
 
     @cmdutils.register(instance='config-commands')
@@ -360,15 +361,15 @@ class ConfigCommands:
             raise cmdutils.CommandError(":config-list-remove can only be used "
                                         "for lists")
 
+        converted = opt.typ.valtype.from_str(value)
+
         with self._handle_config_error():
             option_value = self._config.get_mutable_obj(option)
 
-            if value not in option_value:
-                raise cmdutils.CommandError("{} is not in {}!".format(
-                    value, option))
-
-            option_value.remove(value)
+            if converted not in option_value:
+                raise cmdutils.CommandError(f"{value} is not in {option}!")
 
+            option_value.remove(converted)
             self._config.update_mutables(save_yaml=not temp)
 
     @cmdutils.register(instance='config-commands')
diff --git a/tests/unit/config/test_configcommands.py b/tests/unit/config/test_configcommands.py
index 6dd09cee3..d7031de94 100644
--- a/tests/unit/config/test_configcommands.py
+++ b/tests/unit/config/test_configcommands.py
@@ -319,12 +319,13 @@ class TestAdd:
                 match=":config-list-add can only be used for lists"):
             commands.config_list_add('history_gap_interval', 'value')
 
-    @pytest.mark.parametrize('value', ['', None, 42])
-    def test_list_add_invalid_values(self, commands, value):
-        with pytest.raises(
-                cmdutils.CommandError,
-                match="Invalid value '{}'".format(value)):
-            commands.config_list_add('content.blocking.whitelist', value)
+    def test_list_add_invalid_value(self, commands):
+        with pytest.raises(cmdutils.CommandError, match="Invalid value ''"):
+            commands.config_list_add('content.blocking.whitelist', '')
+
+    # FIXME test value conversion for :list-add like in test_dict_add_value_type
+    # (once we have a List config option using a non-str type, or a way to
+    # dynamically add new option definitions).
 
     @pytest.mark.parametrize('value', ['test1', 'test2'])
     @pytest.mark.parametrize('temp', [True, False])
@@ -368,11 +369,18 @@ class TestAdd:
                 match=":config-dict-add can only be used for dicts"):
             commands.config_dict_add('history_gap_interval', 'key', 'value')
 
-    @pytest.mark.parametrize('value', ['', None, 42])
-    def test_dict_add_invalid_values(self, commands, value):
-        with pytest.raises(cmdutils.CommandError,
-                           match="Invalid value '{}'".format(value)):
-            commands.config_dict_add('aliases', 'missingkey', value)
+    def test_dict_add_invalid_value(self, commands):
+        with pytest.raises(cmdutils.CommandError, match="Invalid value ''"):
+            commands.config_dict_add('aliases', 'missingkey', '')
+
+    def test_dict_add_value_type(self, commands, config_stub):
+        commands.config_dict_add(
+            "content.javascript.log_message.levels",
+            "example",
+            "['error']",
+        )
+        value = config_stub.val.content.javascript.log_message.levels["example"]
+        assert value == ['error']
 
 
 class TestRemove:
@@ -407,8 +415,12 @@ class TestRemove:
     def test_list_remove_no_value(self, commands):
         with pytest.raises(
                 cmdutils.CommandError,
-                match="never is not in colors.completion.fg!"):
-            commands.config_list_remove('colors.completion.fg', 'never')
+                match="#133742 is not in colors.completion.fg!"):
+            commands.config_list_remove('colors.completion.fg', '#133742')
+
+    # FIXME test value conversion for :list-remove like in test_dict_add_value_type
+    # (once we have a List config option using a non-str type, or a way to
+    # dynamically add new option definitions).
 
     @pytest.mark.parametrize('key', ['w', 'q'])
     @pytest.mark.parametrize('temp', [True, False])