From 876a2bdaa16c3beb40aa0f8afdb38e30dc1dfd0b Mon Sep 17 00:00:00 2001 From: Milo Gertjejansen Date: Mon, 1 Oct 2018 22:22:39 -0500 Subject: [PATCH 1/4] New config: More powerful :config- commands: add Adds the config-add command. Modifies #2794 --- qutebrowser/completion/models/configmodel.py | 15 +++++ qutebrowser/config/configcommands.py | 66 ++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/qutebrowser/completion/models/configmodel.py b/qutebrowser/completion/models/configmodel.py index 6ee2f9566..9567f0b12 100644 --- a/qutebrowser/completion/models/configmodel.py +++ b/qutebrowser/completion/models/configmodel.py @@ -47,6 +47,21 @@ def customized_option(*, info): return model +def structure_option(*, info): + """A CompletionModel filled with set settings of structures. + + Gets lists and dicts 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 isinstance(info.config.get_obj(opt.name), (list, dict))) + model.add_category( + listcategory.ListCategory("Structure options", options) + ) + return model + + def value(optname, *values, info): """A CompletionModel filled with setting values. diff --git a/qutebrowser/config/configcommands.py b/qutebrowser/config/configcommands.py index 10e16c370..87a556571 100644 --- a/qutebrowser/config/configcommands.py +++ b/qutebrowser/config/configcommands.py @@ -250,6 +250,72 @@ class ConfigCommands: with self._handle_config_error(): 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. + + This adds an element to a dictionary or list. --replace is needed + to override existing values. + + 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. + temp: Don't touch autoconfig.yml. + replace: Whether or not we should replace, default is not. + """ + with self._handle_config_error(): + option_value = self._config.get_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 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) + @cmdutils.register(instance='config-commands') def config_clear(self, save=False): """Set all settings back to their default. From 7f0ae252cd5fd6223bf3119e2a21d4cd39e7c842 Mon Sep 17 00:00:00 2001 From: Milo Gertjejansen Date: Thu, 4 Oct 2018 18:42:33 -0500 Subject: [PATCH 2/4] 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.""" From a3528dcee81807b69d1986be3cffbdb136d674a0 Mon Sep 17 00:00:00 2001 From: Milo Gertjejansen Date: Sat, 6 Oct 2018 19:42:30 -0500 Subject: [PATCH 3/4] New config: More powerful :config- commands: add #4283 Made minor changes to the second commit which broke tests out into success and failure tests taking advantage of pytests.raises. Additionally updated several grammar issues. Continues #2794 --- qutebrowser/config/configcommands.py | 14 ++-- tests/unit/config/test_configcommands.py | 99 ++++++++++++++++++------ 2 files changed, 80 insertions(+), 33 deletions(-) diff --git a/qutebrowser/config/configcommands.py b/qutebrowser/config/configcommands.py index 04c829957..074680237 100644 --- a/qutebrowser/config/configcommands.py +++ b/qutebrowser/config/configcommands.py @@ -245,7 +245,7 @@ class ConfigCommands: Args: option: The name of the option. - temp: Don't touch autoconfig.yml. + temp: Set value temporarily until qutebrowser is closed. """ with self._handle_config_error(): self._config.unset(option, save_yaml=not temp) @@ -255,12 +255,10 @@ class ConfigCommands: def config_add_list(self, option, value, temp=False): """Append a value to a config option that is a list. - This appends an option to a config setting that is a list. - Args: option: The name of the option. - value: The value to append to the end of the dictionary. - temp: Don't touch autoconfig.yml. + value: The value to append to the end of the list. + temp: Set value temporarily until qutebrowser is closed. """ opt = self._config.get_opt(option) valid_list_types = (configtypes.List, configtypes.ListOrValue) @@ -290,15 +288,15 @@ class ConfigCommands: """ opt = self._config.get_opt(option) if not isinstance(opt.typ, configtypes.Dict): - raise cmdexc.CommandError(":config-add-list can only be used for " + raise cmdexc.CommandError(":config-add-dict can only be used for " "dicts") with self._handle_config_error(): option_value = self._config.get_mutable_obj(option) if key in option_value and not replace: - raise cmdexc.CommandError(("{} already existed in {} - use " - "--replace to overwrite!") + raise cmdexc.CommandError("{} already exists in {} - use " + "--replace to overwrite!" .format(key, option)) option_value[key] = value diff --git a/tests/unit/config/test_configcommands.py b/tests/unit/config/test_configcommands.py index c4906fcb7..3ed1f4620 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, configtypes, configutils +from qutebrowser.config import configcommands, configutils from qutebrowser.commands import cmdexc from qutebrowser.utils import usertypes, urlmatch from qutebrowser.keyinput import keyutils @@ -286,20 +286,12 @@ 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 + @pytest.mark.parametrize('value', ['test1', 'test2']) + def test_add_list(self, commands, config_stub, yaml_value, temp, value): + name = 'content.host_blocking.whitelist' - 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 + commands.config_add_list(name, value, temp=temp) assert str(config_stub.get(name)[-1]) == value if temp: @@ -307,21 +299,37 @@ class TestAdd: else: assert yaml_value(name)[-1] == value - @pytest.mark.parametrize('name', ['aliases', 'history_gap_interval']) - @pytest.mark.parametrize('key', ['w', 'missingkey']) + def test_add_list_non_list(self, commands): + name = 'history_gap_interval' + value = 'value' + with pytest.raises( + cmdexc.CommandError, + match=":config-add-list can only be used for lists"): + commands.config_add_list(name, value) + + def test_add_list_empty_value(self, commands): + name = 'content.host_blocking.whitelist' + value = '' + with pytest.raises( + cmdexc.CommandError, + match="Invalid value '{}' - may not be empty!".format(value)): + commands.config_add_list(name, value) + + def test_add_list_none_value(self, commands): + name = 'content.host_blocking.whitelist' + value = None + with pytest.raises( + cmdexc.CommandError, + match="Invalid value 'None' - may not be null!"): + commands.config_add_list(name, value) + @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 + def test_add_dict(self, commands, config_stub, yaml_value, value, temp): + name = 'aliases' + key = 'missingkey' - 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 + commands.config_add_dict(name, key, value, temp=temp) assert str(config_stub.get(name)[key]) == value if temp: @@ -329,6 +337,47 @@ class TestAdd: else: assert yaml_value(name)[key] == value + @pytest.mark.parametrize('replace', [True, False]) + def test_add_dict_replace(self, commands, config_stub, replace): + name = 'aliases' + key = 'w' + value = 'anything' + + if replace: + commands.config_add_dict(name, key, value, replace=True) + assert str(config_stub.get(name)[key]) == value + else: + with pytest.raises( + cmdexc.CommandError, + match="w already exists in aliases - use --replace to " + "overwrite!"): + commands.config_add_dict(name, key, value, replace=False) + + def test_add_dict_non_dict(self, commands): + name = 'history_gap_interval' + key = 'value' + value = 'value' + with pytest.raises( + cmdexc.CommandError, + match=":config-add-dict can only be used for dicts"): + commands.config_add_dict(name, key, value) + + def test_add_dict_empty_value(self, commands): + name = 'aliases' + key = 'missingkey' + value = '' + with pytest.raises(cmdexc.CommandError, + match="Invalid value '' - may not be empty!"): + commands.config_add_dict(name, key, value) + + def test_add_dict_none_value(self, commands): + name = 'aliases' + key = 'missingkey' + value = None + with pytest.raises(cmdexc.CommandError, + match="Invalid value 'None' - may not be null!"): + commands.config_add_dict(name, key, value) + class TestUnsetAndClear: From bcfc8fa3a84e950d0a8b0009b6129002d02a4c8c Mon Sep 17 00:00:00 2001 From: Milo Gertjejansen Date: Sat, 6 Oct 2018 19:45:36 -0500 Subject: [PATCH 4/4] New config: More powerful :config- commands: add #4283 Missed a small comment change. Continues #2794 --- qutebrowser/config/configcommands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/config/configcommands.py b/qutebrowser/config/configcommands.py index 074680237..228cee59d 100644 --- a/qutebrowser/config/configcommands.py +++ b/qutebrowser/config/configcommands.py @@ -283,7 +283,7 @@ class ConfigCommands: 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. + temp: Set value temporarily until qutebrowser is closed. replace: Whether or not we should replace, default is not. """ opt = self._config.get_opt(option)