From 7f0ae252cd5fd6223bf3119e2a21d4cd39e7c842 Mon Sep 17 00:00:00 2001 From: Milo Gertjejansen Date: Thu, 4 Oct 2018 18:42:33 -0500 Subject: [PATCH] New config: More powerful :config- commands: add #4283 Made requested changes: - Separated list add and dict add commands. - Separated list and dict completion models. - Created tests for each command. - Simplified the configmodel options by breaking them into a separate function to do work that is similar. - General simplification of both add commands. Continues #2794 --- qutebrowser/completion/models/configmodel.py | 35 ++++--- qutebrowser/config/configcommands.py | 96 +++++++++----------- tests/unit/config/test_configcommands.py | 50 +++++++++- 3 files changed, 113 insertions(+), 68 deletions(-) diff --git a/qutebrowser/completion/models/configmodel.py b/qutebrowser/completion/models/configmodel.py index 9567f0b12..3a225fedf 100644 --- a/qutebrowser/completion/models/configmodel.py +++ b/qutebrowser/completion/models/configmodel.py @@ -27,12 +27,7 @@ from qutebrowser.keyinput import keyutils def option(*, info): """A CompletionModel filled with settings and their descriptions.""" - model = completionmodel.CompletionModel(column_widths=(20, 70, 10)) - options = ((opt.name, opt.description, info.config.get_str(opt.name)) - for opt in configdata.DATA.values() - if not opt.no_autoconfig) - model.add_category(listcategory.ListCategory("Options", options)) - return model + return _option(info, "Options", lambda opt: not opt.no_autoconfig) def customized_option(*, info): @@ -47,18 +42,32 @@ def customized_option(*, info): return model -def structure_option(*, info): - """A CompletionModel filled with set settings of structures. +def list_option(*, info): + """A CompletionModel filled with settings whose values are lists.""" + predicate = lambda opt: isinstance(info.config.get_obj(opt.name), list) + return _option(info, "List options", predicate) - Gets lists and dicts and their descriptions. + +def dict_option(*, info): + """A CompletionModel filled with settings whose values are dicts.""" + predicate = lambda opt: isinstance(info.config.get_obj(opt.name), dict) + return _option(info, "Dict options", predicate) + + +def _option(info, title, predicate): + """A CompletionModel that is generified for several option sets. + + Args: + info: The config info that can be passed through. + title: The title of the options. + predicate: The function for filtering out the options. Takes a single + argument. """ model = completionmodel.CompletionModel(column_widths=(20, 70, 10)) options = ((opt.name, opt.description, info.config.get_str(opt.name)) for opt in configdata.DATA.values() - if isinstance(info.config.get_obj(opt.name), (list, dict))) - model.add_category( - listcategory.ListCategory("Structure options", options) - ) + if predicate(opt)) + model.add_category(listcategory.ListCategory(title, options)) return model diff --git a/qutebrowser/config/configcommands.py b/qutebrowser/config/configcommands.py index 87a556571..04c829957 100644 --- a/qutebrowser/config/configcommands.py +++ b/qutebrowser/config/configcommands.py @@ -251,70 +251,58 @@ class ConfigCommands: self._config.unset(option, save_yaml=not temp) @cmdutils.register(instance='config-commands') - @cmdutils.argument('option', completion=configmodel.structure_option) - def config_add( - self, option, key=None, value=None, temp=False, replace=False - ): - """Adds an option. + @cmdutils.argument('option', completion=configmodel.list_option) + def config_add_list(self, option, value, temp=False): + """Append a value to a config option that is a list. - This adds an element to a dictionary or list. --replace is needed - to override existing values. + This appends an option to a config setting that is a list. Args: option: The name of the option. - value: The value to place in either the list or the dictionary. - key: The key to set if it's a dictionary. + value: The value to append to the end of the dictionary. + temp: Don't touch autoconfig.yml. + """ + opt = self._config.get_opt(option) + valid_list_types = (configtypes.List, configtypes.ListOrValue) + if not isinstance(opt.typ, valid_list_types): + raise cmdexc.CommandError(":config-add-list can only be used for " + "lists") + + with self._handle_config_error(): + option_value = self._config.get_mutable_obj(option) + option_value.append(value) + self._config.update_mutables(save_yaml=not temp) + + @cmdutils.register(instance='config-commands') + @cmdutils.argument('option', completion=configmodel.dict_option) + def config_add_dict(self, option, key, value, temp=False, replace=False): + """Add a value at the key within the option specified. + + This adds an element to a dictionary. --replace is needed to override + existing values. + + Args: + option: The name of the option. + key: The key to use. + value: The value to place in the dictionary. temp: Don't touch autoconfig.yml. replace: Whether or not we should replace, default is not. """ + opt = self._config.get_opt(option) + if not isinstance(opt.typ, configtypes.Dict): + raise cmdexc.CommandError(":config-add-list can only be used for " + "dicts") + with self._handle_config_error(): - option_value = self._config.get_obj(option) + option_value = self._config.get_mutable_obj(option) - # Attempt to replace if it's there. - if isinstance(option_value, list): - if value is None: - # In this case our key is teh "value", since we are just - # adding to the list. - option_value.append(key) + if key in option_value and not replace: + raise cmdexc.CommandError(("{} already existed in {} - use " + "--replace to overwrite!") + .format(key, option)) - if value and replace: - # In this case we are trying to replace something at an - # index. - try: - index = int(key) - if index >= len(option_value): - option_value.append(value) - else: - option_value[int(key)] = value - except: - raise cmdexc.CommandError("The index must be a number") - else: - raise cmdexc.CommandError( - "Use --replace to replace a value at an index") - elif isinstance(option_value, dict): - # Here we are trying to add something to a dictionary. - if value is None and not replace: - raise cmdexc.CommandError(( - "{value} should not be empty unless --replace is" - "provided (in which case a 'destroy' will happen)!" - ).format(value=value)) - - if not replace and key in option_value: - raise cmdexc.CommandError(( - "{key} already existed in {option} - use --replace" - "to overwrite!" - ).format(key=key, option=option)) - - # If we made it this far with a None value, we should destroy - # the option in the dictionary, otherwise we should insert it. - if value is None: - option_value.pop(key, None) - else: - option_value[key] = value - - # Set the option in the config. - self._config.set_obj(option, option_value, pattern=None, - save_yaml=not temp) + option_value[key] = value + self._config.update_mutables(save_yaml=not temp) @cmdutils.register(instance='config-commands') def config_clear(self, save=False): diff --git a/tests/unit/config/test_configcommands.py b/tests/unit/config/test_configcommands.py index f70586152..c4906fcb7 100644 --- a/tests/unit/config/test_configcommands.py +++ b/tests/unit/config/test_configcommands.py @@ -25,7 +25,7 @@ import unittest.mock import pytest from PyQt5.QtCore import QUrl -from qutebrowser.config import configcommands, configutils +from qutebrowser.config import configcommands, configtypes, configutils from qutebrowser.commands import cmdexc from qutebrowser.utils import usertypes, urlmatch from qutebrowser.keyinput import keyutils @@ -282,6 +282,54 @@ class TestCycle: assert msg.text == 'auto_save.session = true' +class TestAdd: + + """Test :config-add-list and :config-add-dict.""" + + @pytest.mark.parametrize('name', ['content.host_blocking.whitelist', + 'history_gap_interval']) + @pytest.mark.parametrize('temp', [True, False]) + @pytest.mark.parametrize('value', ['test1', 'test2', '', None]) + def test_add_list(self, commands, config_stub, yaml_value, name, temp, value): + opt_type = config_stub.get_opt(name).typ + + try: + commands.config_add_list(name, value, temp=temp) + except cmdexc.CommandError: + # We attempted to add to the dictionary with replace as false. + valid_list_types = (configtypes.List, configtypes.ListOrValue) + assert not isinstance(opt_type, valid_list_types) or not value + return + + assert str(config_stub.get(name)[-1]) == value + if temp: + assert yaml_value(name) == configutils.UNSET + else: + assert yaml_value(name)[-1] == value + + @pytest.mark.parametrize('name', ['aliases', 'history_gap_interval']) + @pytest.mark.parametrize('key', ['w', 'missingkey']) + @pytest.mark.parametrize('value', ['test1', 'test2']) + @pytest.mark.parametrize('temp', [True, False]) + @pytest.mark.parametrize('replace', [True, False]) + def test_add_dict(self, commands, config_stub, yaml_value, name, key, + value, temp, replace): + opt_type = config_stub.get_opt(name).typ + + try: + commands.config_add_dict(name, key, value, temp=temp, replace=replace) + except cmdexc.CommandError: + # We attempted to add to the dictionary with replace as false. + assert not isinstance(opt_type, configtypes.Dict) or not replace + return + + assert str(config_stub.get(name)[key]) == value + if temp: + assert yaml_value(name) == configutils.UNSET + else: + assert yaml_value(name)[key] == value + + class TestUnsetAndClear: """Test :config-unset and :config-clear."""