From c5ceb6b8803625cf4ced1e8eacde41d85e7a4d23 Mon Sep 17 00:00:00 2001 From: Lakshay Kalbhor Date: Sat, 16 Sep 2017 19:23:05 +0530 Subject: [PATCH 01/77] Added functions to check OS/Platform --- qutebrowser/utils/utils.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index b9aa86f20..bbe79b39d 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -856,3 +856,18 @@ def expand_windows_drive(path): return path + "\\" else: return path + +def is_mac(): + if sys.platform == 'darwin': + return True + return False + +def is_linux(): + if sys.platform == 'linux' or sys.platform == 'linux2': + return True + return False + +def is_windows(): + if sys.platform.startswith('win'): + return True + return False From 476ca6d42f50e9fc519f19fb2b9781c54ed73095 Mon Sep 17 00:00:00 2001 From: Lakshay Kalbhor Date: Sun, 17 Sep 2017 10:27:21 +0530 Subject: [PATCH 02/77] Added 'startswith()' to each OS --- qutebrowser/utils/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index bbe79b39d..02610dbb4 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -858,12 +858,12 @@ def expand_windows_drive(path): return path def is_mac(): - if sys.platform == 'darwin': + if sys.platform.startswith('darwin'): return True return False def is_linux(): - if sys.platform == 'linux' or sys.platform == 'linux2': + if sys.platform.startswith('linux'): return True return False From 813a7b2c3a2fb01df663d0b69e3d9306b0435a1b Mon Sep 17 00:00:00 2001 From: Lakshay Kalbhor Date: Sun, 17 Sep 2017 10:56:53 +0530 Subject: [PATCH 03/77] Removed if statements --- qutebrowser/utils/utils.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index 02610dbb4..3c0916dae 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -858,16 +858,10 @@ def expand_windows_drive(path): return path def is_mac(): - if sys.platform.startswith('darwin'): - return True - return False + return sys.platform.startswith('darwin') def is_linux(): - if sys.platform.startswith('linux'): - return True - return False + return sys.platform.startswith('linux') def is_windows(): - if sys.platform.startswith('win'): - return True - return False + return sys.platform.startswith('win') From 0e743f0e0939b2f5d3a783ad11f7ee12cf254dc0 Mon Sep 17 00:00:00 2001 From: Felix Van der Jeugt Date: Tue, 19 Sep 2017 14:08:59 +0200 Subject: [PATCH 04/77] save only a changed autoconfig file --- qutebrowser/config/configfiles.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 3810bca8c..9bad78394 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -25,6 +25,7 @@ import textwrap import traceback import configparser import contextlib +import copy import yaml from PyQt5.QtCore import QSettings @@ -94,7 +95,10 @@ class YamlConfig: save_manager.add_saveable('yaml-config', self._save) def _save(self): - """Save the changed settings to the YAML file.""" + """Save the settings to the YAML file if they've changed.""" + if self.values == self._initial_values: + return + data = {'config_version': self.VERSION, 'global': self.values} with qtutils.savefile_open(self._filename) as f: f.write(textwrap.dedent(""" @@ -106,6 +110,11 @@ class YamlConfig: def load(self): """Load self.values from the configured YAML file.""" + self._initial_values = self._load() + self.values = copy.deepcopy(self._initial_values) + + def _load(self): + """Load configuration from the configured YAML file.""" try: with open(self._filename, 'r', encoding='utf-8') as f: yaml_data = utils.yaml_load(f) @@ -136,7 +145,7 @@ class YamlConfig: "'global' object is not a dict") raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) - self.values = global_obj + return global_obj class ConfigAPI: From 7b192d426ebe95c4f9054f1ffbac41ece0c039ef Mon Sep 17 00:00:00 2001 From: Felix Van der Jeugt Date: Tue, 19 Sep 2017 15:30:28 +0200 Subject: [PATCH 05/77] add unit test and fix issues with it --- qutebrowser/config/configfiles.py | 3 +- tests/unit/config/test_configfiles.py | 40 +++++++++++++++++++++------ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 9bad78394..89c679976 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -85,6 +85,7 @@ class YamlConfig: self._filename = os.path.join(standarddir.config(auto=True), 'autoconfig.yml') self.values = {} + self._initial_values = {} def init_save_manager(self, save_manager): """Make sure the config gets saved properly. @@ -119,7 +120,7 @@ class YamlConfig: with open(self._filename, 'r', encoding='utf-8') as f: yaml_data = utils.yaml_load(f) except FileNotFoundError: - return + return {} except OSError as e: desc = configexc.ConfigErrorDesc("While reading", e) raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 73b091293..fdf709744 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -72,16 +72,17 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): yaml._save() - text = autoconfig.read_text('utf-8') - lines = text.splitlines() - print(lines) - - assert lines[0].startswith('# DO NOT edit this file by hand,') - assert 'config_version: {}'.format(yaml.VERSION) in lines - - if old_config is None and not insert: - assert 'global: {}' in lines + if not insert and old_config is None: + lines = [] else: + text = autoconfig.read_text('utf-8') + lines = text.splitlines() + print(lines) + + if insert: + assert lines[0].startswith('# DO NOT edit this file by hand,') + assert 'config_version: {}'.format(yaml.VERSION) in lines + assert 'global:' in lines # WORKAROUND for https://github.com/PyCQA/pylint/issues/574 @@ -91,6 +92,27 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): assert ' tabs.show: never' in lines +@pytest.mark.parametrize('old_config', [ + None, + 'global:\n colors.hints.fg: magenta', +]) +def test_yaml_config_unchanged(fake_save_manager, config_tmpdir, old_config): + autoconfig = config_tmpdir / 'autoconfig.yml' + mtime = None + if old_config is not None: + autoconfig.write_text(old_config, 'utf-8') + mtime = autoconfig.stat().mtime + + yaml = configfiles.YamlConfig() + yaml.load() + yaml._save() + + if old_config is None: + assert not autoconfig.exists() + else: + assert autoconfig.stat().mtime == mtime + + @pytest.mark.parametrize('line, text, exception', [ ('%', 'While parsing', 'while scanning a directive'), ('global: 42', 'While loading data', "'global' object is not a dict"), From 8db630d358f6f83f420cf491a5669a29c0ba9365 Mon Sep 17 00:00:00 2001 From: Felix Van der Jeugt Date: Tue, 19 Sep 2017 17:26:03 +0200 Subject: [PATCH 06/77] don't copy values but set dirty --- qutebrowser/config/config.py | 6 +++--- qutebrowser/config/configfiles.py | 30 ++++++++++++++++++--------- tests/helpers/stubs.py | 14 ++++++++++++- tests/unit/config/test_config.py | 22 ++++++++++---------- tests/unit/config/test_configfiles.py | 2 +- 5 files changed, 48 insertions(+), 26 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 678417453..316eff2a0 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -408,7 +408,7 @@ class Config(QObject): def read_yaml(self): """Read the YAML settings from self._yaml.""" self._yaml.load() - for name, value in self._yaml.values.items(): + for name, value in self._yaml: self._set_value(self.get_opt(name), value) def get_opt(self, name): @@ -455,7 +455,7 @@ class Config(QObject): """ self._set_value(self.get_opt(name), value) if save_yaml: - self._yaml.values[name] = value + self._yaml[name] = value def set_str(self, name, value, *, save_yaml=False): """Set the given setting from a string. @@ -469,7 +469,7 @@ class Config(QObject): value)) self._set_value(opt, converted) if save_yaml: - self._yaml.values[name] = converted + self._yaml[name] = converted def update_mutables(self, *, save_yaml=False): """Update mutable settings if they changed. diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 89c679976..0e712ea22 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -84,8 +84,8 @@ class YamlConfig: def __init__(self): self._filename = os.path.join(standarddir.config(auto=True), 'autoconfig.yml') - self.values = {} - self._initial_values = {} + self._values = {} + self._changed = None def init_save_manager(self, save_manager): """Make sure the config gets saved properly. @@ -95,12 +95,26 @@ class YamlConfig: """ save_manager.add_saveable('yaml-config', self._save) + def __getitem__(self, name): + return self._values[name] + + def __setitem__(self, name, value): + if self._values.get(name) != value: + self._changed = True + self._values[name] = value + + def __contains__(self, name): + return name in self._values + + def __iter__(self): + return iter(self._values.items()) + def _save(self): """Save the settings to the YAML file if they've changed.""" - if self.values == self._initial_values: + if not self._changed: return - data = {'config_version': self.VERSION, 'global': self.values} + data = {'config_version': self.VERSION, 'global': self._values} with qtutils.savefile_open(self._filename) as f: f.write(textwrap.dedent(""" # DO NOT edit this file by hand, qutebrowser will overwrite it. @@ -110,11 +124,6 @@ class YamlConfig: utils.yaml_dump(data, f) def load(self): - """Load self.values from the configured YAML file.""" - self._initial_values = self._load() - self.values = copy.deepcopy(self._initial_values) - - def _load(self): """Load configuration from the configured YAML file.""" try: with open(self._filename, 'r', encoding='utf-8') as f: @@ -146,7 +155,8 @@ class YamlConfig: "'global' object is not a dict") raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) - return global_obj + self._values = global_obj + self._changed = False class ConfigAPI: diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py index d7dcb427f..29fbb28e1 100644 --- a/tests/helpers/stubs.py +++ b/tests/helpers/stubs.py @@ -414,12 +414,24 @@ class FakeYamlConfig: """Fake configfiles.YamlConfig object.""" def __init__(self): - self.values = {} self.loaded = False + self._values = {} def load(self): self.loaded = True + def __contains__(self, item): + return item in self._values + + def __iter__(self): + return iter(self._values.items()) + + def __setitem__(self, key, value): + self._values[key] = value + + def __getitem__(self, key): + return self._values[key] + class StatusBarCommandStub(QLineEdit): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 4b86d9a4e..ef0b55d25 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -317,9 +317,9 @@ class TestSetConfigCommand: assert config_stub.get(option) == new_value if temp: - assert option not in config_stub._yaml.values + assert option not in config_stub._yaml else: - assert config_stub._yaml.values[option] == new_value + assert config_stub._yaml[option] == new_value @pytest.mark.parametrize('temp', [True, False]) def test_set_temp_override(self, commands, config_stub, temp): @@ -335,7 +335,7 @@ class TestSetConfigCommand: commands.set(0, 'url.auto_search', 'never', temp=True) assert config_stub.val.url.auto_search == 'never' - assert config_stub._yaml.values['url.auto_search'] == 'dns' + assert config_stub._yaml['url.auto_search'] == 'dns' def test_set_print(self, config_stub, commands, message_mock): """Run ':set -p url.auto_search never'. @@ -357,7 +357,7 @@ class TestSetConfigCommand: assert not config_stub.val.auto_save.session commands.set(0, 'auto_save.session!') assert config_stub.val.auto_save.session - assert config_stub._yaml.values['auto_save.session'] + assert config_stub._yaml['auto_save.session'] def test_set_toggle_nonbool(self, commands, config_stub): """Run ':set url.auto_search!'. @@ -439,7 +439,7 @@ class TestSetConfigCommand: config_stub.set_obj(opt, initial) commands.set(0, opt, 'green', 'magenta', 'blue', 'yellow') assert config_stub.get(opt) == expected - assert config_stub._yaml.values[opt] == expected + assert config_stub._yaml[opt] == expected class TestBindConfigCommand: @@ -464,7 +464,7 @@ class TestBindConfigCommand: commands.bind('a', command) assert keyconf.get_command('a', 'normal') == command - yaml_bindings = config_stub._yaml.values['bindings.commands']['normal'] + yaml_bindings = config_stub._yaml['bindings.commands']['normal'] assert yaml_bindings['a'] == command @pytest.mark.parametrize('key, mode, expected', [ @@ -565,7 +565,7 @@ class TestBindConfigCommand: commands.unbind(key) assert keyconf.get_command(key, 'normal') is None - yaml_bindings = config_stub._yaml.values['bindings.commands']['normal'] + yaml_bindings = config_stub._yaml['bindings.commands']['normal'] if key in 'bc': # Custom binding assert normalized not in yaml_bindings @@ -612,7 +612,7 @@ class TestConfig: def test_read_yaml(self, conf): assert not conf._yaml.loaded - conf._yaml.values['content.plugins'] = True + conf._yaml['content.plugins'] = True conf.read_yaml() @@ -620,7 +620,7 @@ class TestConfig: assert conf._values['content.plugins'] is True def test_read_yaml_invalid(self, conf): - conf._yaml.values['foo.bar'] = True + conf._yaml['foo.bar'] = True with pytest.raises(configexc.NoOptionError): conf.read_yaml() @@ -743,9 +743,9 @@ class TestConfig: meth(option, value, save_yaml=save_yaml) assert conf._values[option] is True if save_yaml: - assert conf._yaml.values[option] is True + assert conf._yaml[option] is True else: - assert option not in conf._yaml.values + assert option not in conf._yaml @pytest.mark.parametrize('method', ['set_obj', 'set_str']) def test_set_invalid(self, conf, qtbot, method): diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index fdf709744..3680cf6b4 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -68,7 +68,7 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): yaml.load() if insert: - yaml.values['tabs.show'] = 'never' + yaml['tabs.show'] = 'never' yaml._save() From 8e14d1b7e6c77e58cbe7fda42875da8e81202149 Mon Sep 17 00:00:00 2001 From: Felix Van der Jeugt Date: Tue, 19 Sep 2017 17:47:38 +0200 Subject: [PATCH 07/77] remove unused import --- qutebrowser/config/configfiles.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 0e712ea22..b187a6c08 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -25,7 +25,6 @@ import textwrap import traceback import configparser import contextlib -import copy import yaml from PyQt5.QtCore import QSettings From 285b5343846929f38ac1284a6c9daee4847f4596 Mon Sep 17 00:00:00 2001 From: Felix Van der Jeugt Date: Wed, 20 Sep 2017 10:04:34 +0200 Subject: [PATCH 08/77] make changed dirty and save on duplicate write --- qutebrowser/config/configfiles.py | 9 ++++----- tests/unit/config/test_configfiles.py | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index b187a6c08..06c3931f6 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -84,7 +84,7 @@ class YamlConfig: self._filename = os.path.join(standarddir.config(auto=True), 'autoconfig.yml') self._values = {} - self._changed = None + self._dirty = None def init_save_manager(self, save_manager): """Make sure the config gets saved properly. @@ -98,8 +98,7 @@ class YamlConfig: return self._values[name] def __setitem__(self, name, value): - if self._values.get(name) != value: - self._changed = True + self._dirty = True self._values[name] = value def __contains__(self, name): @@ -110,7 +109,7 @@ class YamlConfig: def _save(self): """Save the settings to the YAML file if they've changed.""" - if not self._changed: + if not self._dirty: return data = {'config_version': self.VERSION, 'global': self._values} @@ -155,7 +154,7 @@ class YamlConfig: raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) self._values = global_obj - self._changed = False + self._dirty = False class ConfigAPI: diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 3680cf6b4..5736904c2 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -77,7 +77,6 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): else: text = autoconfig.read_text('utf-8') lines = text.splitlines() - print(lines) if insert: assert lines[0].startswith('# DO NOT edit this file by hand,') @@ -85,6 +84,8 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): assert 'global:' in lines + print(lines) + # WORKAROUND for https://github.com/PyCQA/pylint/issues/574 if 'magenta' in (old_config or ''): # pylint: disable=superfluous-parens assert ' colors.hints.fg: magenta' in lines From 9ddc59e8e5237a4dfa387a0954d1b8ed8fb676b7 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 20 Sep 2017 10:08:54 +0200 Subject: [PATCH 09/77] Also add utils.is_posix() --- qutebrowser/utils/utils.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index f6e71cce3..ce3d3a98a 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -19,6 +19,7 @@ """Other utilities which don't fit anywhere else.""" +import os import io import re import sys @@ -886,8 +887,14 @@ def yaml_dump(data, f=None): def is_mac(): return sys.platform.startswith('darwin') + def is_linux(): return sys.platform.startswith('linux') + def is_windows(): return sys.platform.startswith('win') + + +def is_posix(): + return os.name == 'posix' From e4594bd68878727606d5138a686f545eba7364aa Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 20 Sep 2017 10:40:41 +0200 Subject: [PATCH 10/77] Use attributes for utils.is_* --- qutebrowser/utils/utils.py | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index ce3d3a98a..12d85f608 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -50,6 +50,11 @@ from qutebrowser.utils import qtutils, log, debug fake_clipboard = None log_clipboard = False +is_mac = sys.platform.startswith('darwin') +is_linux = sys.platform.startswith('linux') +is_windows = sys.platform.startswith('win') +is_posix = os.name == 'posix' + class ClipboardError(Exception): @@ -882,19 +887,3 @@ def yaml_dump(data, f=None): return None else: return yaml_data.decode('utf-8') - - -def is_mac(): - return sys.platform.startswith('darwin') - - -def is_linux(): - return sys.platform.startswith('linux') - - -def is_windows(): - return sys.platform.startswith('win') - - -def is_posix(): - return os.name == 'posix' From ef1c83862b8166066a860fbfac93a19ec26bb8c3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 20 Sep 2017 10:39:39 +0200 Subject: [PATCH 11/77] Use utils.is_* for platform checks everywhere --- misc/qutebrowser.spec | 5 +- qutebrowser/browser/commands.py | 3 +- qutebrowser/browser/downloads.py | 2 +- .../browser/webengine/webenginesettings.py | 3 +- .../browser/webkit/network/networkmanager.py | 2 +- qutebrowser/browser/webkit/webkittab.py | 9 ++-- qutebrowser/browser/webkit/webview.py | 4 +- qutebrowser/commands/userscripts.py | 6 +-- qutebrowser/misc/crashsignal.py | 4 +- qutebrowser/misc/ipc.py | 13 ++--- qutebrowser/utils/log.py | 2 + qutebrowser/utils/standarddir.py | 19 ++++--- qutebrowser/utils/utils.py | 2 +- qutebrowser/utils/version.py | 6 +-- scripts/dev/build_release.py | 51 ++++++++++--------- scripts/dev/check_coverage.py | 9 ++-- tests/conftest.py | 16 +++--- tests/end2end/conftest.py | 4 +- tests/end2end/features/conftest.py | 15 +++--- .../browser/webkit/network/test_filescheme.py | 7 +-- tests/unit/commands/test_userscripts.py | 6 ++- tests/unit/config/test_configfiles.py | 4 +- tests/unit/keyinput/test_basekeyparser.py | 7 ++- tests/unit/misc/test_ipc.py | 11 ++-- tests/unit/misc/test_msgbox.py | 7 ++- tests/unit/misc/test_utilcmds.py | 4 +- tests/unit/scripts/test_check_coverage.py | 4 +- tests/unit/utils/test_error.py | 5 +- tests/unit/utils/test_jinja.py | 2 +- tests/unit/utils/test_qtutils.py | 7 ++- tests/unit/utils/test_standarddir.py | 16 +++--- tests/unit/utils/test_utils.py | 4 +- tests/unit/utils/test_version.py | 13 +++-- 33 files changed, 133 insertions(+), 139 deletions(-) diff --git a/misc/qutebrowser.spec b/misc/qutebrowser.spec index bcbd67405..e9ccc1619 100644 --- a/misc/qutebrowser.spec +++ b/misc/qutebrowser.spec @@ -5,6 +5,7 @@ import os sys.path.insert(0, os.getcwd()) from scripts import setupcommon +from qutebrowser import utils block_cipher = None @@ -30,9 +31,9 @@ def get_data_files(): setupcommon.write_git_file() -if os.name == 'nt': +if utils.is_windows: icon = 'icons/qutebrowser.ico' -elif sys.platform == 'darwin': +elif utils.is_mac: icon = 'icons/qutebrowser.icns' else: icon = None diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 5bb07f129..f1dcc19e8 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -20,7 +20,6 @@ """Command dispatcher for TabbedBrowser.""" import os -import sys import os.path import shlex import functools @@ -430,7 +429,7 @@ class CommandDispatcher: tab.printing.to_printer(diag.printer(), print_callback) diag = QPrintDialog(tab) - if sys.platform == 'darwin': + if utils.is_mac: # For some reason we get a segfault when using open() on macOS ret = diag.exec_() if ret == QDialog.Accepted: diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 9458e39d0..07e5c7c30 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -174,7 +174,7 @@ def transform_path(path): Returns None if the path is invalid on the current platform. """ - if sys.platform != "win32": + if not utils.is_windows: return path path = utils.expand_windows_drive(path) # Drive dependent working directories are not supported, e.g. diff --git a/qutebrowser/browser/webengine/webenginesettings.py b/qutebrowser/browser/webengine/webenginesettings.py index 0bac53915..7b4ece0e8 100644 --- a/qutebrowser/browser/webengine/webenginesettings.py +++ b/qutebrowser/browser/webengine/webenginesettings.py @@ -28,7 +28,6 @@ Module attributes: """ import os -import sys import ctypes import ctypes.util @@ -207,7 +206,7 @@ def init(args): # WORKAROUND for # https://bugs.launchpad.net/ubuntu/+source/python-qt4/+bug/941826 - if sys.platform == 'linux': + if utils.is_linux: ctypes.CDLL(ctypes.util.find_library("GL"), mode=ctypes.RTLD_GLOBAL) _init_profiles() diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 42de3a65d..beaa690ca 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -58,7 +58,7 @@ def _is_secure_cipher(cipher): # https://codereview.qt-project.org/#/c/75943/ return False # OpenSSL should already protect against this in a better way - elif cipher.keyExchangeMethod() == 'DH' and os.name == 'nt': + elif cipher.keyExchangeMethod() == 'DH' and utils.is_windows: # https://weakdh.org/ return False elif cipher.encryptionMethod().upper().startswith('RC4'): diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index cf2156431..9a9daad77 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -19,7 +19,6 @@ """Wrapper over our (QtWebKit) WebView.""" -import sys import functools import xml.etree.ElementTree @@ -223,11 +222,11 @@ class WebKitCaret(browsertab.AbstractCaret): def move_to_end_of_word(self, count=1): if not self.selection_enabled: act = [QWebPage.MoveToNextWord] - if sys.platform == 'win32': # pragma: no cover + if utils.is_windows: # pragma: no cover act.append(QWebPage.MoveToPreviousChar) else: act = [QWebPage.SelectNextWord] - if sys.platform == 'win32': # pragma: no cover + if utils.is_windows: # pragma: no cover act.append(QWebPage.SelectPreviousChar) for _ in range(count): for a in act: @@ -236,11 +235,11 @@ class WebKitCaret(browsertab.AbstractCaret): def move_to_next_word(self, count=1): if not self.selection_enabled: act = [QWebPage.MoveToNextWord] - if sys.platform != 'win32': # pragma: no branch + if not utils.is_windows: # pragma: no branch act.append(QWebPage.MoveToNextChar) else: act = [QWebPage.SelectNextWord] - if sys.platform != 'win32': # pragma: no branch + if not utils.is_windows: # pragma: no branch act.append(QWebPage.SelectNextChar) for _ in range(count): for a in act: diff --git a/qutebrowser/browser/webkit/webview.py b/qutebrowser/browser/webkit/webview.py index 5608995a4..4f1ff10c8 100644 --- a/qutebrowser/browser/webkit/webview.py +++ b/qutebrowser/browser/webkit/webview.py @@ -19,8 +19,6 @@ """The main browser widgets.""" -import sys - from PyQt5.QtCore import pyqtSignal, pyqtSlot, Qt, QUrl from PyQt5.QtGui import QPalette from PyQt5.QtWidgets import QStyleFactory @@ -57,7 +55,7 @@ class WebView(QWebView): def __init__(self, *, win_id, tab_id, tab, private, parent=None): super().__init__(parent) - if sys.platform == 'darwin': + if utils.is_mac: # WORKAROUND for https://bugreports.qt.io/browse/QTBUG-42948 # See https://github.com/qutebrowser/qutebrowser/issues/462 self.setStyle(QStyleFactory.create('Fusion')) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 5449f1598..10e509ce7 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -25,7 +25,7 @@ import tempfile from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QSocketNotifier -from qutebrowser.utils import message, log, objreg, standarddir +from qutebrowser.utils import message, log, objreg, standarddir, utils from qutebrowser.commands import runners from qutebrowser.config import config from qutebrowser.misc import guiprocess @@ -406,9 +406,9 @@ def run_async(tab, cmd, *args, win_id, env, verbose=False): window=win_id) commandrunner = runners.CommandRunner(win_id, parent=tabbed_browser) - if os.name == 'posix': + if utils.is_posix: runner = _POSIXUserscriptRunner(tabbed_browser) - elif os.name == 'nt': # pragma: no cover + elif utils.is_windows: # pragma: no cover runner = _WindowsUserscriptRunner(tabbed_browser) else: # pragma: no cover raise UnsupportedError diff --git a/qutebrowser/misc/crashsignal.py b/qutebrowser/misc/crashsignal.py index 393aa26f0..9a1b88942 100644 --- a/qutebrowser/misc/crashsignal.py +++ b/qutebrowser/misc/crashsignal.py @@ -40,7 +40,7 @@ from PyQt5.QtWidgets import QApplication, QDialog from qutebrowser.commands import cmdutils from qutebrowser.misc import earlyinit, crashdialog -from qutebrowser.utils import usertypes, standarddir, log, objreg, debug +from qutebrowser.utils import usertypes, standarddir, log, objreg, debug, utils @attr.s @@ -312,7 +312,7 @@ class SignalHandler(QObject): self._orig_handlers[signal.SIGTERM] = signal.signal( signal.SIGTERM, self.interrupt) - if os.name == 'posix' and hasattr(signal, 'set_wakeup_fd'): + if utils.is_posix and hasattr(signal, 'set_wakeup_fd'): # pylint: disable=import-error,no-member,useless-suppression import fcntl read_fd, write_fd = os.pipe() diff --git a/qutebrowser/misc/ipc.py b/qutebrowser/misc/ipc.py index 562cc84cc..e308ac8a0 100644 --- a/qutebrowser/misc/ipc.py +++ b/qutebrowser/misc/ipc.py @@ -30,7 +30,7 @@ from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, Qt from PyQt5.QtNetwork import QLocalSocket, QLocalServer, QAbstractSocket import qutebrowser -from qutebrowser.utils import log, usertypes, error, objreg, standarddir +from qutebrowser.utils import log, usertypes, error, objreg, standarddir, utils CONNECT_TIMEOUT = 100 # timeout for connecting/disconnecting @@ -51,7 +51,7 @@ def _get_socketname_windows(basedir): def _get_socketname(basedir): """Get a socketname to use.""" - if os.name == 'nt': # pragma: no cover + if utils.is_windows: # pragma: no cover return _get_socketname_windows(basedir) parts_to_hash = [getpass.getuser()] @@ -139,8 +139,6 @@ class IPCServer(QObject): _server: A QLocalServer to accept new connections. _socket: The QLocalSocket we're currently connected to. _socketname: The socketname to use. - _socketopts_ok: Set if using setSocketOptions is working with this - OS/Qt version. _atime_timer: Timer to update the atime of the socket regularly. Signals: @@ -169,7 +167,7 @@ class IPCServer(QObject): self._timer.setInterval(READ_TIMEOUT) self._timer.timeout.connect(self.on_timeout) - if os.name == 'nt': # pragma: no cover + if utils.is_windows: # pragma: no cover self._atime_timer = None else: self._atime_timer = usertypes.Timer(self, 'ipc-atime') @@ -182,8 +180,7 @@ class IPCServer(QObject): self._socket = None self._old_socket = None - self._socketopts_ok = os.name == 'nt' - if self._socketopts_ok: # pragma: no cover + if utils.is_windows: # pragma: no cover # If we use setSocketOptions on Unix with Qt < 5.4, we get a # NameError while listening... log.ipc.debug("Calling setSocketOptions") @@ -210,7 +207,7 @@ class IPCServer(QObject): raise AddressInUseError(self._server) else: raise ListenError(self._server) - if not self._socketopts_ok: # pragma: no cover + if not utils.is_windows: # pragma: no cover # If we use setSocketOptions on Unix with Qt < 5.4, we get a # NameError while listening. # (see b135569d5c6e68c735ea83f42e4baf51f7972281) diff --git a/qutebrowser/utils/log.py b/qutebrowser/utils/log.py index 3049d5dfe..fa0208ea7 100644 --- a/qutebrowser/utils/log.py +++ b/qutebrowser/utils/log.py @@ -409,6 +409,8 @@ def qt_message_handler(msg_type, context, msg): # https://codereview.qt-project.org/176831 "QObject::disconnect: Unexpected null parameter", ] + # not using utils.is_mac here, because we can't be sure we can successfully + # import the utils module here. if sys.platform == 'darwin': suppressed_msgs += [ 'libpng warning: iCCP: known incorrect sRGB profile', diff --git a/qutebrowser/utils/standarddir.py b/qutebrowser/utils/standarddir.py index 468fba51e..d6ad7fc39 100644 --- a/qutebrowser/utils/standarddir.py +++ b/qutebrowser/utils/standarddir.py @@ -20,7 +20,6 @@ """Utilities to get and initialize data/config paths.""" import os -import sys import shutil import os.path import contextlib @@ -28,7 +27,7 @@ import contextlib from PyQt5.QtCore import QStandardPaths from PyQt5.QtWidgets import QApplication -from qutebrowser.utils import log, debug, usertypes, message +from qutebrowser.utils import log, debug, usertypes, message, utils # The cached locations _locations = {} @@ -69,7 +68,7 @@ def _init_config(args): typ = QStandardPaths.ConfigLocation overridden, path = _from_args(typ, args) if not overridden: - if os.name == 'nt': + if utils.is_windows: app_data_path = _writable_location( QStandardPaths.AppDataLocation) path = os.path.join(app_data_path, 'config') @@ -80,7 +79,7 @@ def _init_config(args): _locations[Location.auto_config] = path # Override the normal (non-auto) config on macOS - if sys.platform == 'darwin': + if utils.is_mac: overridden, path = _from_args(typ, args) if not overridden: # pragma: no branch path = os.path.expanduser('~/.' + APPNAME) @@ -104,7 +103,7 @@ def _init_data(args): typ = QStandardPaths.DataLocation overridden, path = _from_args(typ, args) if not overridden: - if os.name == 'nt': + if utils.is_windows: app_data_path = _writable_location(QStandardPaths.AppDataLocation) path = os.path.join(app_data_path, 'data') else: @@ -114,7 +113,7 @@ def _init_data(args): # system_data _locations.pop(Location.system_data, None) # Remove old state - if sys.platform.startswith('linux'): + if utils.is_linux: path = '/usr/share/' + APPNAME if os.path.exists(path): _locations[Location.system_data] = path @@ -139,7 +138,7 @@ def _init_cache(args): typ = QStandardPaths.CacheLocation overridden, path = _from_args(typ, args) if not overridden: - if os.name == 'nt': + if utils.is_windows: # Local, not Roaming! data_path = _writable_location(QStandardPaths.DataLocation) path = os.path.join(data_path, 'cache') @@ -172,7 +171,7 @@ def download(): def _init_runtime(args): """Initialize location for runtime data.""" - if sys.platform.startswith('linux'): + if utils.is_linux: typ = QStandardPaths.RuntimeLocation else: # RuntimeLocation is a weird path on macOS and Windows. @@ -312,9 +311,9 @@ def init(args): _init_dirs(args) _init_cachedir_tag() if args is not None and getattr(args, 'basedir', None) is None: - if sys.platform == 'darwin': # pragma: no cover + if utils.is_mac: # pragma: no cover _move_macos() - elif os.name == 'nt': # pragma: no cover + elif utils.is_windows: # pragma: no cover _move_windows() diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index 12d85f608..23c442008 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -383,7 +383,7 @@ def keyevent_to_string(e): A name of the key (combination) as a string or None if only modifiers are pressed.. """ - if sys.platform == 'darwin': + if is_mac: # Qt swaps Ctrl/Meta on macOS, so we switch it back here so the user # can use it in the config as expected. See: # https://github.com/qutebrowser/qutebrowser/issues/110 diff --git a/qutebrowser/utils/version.py b/qutebrowser/utils/version.py index 673590208..0f1b2233d 100644 --- a/qutebrowser/utils/version.py +++ b/qutebrowser/utils/version.py @@ -248,12 +248,12 @@ def _os_info(): """ lines = [] releaseinfo = None - if sys.platform == 'linux': + if utils.is_linux: osver = '' releaseinfo = _release_info() - elif sys.platform == 'win32': + elif utils.is_windows: osver = ', '.join(platform.win32_ver()) - elif sys.platform == 'darwin': + elif utils.is_mac: release, versioninfo, machine = platform.mac_ver() if all(not e for e in versioninfo): versioninfo = '' diff --git a/scripts/dev/build_release.py b/scripts/dev/build_release.py index d5c03ad02..a51c8f2b3 100755 --- a/scripts/dev/build_release.py +++ b/scripts/dev/build_release.py @@ -36,7 +36,8 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir, os.pardir)) import qutebrowser -from scripts import utils +from qutebrowser.utils import utils +from scripts import utils as scriptutils # from scripts.dev import update_3rdparty @@ -70,7 +71,7 @@ def call_tox(toxenv, *args, python=sys.executable): def run_asciidoc2html(args): """Common buildsteps used for all OS'.""" - utils.print_title("Running asciidoc2html.py") + scriptutils.print_title("Running asciidoc2html.py") if args.asciidoc is not None: a2h_args = ['--asciidoc'] + args.asciidoc else: @@ -127,7 +128,7 @@ def patch_mac_app(): def build_mac(): """Build macOS .dmg/.app.""" - utils.print_title("Cleaning up...") + scriptutils.print_title("Cleaning up...") for f in ['wc.dmg', 'template.dmg']: try: os.remove(f) @@ -135,20 +136,20 @@ def build_mac(): pass for d in ['dist', 'build']: shutil.rmtree(d, ignore_errors=True) - utils.print_title("Updating 3rdparty content") + scriptutils.print_title("Updating 3rdparty content") # Currently disabled because QtWebEngine has no pdfjs support # update_3rdparty.run(ace=False, pdfjs=True, fancy_dmg=False) - utils.print_title("Building .app via pyinstaller") + scriptutils.print_title("Building .app via pyinstaller") call_tox('pyinstaller', '-r') - utils.print_title("Patching .app") + scriptutils.print_title("Patching .app") patch_mac_app() - utils.print_title("Building .dmg") + scriptutils.print_title("Building .dmg") subprocess.check_call(['make', '-f', 'scripts/dev/Makefile-dmg']) dmg_name = 'qutebrowser-{}.dmg'.format(qutebrowser.__version__) os.rename('qutebrowser.dmg', dmg_name) - utils.print_title("Running smoke test") + scriptutils.print_title("Running smoke test") try: with tempfile.TemporaryDirectory() as tmpdir: @@ -177,11 +178,11 @@ def patch_windows(out_dir): def build_windows(): """Build windows executables/setups.""" - utils.print_title("Updating 3rdparty content") + scriptutils.print_title("Updating 3rdparty content") # Currently disabled because QtWebEngine has no pdfjs support # update_3rdparty.run(ace=False, pdfjs=True, fancy_dmg=False) - utils.print_title("Building Windows binaries") + scriptutils.print_title("Building Windows binaries") parts = str(sys.version_info.major), str(sys.version_info.minor) ver = ''.join(parts) python_x86 = r'C:\Python{}-32\python.exe'.format(ver) @@ -194,19 +195,19 @@ def build_windows(): artifacts = [] - utils.print_title("Running pyinstaller 32bit") + scriptutils.print_title("Running pyinstaller 32bit") _maybe_remove(out_32) call_tox('pyinstaller', '-r', python=python_x86) shutil.move(out_pyinstaller, out_32) patch_windows(out_32) - utils.print_title("Running pyinstaller 64bit") + scriptutils.print_title("Running pyinstaller 64bit") _maybe_remove(out_64) call_tox('pyinstaller', '-r', python=python_x64) shutil.move(out_pyinstaller, out_64) patch_windows(out_64) - utils.print_title("Building installers") + scriptutils.print_title("Building installers") subprocess.check_call(['makensis.exe', '/DVERSION={}'.format(qutebrowser.__version__), 'misc/qutebrowser.nsi']) @@ -227,12 +228,12 @@ def build_windows(): 'Windows 64bit installer'), ] - utils.print_title("Running 32bit smoke test") + scriptutils.print_title("Running 32bit smoke test") smoke_test(os.path.join(out_32, 'qutebrowser.exe')) - utils.print_title("Running 64bit smoke test") + scriptutils.print_title("Running 64bit smoke test") smoke_test(os.path.join(out_64, 'qutebrowser.exe')) - utils.print_title("Zipping 32bit standalone...") + scriptutils.print_title("Zipping 32bit standalone...") name = 'qutebrowser-{}-windows-standalone-win32'.format( qutebrowser.__version__) shutil.make_archive(name, 'zip', 'dist', os.path.basename(out_32)) @@ -240,7 +241,7 @@ def build_windows(): 'application/zip', 'Windows 32bit standalone')) - utils.print_title("Zipping 64bit standalone...") + scriptutils.print_title("Zipping 64bit standalone...") name = 'qutebrowser-{}-windows-standalone-amd64'.format( qutebrowser.__version__) shutil.make_archive(name, 'zip', 'dist', os.path.basename(out_64)) @@ -253,7 +254,7 @@ def build_windows(): def build_sdist(): """Build an sdist and list the contents.""" - utils.print_title("Building sdist") + scriptutils.print_title("Building sdist") _maybe_remove('dist') @@ -276,10 +277,10 @@ def build_sdist(): assert '.pyc' not in by_ext - utils.print_title("sdist contents") + scriptutils.print_title("sdist contents") for ext, files in sorted(by_ext.items()): - utils.print_subtitle(ext) + scriptutils.print_subtitle(ext) print('\n'.join(files)) filename = 'qutebrowser-{}.tar.gz'.format(qutebrowser.__version__) @@ -308,7 +309,7 @@ def github_upload(artifacts, tag): tag: The name of the release tag """ import github3 - utils.print_title("Uploading to github...") + scriptutils.print_title("Uploading to github...") token = read_github_token() gh = github3.login(token=token) @@ -343,7 +344,7 @@ def main(): parser.add_argument('--upload', help="Tag to upload the release for", nargs=1, required=False, metavar='TAG') args = parser.parse_args() - utils.change_cwd() + scriptutils.change_cwd() upload_to_pypi = False @@ -353,7 +354,7 @@ def main(): import github3 # pylint: disable=unused-variable read_github_token() - if os.name == 'nt': + if utils.is_windows: if sys.maxsize > 2**32: # WORKAROUND print("Due to a python/Windows bug, this script needs to be run ") @@ -364,7 +365,7 @@ def main(): sys.exit(1) run_asciidoc2html(args) artifacts = build_windows() - elif sys.platform == 'darwin': + elif utils.is_mac: run_asciidoc2html(args) artifacts = build_mac() else: @@ -372,7 +373,7 @@ def main(): upload_to_pypi = True if args.upload is not None: - utils.print_title("Press enter to release...") + scriptutils.print_title("Press enter to release...") input() github_upload(artifacts, args.upload[0]) if upload_to_pypi: diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index dfd336d16..aa5072536 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -32,7 +32,8 @@ import attr sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir, os.pardir)) -from scripts import utils +from scripts import utils as scriptutils +from qutebrowser.utils import utils @attr.s @@ -207,7 +208,7 @@ def _get_filename(filename): def check(fileobj, perfect_files): """Main entry point which parses/checks coverage.xml if applicable.""" - if sys.platform != 'linux': + if not utils.is_linux: raise Skipped("on non-Linux system.") elif '-k' in sys.argv[1:]: raise Skipped("because -k is given.") @@ -272,7 +273,7 @@ def main_check(): if messages: print() print() - utils.print_title("Coverage check failed") + scriptutils.print_title("Coverage check failed") for msg in messages: print(msg.text) print() @@ -323,7 +324,7 @@ def main_check_all(): def main(): - utils.change_cwd() + scriptutils.change_cwd() if '--check-all' in sys.argv: return main_check_all() else: diff --git a/tests/conftest.py b/tests/conftest.py index b12f85d6e..0ba48e330 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -35,7 +35,7 @@ from helpers import logfail from helpers.logfail import fail_on_logging from helpers.messagemock import message_mock from helpers.fixtures import * -from qutebrowser.utils import qtutils, standarddir, usertypes +from qutebrowser.utils import qtutils, standarddir, usertypes, utils from qutebrowser.misc import objects import qutebrowser.app # To register commands @@ -50,18 +50,18 @@ hypothesis.settings.load_profile('default') def _apply_platform_markers(config, item): """Apply a skip marker to a given item.""" markers = [ - ('posix', os.name != 'posix', "Requires a POSIX os"), - ('windows', os.name != 'nt', "Requires Windows"), - ('linux', not sys.platform.startswith('linux'), "Requires Linux"), - ('mac', sys.platform != 'darwin', "Requires macOS"), - ('not_mac', sys.platform == 'darwin', "Skipped on macOS"), + ('posix', not utils.is_posix, "Requires a POSIX os"), + ('windows', not utils.is_mac, "Requires Windows"), + ('linux', not utils.is_linux, "Requires Linux"), + ('mac', not utils.is_mac, "Requires macOS"), + ('not_mac', utils.is_mac, "Skipped on macOS"), ('not_frozen', getattr(sys, 'frozen', False), "Can't be run when frozen"), ('frozen', not getattr(sys, 'frozen', False), "Can only run when frozen"), ('ci', 'CI' not in os.environ, "Only runs on CI."), ('no_ci', 'CI' in os.environ, "Skipped on CI."), - ('issue2478', os.name == 'nt' and config.webengine, + ('issue2478', utils.is_windows and config.webengine, "Broken with QtWebEngine on Windows"), ] @@ -181,7 +181,7 @@ def check_display(request): request.config.xvfb is not None): raise Exception("Xvfb is running on buildbot!") - if sys.platform == 'linux' and not os.environ.get('DISPLAY', ''): + if utils.is_linux and not os.environ.get('DISPLAY', ''): raise Exception("No display and no Xvfb available!") diff --git a/tests/end2end/conftest.py b/tests/end2end/conftest.py index 1fd6eb874..51a0497bf 100644 --- a/tests/end2end/conftest.py +++ b/tests/end2end/conftest.py @@ -38,7 +38,7 @@ from end2end.fixtures.webserver import server, server_after_test, ssl_server from end2end.fixtures.quteprocess import (quteproc_process, quteproc, quteproc_new) from end2end.fixtures.testprocess import pytest_runtest_makereport -from qutebrowser.utils import qtutils +from qutebrowser.utils import qtutils, utils def pytest_configure(config): @@ -144,7 +144,7 @@ def pytest_collection_modifyitems(config, items): ('qtwebengine_flaky', 'Flaky with QtWebEngine', pytest.mark.skipif, config.webengine), ('qtwebengine_mac_xfail', 'Fails on macOS with QtWebEngine', - pytest.mark.xfail, config.webengine and sys.platform == 'darwin'), + pytest.mark.xfail, config.webengine and utils.is_mac), ] for item in items: diff --git a/tests/end2end/features/conftest.py b/tests/end2end/features/conftest.py index 52dfe616c..7f5b4e2a6 100644 --- a/tests/end2end/features/conftest.py +++ b/tests/end2end/features/conftest.py @@ -31,9 +31,9 @@ import textwrap import pytest import pytest_bdd as bdd -from qutebrowser.utils import log +from qutebrowser.utils import log, utils from qutebrowser.browser import pdfjs -from helpers import utils +from helpers import utils as testutils def _get_echo_exe_path(): @@ -42,8 +42,9 @@ def _get_echo_exe_path(): Return: Path to the "echo"-utility. """ - if sys.platform == "win32": - return os.path.join(utils.abs_datapath(), 'userscripts', 'echo.bat') + if utils.is_windows: + return os.path.join(testutils.abs_datapath(), 'userscripts', + 'echo.bat') else: return 'echo' @@ -255,7 +256,7 @@ def run_command(quteproc, server, tmpdir, command): invalid = False command = command.replace('(port)', str(server.port)) - command = command.replace('(testdata)', utils.abs_datapath()) + command = command.replace('(testdata)', testutils.abs_datapath()) command = command.replace('(tmpdir)', str(tmpdir)) command = command.replace('(dirsep)', os.sep) command = command.replace('(echo-exe)', _get_echo_exe_path()) @@ -349,7 +350,7 @@ def hint(quteproc, args): @bdd.when(bdd.parsers.parse('I hint with args "{args}" and follow {letter}')) def hint_and_follow(quteproc, args, letter): - args = args.replace('(testdata)', utils.abs_datapath()) + args = args.replace('(testdata)', testutils.abs_datapath()) quteproc.send_cmd(':hint {}'.format(args)) quteproc.wait_for(message='hints: *') quteproc.send_cmd(':follow-hint {}'.format(letter)) @@ -502,7 +503,7 @@ def check_header(quteproc, header, value): assert header not in data['headers'] else: actual = data['headers'][header] - assert utils.pattern_match(pattern=value, value=actual) + assert testutils.pattern_match(pattern=value, value=actual) @bdd.then(bdd.parsers.parse('the page should contain the html "{text}"')) diff --git a/tests/unit/browser/webkit/network/test_filescheme.py b/tests/unit/browser/webkit/network/test_filescheme.py index 0cc485556..13700a103 100644 --- a/tests/unit/browser/webkit/network/test_filescheme.py +++ b/tests/unit/browser/webkit/network/test_filescheme.py @@ -26,7 +26,7 @@ from PyQt5.QtCore import QUrl from PyQt5.QtNetwork import QNetworkRequest from qutebrowser.browser.webkit.network import filescheme -from qutebrowser.utils import urlutils +from qutebrowser.utils import urlutils, utils @pytest.mark.parametrize('create_file, create_dir, filterfunc, expected', [ @@ -228,10 +228,7 @@ class TestDirbrowserHtml: assert parsed.folders == [bar_item] def test_root_dir(self, tmpdir, parser): - if os.name == 'nt': - root_dir = 'C:\\' - else: - root_dir = '/' + root_dir = 'C:\\' if utils.is_windows else '/' parsed = parser(root_dir) assert not parsed.parent diff --git a/tests/unit/commands/test_userscripts.py b/tests/unit/commands/test_userscripts.py index 7829fbca9..be514e788 100644 --- a/tests/unit/commands/test_userscripts.py +++ b/tests/unit/commands/test_userscripts.py @@ -27,6 +27,7 @@ import pytest from PyQt5.QtCore import QFileSystemWatcher from qutebrowser.commands import userscripts +from qutebrowser.utils import utils @pytest.mark.posix @@ -60,7 +61,7 @@ class TestQtFIFOReader: userscripts._WindowsUserscriptRunner, ]) def runner(request, runtime_tmpdir): - if (os.name != 'posix' and + if (not utils.is_posix and request.param is userscripts._POSIXUserscriptRunner): pytest.skip("Requires a POSIX os") else: @@ -246,7 +247,8 @@ def test_unicode_error(caplog, qtbot, py_proc, runner): def test_unsupported(monkeypatch, tabbed_browser_stubs): - monkeypatch.setattr(userscripts.os, 'name', 'toaster') + monkeypatch.setattr(userscripts.utils, 'is_posix', False) + monkeypatch.setattr(userscripts.utils, 'is_windows', False) with pytest.raises(userscripts.UnsupportedError, match="Userscripts are " "not supported on this platform!"): userscripts.run_async(tab=None, cmd=None, win_id=0, env=None) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 1fc939373..b7dba3b18 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -19,11 +19,11 @@ """Tests for qutebrowser.config.configfiles.""" import os -import sys import pytest from qutebrowser.config import config, configfiles, configexc +from qutebrowser.utils import utils from PyQt5.QtCore import QSettings @@ -343,7 +343,7 @@ def test_init(init_patch, config_tmpdir): configfiles.init() # Make sure qsettings land in a subdir - if sys.platform == 'linux': + if utils.is_linux: settings = QSettings() settings.setValue("hello", "world") settings.sync() diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index 18f04266d..e7131f92c 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -19,7 +19,6 @@ """Tests for BaseKeyParser.""" -import sys import logging from unittest import mock @@ -166,7 +165,7 @@ class TestSpecialKeys: keyparser._read_config('prompt') def test_valid_key(self, fake_keyevent_factory, keyparser): - if sys.platform == 'darwin': + if utils.is_mac: modifier = Qt.MetaModifier else: modifier = Qt.ControlModifier @@ -176,7 +175,7 @@ class TestSpecialKeys: 'message-info ctrla', keyparser.Type.special, None) def test_valid_key_count(self, fake_keyevent_factory, keyparser): - if sys.platform == 'darwin': + if utils.is_mac: modifier = Qt.MetaModifier else: modifier = Qt.ControlModifier @@ -210,7 +209,7 @@ class TestKeyChain: keyparser._read_config('prompt') def test_valid_special_key(self, fake_keyevent_factory, keyparser): - if sys.platform == 'darwin': + if utils.is_mac: modifier = Qt.MetaModifier else: modifier = Qt.ControlModifier diff --git a/tests/unit/misc/test_ipc.py b/tests/unit/misc/test_ipc.py index 2b7c17e62..d8024b207 100644 --- a/tests/unit/misc/test_ipc.py +++ b/tests/unit/misc/test_ipc.py @@ -19,7 +19,6 @@ """Tests for qutebrowser.misc.ipc.""" -import sys import os import getpass import logging @@ -35,7 +34,7 @@ from PyQt5.QtTest import QSignalSpy import qutebrowser from qutebrowser.misc import ipc -from qutebrowser.utils import objreg, standarddir +from qutebrowser.utils import objreg, standarddir, utils from helpers import stubs @@ -228,11 +227,11 @@ class TestSocketName: We probably would adjust the code first to make it work on that platform. """ - if os.name == 'nt': + if utils.is_windows: pass - elif sys.platform == 'darwin': + elif utils.is_mac: pass - elif sys.platform.startswith('linux'): + elif utils.is_linux: pass else: raise Exception("Unexpected platform!") @@ -431,7 +430,7 @@ class TestHandleConnection: @pytest.fixture def connected_socket(qtbot, qlocalsocket, ipc_server): - if sys.platform == 'darwin': + if utils.is_mac: pytest.skip("Skipping connected_socket test - " "https://github.com/qutebrowser/qutebrowser/issues/1045") ipc_server.listen() diff --git a/tests/unit/misc/test_msgbox.py b/tests/unit/misc/test_msgbox.py index 2c1268bd8..5f0058dea 100644 --- a/tests/unit/misc/test_msgbox.py +++ b/tests/unit/misc/test_msgbox.py @@ -18,11 +18,10 @@ """Tests for qutebrowser.misc.msgbox.""" -import sys - import pytest from qutebrowser.misc import msgbox +from qutebrowser.utils import utils from PyQt5.QtCore import Qt from PyQt5.QtWidgets import QMessageBox, QWidget @@ -40,7 +39,7 @@ def test_attributes(qtbot): box = msgbox.msgbox(parent=parent, title=title, text=text, icon=icon, buttons=buttons) qtbot.add_widget(box) - if sys.platform != 'darwin': + if not utils.is_mac: assert box.windowTitle() == title assert box.icon() == icon assert box.standardButtons() == buttons @@ -82,7 +81,7 @@ def test_finished_signal(qtbot): def test_information(qtbot): box = msgbox.information(parent=None, title='foo', text='bar') qtbot.add_widget(box) - if sys.platform != 'darwin': + if not utils.is_mac: assert box.windowTitle() == 'foo' assert box.text() == 'bar' assert box.icon() == QMessageBox.Information diff --git a/tests/unit/misc/test_utilcmds.py b/tests/unit/misc/test_utilcmds.py index 238244617..dfb99115d 100644 --- a/tests/unit/misc/test_utilcmds.py +++ b/tests/unit/misc/test_utilcmds.py @@ -21,7 +21,6 @@ import contextlib import logging -import os import signal import time @@ -29,6 +28,7 @@ import pytest from qutebrowser.misc import utilcmds from qutebrowser.commands import cmdexc +from qutebrowser.utils import utils @contextlib.contextmanager @@ -45,7 +45,7 @@ def test_debug_crash_exception(): utilcmds.debug_crash(typ='exception') -@pytest.mark.skipif(os.name == 'nt', +@pytest.mark.skipif(utils.is_windows, reason="current CPython/win can't recover from SIGSEGV") def test_debug_crash_segfault(): """Verify that debug_crash crashes as intended.""" diff --git a/tests/unit/scripts/test_check_coverage.py b/tests/unit/scripts/test_check_coverage.py index 440c743cf..f80dcba44 100644 --- a/tests/unit/scripts/test_check_coverage.py +++ b/tests/unit/scripts/test_check_coverage.py @@ -207,8 +207,8 @@ def test_skipped_args(covtest, args, reason): covtest.check_skipped(args, reason) -def test_skipped_windows(covtest, monkeypatch): - monkeypatch.setattr(check_coverage.sys, 'platform', 'toaster') +def test_skipped_non_linux(covtest, monkeypatch): + monkeypatch.setattr(check_coverage.utils, 'is_linux', False) covtest.check_skipped([], "on non-Linux system.") diff --git a/tests/unit/utils/test_error.py b/tests/unit/utils/test_error.py index 4f4905365..47a1c52d9 100644 --- a/tests/unit/utils/test_error.py +++ b/tests/unit/utils/test_error.py @@ -18,12 +18,11 @@ """Tests for qutebrowser.utils.error.""" -import sys import logging import pytest -from qutebrowser.utils import error +from qutebrowser.utils import error, utils from qutebrowser.misc import ipc from PyQt5.QtCore import QTimer @@ -84,7 +83,7 @@ def test_err_windows(qtbot, qapp, fake_args, pre_text, post_text, expected): w = qapp.activeModalWidget() try: qtbot.add_widget(w) - if sys.platform != 'darwin': + if not utils.is_mac: assert w.windowTitle() == 'title' assert w.icon() == QMessageBox.Critical assert w.standardButtons() == QMessageBox.Ok diff --git a/tests/unit/utils/test_jinja.py b/tests/unit/utils/test_jinja.py index c2a72de1a..1278d7bc1 100644 --- a/tests/unit/utils/test_jinja.py +++ b/tests/unit/utils/test_jinja.py @@ -89,7 +89,7 @@ def test_resource_url(): path = url.path() - if os.name == "nt": + if utils.is_windows: path = path.lstrip('/') path = path.replace('/', os.sep) diff --git a/tests/unit/utils/test_qtutils.py b/tests/unit/utils/test_qtutils.py index f3c1afc04..8045bb0d3 100644 --- a/tests/unit/utils/test_qtutils.py +++ b/tests/unit/utils/test_qtutils.py @@ -21,7 +21,6 @@ import io import os -import sys import os.path import unittest import unittest.mock @@ -36,7 +35,7 @@ import pytest from PyQt5.QtCore import (QDataStream, QPoint, QUrl, QByteArray, QIODevice, QTimer, QBuffer, QFile, QProcess, QFileDevice) -from qutebrowser.utils import qtutils +from qutebrowser.utils import qtutils, utils import overflow_test_cases @@ -458,13 +457,13 @@ class TestSavefileOpen: with qtutils.savefile_open(str(filename)) as f: f.write('foo\nbar\nbaz') data = filename.read_binary() - if os.name == 'nt': + if utils.is_windows: assert data == b'foo\r\nbar\r\nbaz' else: assert data == b'foo\nbar\nbaz' -if test_file is not None and sys.platform != 'darwin': +if test_file is not None and not utils.is_mac: # If we were able to import Python's test_file module, we run some code # here which defines unittest TestCases to run the python tests over # PyQIODevice. diff --git a/tests/unit/utils/test_standarddir.py b/tests/unit/utils/test_standarddir.py index 5230c9c78..b3daec643 100644 --- a/tests/unit/utils/test_standarddir.py +++ b/tests/unit/utils/test_standarddir.py @@ -32,7 +32,7 @@ import attr from PyQt5.QtCore import QStandardPaths import pytest -from qutebrowser.utils import standarddir +from qutebrowser.utils import standarddir, utils # Use a different application name for tests to make sure we don't change real @@ -80,7 +80,7 @@ def test_unset_organization_no_qapp(monkeypatch): def test_fake_mac_config(tmpdir, monkeypatch): """Test standardir.config on a fake Mac.""" - monkeypatch.setattr(sys, 'platform', 'darwin') + monkeypatch.setattr(utils, 'is_mac', True) monkeypatch.setenv('HOME', str(tmpdir)) expected = str(tmpdir) + '/.qute_test' # always with / standarddir._init_config(args=None) @@ -91,7 +91,7 @@ def test_fake_mac_config(tmpdir, monkeypatch): @pytest.mark.not_mac def test_fake_windows(tmpdir, monkeypatch, what): """Make sure the config/data/cache dirs are correct on a fake Windows.""" - monkeypatch.setattr(os, 'name', 'nt') + monkeypatch.setattr(standarddir.utils, 'is_windows', True) monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation', lambda typ: str(tmpdir / APPNAME)) @@ -175,7 +175,7 @@ class TestStandardDir: def test_runtimedir_empty_tempdir(self, monkeypatch, tmpdir): """With an empty tempdir on non-Linux, we should raise.""" - monkeypatch.setattr(standarddir.sys, 'platform', 'nt') + monkeypatch.setattr(standarddir.utils, 'is_linux', False) monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation', lambda typ: '') with pytest.raises(standarddir.EmptyValueError): @@ -294,7 +294,7 @@ class TestCreatingDir: assert basedir.exists() - if os.name == 'posix': + if utils.is_posix: assert basedir.stat().mode & 0o777 == 0o700 @pytest.mark.parametrize('typ', DIR_TYPES) @@ -326,7 +326,7 @@ class TestSystemData: def test_system_datadir_exist_linux(self, monkeypatch): """Test that /usr/share/qute_test is used if path exists.""" - monkeypatch.setattr('sys.platform', "linux") + monkeypatch.setattr(standarddir.utils, 'is_linux', True) monkeypatch.setattr(os.path, 'exists', lambda path: True) standarddir._init_dirs() assert standarddir.data(system=True) == "/usr/share/qute_test" @@ -493,10 +493,10 @@ def test_init(mocker, tmpdir, args_kind): assert standarddir._locations != {} if args_kind == 'normal': - if sys.platform == 'darwin': + if utils.is_mac: assert not m_windows.called assert m_mac.called - elif os.name == 'nt': + elif utils.is_windows: assert m_windows.called assert not m_mac.called else: diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 623f77cc7..e0be5fdb8 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -355,7 +355,7 @@ class TestKeyEventToString: def test_key_and_modifier(self, fake_keyevent_factory): """Test with key and modifier pressed.""" evt = fake_keyevent_factory(key=Qt.Key_A, modifiers=Qt.ControlModifier) - expected = 'meta+a' if sys.platform == 'darwin' else 'ctrl+a' + expected = 'meta+a' if utils.is_mac else 'ctrl+a' assert utils.keyevent_to_string(evt) == expected def test_key_and_modifiers(self, fake_keyevent_factory): @@ -367,7 +367,7 @@ class TestKeyEventToString: def test_mac(self, monkeypatch, fake_keyevent_factory): """Test with a simulated mac.""" - monkeypatch.setattr(sys, 'platform', 'darwin') + monkeypatch.setattr(utils, 'is_mac', True) evt = fake_keyevent_factory(key=Qt.Key_A, modifiers=Qt.ControlModifier) assert utils.keyevent_to_string(evt) == 'meta+a' diff --git a/tests/unit/utils/test_version.py b/tests/unit/utils/test_version.py index c3243862b..7c60d3d1e 100644 --- a/tests/unit/utils/test_version.py +++ b/tests/unit/utils/test_version.py @@ -36,7 +36,7 @@ import attr import pytest import qutebrowser -from qutebrowser.utils import version, usertypes +from qutebrowser.utils import version, usertypes, utils from qutebrowser.browser import pdfjs @@ -333,7 +333,7 @@ class TestGitStrSubprocess: 'GIT_COMMITTER_EMAIL': 'mail@qutebrowser.org', 'GIT_COMMITTER_DATE': 'Thu 1 Jan 01:00:00 CET 1970', }) - if os.name == 'nt': + if utils.is_windows: # If we don't call this with shell=True it might fail under # some environments on Windows... # http://bugs.python.org/issue24493 @@ -696,15 +696,18 @@ class TestOsInfo: mac_ver: The tuple to set platform.mac_ver() to. mac_ver_str: The expected Mac version string in version._os_info(). """ - monkeypatch.setattr(version.sys, 'platform', 'darwin') + monkeypatch.setattr(version.utils, 'is_linux', False) + monkeypatch.setattr(version.utils, 'is_windows', False) + monkeypatch.setattr(version.utils, 'is_mac', True) monkeypatch.setattr(version.platform, 'mac_ver', lambda: mac_ver) ret = version._os_info() expected = ['OS Version: {}'.format(mac_ver_str)] assert ret == expected def test_unknown_fake(self, monkeypatch): - """Test with a fake unknown sys.platform.""" - monkeypatch.setattr(version.sys, 'platform', 'toaster') + """Test with a fake unknown platform.""" + for name in ['is_mac', 'is_windows', 'is_linux']: + monkeypatch.setattr(version.utils, name, False) ret = version._os_info() expected = ['OS Version: ?'] assert ret == expected From 46cfd5353d63352f04b8505466dfcbaa5064f186 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 20 Sep 2017 11:28:19 +0200 Subject: [PATCH 12/77] Use a pytest marker to fake an OS --- pytest.ini | 1 + tests/conftest.py | 31 +++++++++++++++++++++++ tests/unit/commands/test_userscripts.py | 5 ++-- tests/unit/scripts/test_check_coverage.py | 4 +-- tests/unit/utils/test_standarddir.py | 8 +++--- tests/unit/utils/test_utils.py | 4 +-- tests/unit/utils/test_version.py | 13 ++++------ 7 files changed, 47 insertions(+), 19 deletions(-) diff --git a/pytest.ini b/pytest.ini index d47c173f7..b853c8ca8 100644 --- a/pytest.ini +++ b/pytest.ini @@ -25,6 +25,7 @@ markers = this: Used to mark tests during development no_invalid_lines: Don't fail on unparseable lines in end2end tests issue2478: Tests which are broken on Windows with QtWebEngine, https://github.com/qutebrowser/qutebrowser/issues/2478 + fake_os: Fake utils.is_* to a fake operating system qt_log_level_fail = WARNING qt_log_ignore = ^SpellCheck: .* diff --git a/tests/conftest.py b/tests/conftest.py index 0ba48e330..24b87b54b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -193,6 +193,37 @@ def set_backend(monkeypatch, request): monkeypatch.setattr(objects, 'backend', backend) +@pytest.fixture(autouse=True) +def apply_fake_os(monkeypatch, request): + fake_os = request.node.get_marker('fake_os') + if not fake_os: + return + + name = fake_os.args[0] + mac = False + windows = False + linux = False + posix = False + + if name == 'unknown': + pass + elif name == 'mac': + mac = True + posix = True + elif name == 'windows': + windows = True + elif name == 'linux': + linux = True + posix = True + else: + raise ValueError("Invalid fake_os {}".format(name)) + + monkeypatch.setattr('qutebrowser.utils.utils.is_mac', mac) + monkeypatch.setattr('qutebrowser.utils.utils.is_linux', linux) + monkeypatch.setattr('qutebrowser.utils.utils.is_windows', windows) + monkeypatch.setattr('qutebrowser.utils.utils.is_posix', posix) + + @pytest.hookimpl(tryfirst=True, hookwrapper=True) def pytest_runtest_makereport(item, call): """Make test information available in fixtures. diff --git a/tests/unit/commands/test_userscripts.py b/tests/unit/commands/test_userscripts.py index be514e788..9a6490aca 100644 --- a/tests/unit/commands/test_userscripts.py +++ b/tests/unit/commands/test_userscripts.py @@ -246,9 +246,8 @@ def test_unicode_error(caplog, qtbot, py_proc, runner): assert caplog.records[0].message == expected -def test_unsupported(monkeypatch, tabbed_browser_stubs): - monkeypatch.setattr(userscripts.utils, 'is_posix', False) - monkeypatch.setattr(userscripts.utils, 'is_windows', False) +@pytest.mark.fake_os('unknown') +def test_unsupported(tabbed_browser_stubs): with pytest.raises(userscripts.UnsupportedError, match="Userscripts are " "not supported on this platform!"): userscripts.run_async(tab=None, cmd=None, win_id=0, env=None) diff --git a/tests/unit/scripts/test_check_coverage.py b/tests/unit/scripts/test_check_coverage.py index f80dcba44..6b18568c5 100644 --- a/tests/unit/scripts/test_check_coverage.py +++ b/tests/unit/scripts/test_check_coverage.py @@ -207,8 +207,8 @@ def test_skipped_args(covtest, args, reason): covtest.check_skipped(args, reason) -def test_skipped_non_linux(covtest, monkeypatch): - monkeypatch.setattr(check_coverage.utils, 'is_linux', False) +@pytest.mark.fake_os('windows') +def test_skipped_non_linux(covtest): covtest.check_skipped([], "on non-Linux system.") diff --git a/tests/unit/utils/test_standarddir.py b/tests/unit/utils/test_standarddir.py index b3daec643..70a774ed6 100644 --- a/tests/unit/utils/test_standarddir.py +++ b/tests/unit/utils/test_standarddir.py @@ -78,9 +78,9 @@ def test_unset_organization_no_qapp(monkeypatch): pass +@pytest.mark.fake_os('mac') def test_fake_mac_config(tmpdir, monkeypatch): """Test standardir.config on a fake Mac.""" - monkeypatch.setattr(utils, 'is_mac', True) monkeypatch.setenv('HOME', str(tmpdir)) expected = str(tmpdir) + '/.qute_test' # always with / standarddir._init_config(args=None) @@ -89,9 +89,9 @@ def test_fake_mac_config(tmpdir, monkeypatch): @pytest.mark.parametrize('what', ['data', 'config', 'cache']) @pytest.mark.not_mac +@pytest.mark.fake_os('windows') def test_fake_windows(tmpdir, monkeypatch, what): """Make sure the config/data/cache dirs are correct on a fake Windows.""" - monkeypatch.setattr(standarddir.utils, 'is_windows', True) monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation', lambda typ: str(tmpdir / APPNAME)) @@ -173,9 +173,9 @@ class TestStandardDir: standarddir._init_dirs() assert standarddir.runtime() == str(tmpdir / 'temp' / APPNAME) + @pytest.mark.fake_os('windows') def test_runtimedir_empty_tempdir(self, monkeypatch, tmpdir): """With an empty tempdir on non-Linux, we should raise.""" - monkeypatch.setattr(standarddir.utils, 'is_linux', False) monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation', lambda typ: '') with pytest.raises(standarddir.EmptyValueError): @@ -324,9 +324,9 @@ class TestSystemData: """Test system data path.""" + @pytest.mark.fake_os('linux') def test_system_datadir_exist_linux(self, monkeypatch): """Test that /usr/share/qute_test is used if path exists.""" - monkeypatch.setattr(standarddir.utils, 'is_linux', True) monkeypatch.setattr(os.path, 'exists', lambda path: True) standarddir._init_dirs() assert standarddir.data(system=True) == "/usr/share/qute_test" diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index e0be5fdb8..f6eef7f4f 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -365,9 +365,9 @@ class TestKeyEventToString: Qt.MetaModifier | Qt.ShiftModifier)) assert utils.keyevent_to_string(evt) == 'ctrl+alt+meta+shift+a' - def test_mac(self, monkeypatch, fake_keyevent_factory): + @pytest.mark.fake_os('mac') + def test_mac(self, fake_keyevent_factory): """Test with a simulated mac.""" - monkeypatch.setattr(utils, 'is_mac', True) evt = fake_keyevent_factory(key=Qt.Key_A, modifiers=Qt.ControlModifier) assert utils.keyevent_to_string(evt) == 'meta+a' diff --git a/tests/unit/utils/test_version.py b/tests/unit/utils/test_version.py index 7c60d3d1e..489806f8c 100644 --- a/tests/unit/utils/test_version.py +++ b/tests/unit/utils/test_version.py @@ -662,12 +662,12 @@ class TestOsInfo: """Tests for _os_info.""" + @pytest.mark.fake_os('linux') def test_linux_fake(self, monkeypatch): """Test with a fake Linux. No args because osver is set to '' if the OS is linux. """ - monkeypatch.setattr(version.sys, 'platform', 'linux') monkeypatch.setattr(version, '_release_info', lambda: [('releaseinfo', 'Hello World')]) ret = version._os_info() @@ -675,15 +675,16 @@ class TestOsInfo: '--- releaseinfo ---', 'Hello World'] assert ret == expected + @pytest.mark.fake_os('windows') def test_windows_fake(self, monkeypatch): """Test with a fake Windows.""" - monkeypatch.setattr(version.sys, 'platform', 'win32') monkeypatch.setattr(version.platform, 'win32_ver', lambda: ('eggs', 'bacon', 'ham', 'spam')) ret = version._os_info() expected = ['OS Version: eggs, bacon, ham, spam'] assert ret == expected + @pytest.mark.fake_os('mac') @pytest.mark.parametrize('mac_ver, mac_ver_str', [ (('x', ('', '', ''), 'y'), 'x, y'), (('', ('', '', ''), ''), ''), @@ -696,18 +697,14 @@ class TestOsInfo: mac_ver: The tuple to set platform.mac_ver() to. mac_ver_str: The expected Mac version string in version._os_info(). """ - monkeypatch.setattr(version.utils, 'is_linux', False) - monkeypatch.setattr(version.utils, 'is_windows', False) - monkeypatch.setattr(version.utils, 'is_mac', True) monkeypatch.setattr(version.platform, 'mac_ver', lambda: mac_ver) ret = version._os_info() expected = ['OS Version: {}'.format(mac_ver_str)] assert ret == expected - def test_unknown_fake(self, monkeypatch): + @pytest.mark.fake_os('unknown') + def test_unknown_fake(self): """Test with a fake unknown platform.""" - for name in ['is_mac', 'is_windows', 'is_linux']: - monkeypatch.setattr(version.utils, name, False) ret = version._os_info() expected = ['OS Version: ?'] assert ret == expected From 2a4f10f0f532248c9f8a6612fd40c25aab33611b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 20 Sep 2017 11:28:33 +0200 Subject: [PATCH 13/77] Add qapp for tabbed_browser_stubs --- tests/helpers/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/helpers/fixtures.py b/tests/helpers/fixtures.py index 496eb486d..09307711f 100644 --- a/tests/helpers/fixtures.py +++ b/tests/helpers/fixtures.py @@ -279,7 +279,7 @@ def session_manager_stub(stubs): @pytest.fixture -def tabbed_browser_stubs(stubs, win_registry): +def tabbed_browser_stubs(qapp, stubs, win_registry): """Fixture providing a fake tabbed-browser object on win_id 0 and 1.""" win_registry.add_window(1) stubs = [stubs.TabbedBrowserStub(), stubs.TabbedBrowserStub()] From 1d964ccdafbe046cbb6c82e9808e49620a8b9cd3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 20 Sep 2017 12:20:46 +0200 Subject: [PATCH 14/77] Only run system datadir test on Linux --- tests/unit/utils/test_standarddir.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/utils/test_standarddir.py b/tests/unit/utils/test_standarddir.py index 70a774ed6..3dcb282d2 100644 --- a/tests/unit/utils/test_standarddir.py +++ b/tests/unit/utils/test_standarddir.py @@ -324,7 +324,7 @@ class TestSystemData: """Test system data path.""" - @pytest.mark.fake_os('linux') + @pytest.mark.linux def test_system_datadir_exist_linux(self, monkeypatch): """Test that /usr/share/qute_test is used if path exists.""" monkeypatch.setattr(os.path, 'exists', lambda path: True) From f9440b802651f698c841c7104e88f124ce97ea73 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sat, 9 Sep 2017 12:39:04 -0400 Subject: [PATCH 15/77] Use CommandParser for configmodel.bind. The parsing bind() did manually is now available through CommandParser. Resolves #2952. This also adds a unit test for the case when there is no current binding, as I broke that while working on this and there was no test to catch it :) --- qutebrowser/completion/models/configmodel.py | 6 +++--- tests/unit/completion/test_models.py | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/qutebrowser/completion/models/configmodel.py b/qutebrowser/completion/models/configmodel.py index 0ab85e1c9..78ec53338 100644 --- a/qutebrowser/completion/models/configmodel.py +++ b/qutebrowser/completion/models/configmodel.py @@ -21,7 +21,7 @@ from qutebrowser.config import configdata, configexc from qutebrowser.completion.models import completionmodel, listcategory, util -from qutebrowser.commands import cmdutils +from qutebrowser.commands import runners def option(*, info): @@ -71,8 +71,8 @@ def bind(key, *, info): cmd_text = info.keyconf.get_command(key, 'normal') if cmd_text: - cmd_name = cmd_text.split(' ')[0] - cmd = cmdutils.cmd_dict.get(cmd_name) + parser = runners.CommandParser() + cmd = parser.parse(cmd_text).cmd data = [(cmd_text, cmd.desc, key)] model.add_category(listcategory.ListCategory("Current", data)) diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py index ee7726422..b5748f234 100644 --- a/tests/unit/completion/test_models.py +++ b/tests/unit/completion/test_models.py @@ -573,6 +573,24 @@ def test_bind_completion(qtmodeltester, cmdutils_stub, config_stub, }) +def test_bind_completion_no_current(qtmodeltester, cmdutils_stub, config_stub, + key_config_stub, configdata_stub, info): + """Test keybinding completion with no current binding. """ + model = configmodel.bind('x', info=info) + model.set_pattern('') + qtmodeltester.data_display_may_return_none = True + qtmodeltester.check(model) + + _check_completions(model, { + "Commands": [ + ('open', 'open a url', ''), + ('q', "Alias for 'quit'", ''), + ('quit', 'quit qutebrowser', 'ZQ, '), + ('scroll', 'Scroll the current tab in the given direction.', '') + ], + }) + + def test_url_completion_benchmark(benchmark, info, quickmark_manager_stub, bookmark_manager_stub, From 5cd00f699e5f2504dd80f6bac12f59c96a2d0494 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Thu, 14 Sep 2017 12:03:40 -0400 Subject: [PATCH 16/77] Resolve KeyError when deleting URL with space. Resolves #2963. --- qutebrowser/browser/history.py | 8 +++++--- tests/unit/browser/test_history.py | 14 +++++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index 72fca984b..c9609bc03 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -26,7 +26,7 @@ from PyQt5.QtCore import pyqtSlot, QUrl, QTimer from qutebrowser.commands import cmdutils, cmdexc from qutebrowser.utils import (utils, objreg, log, usertypes, message, - debug, standarddir) + debug, standarddir, qtutils) from qutebrowser.misc import objects, sql @@ -144,8 +144,10 @@ class WebHistory(sql.SqlTable): Args: url: URL string to delete. """ - self.delete('url', url) - self.completion.delete('url', url) + qurl = QUrl(url) + qtutils.ensure_valid(qurl) + self.delete('url', self._format_url(qurl)) + self.completion.delete('url', self._format_completion_url(qurl)) @pyqtSlot(QUrl, QUrl, str) def add_from_tab(self, url, requested_url, title): diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py index 7b34090f2..51157fccb 100644 --- a/tests/unit/browser/test_history.py +++ b/tests/unit/browser/test_history.py @@ -127,21 +127,25 @@ def test_clear_force(qtbot, tmpdir, hist): assert not len(hist.completion) -def test_delete_url(hist): +@pytest.mark.parametrize('raw, escaped', [ + ('http://example.com/1', 'http://example.com/1'), + ('http://example.com/1 2', 'http://example.com/1%202'), +]) +def test_delete_url(hist, raw, escaped): hist.add_url(QUrl('http://example.com/'), atime=0) - hist.add_url(QUrl('http://example.com/1'), atime=0) + hist.add_url(QUrl(escaped), atime=0) hist.add_url(QUrl('http://example.com/2'), atime=0) before = set(hist) completion_before = set(hist.completion) - hist.delete_url(QUrl('http://example.com/1')) + hist.delete_url(QUrl(raw)) diff = before.difference(set(hist)) - assert diff == {('http://example.com/1', '', 0, False)} + assert diff == {(escaped, '', 0, False)} completion_diff = completion_before.difference(set(hist.completion)) - assert completion_diff == {('http://example.com/1', '', 0)} + assert completion_diff == {(raw, '', 0)} @pytest.mark.parametrize( From 6892705e186462cde399a55b3fc047516c73159a Mon Sep 17 00:00:00 2001 From: Felix Van der Jeugt Date: Wed, 20 Sep 2017 15:52:42 +0200 Subject: [PATCH 17/77] cover setting-saving-loading-getting yaml config --- tests/unit/config/test_configfiles.py | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 5736904c2..e252762b6 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -93,6 +93,37 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): assert ' tabs.show: never' in lines +@pytest.mark.parametrize('old_config', [ + None, + 'global:\n colors.hints.fg: magenta', +]) +@pytest.mark.parametrize('key, value', [ + ('colors.hints.fg', 'green'), + ('colors.hints.bg', None), + ('confirm_quit', True), + ('confirm_quit', False), +]) +def test_yaml_config_changed(fake_save_manager, config_tmpdir, old_config, key, value): + autoconfig = config_tmpdir / 'autoconfig.yml' + if old_config is not None: + autoconfig.write_text(old_config, 'utf-8') + + yaml = configfiles.YamlConfig() + yaml.load() + + yaml[key] = value + assert key in yaml + assert yaml[key] == value + + yaml._save() + + yaml = configfiles.YamlConfig() + yaml.load() + + assert key in yaml + assert yaml[key] == value + + @pytest.mark.parametrize('old_config', [ None, 'global:\n colors.hints.fg: magenta', From 991355068868d6779b5a01320057f22a00e775e0 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 20 Sep 2017 18:28:18 +0200 Subject: [PATCH 18/77] Fix windows condition --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 24b87b54b..db50afcd6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -51,7 +51,7 @@ def _apply_platform_markers(config, item): """Apply a skip marker to a given item.""" markers = [ ('posix', not utils.is_posix, "Requires a POSIX os"), - ('windows', not utils.is_mac, "Requires Windows"), + ('windows', not utils.is_windows, "Requires Windows"), ('linux', not utils.is_linux, "Requires Linux"), ('mac', not utils.is_mac, "Requires macOS"), ('not_mac', utils.is_mac, "Skipped on macOS"), From 192c063743328916ec00aa26c017aecd76533ced Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 08:58:56 +0200 Subject: [PATCH 19/77] Mark another window.open test as flaky See https://travis-ci.org/qutebrowser/qutebrowser/jobs/277846887 --- tests/end2end/features/javascript.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/end2end/features/javascript.feature b/tests/end2end/features/javascript.feature index c685722fa..a309d6187 100644 --- a/tests/end2end/features/javascript.feature +++ b/tests/end2end/features/javascript.feature @@ -61,6 +61,7 @@ Feature: Javascript stuff And I run :jseval if (window.open('about:blank')) { console.log('window opened'); } else { console.log('error while opening window'); } Then the javascript message "window opened" should be logged + @flaky Scenario: Opening window without user interaction with javascript.can_open_tabs_automatically set to false When I open data/hello.txt And I set content.javascript.can_open_tabs_automatically to false From 9a6de48efa0191da7db97fd6a0f489de3b5e2730 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 09:12:25 +0200 Subject: [PATCH 20/77] Break long line --- tests/unit/config/test_configfiles.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index c18baa7de..219b92395 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -103,7 +103,8 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): ('confirm_quit', True), ('confirm_quit', False), ]) -def test_yaml_config_changed(fake_save_manager, config_tmpdir, old_config, key, value): +def test_yaml_config_changed(fake_save_manager, config_tmpdir, old_config, + key, value): autoconfig = config_tmpdir / 'autoconfig.yml' if old_config is not None: autoconfig.write_text(old_config, 'utf-8') From e0e7d4ca6717971f3c781af4d17566787afbe0ab Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 13:35:57 +0200 Subject: [PATCH 21/77] Stabilize test_quitting_process_expected --- tests/end2end/fixtures/test_testprocess.py | 2 +- tests/end2end/fixtures/testprocess.py | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/end2end/fixtures/test_testprocess.py b/tests/end2end/fixtures/test_testprocess.py index b38e79f56..1811b7fb1 100644 --- a/tests/end2end/fixtures/test_testprocess.py +++ b/tests/end2end/fixtures/test_testprocess.py @@ -138,9 +138,9 @@ def test_quitting_process(qtbot, quit_pyproc): def test_quitting_process_expected(qtbot, quit_pyproc): + quit_pyproc.exit_expected = True with qtbot.waitSignal(quit_pyproc.proc.finished): quit_pyproc.start() - quit_pyproc.exit_expected = True quit_pyproc.after_test() diff --git a/tests/end2end/fixtures/testprocess.py b/tests/end2end/fixtures/testprocess.py index e955937bc..60874e622 100644 --- a/tests/end2end/fixtures/testprocess.py +++ b/tests/end2end/fixtures/testprocess.py @@ -125,6 +125,7 @@ class Process(QObject): Attributes: _invalid: A list of lines which could not be parsed. _data: A list of parsed lines. + _started: Whether the process was ever started. proc: The QProcess for the underlying process. exit_expected: Whether the process is expected to quit. @@ -140,11 +141,12 @@ class Process(QObject): def __init__(self, parent=None): super().__init__(parent) self.captured_log = [] + self._started = False self._invalid = [] self._data = [] self.proc = QProcess() self.proc.setReadChannel(QProcess.StandardError) - self.exit_expected = True # Not started at all yet + self.exit_expected = None # Not started at all yet def _log(self, line): """Add the given line to the captured log output.""" @@ -221,8 +223,8 @@ class Process(QObject): def start(self, args=None, *, env=None): """Start the process and wait until it started.""" - self.exit_expected = False self._start(args, env=env) + self._started = True timeout = 60 if 'CI' in os.environ else 20 for _ in range(timeout): with self._wait_signal(self.ready, timeout=1000, @@ -230,6 +232,8 @@ class Process(QObject): pass if not self.is_running(): + if self.exit_expected: + return # _start ensures it actually started, but it might quit shortly # afterwards raise ProcessExited('\n' + _render_log(self.captured_log)) @@ -285,7 +289,7 @@ class Process(QObject): raise InvalidLine('\n' + '\n'.join(self._invalid)) self.clear_data() - if not self.is_running() and not self.exit_expected: + if not self.is_running() and not self.exit_expected and self._started: raise ProcessExited self.exit_expected = False From cb57525f696a4705a265cb1dee5424d8de24c9bb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 13:43:30 +0200 Subject: [PATCH 22/77] Fix whitespace --- tests/unit/completion/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py index b5748f234..4e65254c9 100644 --- a/tests/unit/completion/test_models.py +++ b/tests/unit/completion/test_models.py @@ -575,7 +575,7 @@ def test_bind_completion(qtmodeltester, cmdutils_stub, config_stub, def test_bind_completion_no_current(qtmodeltester, cmdutils_stub, config_stub, key_config_stub, configdata_stub, info): - """Test keybinding completion with no current binding. """ + """Test keybinding completion with no current binding.""" model = configmodel.bind('x', info=info) model.set_pattern('') qtmodeltester.data_display_may_return_none = True From 7cad8f41f2d80806f15f4e3983996fcf5324ebcd Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 16:29:40 +0200 Subject: [PATCH 23/77] Remove unknown YAML data from config I considered introducing another list of deleted options (or a "deleted: True" in configdata.yml), similar to what we had with the old config. However, let's take the easier route and just delete everything we don't know from configdata.yml. If someone edits it by hand, it's their fault :P See #2772, #2847 --- qutebrowser/config/configfiles.py | 8 +++++++- tests/unit/config/test_config.py | 9 ++------- tests/unit/config/test_configfiles.py | 3 +++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index e26a03454..a155726a8 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -30,7 +30,7 @@ import yaml from PyQt5.QtCore import QSettings import qutebrowser -from qutebrowser.config import configexc, config +from qutebrowser.config import configexc, config, configdata from qutebrowser.utils import standarddir, utils, qtutils @@ -153,6 +153,12 @@ class YamlConfig: "'global' object is not a dict") raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + # Delete unknown values + # (e.g. options which were removed from configdata.yml) + for name in list(global_obj): + if name not in configdata.DATA: + del global_obj[name] + self._values = global_obj self._dirty = False diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 6dff3c30a..36dc90c10 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -932,14 +932,9 @@ def test_early_init(init_patch, config_tmpdir, caplog, fake_args, expected_errors.append( "Errors occurred while reading config.py:\n" " While setting 'foo': No option 'foo'") - if invalid_yaml and (load_autoconfig or not config_py): + if invalid_yaml == '42' and (load_autoconfig or not config_py): error = "Errors occurred while reading autoconfig.yml:\n" - if invalid_yaml == '42': - error += " While loading data: Toplevel object is not a dict" - elif invalid_yaml == 'unknown': - error += " Error: No option 'colors.foobar'" - else: - assert False, invalid_yaml + error += " While loading data: Toplevel object is not a dict" expected_errors.append(error) actual_errors = [str(err) for err in config._init_errors] diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 219b92395..fdfda4033 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -57,6 +57,8 @@ def test_state_config(fake_save_manager, data_tmpdir, @pytest.mark.parametrize('old_config', [ None, 'global:\n colors.hints.fg: magenta', + # Unknown key + 'global:\n hello: world', ]) @pytest.mark.parametrize('insert', [True, False]) def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): @@ -91,6 +93,7 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): assert ' colors.hints.fg: magenta' in lines if insert: assert ' tabs.show: never' in lines + assert ' hello:' not in lines @pytest.mark.parametrize('old_config', [ From 2f7cbfa1ee5a5c618d0266ec8a62506d61270eb5 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 17:42:30 +0200 Subject: [PATCH 24/77] Make sure the changelog is in releases [ci skip] --- MANIFEST.in | 1 + 1 file changed, 1 insertion(+) diff --git a/MANIFEST.in b/MANIFEST.in index ec906aaf4..a3ae1ee28 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -27,6 +27,7 @@ exclude scripts/asciidoc2html.py exclude doc/notes recursive-exclude doc *.asciidoc include doc/qutebrowser.1.asciidoc +include doc/changelog.asciidoc prune tests prune qutebrowser/3rdparty prune misc/requirements From f821fb793a6fb271ca043ebbb29ec06c509fb4d7 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 19:37:22 +0200 Subject: [PATCH 25/77] Initialize configdata in test_configfiles --- tests/unit/config/test_configfiles.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index fdfda4033..c7f611f44 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -22,12 +22,19 @@ import os import pytest -from qutebrowser.config import config, configfiles, configexc +from qutebrowser.config import config, configfiles, configexc, configdata from qutebrowser.utils import utils from PyQt5.QtCore import QSettings +@pytest.fixture(autouse=True) +def configdata_init(): + """Initialize configdata if needed.""" + if configdata.DATA is None: + configdata.init() + + @pytest.mark.parametrize('old_data, insert, new_data', [ (None, False, '[general]\n\n[geometry]\n\n'), ('[general]\nfooled = true', False, '[general]\n\n[geometry]\n\n'), From 3e0d49a4b32bd586e8bcc80c18df01ea92dabbe3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 19:57:54 +0200 Subject: [PATCH 26/77] Add TestYaml class to test_configfiles --- tests/unit/config/test_configfiles.py | 236 +++++++++++++------------- 1 file changed, 118 insertions(+), 118 deletions(-) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index c7f611f44..f1b7271f6 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -61,142 +61,142 @@ def test_state_config(fake_save_manager, data_tmpdir, assert statefile.read_text('utf-8') == new_data -@pytest.mark.parametrize('old_config', [ - None, - 'global:\n colors.hints.fg: magenta', - # Unknown key - 'global:\n hello: world', -]) -@pytest.mark.parametrize('insert', [True, False]) -def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): - autoconfig = config_tmpdir / 'autoconfig.yml' - if old_config is not None: - autoconfig.write_text(old_config, 'utf-8') +class TestYaml: - yaml = configfiles.YamlConfig() - yaml.load() + @pytest.mark.parametrize('old_config', [ + None, + 'global:\n colors.hints.fg: magenta', + # Unknown key + 'global:\n hello: world', + ]) + @pytest.mark.parametrize('insert', [True, False]) + def test_yaml_config(self, fake_save_manager, config_tmpdir, + old_config, insert): + autoconfig = config_tmpdir / 'autoconfig.yml' + if old_config is not None: + autoconfig.write_text(old_config, 'utf-8') - if insert: - yaml['tabs.show'] = 'never' - - yaml._save() - - if not insert and old_config is None: - lines = [] - else: - text = autoconfig.read_text('utf-8') - lines = text.splitlines() + yaml = configfiles.YamlConfig() + yaml.load() if insert: - assert lines[0].startswith('# DO NOT edit this file by hand,') - assert 'config_version: {}'.format(yaml.VERSION) in lines + yaml['tabs.show'] = 'never' - assert 'global:' in lines + yaml._save() - print(lines) + if not insert and old_config is None: + lines = [] + else: + text = autoconfig.read_text('utf-8') + lines = text.splitlines() - # WORKAROUND for https://github.com/PyCQA/pylint/issues/574 - if 'magenta' in (old_config or ''): # pylint: disable=superfluous-parens - assert ' colors.hints.fg: magenta' in lines - if insert: - assert ' tabs.show: never' in lines - assert ' hello:' not in lines + if insert: + assert lines[0].startswith('# DO NOT edit this file by hand,') + assert 'config_version: {}'.format(yaml.VERSION) in lines + assert 'global:' in lines -@pytest.mark.parametrize('old_config', [ - None, - 'global:\n colors.hints.fg: magenta', -]) -@pytest.mark.parametrize('key, value', [ - ('colors.hints.fg', 'green'), - ('colors.hints.bg', None), - ('confirm_quit', True), - ('confirm_quit', False), -]) -def test_yaml_config_changed(fake_save_manager, config_tmpdir, old_config, - key, value): - autoconfig = config_tmpdir / 'autoconfig.yml' - if old_config is not None: - autoconfig.write_text(old_config, 'utf-8') + print(lines) - yaml = configfiles.YamlConfig() - yaml.load() + # WORKAROUND for https://github.com/PyCQA/pylint/issues/574 + # pylint: disable=superfluous-parens + if 'magenta' in (old_config or ''): + assert ' colors.hints.fg: magenta' in lines + if insert: + assert ' tabs.show: never' in lines + assert ' hello:' not in lines - yaml[key] = value - assert key in yaml - assert yaml[key] == value + @pytest.mark.parametrize('old_config', [ + None, + 'global:\n colors.hints.fg: magenta', + ]) + @pytest.mark.parametrize('key, value', [ + ('colors.hints.fg', 'green'), + ('colors.hints.bg', None), + ('confirm_quit', True), + ('confirm_quit', False), + ]) + def test_changed(self, fake_save_manager, config_tmpdir, old_config, + key, value): + autoconfig = config_tmpdir / 'autoconfig.yml' + if old_config is not None: + autoconfig.write_text(old_config, 'utf-8') - yaml._save() - - yaml = configfiles.YamlConfig() - yaml.load() - - assert key in yaml - assert yaml[key] == value - - -@pytest.mark.parametrize('old_config', [ - None, - 'global:\n colors.hints.fg: magenta', -]) -def test_yaml_config_unchanged(fake_save_manager, config_tmpdir, old_config): - autoconfig = config_tmpdir / 'autoconfig.yml' - mtime = None - if old_config is not None: - autoconfig.write_text(old_config, 'utf-8') - mtime = autoconfig.stat().mtime - - yaml = configfiles.YamlConfig() - yaml.load() - yaml._save() - - if old_config is None: - assert not autoconfig.exists() - else: - assert autoconfig.stat().mtime == mtime - - -@pytest.mark.parametrize('line, text, exception', [ - ('%', 'While parsing', 'while scanning a directive'), - ('global: 42', 'While loading data', "'global' object is not a dict"), - ('foo: 42', 'While loading data', - "Toplevel object does not contain 'global' key"), - ('42', 'While loading data', "Toplevel object is not a dict"), -]) -def test_yaml_config_invalid(fake_save_manager, config_tmpdir, - line, text, exception): - autoconfig = config_tmpdir / 'autoconfig.yml' - autoconfig.write_text(line, 'utf-8', ensure=True) - - yaml = configfiles.YamlConfig() - - with pytest.raises(configexc.ConfigFileErrors) as excinfo: + yaml = configfiles.YamlConfig() yaml.load() - assert len(excinfo.value.errors) == 1 - error = excinfo.value.errors[0] - assert error.text == text - assert str(error.exception).splitlines()[0] == exception - assert error.traceback is None + yaml[key] = value + assert key in yaml + assert yaml[key] == value + yaml._save() -def test_yaml_oserror(fake_save_manager, config_tmpdir): - autoconfig = config_tmpdir / 'autoconfig.yml' - autoconfig.ensure() - autoconfig.chmod(0) - if os.access(str(autoconfig), os.R_OK): - # Docker container or similar - pytest.skip("File was still readable") - - yaml = configfiles.YamlConfig() - with pytest.raises(configexc.ConfigFileErrors) as excinfo: + yaml = configfiles.YamlConfig() yaml.load() - assert len(excinfo.value.errors) == 1 - error = excinfo.value.errors[0] - assert error.text == "While reading" - assert isinstance(error.exception, OSError) - assert error.traceback is None + assert key in yaml + assert yaml[key] == value + + @pytest.mark.parametrize('old_config', [ + None, + 'global:\n colors.hints.fg: magenta', + ]) + def test_unchanged(self, fake_save_manager, config_tmpdir, old_config): + autoconfig = config_tmpdir / 'autoconfig.yml' + mtime = None + if old_config is not None: + autoconfig.write_text(old_config, 'utf-8') + mtime = autoconfig.stat().mtime + + yaml = configfiles.YamlConfig() + yaml.load() + yaml._save() + + if old_config is None: + assert not autoconfig.exists() + else: + assert autoconfig.stat().mtime == mtime + + @pytest.mark.parametrize('line, text, exception', [ + ('%', 'While parsing', 'while scanning a directive'), + ('global: 42', 'While loading data', "'global' object is not a dict"), + ('foo: 42', 'While loading data', + "Toplevel object does not contain 'global' key"), + ('42', 'While loading data', "Toplevel object is not a dict"), + ]) + def test_invalid(self, fake_save_manager, config_tmpdir, + line, text, exception): + autoconfig = config_tmpdir / 'autoconfig.yml' + autoconfig.write_text(line, 'utf-8', ensure=True) + + yaml = configfiles.YamlConfig() + + with pytest.raises(configexc.ConfigFileErrors) as excinfo: + yaml.load() + + assert len(excinfo.value.errors) == 1 + error = excinfo.value.errors[0] + assert error.text == text + assert str(error.exception).splitlines()[0] == exception + assert error.traceback is None + + def test_oserror(self, fake_save_manager, config_tmpdir): + autoconfig = config_tmpdir / 'autoconfig.yml' + autoconfig.ensure() + autoconfig.chmod(0) + if os.access(str(autoconfig), os.R_OK): + # Docker container or similar + pytest.skip("File was still readable") + + yaml = configfiles.YamlConfig() + with pytest.raises(configexc.ConfigFileErrors) as excinfo: + yaml.load() + + assert len(excinfo.value.errors) == 1 + error = excinfo.value.errors[0] + assert error.text == "While reading" + assert isinstance(error.exception, OSError) + assert error.traceback is None class TestConfigPy: From 691cd2d09bbc4b89e4ba1db19390b62a3efbc1ff Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 20:19:02 +0200 Subject: [PATCH 27/77] More test_configfiles cleanups --- tests/unit/config/test_configfiles.py | 28 +++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index f1b7271f6..0453314bd 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -63,15 +63,14 @@ def test_state_config(fake_save_manager, data_tmpdir, class TestYaml: + pytestmark = pytest.mark.usefixtures('fake_save_manager') + @pytest.mark.parametrize('old_config', [ None, 'global:\n colors.hints.fg: magenta', - # Unknown key - 'global:\n hello: world', ]) @pytest.mark.parametrize('insert', [True, False]) - def test_yaml_config(self, fake_save_manager, config_tmpdir, - old_config, insert): + def test_yaml_config(self, config_tmpdir, old_config, insert): autoconfig = config_tmpdir / 'autoconfig.yml' if old_config is not None: autoconfig.write_text(old_config, 'utf-8') @@ -104,6 +103,17 @@ class TestYaml: assert ' colors.hints.fg: magenta' in lines if insert: assert ' tabs.show: never' in lines + + def test_unknown_key(self, config_tmpdir): + """An unknown setting should be deleted.""" + autoconfig = config_tmpdir / 'autoconfig.yml' + autoconfig.write_text('global:\n hello: world', encoding='utf-8') + + yaml = configfiles.YamlConfig() + yaml.load() + yaml._save() + + lines = autoconfig.read_text('utf-8').splitlines() assert ' hello:' not in lines @pytest.mark.parametrize('old_config', [ @@ -116,8 +126,7 @@ class TestYaml: ('confirm_quit', True), ('confirm_quit', False), ]) - def test_changed(self, fake_save_manager, config_tmpdir, old_config, - key, value): + def test_changed(self, config_tmpdir, old_config, key, value): autoconfig = config_tmpdir / 'autoconfig.yml' if old_config is not None: autoconfig.write_text(old_config, 'utf-8') @@ -141,7 +150,7 @@ class TestYaml: None, 'global:\n colors.hints.fg: magenta', ]) - def test_unchanged(self, fake_save_manager, config_tmpdir, old_config): + def test_unchanged(self, config_tmpdir, old_config): autoconfig = config_tmpdir / 'autoconfig.yml' mtime = None if old_config is not None: @@ -164,8 +173,7 @@ class TestYaml: "Toplevel object does not contain 'global' key"), ('42', 'While loading data', "Toplevel object is not a dict"), ]) - def test_invalid(self, fake_save_manager, config_tmpdir, - line, text, exception): + def test_invalid(self, config_tmpdir, line, text, exception): autoconfig = config_tmpdir / 'autoconfig.yml' autoconfig.write_text(line, 'utf-8', ensure=True) @@ -180,7 +188,7 @@ class TestYaml: assert str(error.exception).splitlines()[0] == exception assert error.traceback is None - def test_oserror(self, fake_save_manager, config_tmpdir): + def test_oserror(self, config_tmpdir): autoconfig = config_tmpdir / 'autoconfig.yml' autoconfig.ensure() autoconfig.chmod(0) From b1ddb9a6df26dbd28bef717945635316d34d5960 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 20:27:45 +0200 Subject: [PATCH 28/77] Remove confusing test That's not the behavior we actually have in the config anymore when using conf._yaml.load(). --- tests/unit/config/test_config.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 36dc90c10..bf423fede 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -619,11 +619,6 @@ class TestConfig: assert conf._yaml.loaded assert conf._values['content.plugins'] is True - def test_read_yaml_invalid(self, conf): - conf._yaml['foo.bar'] = True - with pytest.raises(configexc.NoOptionError): - conf.read_yaml() - def test_get_opt_valid(self, conf): assert conf.get_opt('tabs.show') == configdata.DATA['tabs.show'] From 32b2b3dfd96e8d0cd253d0788f77ca6d83a34c8c Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 20:41:30 +0200 Subject: [PATCH 29/77] Add test for invalid value type in YAML file --- tests/unit/config/test_config.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index bf423fede..3f419e8d0 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -891,7 +891,8 @@ def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, @pytest.mark.parametrize('load_autoconfig', [True, False]) # noqa @pytest.mark.parametrize('config_py', [True, 'error', False]) -@pytest.mark.parametrize('invalid_yaml', ['42', 'unknown', False]) +@pytest.mark.parametrize('invalid_yaml', + ['42', 'unknown', 'wrong-type', False]) # pylint: disable=too-many-branches def test_early_init(init_patch, config_tmpdir, caplog, fake_args, load_autoconfig, config_py, invalid_yaml): @@ -900,14 +901,15 @@ def test_early_init(init_patch, config_tmpdir, caplog, fake_args, config_py_file = config_tmpdir / 'config.py' if invalid_yaml == '42': - autoconfig_file.write_text('42', 'utf-8', ensure=True) + text = '42' elif invalid_yaml == 'unknown': - autoconfig_file.write_text('global:\n colors.foobar: magenta\n', - 'utf-8', ensure=True) + text = 'global:\n colors.foobar: magenta\n' + elif invalid_yaml == 'wrong-type': + text = 'global:\n tabs.position: true\n' else: assert not invalid_yaml - autoconfig_file.write_text('global:\n colors.hints.fg: magenta\n', - 'utf-8', ensure=True) + text = 'global:\n colors.hints.fg: magenta\n' + autoconfig_file.write_text(text, 'utf-8', ensure=True) if config_py: config_py_lines = ['c.colors.hints.bg = "red"'] @@ -927,10 +929,15 @@ def test_early_init(init_patch, config_tmpdir, caplog, fake_args, expected_errors.append( "Errors occurred while reading config.py:\n" " While setting 'foo': No option 'foo'") - if invalid_yaml == '42' and (load_autoconfig or not config_py): + if load_autoconfig or not config_py: error = "Errors occurred while reading autoconfig.yml:\n" - error += " While loading data: Toplevel object is not a dict" - expected_errors.append(error) + if invalid_yaml == '42': + error += " While loading data: Toplevel object is not a dict" + expected_errors.append(error) + elif invalid_yaml == 'wrong-type': + error += (" Error: Invalid value 'True' - expected a value of " + "type str but got bool.") + expected_errors.append(error) actual_errors = [str(err) for err in config._init_errors] assert actual_errors == expected_errors From f97f42710091b375b5249a6466f1d7124ef0d15e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 21:50:33 +0200 Subject: [PATCH 30/77] Add an assertion for Completer._partition --- qutebrowser/completion/completer.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 854231305..efd6b3490 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -154,6 +154,9 @@ class Completer(QObject): "partitioned: {} '{}' {}".format(prefix, center, postfix)) return prefix, center, postfix + # We should always return above + assert False, parts + @pyqtSlot(str) def on_selection_changed(self, text): """Change the completed part if a new item was selected. From 64b783d9c056045bcb5caa3d9ab8543b54fc4e18 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 22:28:51 +0200 Subject: [PATCH 31/77] Do not validate commands in the config and with :bind There are just way too many gotchas related to valid modes, aliases, and circular dependencies when validating aliases/bindings in the config. Let's just remove this and let invalid commands fail late, when they're actually used. --- qutebrowser/config/config.py | 15 +-------- qutebrowser/config/configtypes.py | 36 ++++++--------------- tests/unit/config/test_config.py | 46 ++++++++++++--------------- tests/unit/config/test_configfiles.py | 9 ++++++ tests/unit/config/test_configtypes.py | 27 ---------------- 5 files changed, 41 insertions(+), 92 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 1efc96856..e99498947 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -31,7 +31,7 @@ from qutebrowser.config import configdata, configexc, configtypes, configfiles from qutebrowser.utils import (utils, objreg, message, log, usertypes, jinja, qtutils) from qutebrowser.misc import objects, msgbox, earlyinit -from qutebrowser.commands import cmdexc, cmdutils, runners +from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.completion.models import configmodel # An easy way to access the config from other code via config.val.foo @@ -178,19 +178,6 @@ class KeyConfig: def bind(self, key, command, *, mode, force=False, save_yaml=False): """Add a new binding from key to command.""" key = self._prepare(key, mode) - - parser = runners.CommandParser() - try: - results = parser.parse_all(command) - except cmdexc.Error as e: - raise configexc.KeybindingError("Invalid command: {}".format(e)) - - for result in results: # pragma: no branch - try: - result.cmd.validate_mode(usertypes.KeyMode[mode]) - except cmdexc.PrerequisitesError as e: - raise configexc.KeybindingError(str(e)) - log.keyboard.vdebug("Adding binding {} -> {} in mode {}.".format( key, command, mode)) if key in self.get_bindings_for(mode) and not force: diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index c669ff426..aca9fffda 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -59,7 +59,7 @@ from PyQt5.QtCore import QUrl, Qt from PyQt5.QtGui import QColor, QFont from PyQt5.QtWidgets import QTabWidget, QTabBar -from qutebrowser.commands import cmdutils, runners, cmdexc +from qutebrowser.commands import cmdutils from qutebrowser.config import configexc from qutebrowser.utils import standarddir, utils, qtutils, urlutils @@ -773,33 +773,13 @@ class PercOrInt(_Numeric): class Command(BaseType): - """Base class for a command value with arguments.""" + """Base class for a command value with arguments. - # See to_py for details - unvalidated = False + // - def to_py(self, value): - self._basic_py_validation(value, str) - if not value: - return None - - # This requires some trickery, as runners.CommandParser uses - # conf.val.aliases, which in turn map to a command again, - # leading to an endless recursion. - # To fix that, we turn off validating other commands (alias values) - # while validating a command. - if not Command.unvalidated: - Command.unvalidated = True - try: - parser = runners.CommandParser() - try: - parser.parse_all(value) - except cmdexc.Error as e: - raise configexc.ValidationError(value, str(e)) - finally: - Command.unvalidated = False - - return value + Since validation is quite tricky here, we don't do so, and instead let + invalid commands (in bindings/aliases) fail when used. + """ def complete(self): out = [] @@ -807,6 +787,10 @@ class Command(BaseType): out.append((cmdname, obj.desc)) return out + def to_py(self, value): + self._basic_py_validation(value, str) + return value + class ColorSystem(MappingType): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 3f419e8d0..4e8bd684b 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -182,17 +182,6 @@ class TestKeyConfig: config_stub.val.bindings.commands = {'normal': bindings} assert keyconf.get_reverse_bindings_for('normal') == expected - def test_bind_invalid_command(self, keyconf): - with pytest.raises(configexc.KeybindingError, - match='Invalid command: foobar'): - keyconf.bind('a', 'foobar', mode='normal') - - def test_bind_invalid_mode(self, keyconf): - with pytest.raises(configexc.KeybindingError, - match='completion-item-del: This command is only ' - 'allowed in command mode, not normal.'): - keyconf.bind('a', 'completion-item-del', mode='normal') - @pytest.mark.parametrize('force', [True, False]) @pytest.mark.parametrize('key', ['a', '', 'b']) def test_bind_duplicate(self, keyconf, config_stub, force, key): @@ -208,12 +197,15 @@ class TestKeyConfig: assert keyconf.get_command(key, 'normal') == 'nop' @pytest.mark.parametrize('mode', ['normal', 'caret']) - def test_bind(self, keyconf, config_stub, qtbot, no_bindings, mode): + @pytest.mark.parametrize('command', [ + 'message-info foo', + 'nop ;; wq', # https://github.com/qutebrowser/qutebrowser/issues/3002 + ]) + def test_bind(self, keyconf, config_stub, qtbot, no_bindings, + mode, command): config_stub.val.bindings.default = no_bindings config_stub.val.bindings.commands = no_bindings - command = 'message-info foo' - with qtbot.wait_signal(config_stub.changed): keyconf.bind('a', command, mode=mode) @@ -221,6 +213,16 @@ class TestKeyConfig: assert keyconf.get_bindings_for(mode)['a'] == command assert keyconf.get_command('a', mode) == command + def test_bind_mode_changing(self, keyconf, config_stub, no_bindings): + """Make sure we can bind to a command which changes the mode. + + https://github.com/qutebrowser/qutebrowser/issues/2989 + """ + config_stub.val.bindings.default = no_bindings + config_stub.val.bindings.commands = no_bindings + keyconf.bind('a', 'set-cmd-text :nop ;; rl-beginning-of-line', + mode='normal') + @pytest.mark.parametrize('key, normalized', [ ('a', 'a'), # default bindings ('b', 'b'), # custom bindings @@ -504,20 +506,14 @@ class TestBindConfigCommand: msg = message_mock.getmsg(usertypes.MessageLevel.info) assert msg.text == expected - @pytest.mark.parametrize('command, mode, expected', [ - ('foobar', 'normal', "bind: Invalid command: foobar"), - ('completion-item-del', 'normal', - "bind: completion-item-del: This command is only allowed in " - "command mode, not normal."), - ('nop', 'wrongmode', "bind: Invalid mode wrongmode!"), - ]) - def test_bind_invalid(self, commands, command, mode, expected): - """Run ':bind a foobar' / ':bind a completion-item-del'. + def test_bind_invalid_mode(self, commands): + """Run ':bind --mode=wrongmode nop'. Should show an error. """ - with pytest.raises(cmdexc.CommandError, match=expected): - commands.bind('a', command, mode=mode) + with pytest.raises(cmdexc.CommandError, + match='bind: Invalid mode wrongmode!'): + commands.bind('a', 'nop', mode='wrongmode') @pytest.mark.parametrize('force', [True, False]) @pytest.mark.parametrize('key', ['a', 'b', '']) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 0453314bd..975e9c46a 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -279,6 +279,15 @@ class TestConfigPy: expected = {mode: {',a': 'message-info foo'}} assert config.instance._values['bindings.commands'] == expected + def test_bind_freshly_defined_alias(self, confpy): + """Make sure we can bind to a new alias. + + https://github.com/qutebrowser/qutebrowser/issues/3001 + """ + confpy.write("c.aliases['foo'] = 'message-info foo'", + "config.bind(',f', 'foo')") + confpy.read() + @pytest.mark.parametrize('line, key, mode', [ ('config.unbind("o")', 'o', 'normal'), ('config.unbind("y", mode="prompt")', 'y', 'prompt'), diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 122a1c20c..daf785d9c 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1072,37 +1072,10 @@ class TestCommand: monkeypatch.setattr(configtypes, 'cmdutils', cmd_utils) monkeypatch.setattr('qutebrowser.commands.runners.cmdutils', cmd_utils) - @pytest.fixture(autouse=True) - def patch_aliases(self, config_stub): - """Patch the aliases setting.""" - configtypes.Command.unvalidated = True - config_stub.val.aliases = {'alias': 'cmd1'} - configtypes.Command.unvalidated = False - @pytest.fixture def klass(self): return configtypes.Command - @pytest.mark.parametrize('val', ['cmd1', 'cmd2', 'cmd1 foo bar', - 'cmd2 baz fish', 'alias foo']) - def test_to_py_valid(self, patch_cmdutils, klass, val): - expected = None if not val else val - assert klass().to_py(val) == expected - - @pytest.mark.parametrize('val', ['cmd3', 'cmd3 foo bar', ' ']) - def test_to_py_invalid(self, patch_cmdutils, klass, val): - with pytest.raises(configexc.ValidationError): - klass().to_py(val) - - def test_cmdline(self, klass, cmdline_test): - """Test some commandlines from the cmdline_test fixture.""" - typ = klass() - if cmdline_test.valid: - typ.to_py(cmdline_test.cmd) - else: - with pytest.raises(configexc.ValidationError): - typ.to_py(cmdline_test.cmd) - def test_complete(self, patch_cmdutils, klass): """Test completion.""" items = klass().complete() From 1c76a51c1e53186a7a3679d54b74b03acd50fe68 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 22:31:11 +0200 Subject: [PATCH 32/77] Improve configtypes.Command docs --- doc/help/settings.asciidoc | 2 +- qutebrowser/config/configtypes.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 243e60023..3fdb32e3c 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -3166,7 +3166,7 @@ This setting is only available with the QtWebKit backend. When setting from a string, `1`, `yes`, `on` and `true` count as true, while `0`, `no`, `off` and `false` count as false (case-insensitive). |BoolAsk|Like `Bool`, but `ask` is allowed as additional value. |ColorSystem|The color system to use for color interpolation. -|Command|Base class for a command value with arguments. +|Command|A qutebrowser command with arguments. |ConfirmQuit|Whether to display a confirmation when the window is closed. |Dict|A dictionary of values. diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index aca9fffda..76fc61a8b 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -773,7 +773,7 @@ class PercOrInt(_Numeric): class Command(BaseType): - """Base class for a command value with arguments. + """A qutebrowser command with arguments. // From 599a5b96482e37b5a8f76bcb73420bb1da8dccce Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 22:47:46 +0200 Subject: [PATCH 33/77] Remove windows/pip instructions from earlyinit Windows: The instructions are outdated and not really relevant anymore with the standalone packages; pip: Let's recommend tox/virtualenv by just linking to the install docs. Closes #2998 --- qutebrowser/misc/earlyinit.py | 53 ++++++++--------------------------- 1 file changed, 11 insertions(+), 42 deletions(-) diff --git a/qutebrowser/misc/earlyinit.py b/qutebrowser/misc/earlyinit.py index 032dfc53b..e589cbeef 100644 --- a/qutebrowser/misc/earlyinit.py +++ b/qutebrowser/misc/earlyinit.py @@ -47,33 +47,25 @@ except ImportError: START_TIME = datetime.datetime.now() -def _missing_str(name, *, windows=None, pip=None, webengine=False): +def _missing_str(name, *, webengine=False): """Get an error string for missing packages. Args: name: The name of the package. - windows: String to be displayed for Windows. - pip: pypi package name. webengine: Whether this is checking the QtWebEngine package """ blocks = ["Fatal error: {} is required to run qutebrowser but " "could not be imported! Maybe it's not installed?".format(name), "The error encountered was:
%ERROR%"] lines = ['Please search for the python3 version of {} in your ' - 'distributions packages, or install it via pip.'.format(name)] + 'distributions packages, or see ' + 'https://github.com/qutebrowser/qutebrowser/blob/master/doc/install.asciidoc' + .format(name)] blocks.append('
'.join(lines)) if not webengine: lines = ['If you installed a qutebrowser package for your ' 'distribution, please report this as a bug.'] blocks.append('
'.join(lines)) - if windows is not None: - lines = ["On Windows:"] - lines += windows.splitlines() - blocks.append('
'.join(lines)) - if pip is not None: - lines = ["Using pip:"] - lines.append("pip3 install {}".format(pip)) - blocks.append('
'.join(lines)) return '

'.join(blocks) @@ -142,11 +134,7 @@ def check_pyqt_core(): try: import PyQt5.QtCore # pylint: disable=unused-variable except ImportError as e: - text = _missing_str('PyQt5', - windows="Use the installer by Riverbank computing " - "or the standalone qutebrowser exe.
" - "http://www.riverbankcomputing.co.uk/" - "software/pyqt/download5") + text = _missing_str('PyQt5') text = text.replace('', '') text = text.replace('', '') text = text.replace('
', '\n') @@ -239,31 +227,12 @@ def _check_modules(modules): def check_libraries(): """Check if all needed Python libraries are installed.""" modules = { - 'pkg_resources': - _missing_str("pkg_resources/setuptools", - windows="Run python -m ensurepip."), - 'pypeg2': - _missing_str("pypeg2", - pip="pypeg2"), - 'jinja2': - _missing_str("jinja2", - windows="Install from http://www.lfd.uci.edu/" - "~gohlke/pythonlibs/#jinja2 or via pip.", - pip="jinja2"), - 'pygments': - _missing_str("pygments", - windows="Install from http://www.lfd.uci.edu/" - "~gohlke/pythonlibs/#pygments or via pip.", - pip="pygments"), - 'yaml': - _missing_str("PyYAML", - windows="Use the installers at " - "http://pyyaml.org/download/pyyaml/ (py3.4) " - "or Install via pip.", - pip="PyYAML"), - 'attr': - _missing_str("attrs", - pip="attrs"), + 'pkg_resources': _missing_str("pkg_resources/setuptools"), + 'pypeg2': _missing_str("pypeg2"), + 'jinja2': _missing_str("jinja2"), + 'pygments': _missing_str("pygments"), + 'yaml': _missing_str("PyYAML"), + 'attr': _missing_str("attrs"), 'PyQt5.QtQml': _missing_str("PyQt5.QtQml"), 'PyQt5.QtSql': _missing_str("PyQt5.QtSql"), 'PyQt5.QtOpenGL': _missing_str("PyQt5.QtOpenGL"), From c74236dd96a05829b086f9d8853d1cba3eb606a2 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 22:53:43 +0200 Subject: [PATCH 34/77] Move some data from setupcommon to setup.py We can't get rid of setupcommon entirely (it's needed by PyInstaller), but at least we can get the data back to setup.py. Fixes #2996 --- scripts/setupcommon.py | 68 +++--------------------------------------- setup.py | 57 ++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 65 deletions(-) diff --git a/scripts/setupcommon.py b/scripts/setupcommon.py index d5e13ec72..a2e4dfca9 100644 --- a/scripts/setupcommon.py +++ b/scripts/setupcommon.py @@ -18,11 +18,9 @@ # along with qutebrowser. If not, see . -"""Data used by setup.py and scripts/freeze.py.""" +"""Data used by setup.py and the PyInstaller qutebrowser.spec.""" import sys -import re -import ast import os import os.path import subprocess @@ -30,42 +28,16 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir)) if sys.hexversion >= 0x03000000: - _open = open + open_file = open else: import codecs - _open = codecs.open + open_file = codecs.open BASEDIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), os.path.pardir) -def read_file(name): - """Get the string contained in the file named name.""" - with _open(name, 'r', encoding='utf-8') as f: - return f.read() - - -def _get_constant(name): - """Read a __magic__ constant from qutebrowser/__init__.py. - - We don't import qutebrowser here because it can go wrong for multiple - reasons. Instead we use re/ast to get the value directly from the source - file. - - Args: - name: The name of the argument to get. - - Return: - The value of the argument. - """ - field_re = re.compile(r'__{}__\s+=\s+(.*)'.format(re.escape(name))) - path = os.path.join(BASEDIR, 'qutebrowser', '__init__.py') - line = field_re.search(read_file(path)).group(1) - value = ast.literal_eval(line) - return value - - def _git_str(): """Try to find out git version. @@ -95,37 +67,5 @@ def write_git_file(): if gitstr is None: gitstr = '' path = os.path.join(BASEDIR, 'qutebrowser', 'git-commit-id') - with _open(path, 'w', encoding='ascii') as f: + with open_file(path, 'w', encoding='ascii') as f: f.write(gitstr) - - -setupdata = { - 'name': 'qutebrowser', - 'version': '.'.join(str(e) for e in _get_constant('version_info')), - 'description': _get_constant('description'), - 'long_description': read_file('README.asciidoc'), - 'url': 'https://www.qutebrowser.org/', - 'requires': ['pypeg2', 'jinja2', 'pygments', 'PyYAML', 'attrs'], - 'author': _get_constant('author'), - 'author_email': _get_constant('email'), - 'license': _get_constant('license'), - 'classifiers': [ - 'Development Status :: 3 - Alpha', - 'Environment :: X11 Applications :: Qt', - 'Intended Audience :: End Users/Desktop', - 'License :: OSI Approved :: GNU General Public License v3 or later ' - '(GPLv3+)', - 'Natural Language :: English', - 'Operating System :: Microsoft :: Windows', - 'Operating System :: Microsoft :: Windows :: Windows XP', - 'Operating System :: Microsoft :: Windows :: Windows 7', - 'Operating System :: POSIX :: Linux', - 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.5', - 'Programming Language :: Python :: 3.6', - 'Topic :: Internet', - 'Topic :: Internet :: WWW/HTTP', - 'Topic :: Internet :: WWW/HTTP :: Browsers', - ], - 'keywords': 'pyqt browser web qt webkit', -} diff --git a/setup.py b/setup.py index 7bfd968f6..c90b163ed 100755 --- a/setup.py +++ b/setup.py @@ -21,6 +21,8 @@ """setuptools installer script for qutebrowser.""" +import re +import ast import os import os.path @@ -35,6 +37,32 @@ except NameError: BASEDIR = None +def read_file(name): + """Get the string contained in the file named name.""" + with common.open_file(name, 'r', encoding='utf-8') as f: + return f.read() + + +def _get_constant(name): + """Read a __magic__ constant from qutebrowser/__init__.py. + + We don't import qutebrowser here because it can go wrong for multiple + reasons. Instead we use re/ast to get the value directly from the source + file. + + Args: + name: The name of the argument to get. + + Return: + The value of the argument. + """ + field_re = re.compile(r'__{}__\s+=\s+(.*)'.format(re.escape(name))) + path = os.path.join(BASEDIR, 'qutebrowser', '__init__.py') + line = field_re.search(read_file(path)).group(1) + value = ast.literal_eval(line) + return value + + try: common.write_git_file() setuptools.setup( @@ -45,7 +73,34 @@ try: test_suite='qutebrowser.test', zip_safe=True, install_requires=['pypeg2', 'jinja2', 'pygments', 'PyYAML', 'attrs'], - **common.setupdata + name='qutebrowser', + version='.'.join(str(e) for e in _get_constant('version_info')), + description=_get_constant('description'), + long_description=read_file('README.asciidoc'), + url='https://www.qutebrowser.org/', + requires=['pypeg2', 'jinja2', 'pygments', 'PyYAML', 'attrs'], + author=_get_constant('author'), + author_email=_get_constant('email'), + license=_get_constant('license'), + classifiers=[ + 'Development Status :: 3 - Alpha', + 'Environment :: X11 Applications :: Qt', + 'Intended Audience :: End Users/Desktop', + 'License :: OSI Approved :: GNU General Public License v3 or later ' + '(GPLv3+)', + 'Natural Language :: English', + 'Operating System :: Microsoft :: Windows', + 'Operating System :: Microsoft :: Windows :: Windows XP', + 'Operating System :: Microsoft :: Windows :: Windows 7', + 'Operating System :: POSIX :: Linux', + 'Programming Language :: Python :: 3', + 'Programming Language :: Python :: 3.5', + 'Programming Language :: Python :: 3.6', + 'Topic :: Internet', + 'Topic :: Internet :: WWW/HTTP', + 'Topic :: Internet :: WWW/HTTP :: Browsers', + ], + keywords='pyqt browser web qt webkit', ) finally: if BASEDIR is not None: From 3f18a5ada7c585faaaa1c8f9a175a2a8950e1a73 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 22:57:29 +0200 Subject: [PATCH 35/77] Update metainfo in setup.py --- setup.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/setup.py b/setup.py index c90b163ed..712383839 100755 --- a/setup.py +++ b/setup.py @@ -70,7 +70,6 @@ try: include_package_data=True, entry_points={'gui_scripts': ['qutebrowser = qutebrowser.qutebrowser:main']}, - test_suite='qutebrowser.test', zip_safe=True, install_requires=['pypeg2', 'jinja2', 'pygments', 'PyYAML', 'attrs'], name='qutebrowser', @@ -78,21 +77,20 @@ try: description=_get_constant('description'), long_description=read_file('README.asciidoc'), url='https://www.qutebrowser.org/', - requires=['pypeg2', 'jinja2', 'pygments', 'PyYAML', 'attrs'], author=_get_constant('author'), author_email=_get_constant('email'), license=_get_constant('license'), classifiers=[ - 'Development Status :: 3 - Alpha', + 'Development Status :: 4 - Beta', 'Environment :: X11 Applications :: Qt', 'Intended Audience :: End Users/Desktop', 'License :: OSI Approved :: GNU General Public License v3 or later ' '(GPLv3+)', 'Natural Language :: English', 'Operating System :: Microsoft :: Windows', - 'Operating System :: Microsoft :: Windows :: Windows XP', - 'Operating System :: Microsoft :: Windows :: Windows 7', 'Operating System :: POSIX :: Linux', + 'Operating System :: MacOS', + 'Operating System :: POSIX :: BSD', 'Programming Language :: Python :: 3', 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', @@ -100,7 +98,7 @@ try: 'Topic :: Internet :: WWW/HTTP', 'Topic :: Internet :: WWW/HTTP :: Browsers', ], - keywords='pyqt browser web qt webkit', + keywords='pyqt browser web qt webkit qtwebkit qtwebengine', ) finally: if BASEDIR is not None: From cd9fe57d84c102213bffbcfb8bbab2f0270e33eb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 23:03:02 +0200 Subject: [PATCH 36/77] build_release: Also run asciidoc2html on Linux --- scripts/dev/build_release.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/dev/build_release.py b/scripts/dev/build_release.py index a51c8f2b3..0019752c2 100755 --- a/scripts/dev/build_release.py +++ b/scripts/dev/build_release.py @@ -354,6 +354,7 @@ def main(): import github3 # pylint: disable=unused-variable read_github_token() + run_asciidoc2html(args) if utils.is_windows: if sys.maxsize > 2**32: # WORKAROUND @@ -363,10 +364,8 @@ def main(): print("See http://bugs.python.org/issue24493 and ") print("https://github.com/pypa/virtualenv/issues/774") sys.exit(1) - run_asciidoc2html(args) artifacts = build_windows() elif utils.is_mac: - run_asciidoc2html(args) artifacts = build_mac() else: artifacts = build_sdist() From f4017eb5b6428071aceaa5f4023d8e59bf9c7fa8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 23:24:22 +0200 Subject: [PATCH 37/77] Ignore more Python warnings when importing in earlyinit With a17c4767d6de3259b295ed7c15e81f8279df165a we moved the first time pkg_resources is imported to earlyinit.py, which means less warnings were suppressed. Fixes #2990 --- qutebrowser/misc/earlyinit.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/qutebrowser/misc/earlyinit.py b/qutebrowser/misc/earlyinit.py index e589cbeef..c2ab454ac 100644 --- a/qutebrowser/misc/earlyinit.py +++ b/qutebrowser/misc/earlyinit.py @@ -218,7 +218,14 @@ def _check_modules(modules): 'Flags not at the start of the expression'] with log.ignore_py_warnings( category=DeprecationWarning, - message=r'({})'.format('|'.join(messages))): + message=r'({})'.format('|'.join(messages)) + ), log.ignore_py_warnings( + category=PendingDeprecationWarning, + module='imp' + ), log.ignore_py_warnings( + category=ImportWarning, + message=r'Not importing directory .*: missing __init__' + ): importlib.import_module(name) except ImportError as e: _die(text, e) From c652b0f96c3771fddd15f8a680cf2048d4ebdff5 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 21 Sep 2017 23:59:16 +0200 Subject: [PATCH 38/77] Remove old monkeypatch --- tests/unit/commands/test_runners.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/commands/test_runners.py b/tests/unit/commands/test_runners.py index 5e9aa3dfc..4e97c2385 100644 --- a/tests/unit/commands/test_runners.py +++ b/tests/unit/commands/test_runners.py @@ -47,7 +47,6 @@ class TestCommandParser: if not cmdline_test.cmd: pytest.skip("Empty command") - monkeypatch.setattr(configtypes.Command, 'unvalidated', True) config_stub.val.aliases = {'alias_name': cmdline_test.cmd} parser = runners.CommandParser() From a2952e13a896fffbb16bc31549d8919bbd4f40e8 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Fri, 15 Sep 2017 20:43:17 -0400 Subject: [PATCH 39/77] Add qutebrowser config directory to python path This is done so config.py can import other python files in the config directory. For example, config.py can 'import theme' which would load a theme.py. The previous path is restored at the end of this function, to avoid tainting qutebrowser's path --- qutebrowser/config/configfiles.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index a155726a8..155a2c99f 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -21,6 +21,7 @@ import types import os.path +import sys import textwrap import traceback import configparser @@ -222,6 +223,12 @@ def read_config_py(filename=None): if not os.path.exists(filename): return api + # Add config directory to python path, so config.py can import other files + # in logical places + old_path = sys.path.copy() + if standarddir.config() not in sys.path: + sys.path.insert(0, standarddir.config()) + container = config.ConfigContainer(config.instance, configapi=api) basename = os.path.basename(filename) @@ -256,6 +263,9 @@ def read_config_py(filename=None): "Unhandled exception", exception=e, traceback=traceback.format_exc())) + # Restore previous path, to protect qutebrowser's imports + sys.path = old_path + api.finalize() return api From 0332dce458e249e73efd61cdf556d60f1062406c Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Sat, 16 Sep 2017 01:07:13 -0400 Subject: [PATCH 40/77] Get config path from config.py location, rather than standarddir --- qutebrowser/config/configfiles.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 155a2c99f..450b7b644 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -226,8 +226,9 @@ def read_config_py(filename=None): # Add config directory to python path, so config.py can import other files # in logical places old_path = sys.path.copy() - if standarddir.config() not in sys.path: - sys.path.insert(0, standarddir.config()) + config_dir = os.path.dirname(filename) + if config_dir not in sys.path: + sys.path.insert(0, config_dir) container = config.ConfigContainer(config.instance, configapi=api) basename = os.path.basename(filename) From 333c0d848b9564ed000b66774b63e3014277f451 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Sat, 16 Sep 2017 23:22:19 -0400 Subject: [PATCH 41/77] Restructure save/load of state to be more extensible Also save/load sys.modules as well - This is a little rough, but I can't find a better way... --- qutebrowser/config/configfiles.py | 33 +++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 450b7b644..57d6e851b 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -225,10 +225,10 @@ def read_config_py(filename=None): # Add config directory to python path, so config.py can import other files # in logical places - old_path = sys.path.copy() + old_state = _pre_config_save() config_dir = os.path.dirname(filename) if config_dir not in sys.path: - sys.path.insert(0, config_dir) + sys.path = [config_dir] + sys.path container = config.ConfigContainer(config.instance, configapi=api) basename = os.path.basename(filename) @@ -238,6 +238,20 @@ def read_config_py(filename=None): module.c = container module.__file__ = filename + try: + _run_python_config_helper(filename, basename, api, module) + except: + _post_config_load(old_state) + raise + + # Restore previous path, to protect qutebrowser's imports + _post_config_load(old_state) + + api.finalize() + return api + + +def _run_python_config_helper(filename, basename, api, module): try: with open(filename, mode='rb') as f: source = f.read() @@ -264,11 +278,18 @@ def read_config_py(filename=None): "Unhandled exception", exception=e, traceback=traceback.format_exc())) - # Restore previous path, to protect qutebrowser's imports - sys.path = old_path - api.finalize() - return api +def _pre_config_save(): + old_path = sys.path + old_modules = sys.modules.copy() + return (old_path, old_modules) + + +def _post_config_load(save_tuple): + sys.path = save_tuple[0] + for module in set(sys.modules).difference(save_tuple[1]): + del sys.modules[module] + pass def init(): From 7ddde334dacaa21661f41de893864319b2245b8a Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Sun, 17 Sep 2017 01:18:20 -0400 Subject: [PATCH 42/77] Add tests for module/path isolation --- qutebrowser/config/configfiles.py | 1 - tests/unit/config/test_configfiles.py | 138 ++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 57d6e851b..91dafe172 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -289,7 +289,6 @@ def _post_config_load(save_tuple): sys.path = save_tuple[0] for module in set(sys.modules).difference(save_tuple[1]): del sys.modules[module] - pass def init(): diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 975e9c46a..0e782efd7 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -207,6 +207,144 @@ class TestYaml: assert error.traceback is None +class TestConfigPyModules: + + """Test for ConfigPy Modules.""" + + pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') + _old_sys_path = sys.path.copy() + + class ConfPy: + + """Helper class to get a confpy fixture.""" + + def __init__(self, tmpdir): + self._confpy = tmpdir / 'config.py' + self.filename = str(self._confpy) + + def write(self, *lines): + text = '\n'.join(lines) + self._confpy.write_text(text, 'utf-8', ensure=True) + + class QbModulePy: + + """Helper class to get a QbModulePy fixture.""" + + def __init__(self, tmpdir): + self._qbmodulepy = tmpdir / 'qbmodule.py' + self.filename = str(self._qbmodulepy) + + def write(self, *lines): + text = '\n'.join(lines) + self._qbmodulepy.write_text(text, 'utf-8', ensure=True) + + @pytest.fixture + def confpy(self, tmpdir): + return self.ConfPy(tmpdir) + + @pytest.fixture + def qbmodulepy(self, tmpdir): + return self.QbModulePy(tmpdir) + + def setup_method(self, method): + # If we plan to add tests that modify modules themselves, that should + # be saved as well + TestConfigPyModules._old_sys_path = sys.path.copy() + + def teardown_method(self, method): + # Restore path to save the rest of the tests + sys.path = TestConfigPyModules._old_sys_path + + def test_bind_in_module(self, confpy, qbmodulepy): + qbmodulepy.write("""def run(config): + config.bind(",a", "message-info foo", mode="normal")""") + confpy.write("""import qbmodule +qbmodule.run(config)""") + api = configfiles.read_config_py(confpy.filename) + expected = {'normal': {',a': 'message-info foo'}} + assert len(api.errors) == 0 + assert config.instance._values['bindings.commands'] == expected + + def test_clear_path(self, confpy, qbmodulepy, tmpdir): + qbmodulepy.write("""def run(config): + config.bind(",a", "message-info foo", mode="normal")""") + confpy.write("""import qbmodule +qbmodule.run(config)""") + api = configfiles.read_config_py(confpy.filename) + assert len(api.errors) == 0 + assert tmpdir not in sys.path + + def test_clear_modules(self, confpy, qbmodulepy): + qbmodulepy.write("""def run(config): + config.bind(",a", "message-info foo", mode="normal")""") + confpy.write("""import qbmodule +qbmodule.run(config)""") + api = configfiles.read_config_py(confpy.filename) + assert len(api.errors) == 0 + assert "qbmodule" not in sys.modules.keys() + + def test_clear_modules_on_err(self, confpy, qbmodulepy): + qbmodulepy.write("""def run(config): + 1/0""") + confpy.write("""import qbmodule +qbmodule.run(config)""") + api = configfiles.read_config_py(confpy.filename) + + assert len(api.errors) == 1 + error = api.errors[0] + assert error.text == "Unhandled exception" + assert isinstance(error.exception, ZeroDivisionError) + + tblines = error.traceback.strip().splitlines() + assert tblines[0] == "Traceback (most recent call last):" + assert tblines[-1] == "ZeroDivisionError: division by zero" + assert " 1/0" in tblines + assert "qbmodule" not in sys.modules.keys() + + def test_clear_path_on_err(self, confpy, qbmodulepy, tmpdir): + qbmodulepy.write("""def run(config): + 1/0""") + confpy.write("""import qbmodule +qbmodule.run(config)""") + api = configfiles.read_config_py(confpy.filename) + + assert len(api.errors) == 1 + error = api.errors[0] + assert error.text == "Unhandled exception" + assert isinstance(error.exception, ZeroDivisionError) + + tblines = error.traceback.strip().splitlines() + assert tblines[0] == "Traceback (most recent call last):" + assert tblines[-1] == "ZeroDivisionError: division by zero" + assert " 1/0" in tblines + assert tmpdir not in sys.path + + def test_fail_on_nonexistent_module(self, confpy, qbmodulepy, tmpdir): + qbmodulepy.write("""def run(config): + pass""") + confpy.write("""import foobar +foobar.run(config)""") + api = configfiles.read_config_py(confpy.filename) + + assert len(api.errors) == 1 + error = api.errors[0] + assert error.text == "Unhandled exception" + assert isinstance(error.exception, ImportError) + + tblines = error.traceback.strip().splitlines() + assert tblines[0] == "Traceback (most recent call last):" + assert tblines[-1].endswith("Error: No module named 'foobar'") + + def test_no_double_if_path_exists(self, confpy, qbmodulepy, tmpdir): + sys.path.insert(0, tmpdir) + confpy.write("""import sys +if sys.path[1:].count(sys.path[0]) != 0: + raise Exception('Path not expected')""") + api = configfiles.read_config_py(confpy.filename) + assert len(api.errors) == 0 + assert sys.path.count(tmpdir) == 1 + + class TestConfigPy: """Tests for ConfigAPI and read_config_py().""" From 4e22b4666d79b6b7f4f9b56aff12d72dae42217d Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Wed, 20 Sep 2017 21:26:56 -0400 Subject: [PATCH 43/77] Convert save-restore of sys to a context-manager Also improve and simplify tests for save/load of sys.module and sys.path --- qutebrowser/config/configfiles.py | 53 +++++------ tests/unit/config/test_configfiles.py | 130 ++++++-------------------- 2 files changed, 54 insertions(+), 129 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 91dafe172..561eec0eb 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -223,13 +223,6 @@ def read_config_py(filename=None): if not os.path.exists(filename): return api - # Add config directory to python path, so config.py can import other files - # in logical places - old_state = _pre_config_save() - config_dir = os.path.dirname(filename) - if config_dir not in sys.path: - sys.path = [config_dir] + sys.path - container = config.ConfigContainer(config.instance, configapi=api) basename = os.path.basename(filename) @@ -238,20 +231,6 @@ def read_config_py(filename=None): module.c = container module.__file__ = filename - try: - _run_python_config_helper(filename, basename, api, module) - except: - _post_config_load(old_state) - raise - - # Restore previous path, to protect qutebrowser's imports - _post_config_load(old_state) - - api.finalize() - return api - - -def _run_python_config_helper(filename, basename, api, module): try: with open(filename, mode='rb') as f: source = f.read() @@ -268,27 +247,41 @@ def _run_python_config_helper(filename, basename, api, module): raise configexc.ConfigFileErrors(basename, [desc]) except SyntaxError as e: desc = configexc.ConfigErrorDesc("Syntax Error", e, - traceback=traceback.format_exc()) + traceback=traceback.format_exc()) raise configexc.ConfigFileErrors(basename, [desc]) try: - exec(code, module.__dict__) + # Save and restore sys variables + with saved_sys_properties(): + # Add config directory to python path, so config.py can import + # other files in logical places + config_dir = os.path.dirname(filename) + if config_dir not in sys.path: + sys.path = [config_dir] + sys.path + + exec(code, module.__dict__) except Exception as e: api.errors.append(configexc.ConfigErrorDesc( "Unhandled exception", exception=e, traceback=traceback.format_exc())) + api.finalize() + return api -def _pre_config_save(): +@contextlib.contextmanager +def saved_sys_properties(): + """Save various sys properties such as sys.path and sys.modules.""" old_path = sys.path old_modules = sys.modules.copy() - return (old_path, old_modules) - -def _post_config_load(save_tuple): - sys.path = save_tuple[0] - for module in set(sys.modules).difference(save_tuple[1]): - del sys.modules[module] + try: + yield + except: + raise + finally: + sys.path = old_path + for module in set(sys.modules).difference(old_modules): + del sys.modules[module] def init(): diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 0e782efd7..2d8d4cd94 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -209,121 +209,50 @@ class TestYaml: class TestConfigPyModules: - """Test for ConfigPy Modules.""" - pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') - _old_sys_path = sys.path.copy() - - class ConfPy: - - """Helper class to get a confpy fixture.""" - - def __init__(self, tmpdir): - self._confpy = tmpdir / 'config.py' - self.filename = str(self._confpy) - - def write(self, *lines): - text = '\n'.join(lines) - self._confpy.write_text(text, 'utf-8', ensure=True) - - class QbModulePy: - - """Helper class to get a QbModulePy fixture.""" - - def __init__(self, tmpdir): - self._qbmodulepy = tmpdir / 'qbmodule.py' - self.filename = str(self._qbmodulepy) - - def write(self, *lines): - text = '\n'.join(lines) - self._qbmodulepy.write_text(text, 'utf-8', ensure=True) @pytest.fixture def confpy(self, tmpdir): - return self.ConfPy(tmpdir) + return TestConfigPy.ConfPy(tmpdir) @pytest.fixture def qbmodulepy(self, tmpdir): - return self.QbModulePy(tmpdir) + return TestConfigPy.ConfPy(tmpdir, filename="qbmodule.py") - def setup_method(self, method): - # If we plan to add tests that modify modules themselves, that should - # be saved as well - TestConfigPyModules._old_sys_path = sys.path.copy() + @pytest.fixture(autouse=True) + def restore_sys_path(self): + old_path = sys.path.copy() + yield + sys.path = old_path - def teardown_method(self, method): - # Restore path to save the rest of the tests - sys.path = TestConfigPyModules._old_sys_path - - def test_bind_in_module(self, confpy, qbmodulepy): - qbmodulepy.write("""def run(config): - config.bind(",a", "message-info foo", mode="normal")""") - confpy.write("""import qbmodule -qbmodule.run(config)""") - api = configfiles.read_config_py(confpy.filename) + def test_bind_in_module(self, confpy, qbmodulepy, tmpdir): + qbmodulepy.write('def run(config):', + ' config.bind(",a", "message-info foo", mode="normal")') + confpy.write_qbmodule() + confpy.read() expected = {'normal': {',a': 'message-info foo'}} - assert len(api.errors) == 0 assert config.instance._values['bindings.commands'] == expected - - def test_clear_path(self, confpy, qbmodulepy, tmpdir): - qbmodulepy.write("""def run(config): - config.bind(",a", "message-info foo", mode="normal")""") - confpy.write("""import qbmodule -qbmodule.run(config)""") - api = configfiles.read_config_py(confpy.filename) - assert len(api.errors) == 0 + assert "qbmodule" not in sys.modules.keys() assert tmpdir not in sys.path - def test_clear_modules(self, confpy, qbmodulepy): - qbmodulepy.write("""def run(config): - config.bind(",a", "message-info foo", mode="normal")""") - confpy.write("""import qbmodule -qbmodule.run(config)""") - api = configfiles.read_config_py(confpy.filename) - assert len(api.errors) == 0 - assert "qbmodule" not in sys.modules.keys() - - def test_clear_modules_on_err(self, confpy, qbmodulepy): - qbmodulepy.write("""def run(config): - 1/0""") - confpy.write("""import qbmodule -qbmodule.run(config)""") + def test_restore_sys_on_err(self, confpy, qbmodulepy, tmpdir): + confpy.write_qbmodule() + qbmodulepy.write('def run(config):', + ' 1/0') api = configfiles.read_config_py(confpy.filename) assert len(api.errors) == 1 error = api.errors[0] assert error.text == "Unhandled exception" assert isinstance(error.exception, ZeroDivisionError) - - tblines = error.traceback.strip().splitlines() - assert tblines[0] == "Traceback (most recent call last):" - assert tblines[-1] == "ZeroDivisionError: division by zero" - assert " 1/0" in tblines assert "qbmodule" not in sys.modules.keys() - - def test_clear_path_on_err(self, confpy, qbmodulepy, tmpdir): - qbmodulepy.write("""def run(config): - 1/0""") - confpy.write("""import qbmodule -qbmodule.run(config)""") - api = configfiles.read_config_py(confpy.filename) - - assert len(api.errors) == 1 - error = api.errors[0] - assert error.text == "Unhandled exception" - assert isinstance(error.exception, ZeroDivisionError) - - tblines = error.traceback.strip().splitlines() - assert tblines[0] == "Traceback (most recent call last):" - assert tblines[-1] == "ZeroDivisionError: division by zero" - assert " 1/0" in tblines assert tmpdir not in sys.path def test_fail_on_nonexistent_module(self, confpy, qbmodulepy, tmpdir): - qbmodulepy.write("""def run(config): - pass""") - confpy.write("""import foobar -foobar.run(config)""") + qbmodulepy.write('def run(config):', + ' pass') + confpy.write('import foobar', + 'foobar.run(config)') api = configfiles.read_config_py(confpy.filename) assert len(api.errors) == 1 @@ -337,11 +266,10 @@ foobar.run(config)""") def test_no_double_if_path_exists(self, confpy, qbmodulepy, tmpdir): sys.path.insert(0, tmpdir) - confpy.write("""import sys -if sys.path[1:].count(sys.path[0]) != 0: - raise Exception('Path not expected')""") - api = configfiles.read_config_py(confpy.filename) - assert len(api.errors) == 0 + confpy.write('import sys', + 'if sys.path[0] in sys.path[1:]:', + ' raise Exception("Path not expected")') + confpy.read() assert sys.path.count(tmpdir) == 1 @@ -355,8 +283,8 @@ class TestConfigPy: """Helper class to get a confpy fixture.""" - def __init__(self, tmpdir): - self._confpy = tmpdir / 'config.py' + def __init__(self, tmpdir, filename: str = "config.py"): + self._confpy = tmpdir / filename self.filename = str(self._confpy) def write(self, *lines): @@ -368,6 +296,10 @@ class TestConfigPy: api = configfiles.read_config_py(self.filename) assert not api.errors + def write_qbmodule(self): + self.write('import qbmodule', + 'qbmodule.run(config)') + @pytest.fixture def confpy(self, tmpdir): return self.ConfPy(tmpdir) From 43ce10efc34188825421f1db94b60117b46b57b3 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Thu, 21 Sep 2017 10:17:25 -0400 Subject: [PATCH 44/77] Simplify and reorganize configfile tests Also make save/load of sys.path a little more robust --- qutebrowser/config/configfiles.py | 9 +++-- tests/unit/config/test_configfiles.py | 50 ++++++++++++++------------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 561eec0eb..2ee973730 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -247,7 +247,7 @@ def read_config_py(filename=None): raise configexc.ConfigFileErrors(basename, [desc]) except SyntaxError as e: desc = configexc.ConfigErrorDesc("Syntax Error", e, - traceback=traceback.format_exc()) + traceback=traceback.format_exc()) raise configexc.ConfigFileErrors(basename, [desc]) try: @@ -257,13 +257,14 @@ def read_config_py(filename=None): # other files in logical places config_dir = os.path.dirname(filename) if config_dir not in sys.path: - sys.path = [config_dir] + sys.path + sys.path.insert(0, config_dir) exec(code, module.__dict__) except Exception as e: api.errors.append(configexc.ConfigErrorDesc( "Unhandled exception", exception=e, traceback=traceback.format_exc())) + api.finalize() return api @@ -271,13 +272,11 @@ def read_config_py(filename=None): @contextlib.contextmanager def saved_sys_properties(): """Save various sys properties such as sys.path and sys.modules.""" - old_path = sys.path + old_path = sys.path.copy() old_modules = sys.modules.copy() try: yield - except: - raise finally: sys.path = old_path for module in set(sys.modules).difference(old_modules): diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 2d8d4cd94..bf214fed3 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -19,6 +19,7 @@ """Tests for qutebrowser.config.configfiles.""" import os +import sys import pytest @@ -207,17 +208,39 @@ class TestYaml: assert error.traceback is None +class ConfPy: + + """Helper class to get a confpy fixture.""" + + def __init__(self, tmpdir, filename: str = "config.py"): + self._file = tmpdir / filename + self.filename = str(self._file) + + def write(self, *lines): + text = '\n'.join(lines) + self._file.write_text(text, 'utf-8', ensure=True) + + def read(self): + """Read the config.py via configfiles and check for errors.""" + api = configfiles.read_config_py(self.filename) + assert not api.errors + + def write_qbmodule(self): + self.write('import qbmodule', + 'qbmodule.run(config)') + + class TestConfigPyModules: pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') @pytest.fixture def confpy(self, tmpdir): - return TestConfigPy.ConfPy(tmpdir) + return ConfPy(tmpdir) @pytest.fixture def qbmodulepy(self, tmpdir): - return TestConfigPy.ConfPy(tmpdir, filename="qbmodule.py") + return ConfPy(tmpdir, filename="qbmodule.py") @pytest.fixture(autouse=True) def restore_sys_path(self): @@ -279,30 +302,9 @@ class TestConfigPy: pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') - class ConfPy: - - """Helper class to get a confpy fixture.""" - - def __init__(self, tmpdir, filename: str = "config.py"): - self._confpy = tmpdir / filename - self.filename = str(self._confpy) - - def write(self, *lines): - text = '\n'.join(lines) - self._confpy.write_text(text, 'utf-8', ensure=True) - - def read(self): - """Read the config.py via configfiles and check for errors.""" - api = configfiles.read_config_py(self.filename) - assert not api.errors - - def write_qbmodule(self): - self.write('import qbmodule', - 'qbmodule.run(config)') - @pytest.fixture def confpy(self, tmpdir): - return self.ConfPy(tmpdir) + return ConfPy(tmpdir) @pytest.mark.parametrize('line', [ 'c.colors.hints.bg = "red"', From 10016ae240863d68949681f789a84d129698b7a1 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 08:23:06 +0200 Subject: [PATCH 45/77] Remove unused import --- tests/unit/commands/test_runners.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/commands/test_runners.py b/tests/unit/commands/test_runners.py index 4e97c2385..75558b390 100644 --- a/tests/unit/commands/test_runners.py +++ b/tests/unit/commands/test_runners.py @@ -22,7 +22,6 @@ import pytest from qutebrowser.commands import runners, cmdexc -from qutebrowser.config import configtypes class TestCommandParser: From 1dbd156c2f34bd9b2bdba47bbae0083493d2d0a1 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 08:43:07 +0200 Subject: [PATCH 46/77] Simplify some config.py tests --- tests/unit/config/test_configfiles.py | 54 ++++++++++++--------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index bf214fed3..4f1aba9d4 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -220,10 +220,11 @@ class ConfPy: text = '\n'.join(lines) self._file.write_text(text, 'utf-8', ensure=True) - def read(self): + def read(self, error=False): """Read the config.py via configfiles and check for errors.""" api = configfiles.read_config_py(self.filename) - assert not api.errors + assert len(api.errors) == (1 if error else 0) + return api def write_qbmodule(self): self.write('import qbmodule', @@ -262,10 +263,9 @@ class TestConfigPyModules: confpy.write_qbmodule() qbmodulepy.write('def run(config):', ' 1/0') - api = configfiles.read_config_py(confpy.filename) - - assert len(api.errors) == 1 + api = confpy.read(error=True) error = api.errors[0] + assert error.text == "Unhandled exception" assert isinstance(error.exception, ZeroDivisionError) assert "qbmodule" not in sys.modules.keys() @@ -276,10 +276,10 @@ class TestConfigPyModules: ' pass') confpy.write('import foobar', 'foobar.run(config)') - api = configfiles.read_config_py(confpy.filename) - assert len(api.errors) == 1 + api = confpy.read(error=True) error = api.errors[0] + assert error.text == "Unhandled exception" assert isinstance(error.exception, ImportError) @@ -306,6 +306,13 @@ class TestConfigPy: def confpy(self, tmpdir): return ConfPy(tmpdir) + def test_assertions(self, confpy): + """Make sure assertions in config.py work for these tests.""" + confpy.write('assert False') + api = confpy.read(error=True) + error = api.errors[0] + assert isinstance(error.exception, AssertionError) + @pytest.mark.parametrize('line', [ 'c.colors.hints.bg = "red"', 'config.set("colors.hints.bg", "red")', @@ -321,25 +328,15 @@ class TestConfigPy: 'config.get("colors.hints.fg")', ]) def test_get(self, confpy, set_first, get_line): - """Test whether getting options works correctly. - - We test this by doing the following: - - Set colors.hints.fg to some value (inside the config.py with - set_first, outside of it otherwise). - - In the config.py, read .fg and set .bg to the same value. - - Verify that .bg has been set correctly. - """ + """Test whether getting options works correctly.""" # pylint: disable=bad-config-option config.val.colors.hints.fg = 'green' if set_first: confpy.write('c.colors.hints.fg = "red"', - 'c.colors.hints.bg = {}'.format(get_line)) - expected = 'red' + 'assert {} == "red"'.format(get_line)) else: - confpy.write('c.colors.hints.bg = {}'.format(get_line)) - expected = 'green' + confpy.write('assert {} == "green"'.format(get_line)) confpy.read() - assert config.instance._values['colors.hints.bg'] == expected @pytest.mark.parametrize('line, mode', [ ('config.bind(",a", "message-info foo")', 'normal'), @@ -430,12 +427,11 @@ class TestConfigPy: def test_unhandled_exception(self, confpy): confpy.write("config.load_autoconfig = False", "1/0") - api = configfiles.read_config_py(confpy.filename) + + api = confpy.read(error=True) + error = api.errors[0] assert not api.load_autoconfig - - assert len(api.errors) == 1 - error = api.errors[0] assert error.text == "Unhandled exception" assert isinstance(error.exception, ZeroDivisionError) @@ -447,9 +443,9 @@ class TestConfigPy: def test_config_val(self, confpy): """Using config.val should not work in config.py files.""" confpy.write("config.val.colors.hints.bg = 'red'") - api = configfiles.read_config_py(confpy.filename) - assert len(api.errors) == 1 + api = confpy.read(error=True) error = api.errors[0] + assert error.text == "Unhandled exception" assert isinstance(error.exception, AttributeError) message = "'ConfigAPI' object has no attribute 'val'" @@ -458,12 +454,10 @@ class TestConfigPy: @pytest.mark.parametrize('line', ["c.foo = 42", "config.set('foo', 42)"]) def test_config_error(self, confpy, line): confpy.write(line, "config.load_autoconfig = False") - api = configfiles.read_config_py(confpy.filename) + api = confpy.read(error=True) + error = api.errors[0] assert not api.load_autoconfig - - assert len(api.errors) == 1 - error = api.errors[0] assert error.text == "While setting 'foo'" assert isinstance(error.exception, configexc.NoOptionError) assert str(error.exception) == "No option 'foo'" From ebf378a945f99eec7e07e96e6f593c1e16949b87 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 08:58:41 +0200 Subject: [PATCH 47/77] Add docs about importing modules in config.py --- doc/help/configuring.asciidoc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/help/configuring.asciidoc b/doc/help/configuring.asciidoc index dd2612045..3425fc924 100644 --- a/doc/help/configuring.asciidoc +++ b/doc/help/configuring.asciidoc @@ -211,6 +211,20 @@ doing: config.load_autoconfig = False ---- +Importing other modules +~~~~~~~~~~~~~~~~~~~~~~~ + +You can import any module from the +https://docs.python.org/3/library/index.html[Python standard library] (e.g. +`import os.path`), as well as any module installed in the environment +qutebrowser is run with. + +If you have an `utils.py` file in your qutebrowser config folder, you can import +that via `import utils` as well. + +While it's in some cases possible to import code from the qutebrowser +installation, doing so is unsupported and discouraged. + Handling errors ~~~~~~~~~~~~~~~ From 9b22480b07e50bd3138ee3fe6f858448659151ee Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 09:09:45 +0200 Subject: [PATCH 48/77] Raise config.py errors happening in tests --- qutebrowser/config/configfiles.py | 11 +++++++++-- tests/unit/config/test_configfiles.py | 7 +++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 2ee973730..55c6d7e3a 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -214,8 +214,13 @@ class ConfigAPI: self._keyconfig.unbind(key, mode=mode) -def read_config_py(filename=None): - """Read a config.py file.""" +def read_config_py(filename=None, raising=False): + """Read a config.py file. + + Arguments; + raising: Raise exceptions happening in config.py. + This is needed during tests to use pytest's inspection. + """ api = ConfigAPI(config.instance, config.key_instance) if filename is None: @@ -261,6 +266,8 @@ def read_config_py(filename=None): exec(code, module.__dict__) except Exception as e: + if raising: + raise api.errors.append(configexc.ConfigErrorDesc( "Unhandled exception", exception=e, traceback=traceback.format_exc())) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 4f1aba9d4..5661c3ff9 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -222,7 +222,7 @@ class ConfPy: def read(self, error=False): """Read the config.py via configfiles and check for errors.""" - api = configfiles.read_config_py(self.filename) + api = configfiles.read_config_py(self.filename, raising=not error) assert len(api.errors) == (1 if error else 0) return api @@ -309,9 +309,8 @@ class TestConfigPy: def test_assertions(self, confpy): """Make sure assertions in config.py work for these tests.""" confpy.write('assert False') - api = confpy.read(error=True) - error = api.errors[0] - assert isinstance(error.exception, AssertionError) + with pytest.raises(AssertionError): + confpy.read() # no errors=True so it gets raised @pytest.mark.parametrize('line', [ 'c.colors.hints.bg = "red"', From 7f8ae531aa43350c22b19d1389bac014f4fb3a28 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 09:57:06 +0200 Subject: [PATCH 49/77] Add config.configdir and config.datadir to config API. Fixes #1419 --- doc/help/configuring.asciidoc | 17 +++++++++++++++++ qutebrowser/config/configfiles.py | 5 +++++ tests/unit/config/test_configfiles.py | 19 ++++++++++++++----- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/doc/help/configuring.asciidoc b/doc/help/configuring.asciidoc index 3425fc924..23368a5ff 100644 --- a/doc/help/configuring.asciidoc +++ b/doc/help/configuring.asciidoc @@ -225,6 +225,23 @@ that via `import utils` as well. While it's in some cases possible to import code from the qutebrowser installation, doing so is unsupported and discouraged. +Getting the config directory +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +If you need to get the qutebrowser config directory, you can do so by reading +`config.configdir`. Similarily, you can get the qutebrowser data directory via +`config.datadir`. + +This gives you a https://docs.python.org/3/library/pathlib.html[`pathlib.Path` +object], on which you can use `/` to add more directory parts, or `str(...)` to +get a string: + +.config.py: +[source,python] +---- +print(str(config.configdir / 'config.py') +---- + Handling errors ~~~~~~~~~~~~~~~ diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 55c6d7e3a..622280ee7 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -19,6 +19,7 @@ """Configuration files residing on disk.""" +import pathlib import types import os.path import sys @@ -177,6 +178,8 @@ class ConfigAPI: _keyconfig: The KeyConfig object. load_autoconfig: Whether autoconfig.yml should be loaded. errors: Errors which occurred while setting options. + configdir: The qutebrowser config directory, as pathlib.Path. + datadir: The qutebrowser data directory, as pathlib.Path. """ def __init__(self, conf, keyconfig): @@ -184,6 +187,8 @@ class ConfigAPI: self._keyconfig = keyconfig self.load_autoconfig = True self.errors = [] + self.configdir = pathlib.Path(standarddir.config()) + self.datadir = pathlib.Path(standarddir.data()) @contextlib.contextmanager def _handle_error(self, action, name): diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 5661c3ff9..ed5d2b882 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -236,7 +236,7 @@ class TestConfigPyModules: pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') @pytest.fixture - def confpy(self, tmpdir): + def confpy(self, tmpdir, config_tmpdir, data_tmpdir): return ConfPy(tmpdir) @pytest.fixture @@ -303,7 +303,7 @@ class TestConfigPy: pytestmark = pytest.mark.usefixtures('config_stub', 'key_config_stub') @pytest.fixture - def confpy(self, tmpdir): + def confpy(self, tmpdir, config_tmpdir, data_tmpdir): return ConfPy(tmpdir) def test_assertions(self, confpy): @@ -312,6 +312,14 @@ class TestConfigPy: with pytest.raises(AssertionError): confpy.read() # no errors=True so it gets raised + @pytest.mark.parametrize('what', ['configdir', 'datadir']) + def test_getting_dirs(self, confpy, what): + confpy.write('import pathlib', + 'directory = config.{}'.format(what), + 'assert isinstance(directory, pathlib.Path)', + 'assert directory.exists()') + confpy.read() + @pytest.mark.parametrize('line', [ 'c.colors.hints.bg = "red"', 'config.set("colors.hints.bg", "red")', @@ -373,17 +381,18 @@ class TestConfigPy: assert config.instance._values['aliases']['foo'] == 'message-info foo' assert config.instance._values['aliases']['bar'] == 'message-info bar' - def test_reading_default_location(self, config_tmpdir): + def test_reading_default_location(self, config_tmpdir, data_tmpdir): (config_tmpdir / 'config.py').write_text( 'c.colors.hints.bg = "red"', 'utf-8') configfiles.read_config_py() assert config.instance._values['colors.hints.bg'] == 'red' - def test_reading_missing_default_location(self, config_tmpdir): + def test_reading_missing_default_location(self, config_tmpdir, + data_tmpdir): assert not (config_tmpdir / 'config.py').exists() configfiles.read_config_py() # Should not crash - def test_oserror(self, tmpdir): + def test_oserror(self, tmpdir, data_tmpdir, config_tmpdir): with pytest.raises(configexc.ConfigFileErrors) as excinfo: configfiles.read_config_py(str(tmpdir / 'foo')) From 43ab27634fd4c993ceb2923c26de71e0ae1367d4 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 11:07:54 +0200 Subject: [PATCH 50/77] Fix vulture --- scripts/dev/run_vulture.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/dev/run_vulture.py b/scripts/dev/run_vulture.py index 3e88da43b..ab93ab842 100755 --- a/scripts/dev/run_vulture.py +++ b/scripts/dev/run_vulture.py @@ -108,6 +108,8 @@ def whitelist_generator(): yield 'qutebrowser.config.configexc.ConfigErrorDesc.traceback' yield 'qutebrowser.config.configfiles.ConfigAPI.load_autoconfig' yield 'types.ModuleType.c' # configfiles:read_config_py + for name in ['configdir', 'datadir']: + yield 'qutebrowser.config.configfiles.ConfigAPI.' + name yield 'include_aliases' From d9a32684054b8c5f7b7cf7bd38294d655ed76744 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 11:33:42 +0200 Subject: [PATCH 51/77] Explain relationship between 'c' and 'config.set' better [ci skip] --- doc/help/configuring.asciidoc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/help/configuring.asciidoc b/doc/help/configuring.asciidoc index 23368a5ff..38bc8ede7 100644 --- a/doc/help/configuring.asciidoc +++ b/doc/help/configuring.asciidoc @@ -88,7 +88,9 @@ Two global objects are pre-defined when running `config.py`: `c` and `config`. Changing settings ~~~~~~~~~~~~~~~~~ -`c` is a shorthand object to easily set settings like this: +While you can set settings using the `config.set()` method (which is explained +in the next section), it's easier to use the `c` shorthand object to easily set +settings like this: .config.py: [source,python] @@ -136,6 +138,8 @@ If you want to set settings based on their name as a string, use the .config.py: [source,python] ---- +# Equivalent to: +# c.content.javascript.enabled = False config.set('content.javascript.enabled', False) ---- @@ -143,6 +147,8 @@ To read a setting, use the `config.get` method: [source,python] ---- +# Equivalent to: +# color = c.colors.completion.fg color = config.get('colors.completion.fg') ---- From 501764d1cbbf09058234bf0f0a94d6c863fb8500 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 13:18:27 +0200 Subject: [PATCH 52/77] Fix documented default values for falsey non-strings Fixes #3015. --- doc/help/settings.asciidoc | 70 +++++++++++++-------------- qutebrowser/config/configtypes.py | 5 +- tests/unit/config/test_configtypes.py | 6 ++- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 3fdb32e3c..47524daa3 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -284,7 +284,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[backend]] === backend @@ -1342,7 +1342,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[completion.timestamp_format]] === completion.timestamp_format @@ -1402,7 +1402,7 @@ For more information about the feature, please refer to: http://webkit.org/blog/ Type: <> -Default: empty +Default: +pass:[0]+ This setting is only available with the QtWebKit backend. @@ -1466,7 +1466,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ This setting is only available with the QtWebKit backend. @@ -1497,7 +1497,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ This setting is only available with the QtWebKit backend. @@ -1628,7 +1628,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[content.images]] === content.images @@ -1668,7 +1668,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[content.javascript.can_close_tabs]] === content.javascript.can_close_tabs @@ -1681,7 +1681,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ This setting is only available with the QtWebKit backend. @@ -1696,7 +1696,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[content.javascript.enabled]] === content.javascript.enabled @@ -1737,7 +1737,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[content.javascript.prompt]] === content.javascript.prompt @@ -1776,7 +1776,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[content.local_storage]] === content.local_storage @@ -1842,7 +1842,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ This setting is only available with the QtWebKit backend. @@ -1857,7 +1857,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[content.print_element_backgrounds]] === content.print_element_backgrounds @@ -1885,7 +1885,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[content.proxy]] === content.proxy @@ -1963,7 +1963,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[downloads.location.directory]] === downloads.location.directory @@ -2243,7 +2243,7 @@ The hard minimum font size. Type: <> -Default: empty +Default: +pass:[0]+ [[fonts.web.size.minimum_logical]] === fonts.web.size.minimum_logical @@ -2274,7 +2274,7 @@ A timeout (in milliseconds) to ignore normal-mode key bindings after a successfu Type: <> -Default: empty +Default: +pass:[0]+ [[hints.border]] === hints.border @@ -2404,7 +2404,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[history_gap_interval]] === history_gap_interval @@ -2467,7 +2467,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[input.insert_mode.plugins]] === input.insert_mode.plugins @@ -2480,7 +2480,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[input.links_included_in_focus_chain]] === input.links_included_in_focus_chain @@ -2516,7 +2516,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[input.spatial_navigation]] === input.spatial_navigation @@ -2530,7 +2530,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[keyhint.blacklist]] === keyhint.blacklist @@ -2569,7 +2569,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[new_instance_open_target]] === new_instance_open_target @@ -2646,7 +2646,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[scrolling.smooth]] === scrolling.smooth @@ -2660,7 +2660,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[session_default_name]] === session_default_name @@ -2682,7 +2682,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[statusbar.padding]] === statusbar.padding @@ -2693,8 +2693,8 @@ Type: <> Default: - +pass:[bottom]+: +pass:[1]+ -- +pass:[left]+: empty -- +pass:[right]+: empty +- +pass:[left]+: +pass:[0]+ +- +pass:[right]+: +pass:[0]+ - +pass:[top]+: +pass:[1]+ [[statusbar.position]] @@ -2721,7 +2721,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[tabs.close_mouse_button]] === tabs.close_mouse_button @@ -2768,7 +2768,7 @@ Type: <> Default: - +pass:[bottom]+: +pass:[2]+ -- +pass:[left]+: empty +- +pass:[left]+: +pass:[0]+ - +pass:[right]+: +pass:[4]+ - +pass:[top]+: +pass:[2]+ @@ -2839,10 +2839,10 @@ Type: <> Default: -- +pass:[bottom]+: empty +- +pass:[bottom]+: +pass:[0]+ - +pass:[left]+: +pass:[5]+ - +pass:[right]+: +pass:[5]+ -- +pass:[top]+: empty +- +pass:[top]+: +pass:[0]+ [[tabs.position]] === tabs.position @@ -2907,7 +2907,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[tabs.title.alignment]] === tabs.title.alignment @@ -3078,7 +3078,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ [[window.title_format]] === window.title_format @@ -3152,7 +3152,7 @@ Valid values: * +true+ * +false+ -Default: empty +Default: +pass:[false]+ This setting is only available with the QtWebKit backend. diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index 76fc61a8b..154925734 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -257,9 +257,10 @@ class BaseType: This currently uses asciidoc syntax. """ utils.unused(indent) # only needed for Dict/List - if not value: + str_value = self.to_str(value) + if str_value == '': return 'empty' - return '+pass:[{}]+'.format(html.escape(self.to_str(value))) + return '+pass:[{}]+'.format(html.escape(str_value)) def complete(self): """Return a list of possible values for completion. diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index daf785d9c..55a321307 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -718,8 +718,10 @@ class TestBool: def test_to_str(self, klass, val, expected): assert klass().to_str(val) == expected - def test_to_doc(self, klass): - assert klass().to_doc(True) == '+pass:[true]+' + @pytest.mark.parametrize('value, expected', [(True, '+pass:[true]+'), + (False, '+pass:[false]+')]) + def test_to_doc(self, klass, value, expected): + assert klass().to_doc(value) == expected class TestBoolAsk: From 69d19e49df83355872b918293527067efd1ecc34 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 13:20:18 +0200 Subject: [PATCH 53/77] Fix flake8 --- scripts/dev/run_vulture.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/dev/run_vulture.py b/scripts/dev/run_vulture.py index ab93ab842..1c0b2224b 100755 --- a/scripts/dev/run_vulture.py +++ b/scripts/dev/run_vulture.py @@ -41,7 +41,7 @@ from qutebrowser.browser import qutescheme from qutebrowser.config import configtypes -def whitelist_generator(): +def whitelist_generator(): # noqa """Generator which yields lines to add to a vulture whitelist.""" # qutebrowser commands for cmd in cmdutils.cmd_dict.values(): From d1a4a028cd1d1b2ec8c6e87f4b80e31799170d8e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 13:24:26 +0200 Subject: [PATCH 54/77] Use more idiomatic comparison --- qutebrowser/config/configtypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index 154925734..32b1fc872 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -258,7 +258,7 @@ class BaseType: """ utils.unused(indent) # only needed for Dict/List str_value = self.to_str(value) - if str_value == '': + if not str_value: return 'empty' return '+pass:[{}]+'.format(html.escape(str_value)) From d5a1fff637c5261443562e2078707489f93d7f2b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 14:08:06 +0200 Subject: [PATCH 55/77] Move init stuff from config.py to configinit.py Closes #2997 --- qutebrowser/app.py | 9 +- qutebrowser/config/config.py | 124 +------------- qutebrowser/config/configinit.py | 144 +++++++++++++++++ tests/unit/config/test_config.py | 216 +------------------------ tests/unit/config/test_configinit.py | 234 +++++++++++++++++++++++++++ 5 files changed, 391 insertions(+), 336 deletions(-) create mode 100644 qutebrowser/config/configinit.py create mode 100644 tests/unit/config/test_configinit.py diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 5bca765eb..1f938c57e 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -43,7 +43,8 @@ import qutebrowser import qutebrowser.resources from qutebrowser.completion.models import miscmodels from qutebrowser.commands import cmdutils, runners, cmdexc -from qutebrowser.config import config, websettings, configexc, configfiles +from qutebrowser.config import (config, websettings, configexc, configfiles, + configinit) from qutebrowser.browser import (urlmarks, adblock, history, browsertab, downloads) from qutebrowser.browser.network import proxy @@ -77,7 +78,7 @@ def run(args): standarddir.init(args) log.init.debug("Initializing config...") - config.early_init(args) + configinit.early_init(args) global qApp qApp = Application(args) @@ -393,7 +394,7 @@ def _init_modules(args, crash_handler): log.init.debug("Initializing save manager...") save_manager = savemanager.SaveManager(qApp) objreg.register('save-manager', save_manager) - config.late_init(save_manager) + configinit.late_init(save_manager) log.init.debug("Initializing network...") networkmanager.init() @@ -762,7 +763,7 @@ class Application(QApplication): """ self._last_focus_object = None - qt_args = config.qt_args(args) + qt_args = configinit.qt_args(args) log.init.debug("Qt arguments: {}, based on {}".format(qt_args, args)) super().__init__(qt_args) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index e99498947..0365eb049 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -19,18 +19,15 @@ """Configuration storage and config-related utilities.""" -import sys import copy import contextlib import functools from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QUrl -from PyQt5.QtWidgets import QMessageBox from qutebrowser.config import configdata, configexc, configtypes, configfiles -from qutebrowser.utils import (utils, objreg, message, log, usertypes, jinja, - qtutils) -from qutebrowser.misc import objects, msgbox, earlyinit +from qutebrowser.utils import utils, objreg, message, log, jinja, qtutils +from qutebrowser.misc import objects from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.completion.models import configmodel @@ -40,9 +37,7 @@ instance = None key_instance = None # Keeping track of all change filters to validate them later. -_change_filters = [] -# Errors which happened during init, so we can show a message box. -_init_errors = [] +change_filters = [] class change_filter: # pylint: disable=invalid-name @@ -68,7 +63,7 @@ class change_filter: # pylint: disable=invalid-name """ self._option = option self._function = function - _change_filters.append(self) + change_filters.append(self) def validate(self): """Make sure the configured option or prefix exists. @@ -634,114 +629,3 @@ class StyleSheetObserver(QObject): self._obj.setStyleSheet(qss) if update: instance.changed.connect(self._update_stylesheet) - - -def early_init(args): - """Initialize the part of the config which works without a QApplication.""" - configdata.init() - - yaml_config = configfiles.YamlConfig() - - global val, instance, key_instance - instance = Config(yaml_config=yaml_config) - val = ConfigContainer(instance) - key_instance = KeyConfig(instance) - - for cf in _change_filters: - cf.validate() - - configtypes.Font.monospace_fonts = val.fonts.monospace - - config_commands = ConfigCommands(instance, key_instance) - objreg.register('config-commands', config_commands) - - config_api = None - - try: - config_api = configfiles.read_config_py() - # Raised here so we get the config_api back. - if config_api.errors: - raise configexc.ConfigFileErrors('config.py', config_api.errors) - except configexc.ConfigFileErrors as e: - log.config.exception("Error while loading config.py") - _init_errors.append(e) - - try: - if getattr(config_api, 'load_autoconfig', True): - try: - instance.read_yaml() - except configexc.ConfigFileErrors as e: - raise # caught in outer block - except configexc.Error as e: - desc = configexc.ConfigErrorDesc("Error", e) - raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) - except configexc.ConfigFileErrors as e: - log.config.exception("Error while loading config.py") - _init_errors.append(e) - - configfiles.init() - - objects.backend = get_backend(args) - earlyinit.init_with_backend(objects.backend) - - -def get_backend(args): - """Find out what backend to use based on available libraries.""" - try: - import PyQt5.QtWebKit # pylint: disable=unused-variable - except ImportError: - webkit_available = False - else: - webkit_available = qtutils.is_new_qtwebkit() - - str_to_backend = { - 'webkit': usertypes.Backend.QtWebKit, - 'webengine': usertypes.Backend.QtWebEngine, - } - - if args.backend is not None: - return str_to_backend[args.backend] - elif val.backend != 'auto': - return str_to_backend[val.backend] - elif webkit_available: - return usertypes.Backend.QtWebKit - else: - return usertypes.Backend.QtWebEngine - - -def late_init(save_manager): - """Initialize the rest of the config after the QApplication is created.""" - global _init_errors - for err in _init_errors: - errbox = msgbox.msgbox(parent=None, - title="Error while reading config", - text=err.to_html(), - icon=QMessageBox.Warning, - plain_text=False) - errbox.exec_() - _init_errors = [] - - instance.init_save_manager(save_manager) - configfiles.state.init_save_manager(save_manager) - - -def qt_args(namespace): - """Get the Qt QApplication arguments based on an argparse namespace. - - Args: - namespace: The argparse namespace. - - Return: - The argv list to be passed to Qt. - """ - argv = [sys.argv[0]] - - if namespace.qt_flag is not None: - argv += ['--' + flag[0] for flag in namespace.qt_flag] - - if namespace.qt_arg is not None: - for name, value in namespace.qt_arg: - argv += ['--' + name, value] - - argv += ['--' + arg for arg in val.qt_args] - return argv diff --git a/qutebrowser/config/configinit.py b/qutebrowser/config/configinit.py new file mode 100644 index 000000000..27a37b5e9 --- /dev/null +++ b/qutebrowser/config/configinit.py @@ -0,0 +1,144 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2017 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + +"""Initialization of the configuration.""" + +import sys + +from PyQt5.QtWidgets import QMessageBox + +from qutebrowser.config import (config, configdata, configfiles, configtypes, + configexc) +from qutebrowser.utils import objreg, qtutils, usertypes, log +from qutebrowser.misc import earlyinit, msgbox, objects + + +# Errors which happened during init, so we can show a message box. +_init_errors = [] + + +def early_init(args): + """Initialize the part of the config which works without a QApplication.""" + configdata.init() + + yaml_config = configfiles.YamlConfig() + + config.instance = config.Config(yaml_config=yaml_config) + config.val = config.ConfigContainer(config.instance) + config.key_instance = config.KeyConfig(config.instance) + + for cf in config.change_filters: + cf.validate() + + configtypes.Font.monospace_fonts = config.val.fonts.monospace + + config_commands = config.ConfigCommands(config.instance, + config.key_instance) + objreg.register('config-commands', config_commands) + + config_api = None + + try: + config_api = configfiles.read_config_py() + # Raised here so we get the config_api back. + if config_api.errors: + raise configexc.ConfigFileErrors('config.py', config_api.errors) + except configexc.ConfigFileErrors as e: + log.config.exception("Error while loading config.py") + _init_errors.append(e) + + try: + if getattr(config_api, 'load_autoconfig', True): + try: + config.instance.read_yaml() + except configexc.ConfigFileErrors as e: + raise # caught in outer block + except configexc.Error as e: + desc = configexc.ConfigErrorDesc("Error", e) + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) + except configexc.ConfigFileErrors as e: + log.config.exception("Error while loading config.py") + _init_errors.append(e) + + configfiles.init() + + objects.backend = get_backend(args) + earlyinit.init_with_backend(objects.backend) + + +def get_backend(args): + """Find out what backend to use based on available libraries.""" + try: + import PyQt5.QtWebKit # pylint: disable=unused-variable + except ImportError: + webkit_available = False + else: + webkit_available = qtutils.is_new_qtwebkit() + + str_to_backend = { + 'webkit': usertypes.Backend.QtWebKit, + 'webengine': usertypes.Backend.QtWebEngine, + } + + if args.backend is not None: + return str_to_backend[args.backend] + elif config.val.backend != 'auto': + return str_to_backend[config.val.backend] + elif webkit_available: + return usertypes.Backend.QtWebKit + else: + return usertypes.Backend.QtWebEngine + + +def late_init(save_manager): + """Initialize the rest of the config after the QApplication is created.""" + global _init_errors + for err in _init_errors: + errbox = msgbox.msgbox(parent=None, + title="Error while reading config", + text=err.to_html(), + icon=QMessageBox.Warning, + plain_text=False) + errbox.exec_() + _init_errors = [] + + config.instance.init_save_manager(save_manager) + configfiles.state.init_save_manager(save_manager) + + +def qt_args(namespace): + """Get the Qt QApplication arguments based on an argparse namespace. + + Args: + namespace: The argparse namespace. + + Return: + The argv list to be passed to Qt. + """ + argv = [sys.argv[0]] + + if namespace.qt_flag is not None: + argv += ['--' + flag[0] for flag in namespace.qt_flag] + + if namespace.qt_arg is not None: + for name, value in namespace.qt_arg: + argv += ['--' + name, value] + + argv += ['--' + arg for arg in config.val.qt_args] + return argv diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 4e8bd684b..24c6814b5 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -18,19 +18,15 @@ """Tests for qutebrowser.config.config.""" -import sys import copy import types -import logging -import unittest.mock import pytest from PyQt5.QtCore import QObject, QUrl from PyQt5.QtGui import QColor -from qutebrowser import qutebrowser from qutebrowser.commands import cmdexc -from qutebrowser.config import config, configdata, configexc, configfiles +from qutebrowser.config import config, configdata, configexc from qutebrowser.utils import objreg, usertypes from qutebrowser.misc import objects @@ -52,8 +48,8 @@ class TestChangeFilter: @pytest.fixture(autouse=True) def cleanup_globals(self, monkeypatch): - """Make sure config._change_filters is cleaned up.""" - monkeypatch.setattr(config, '_change_filters', []) + """Make sure config.change_filters is cleaned up.""" + monkeypatch.setattr(config, 'change_filters', []) @pytest.mark.parametrize('option', ['foobar', 'tab', 'tabss', 'tabs.']) def test_unknown_option(self, option): @@ -65,7 +61,7 @@ class TestChangeFilter: def test_validate(self, option): cf = config.change_filter(option) cf.validate() - assert cf in config._change_filters + assert cf in config.change_filters @pytest.mark.parametrize('method', [True, False]) @pytest.mark.parametrize('option, changed, matches', [ @@ -864,207 +860,3 @@ def test_set_register_stylesheet(delete, stylesheet_param, update, qtbot, expected = 'yellow' assert obj.rendered_stylesheet == expected - - -@pytest.fixture -def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, - data_tmpdir): - monkeypatch.setattr(configdata, 'DATA', None) - monkeypatch.setattr(configfiles, 'state', None) - monkeypatch.setattr(config, 'instance', None) - monkeypatch.setattr(config, 'key_instance', None) - monkeypatch.setattr(config, '_change_filters', []) - monkeypatch.setattr(config, '_init_errors', []) - # Make sure we get no SSL warning - monkeypatch.setattr(config.earlyinit, 'check_backend_ssl_support', - lambda _backend: None) - yield - try: - objreg.delete('config-commands') - except KeyError: - pass - - -@pytest.mark.parametrize('load_autoconfig', [True, False]) # noqa -@pytest.mark.parametrize('config_py', [True, 'error', False]) -@pytest.mark.parametrize('invalid_yaml', - ['42', 'unknown', 'wrong-type', False]) -# pylint: disable=too-many-branches -def test_early_init(init_patch, config_tmpdir, caplog, fake_args, - load_autoconfig, config_py, invalid_yaml): - # Prepare files - autoconfig_file = config_tmpdir / 'autoconfig.yml' - config_py_file = config_tmpdir / 'config.py' - - if invalid_yaml == '42': - text = '42' - elif invalid_yaml == 'unknown': - text = 'global:\n colors.foobar: magenta\n' - elif invalid_yaml == 'wrong-type': - text = 'global:\n tabs.position: true\n' - else: - assert not invalid_yaml - text = 'global:\n colors.hints.fg: magenta\n' - autoconfig_file.write_text(text, 'utf-8', ensure=True) - - if config_py: - config_py_lines = ['c.colors.hints.bg = "red"'] - if not load_autoconfig: - config_py_lines.append('config.load_autoconfig = False') - if config_py == 'error': - config_py_lines.append('c.foo = 42') - config_py_file.write_text('\n'.join(config_py_lines), - 'utf-8', ensure=True) - - with caplog.at_level(logging.ERROR): - config.early_init(fake_args) - - # Check error messages - expected_errors = [] - if config_py == 'error': - expected_errors.append( - "Errors occurred while reading config.py:\n" - " While setting 'foo': No option 'foo'") - if load_autoconfig or not config_py: - error = "Errors occurred while reading autoconfig.yml:\n" - if invalid_yaml == '42': - error += " While loading data: Toplevel object is not a dict" - expected_errors.append(error) - elif invalid_yaml == 'wrong-type': - error += (" Error: Invalid value 'True' - expected a value of " - "type str but got bool.") - expected_errors.append(error) - - actual_errors = [str(err) for err in config._init_errors] - assert actual_errors == expected_errors - - # Make sure things have been init'ed - objreg.get('config-commands') - assert isinstance(config.instance, config.Config) - assert isinstance(config.key_instance, config.KeyConfig) - - # Check config values - if config_py and load_autoconfig and not invalid_yaml: - assert config.instance._values == { - 'colors.hints.bg': 'red', - 'colors.hints.fg': 'magenta', - } - elif config_py: - assert config.instance._values == {'colors.hints.bg': 'red'} - elif invalid_yaml: - assert config.instance._values == {} - else: - assert config.instance._values == {'colors.hints.fg': 'magenta'} - - -def test_early_init_invalid_change_filter(init_patch, fake_args): - config.change_filter('foobar') - with pytest.raises(configexc.NoOptionError): - config.early_init(fake_args) - - -@pytest.mark.parametrize('errors', [True, False]) -def test_late_init(init_patch, monkeypatch, fake_save_manager, fake_args, - mocker, errors): - config.early_init(fake_args) - if errors: - err = configexc.ConfigErrorDesc("Error text", Exception("Exception")) - errs = configexc.ConfigFileErrors("config.py", [err]) - monkeypatch.setattr(config, '_init_errors', [errs]) - msgbox_mock = mocker.patch('qutebrowser.config.config.msgbox.msgbox', - autospec=True) - - config.late_init(fake_save_manager) - - fake_save_manager.add_saveable.assert_any_call( - 'state-config', unittest.mock.ANY) - fake_save_manager.add_saveable.assert_any_call( - 'yaml-config', unittest.mock.ANY) - if errors: - assert len(msgbox_mock.call_args_list) == 1 - _call_posargs, call_kwargs = msgbox_mock.call_args_list[0] - text = call_kwargs['text'].strip() - assert text.startswith('Errors occurred while reading config.py:') - assert 'Error text: Exception' in text - else: - assert not msgbox_mock.called - - -class TestQtArgs: - - @pytest.fixture - def parser(self, mocker): - """Fixture to provide an argparser. - - Monkey-patches .exit() of the argparser so it doesn't exit on errors. - """ - parser = qutebrowser.get_argparser() - mocker.patch.object(parser, 'exit', side_effect=Exception) - return parser - - @pytest.mark.parametrize('args, expected', [ - # No Qt arguments - (['--debug'], [sys.argv[0]]), - # Qt flag - (['--debug', '--qt-flag', 'reverse'], [sys.argv[0], '--reverse']), - # Qt argument with value - (['--qt-arg', 'stylesheet', 'foo'], - [sys.argv[0], '--stylesheet', 'foo']), - # --qt-arg given twice - (['--qt-arg', 'stylesheet', 'foo', '--qt-arg', 'geometry', 'bar'], - [sys.argv[0], '--stylesheet', 'foo', '--geometry', 'bar']), - # --qt-flag given twice - (['--qt-flag', 'foo', '--qt-flag', 'bar'], - [sys.argv[0], '--foo', '--bar']), - ]) - def test_qt_args(self, config_stub, args, expected, parser): - """Test commandline with no Qt arguments given.""" - parsed = parser.parse_args(args) - assert config.qt_args(parsed) == expected - - def test_qt_both(self, config_stub, parser): - """Test commandline with a Qt argument and flag.""" - args = parser.parse_args(['--qt-arg', 'stylesheet', 'foobar', - '--qt-flag', 'reverse']) - qt_args = config.qt_args(args) - assert qt_args[0] == sys.argv[0] - assert '--reverse' in qt_args - assert '--stylesheet' in qt_args - assert 'foobar' in qt_args - - def test_with_settings(self, config_stub, parser): - parsed = parser.parse_args(['--qt-flag', 'foo']) - config_stub.val.qt_args = ['bar'] - assert config.qt_args(parsed) == [sys.argv[0], '--foo', '--bar'] - - -@pytest.mark.parametrize('arg, confval, can_import, is_new_webkit, used', [ - # overridden by commandline arg - ('webkit', 'auto', False, False, usertypes.Backend.QtWebKit), - # overridden by config - (None, 'webkit', False, False, usertypes.Backend.QtWebKit), - # WebKit available but too old - (None, 'auto', True, False, usertypes.Backend.QtWebEngine), - # WebKit available and new - (None, 'auto', True, True, usertypes.Backend.QtWebKit), - # WebKit unavailable - (None, 'auto', False, False, usertypes.Backend.QtWebEngine), -]) -def test_get_backend(monkeypatch, fake_args, config_stub, - arg, confval, can_import, is_new_webkit, used): - real_import = __import__ - - def fake_import(name, *args, **kwargs): - if name != 'PyQt5.QtWebKit': - return real_import(name, *args, **kwargs) - if can_import: - return None - raise ImportError - - fake_args.backend = arg - config_stub.val.backend = confval - monkeypatch.setattr(config.qtutils, 'is_new_qtwebkit', - lambda: is_new_webkit) - monkeypatch.setattr('builtins.__import__', fake_import) - - assert config.get_backend(fake_args) == used diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py new file mode 100644 index 000000000..c5108d95f --- /dev/null +++ b/tests/unit/config/test_configinit.py @@ -0,0 +1,234 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: +# Copyright 2017 Florian Bruhin (The Compiler) + +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + +"""Tests for qutebrowser.config.configinit.""" + +import sys +import logging +import unittest.mock + +import pytest + +from qutebrowser import qutebrowser +from qutebrowser.config import (config, configdata, configexc, configfiles, + configinit) +from qutebrowser.utils import objreg, usertypes + + +@pytest.fixture +def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, + data_tmpdir): + monkeypatch.setattr(configdata, 'DATA', None) + monkeypatch.setattr(configfiles, 'state', None) + monkeypatch.setattr(config, 'instance', None) + monkeypatch.setattr(config, 'key_instance', None) + monkeypatch.setattr(config, 'change_filters', []) + monkeypatch.setattr(configinit, '_init_errors', []) + # Make sure we get no SSL warning + monkeypatch.setattr(configinit.earlyinit, 'check_backend_ssl_support', + lambda _backend: None) + yield + try: + objreg.delete('config-commands') + except KeyError: + pass + + +@pytest.mark.parametrize('load_autoconfig', [True, False]) # noqa +@pytest.mark.parametrize('config_py', [True, 'error', False]) +@pytest.mark.parametrize('invalid_yaml', + ['42', 'unknown', 'wrong-type', False]) +# pylint: disable=too-many-branches +def test_early_init(init_patch, config_tmpdir, caplog, fake_args, + load_autoconfig, config_py, invalid_yaml): + # Prepare files + autoconfig_file = config_tmpdir / 'autoconfig.yml' + config_py_file = config_tmpdir / 'config.py' + + if invalid_yaml == '42': + text = '42' + elif invalid_yaml == 'unknown': + text = 'global:\n colors.foobar: magenta\n' + elif invalid_yaml == 'wrong-type': + text = 'global:\n tabs.position: true\n' + else: + assert not invalid_yaml + text = 'global:\n colors.hints.fg: magenta\n' + autoconfig_file.write_text(text, 'utf-8', ensure=True) + + if config_py: + config_py_lines = ['c.colors.hints.bg = "red"'] + if not load_autoconfig: + config_py_lines.append('config.load_autoconfig = False') + if config_py == 'error': + config_py_lines.append('c.foo = 42') + config_py_file.write_text('\n'.join(config_py_lines), + 'utf-8', ensure=True) + + with caplog.at_level(logging.ERROR): + configinit.early_init(fake_args) + + # Check error messages + expected_errors = [] + if config_py == 'error': + expected_errors.append( + "Errors occurred while reading config.py:\n" + " While setting 'foo': No option 'foo'") + if load_autoconfig or not config_py: + error = "Errors occurred while reading autoconfig.yml:\n" + if invalid_yaml == '42': + error += " While loading data: Toplevel object is not a dict" + expected_errors.append(error) + elif invalid_yaml == 'wrong-type': + error += (" Error: Invalid value 'True' - expected a value of " + "type str but got bool.") + expected_errors.append(error) + + actual_errors = [str(err) for err in configinit._init_errors] + assert actual_errors == expected_errors + + # Make sure things have been init'ed + objreg.get('config-commands') + assert isinstance(config.instance, config.Config) + assert isinstance(config.key_instance, config.KeyConfig) + + # Check config values + if config_py and load_autoconfig and not invalid_yaml: + assert config.instance._values == { + 'colors.hints.bg': 'red', + 'colors.hints.fg': 'magenta', + } + elif config_py: + assert config.instance._values == {'colors.hints.bg': 'red'} + elif invalid_yaml: + assert config.instance._values == {} + else: + assert config.instance._values == {'colors.hints.fg': 'magenta'} + + +def test_early_init_invalid_change_filter(init_patch, fake_args): + config.change_filter('foobar') + with pytest.raises(configexc.NoOptionError): + configinit.early_init(fake_args) + + +@pytest.mark.parametrize('errors', [True, False]) +def test_late_init(init_patch, monkeypatch, fake_save_manager, fake_args, + mocker, errors): + configinit.early_init(fake_args) + if errors: + err = configexc.ConfigErrorDesc("Error text", Exception("Exception")) + errs = configexc.ConfigFileErrors("config.py", [err]) + monkeypatch.setattr(configinit, '_init_errors', [errs]) + msgbox_mock = mocker.patch('qutebrowser.config.configinit.msgbox.msgbox', + autospec=True) + + configinit.late_init(fake_save_manager) + + fake_save_manager.add_saveable.assert_any_call( + 'state-config', unittest.mock.ANY) + fake_save_manager.add_saveable.assert_any_call( + 'yaml-config', unittest.mock.ANY) + if errors: + assert len(msgbox_mock.call_args_list) == 1 + _call_posargs, call_kwargs = msgbox_mock.call_args_list[0] + text = call_kwargs['text'].strip() + assert text.startswith('Errors occurred while reading config.py:') + assert 'Error text: Exception' in text + else: + assert not msgbox_mock.called + + +class TestQtArgs: + + @pytest.fixture + def parser(self, mocker): + """Fixture to provide an argparser. + + Monkey-patches .exit() of the argparser so it doesn't exit on errors. + """ + parser = qutebrowser.get_argparser() + mocker.patch.object(parser, 'exit', side_effect=Exception) + return parser + + @pytest.mark.parametrize('args, expected', [ + # No Qt arguments + (['--debug'], [sys.argv[0]]), + # Qt flag + (['--debug', '--qt-flag', 'reverse'], [sys.argv[0], '--reverse']), + # Qt argument with value + (['--qt-arg', 'stylesheet', 'foo'], + [sys.argv[0], '--stylesheet', 'foo']), + # --qt-arg given twice + (['--qt-arg', 'stylesheet', 'foo', '--qt-arg', 'geometry', 'bar'], + [sys.argv[0], '--stylesheet', 'foo', '--geometry', 'bar']), + # --qt-flag given twice + (['--qt-flag', 'foo', '--qt-flag', 'bar'], + [sys.argv[0], '--foo', '--bar']), + ]) + def test_qt_args(self, config_stub, args, expected, parser): + """Test commandline with no Qt arguments given.""" + parsed = parser.parse_args(args) + assert configinit.qt_args(parsed) == expected + + def test_qt_both(self, config_stub, parser): + """Test commandline with a Qt argument and flag.""" + args = parser.parse_args(['--qt-arg', 'stylesheet', 'foobar', + '--qt-flag', 'reverse']) + qt_args = configinit.qt_args(args) + assert qt_args[0] == sys.argv[0] + assert '--reverse' in qt_args + assert '--stylesheet' in qt_args + assert 'foobar' in qt_args + + def test_with_settings(self, config_stub, parser): + parsed = parser.parse_args(['--qt-flag', 'foo']) + config_stub.val.qt_args = ['bar'] + assert configinit.qt_args(parsed) == [sys.argv[0], '--foo', '--bar'] + + +@pytest.mark.parametrize('arg, confval, can_import, is_new_webkit, used', [ + # overridden by commandline arg + ('webkit', 'auto', False, False, usertypes.Backend.QtWebKit), + # overridden by config + (None, 'webkit', False, False, usertypes.Backend.QtWebKit), + # WebKit available but too old + (None, 'auto', True, False, usertypes.Backend.QtWebEngine), + # WebKit available and new + (None, 'auto', True, True, usertypes.Backend.QtWebKit), + # WebKit unavailable + (None, 'auto', False, False, usertypes.Backend.QtWebEngine), +]) +def test_get_backend(monkeypatch, fake_args, config_stub, + arg, confval, can_import, is_new_webkit, used): + real_import = __import__ + + def fake_import(name, *args, **kwargs): + if name != 'PyQt5.QtWebKit': + return real_import(name, *args, **kwargs) + if can_import: + return None + raise ImportError + + fake_args.backend = arg + config_stub.val.backend = confval + monkeypatch.setattr(config.qtutils, 'is_new_qtwebkit', + lambda: is_new_webkit) + monkeypatch.setattr('builtins.__import__', fake_import) + + assert configinit.get_backend(fake_args) == used From 7f4cba8bc23223dabb81d448187c61ff56a03a25 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 14:23:41 +0200 Subject: [PATCH 56/77] Improve load_autoconfig docs Closes #2993 --- doc/help/configuring.asciidoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/help/configuring.asciidoc b/doc/help/configuring.asciidoc index 38bc8ede7..b68e0bedc 100644 --- a/doc/help/configuring.asciidoc +++ b/doc/help/configuring.asciidoc @@ -217,6 +217,9 @@ doing: config.load_autoconfig = False ---- +Note that the settings are still saved in `autoconfig.yml` that way, but then +not loaded on start. + Importing other modules ~~~~~~~~~~~~~~~~~~~~~~~ From 1e2015be651c24c4e37ccb0621f5969233afb10f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 15:28:45 +0200 Subject: [PATCH 57/77] Make bindings win over mappings Fixes #2995 --- doc/help/settings.asciidoc | 1 + qutebrowser/config/configdata.yml | 3 ++ qutebrowser/keyinput/basekeyparser.py | 49 +++++++++++++---------- tests/unit/keyinput/conftest.py | 7 ++++ tests/unit/keyinput/test_basekeyparser.py | 41 ++++++++++++++----- 5 files changed, 69 insertions(+), 32 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 47524daa3..620e4467c 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -627,6 +627,7 @@ Default: This setting can be used to map keys to other keys. When the key used as dictionary-key is pressed, the binding for the key used as dictionary-value is invoked instead. This is useful for global remappings of keys, for example to map Ctrl-[ to Escape. +Note that when a key is bound (via `bindings.default` or `bindings.commands`), the mapping is ignored. Type: <> diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index f0e5ae74d..9f67689b0 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -1903,6 +1903,9 @@ bindings.key_mappings: This is useful for global remappings of keys, for example to map Ctrl-[ to Escape. + Note that when a key is bound (via `bindings.default` or + `bindings.commands`), the mapping is ignored. + bindings.default: default: normal: diff --git a/qutebrowser/keyinput/basekeyparser.py b/qutebrowser/keyinput/basekeyparser.py index 9eec4b28f..2f75cbdfe 100644 --- a/qutebrowser/keyinput/basekeyparser.py +++ b/qutebrowser/keyinput/basekeyparser.py @@ -122,37 +122,40 @@ class BaseKeyParser(QObject): self._debug_log("Ignoring only-modifier keyeevent.") return False - key_mappings = config.val.bindings.key_mappings - try: - binding = key_mappings['<{}>'.format(binding)][1:-1] - except KeyError: - pass + if binding not in self.special_bindings: + key_mappings = config.val.bindings.key_mappings + try: + binding = key_mappings['<{}>'.format(binding)][1:-1] + except KeyError: + pass try: cmdstr = self.special_bindings[binding] except KeyError: self._debug_log("No special binding found for {}.".format(binding)) return False - count, _command = self._split_count() + count, _command = self._split_count(self._keystring) self.execute(cmdstr, self.Type.special, count) self.clear_keystring() return True - def _split_count(self): + def _split_count(self, keystring): """Get count and command from the current keystring. + Args: + keystring: The key string to split. + Return: A (count, command) tuple. """ if self._supports_count: - (countstr, cmd_input) = re.match(r'^(\d*)(.*)', - self._keystring).groups() + (countstr, cmd_input) = re.match(r'^(\d*)(.*)', keystring).groups() count = int(countstr) if countstr else None if count == 0 and not cmd_input: - cmd_input = self._keystring + cmd_input = keystring count = None else: - cmd_input = self._keystring + cmd_input = keystring count = None return count, cmd_input @@ -183,18 +186,17 @@ class BaseKeyParser(QObject): self._debug_log("Ignoring, no text char") return self.Match.none - key_mappings = config.val.bindings.key_mappings - txt = key_mappings.get(txt, txt) - self._keystring += txt - - count, cmd_input = self._split_count() - - if not cmd_input: - # Only a count, no command yet, but we handled it - return self.Match.other - + count, cmd_input = self._split_count(self._keystring + txt) match, binding = self._match_key(cmd_input) + if match == self.Match.none: + mappings = config.val.bindings.key_mappings + mapped = mappings.get(txt, None) + if mapped is not None: + txt = mapped + count, cmd_input = self._split_count(self._keystring + txt) + match, binding = self._match_key(cmd_input) + self._keystring += txt if match == self.Match.definitive: self._debug_log("Definitive match for '{}'.".format( self._keystring)) @@ -207,6 +209,8 @@ class BaseKeyParser(QObject): self._debug_log("Giving up with '{}', no matches".format( self._keystring)) self.clear_keystring() + elif match == self.Match.other: + pass else: raise AssertionError("Invalid match value {!r}".format(match)) return match @@ -223,6 +227,9 @@ class BaseKeyParser(QObject): binding: - None with Match.partial/Match.none. - The found binding with Match.definitive. """ + if not cmd_input: + # Only a count, no command yet, but we handled it + return (self.Match.other, None) # A (cmd_input, binding) tuple (k, v of bindings) or None. definitive_match = None partial_match = False diff --git a/tests/unit/keyinput/conftest.py b/tests/unit/keyinput/conftest.py index bdae15272..684e5792e 100644 --- a/tests/unit/keyinput/conftest.py +++ b/tests/unit/keyinput/conftest.py @@ -31,6 +31,12 @@ BINDINGS = {'prompt': {'': 'message-info ctrla', 'command': {'foo': 'message-info bar', '': 'message-info ctrlx'}, 'normal': {'a': 'message-info a', 'ba': 'message-info ba'}} +MAPPINGS = { + '': 'a', + '': '', + 'x': 'a', + 'b': 'a', +} @pytest.fixture @@ -38,3 +44,4 @@ def keyinput_bindings(config_stub, key_config_stub): """Register some test bindings.""" config_stub.val.bindings.default = {} config_stub.val.bindings.commands = dict(BINDINGS) + config_stub.val.bindings.key_mappings = dict(MAPPINGS) diff --git a/tests/unit/keyinput/test_basekeyparser.py b/tests/unit/keyinput/test_basekeyparser.py index e7131f92c..c4ce838da 100644 --- a/tests/unit/keyinput/test_basekeyparser.py +++ b/tests/unit/keyinput/test_basekeyparser.py @@ -91,8 +91,7 @@ class TestDebugLog: ]) def test_split_count(config_stub, input_key, supports_count, expected): kp = basekeyparser.BaseKeyParser(0, supports_count=supports_count) - kp._keystring = input_key - assert kp._split_count() == expected + assert kp._split_count(input_key) == expected @pytest.mark.usefixtures('keyinput_bindings') @@ -165,20 +164,14 @@ class TestSpecialKeys: keyparser._read_config('prompt') def test_valid_key(self, fake_keyevent_factory, keyparser): - if utils.is_mac: - modifier = Qt.MetaModifier - else: - modifier = Qt.ControlModifier + modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier keyparser.handle(fake_keyevent_factory(Qt.Key_A, modifier)) keyparser.handle(fake_keyevent_factory(Qt.Key_X, modifier)) keyparser.execute.assert_called_once_with( 'message-info ctrla', keyparser.Type.special, None) def test_valid_key_count(self, fake_keyevent_factory, keyparser): - if utils.is_mac: - modifier = Qt.MetaModifier - else: - modifier = Qt.ControlModifier + modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier keyparser.handle(fake_keyevent_factory(5, text='5')) keyparser.handle(fake_keyevent_factory(Qt.Key_A, modifier, text='A')) keyparser.execute.assert_called_once_with( @@ -199,6 +192,22 @@ class TestSpecialKeys: keyparser.handle(fake_keyevent_factory(Qt.Key_A, Qt.NoModifier)) assert not keyparser.execute.called + def test_mapping(self, config_stub, fake_keyevent_factory, keyparser): + modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier + + keyparser.handle(fake_keyevent_factory(Qt.Key_B, modifier)) + keyparser.execute.assert_called_once_with( + 'message-info ctrla', keyparser.Type.special, None) + + def test_binding_and_mapping(self, config_stub, fake_keyevent_factory, + keyparser): + """with a conflicting binding/mapping, the binding should win.""" + modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier + + keyparser.handle(fake_keyevent_factory(Qt.Key_A, modifier)) + keyparser.execute.assert_called_once_with( + 'message-info ctrla', keyparser.Type.special, None) + class TestKeyChain: @@ -230,7 +239,7 @@ class TestKeyChain: handle_text((Qt.Key_X, 'x'), # Then start the real chain (Qt.Key_B, 'b'), (Qt.Key_A, 'a')) - keyparser.execute.assert_called_once_with( + keyparser.execute.assert_called_with( 'message-info ba', keyparser.Type.chain, None) assert keyparser._keystring == '' @@ -249,6 +258,16 @@ class TestKeyChain: handle_text((Qt.Key_C, 'c')) assert keyparser._keystring == '' + def test_mapping(self, config_stub, handle_text, keyparser): + handle_text((Qt.Key_X, 'x')) + keyparser.execute.assert_called_once_with( + 'message-info a', keyparser.Type.chain, None) + + def test_binding_and_mapping(self, config_stub, handle_text, keyparser): + """with a conflicting binding/mapping, the binding should win.""" + handle_text((Qt.Key_B, 'b')) + assert not keyparser.execute.called + class TestCount: From 5be44756e3632be40558ce93eeacc8d4d7dda466 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 17:13:45 +0200 Subject: [PATCH 58/77] Remove unused imports --- qutebrowser/config/config.py | 4 ++-- tests/unit/config/test_configinit.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 0365eb049..8d93a2344 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -25,8 +25,8 @@ import functools from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QUrl -from qutebrowser.config import configdata, configexc, configtypes, configfiles -from qutebrowser.utils import utils, objreg, message, log, jinja, qtutils +from qutebrowser.config import configdata, configexc, configtypes +from qutebrowser.utils import utils, objreg, message, log, jinja from qutebrowser.misc import objects from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.completion.models import configmodel diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index c5108d95f..fef352584 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -227,7 +227,7 @@ def test_get_backend(monkeypatch, fake_args, config_stub, fake_args.backend = arg config_stub.val.backend = confval - monkeypatch.setattr(config.qtutils, 'is_new_qtwebkit', + monkeypatch.setattr(configinit.qtutils, 'is_new_qtwebkit', lambda: is_new_webkit) monkeypatch.setattr('builtins.__import__', fake_import) From e27c54a5c1a0fa06a627df99e34fafb176d92fc7 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 19:49:52 +0200 Subject: [PATCH 59/77] Fix modeparser tests --- tests/unit/keyinput/test_modeparsers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/keyinput/test_modeparsers.py b/tests/unit/keyinput/test_modeparsers.py index 5010b8efa..4d2024eac 100644 --- a/tests/unit/keyinput/test_modeparsers.py +++ b/tests/unit/keyinput/test_modeparsers.py @@ -56,7 +56,7 @@ class TestsNormalKeyParser: # Then start the real chain keyparser.handle(fake_keyevent_factory(Qt.Key_B, text='b')) keyparser.handle(fake_keyevent_factory(Qt.Key_A, text='a')) - keyparser.execute.assert_called_once_with( + keyparser.execute.assert_called_with( 'message-info ba', keyparser.Type.chain, None) assert keyparser._keystring == '' From 4e46c34e5a93ad44abf5e579171f487c21bd8a7f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 19:58:38 +0200 Subject: [PATCH 60/77] Use .assert_not_called() for mocks --- tests/helpers/test_stubs.py | 12 ++++++------ tests/unit/completion/test_completionmodel.py | 4 ++-- tests/unit/completion/test_completionwidget.py | 2 +- tests/unit/config/test_configinit.py | 2 +- tests/unit/misc/test_ipc.py | 2 +- tests/unit/utils/test_standarddir.py | 12 ++++++------ 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/helpers/test_stubs.py b/tests/helpers/test_stubs.py index 10fa9e5db..a5c1d27fb 100644 --- a/tests/helpers/test_stubs.py +++ b/tests/helpers/test_stubs.py @@ -36,8 +36,8 @@ def test_timeout(timer): func2 = mock.Mock() timer.timeout.connect(func) timer.timeout.connect(func2) - assert not func.called - assert not func2.called + func.assert_not_called() + func2.assert_not_called() timer.timeout.emit() func.assert_called_once_with() func2.assert_called_once_with() @@ -49,7 +49,7 @@ def test_disconnect_all(timer): timer.timeout.connect(func) timer.timeout.disconnect() timer.timeout.emit() - assert not func.called + func.assert_not_called() def test_disconnect_one(timer): @@ -58,7 +58,7 @@ def test_disconnect_one(timer): timer.timeout.connect(func) timer.timeout.disconnect(func) timer.timeout.emit() - assert not func.called + func.assert_not_called() def test_disconnect_all_invalid(timer): @@ -74,8 +74,8 @@ def test_disconnect_one_invalid(timer): timer.timeout.connect(func1) with pytest.raises(TypeError): timer.timeout.disconnect(func2) - assert not func1.called - assert not func2.called + func1.assert_not_called() + func2.assert_not_called() timer.timeout.emit() func1.assert_called_once_with() diff --git a/tests/unit/completion/test_completionmodel.py b/tests/unit/completion/test_completionmodel.py index 9e73e533a..292349730 100644 --- a/tests/unit/completion/test_completionmodel.py +++ b/tests/unit/completion/test_completionmodel.py @@ -103,7 +103,7 @@ def test_delete_cur_item_no_func(): parent = model.index(0, 0) with pytest.raises(cmdexc.CommandError): model.delete_cur_item(model.index(0, 0, parent)) - assert not callback.called + callback.assert_not_called() def test_delete_cur_item_no_cat(): @@ -114,4 +114,4 @@ def test_delete_cur_item_no_cat(): model.rowsRemoved.connect(callback) with pytest.raises(qtutils.QtValueError): model.delete_cur_item(QModelIndex()) - assert not callback.called + callback.assert_not_called() diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 7eb7fe2b5..22c71a2b7 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -242,7 +242,7 @@ def test_completion_item_del_no_selection(completionview): completionview.set_model(model) with pytest.raises(cmdexc.CommandError, match='No item selected!'): completionview.completion_item_del() - assert not func.called + func.assert_not_called() def test_resize_no_model(completionview, qtbot): diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index fef352584..767ffd6ca 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -151,7 +151,7 @@ def test_late_init(init_patch, monkeypatch, fake_save_manager, fake_args, assert text.startswith('Errors occurred while reading config.py:') assert 'Error text: Exception' in text else: - assert not msgbox_mock.called + msgbox_mock.assert_not_called() class TestQtArgs: diff --git a/tests/unit/misc/test_ipc.py b/tests/unit/misc/test_ipc.py index d8024b207..874419511 100644 --- a/tests/unit/misc/test_ipc.py +++ b/tests/unit/misc/test_ipc.py @@ -380,7 +380,7 @@ class TestHandleConnection: monkeypatch.setattr(ipc_server._server, 'nextPendingConnection', m) ipc_server.ignored = True ipc_server.handle_connection() - assert not m.called + m.assert_not_called() def test_no_connection(self, ipc_server, caplog): ipc_server.handle_connection() diff --git a/tests/unit/utils/test_standarddir.py b/tests/unit/utils/test_standarddir.py index 3dcb282d2..20ea88645 100644 --- a/tests/unit/utils/test_standarddir.py +++ b/tests/unit/utils/test_standarddir.py @@ -494,17 +494,17 @@ def test_init(mocker, tmpdir, args_kind): assert standarddir._locations != {} if args_kind == 'normal': if utils.is_mac: - assert not m_windows.called + m_windows.assert_not_called() assert m_mac.called elif utils.is_windows: assert m_windows.called - assert not m_mac.called + m_mac.assert_not_called() else: - assert not m_windows.called - assert not m_mac.called + m_windows.assert_not_called() + m_mac.assert_not_called() else: - assert not m_windows.called - assert not m_mac.called + m_windows.assert_not_called() + m_mac.assert_not_called() @pytest.mark.linux From 459bbc3a6f577474cb10aa7c85915bb2b1d0e86c Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 20:26:56 +0200 Subject: [PATCH 61/77] Add configinit to PERFECT_FILES --- scripts/dev/check_coverage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index aa5072536..99bd1277d 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -141,6 +141,8 @@ PERFECT_FILES = [ 'config/configfiles.py'), ('tests/unit/config/test_configtypes.py', 'config/configtypes.py'), + ('tests/unit/config/test_configinit.py', + 'config/configinit.py'), ('tests/unit/utils/test_qtutils.py', 'utils/qtutils.py'), From e8ceeceac8c3603af8937de961bf536ad0755458 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 22:28:40 +0200 Subject: [PATCH 62/77] Fix mock check with Python 3.5 Looks like .assert_not_called() doesn't work on function mocks with 3.5. --- tests/unit/config/test_configinit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index 767ffd6ca..fef352584 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -151,7 +151,7 @@ def test_late_init(init_patch, monkeypatch, fake_save_manager, fake_args, assert text.startswith('Errors occurred while reading config.py:') assert 'Error text: Exception' in text else: - msgbox_mock.assert_not_called() + assert not msgbox_mock.called class TestQtArgs: From b8389e4496028fce178031fc79eee478f4d8e4c9 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 Sep 2017 22:30:02 +0200 Subject: [PATCH 63/77] Revert "Fix NUL byte error handling on Python 3.4" This reverts commit a7d5a98cc4843437244e7633f622dd71be9f500e. Not needed anymore now that we dropped support. --- qutebrowser/config/configfiles.py | 2 +- tests/unit/config/test_configfiles.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 622280ee7..c9720a6d8 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -251,7 +251,7 @@ def read_config_py(filename=None, raising=False): try: code = compile(source, filename, 'exec') - except (ValueError, TypeError) as e: + except ValueError as e: # source contains NUL bytes desc = configexc.ConfigErrorDesc("Error while compiling", e) raise configexc.ConfigFileErrors(basename, [desc]) diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index ed5d2b882..6a58659fa 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -409,7 +409,7 @@ class TestConfigPy: assert len(excinfo.value.errors) == 1 error = excinfo.value.errors[0] - assert isinstance(error.exception, (TypeError, ValueError)) + assert isinstance(error.exception, ValueError) assert error.text == "Error while compiling" exception_text = 'source code string cannot contain null bytes' assert str(error.exception) == exception_text From 888a1b8c578bfd4f0481bafbc5bcdb67cd178f7c Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Fri, 22 Sep 2017 08:30:41 -0400 Subject: [PATCH 64/77] Append multiple history backups on import. Previously, a successful import of the text history into sqlite would move 'history' to 'history.bak'. If history.bak already existed, this would overwrite it on unix and fail on windows. With this patch, the most recently imported history is appended to history.bak to avoid data loss. Resolves #3005. A few other options I considered: 1. os.replace: - fast, simple, no error on Windows - potential data loss 2. numbered backups (.bak.1, .bak.2, ...): - fast, no data loss, but more complex 3. append each line to the backup as it is read: - more efficient than current patch (no need to read history twice) - potentially duplicate data if backup fails --- qutebrowser/browser/history.py | 15 +++++++++++---- tests/unit/browser/test_history.py | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index c9609bc03..ca62d9e6c 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -252,10 +252,7 @@ class WebHistory(sql.SqlTable): except ValueError as ex: message.error('Failed to import history: {}'.format(ex)) else: - bakpath = path + '.bak' - message.info('History import complete. Moving {} to {}' - .format(path, bakpath)) - os.rename(path, bakpath) + self._write_backup(path) # delay to give message time to appear before locking down for import message.info('Converting {} to sqlite...'.format(path)) @@ -287,6 +284,16 @@ class WebHistory(sql.SqlTable): self.insert_batch(data) self.completion.insert_batch(completion_data, replace=True) + def _write_backup(self, path): + bak = path + '.bak' + message.info('History import complete. Appending {} to {}' + .format(path, bak)) + with open(path, 'r', encoding='utf-8') as infile: + with open(bak, 'a', encoding='utf-8') as outfile: + for line in infile: + outfile.write('\n' + line) + os.remove(path) + def _format_url(self, url): return url.toString(QUrl.RemovePassword | QUrl.FullyEncoded) diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py index 51157fccb..1454e259b 100644 --- a/tests/unit/browser/test_history.py +++ b/tests/unit/browser/test_history.py @@ -284,6 +284,22 @@ def test_import_txt(hist, data_tmpdir, monkeypatch, stubs): assert (data_tmpdir / 'history.bak').exists() +def test_import_txt_existing_backup(hist, data_tmpdir, monkeypatch, stubs): + monkeypatch.setattr(history, 'QTimer', stubs.InstaTimer) + histfile = data_tmpdir / 'history' + bakfile = data_tmpdir / 'history.bak' + histfile.write('12345 http://example.com/ title') + bakfile.write('12346 http://qutebrowser.org/') + + hist.import_txt() + + assert list(hist) == [('http://example.com/', 'title', 12345, False)] + + assert not histfile.exists() + assert bakfile.read().split('\n') == ['12346 http://qutebrowser.org/', + '12345 http://example.com/ title'] + + @pytest.mark.parametrize('line', [ '', '#12345 http://example.com/commented', From 1784dc777d6a5c0ebd1cd616d1e88a72bceb2efc Mon Sep 17 00:00:00 2001 From: arza Date: Sat, 23 Sep 2017 22:24:17 +0300 Subject: [PATCH 65/77] Add table headers and widen input fields in qute://settings --- qutebrowser/html/settings.html | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/qutebrowser/html/settings.html b/qutebrowser/html/settings.html index 217e052af..b370c0d91 100644 --- a/qutebrowser/html/settings.html +++ b/qutebrowser/html/settings.html @@ -17,6 +17,9 @@ pre { margin: 2px; } th, td { border: 1px solid grey; padding: 0px 5px; } th { background: lightgrey; } th pre { color: grey; text-align: left; } +input { width: 98%; } +.setting { width: 75%; } +.value { width: 25%; text-align: center; } .noscript, .noscript-text { color:red; } .noscript-text { margin-bottom: 5cm; } .option_description { margin: .5ex 0; color: grey; font-size: 80%; font-style: italic; white-space: pre-line; } @@ -26,15 +29,19 @@ th pre { color: grey; text-align: left; }

{{ title }}

+ + + + {% for option in configdata.DATA.values() %} - -
SettingValue
{{ option.name }} (Current: {{ confget(option.name) | string |truncate(100) }}) + {{ option.name }} (Current: {{ confget(option.name) | string |truncate(100) }}) {% if option.description %}

{{ option.description|e }}

{% endif %}
+ Date: Sat, 23 Sep 2017 22:38:36 +0300 Subject: [PATCH 66/77] Remove extra backslashes in configdata.yml --- qutebrowser/config/configdata.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index 9f67689b0..642afb634 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -562,7 +562,7 @@ content.xss_auditing: desc: >- Whether load requests should be monitored for cross-site scripting attempts. - Suspicious scripts will be blocked and reported in the inspector\'s + Suspicious scripts will be blocked and reported in the inspector's JavaScript console. Enabling this feature might have an impact on performance. @@ -917,7 +917,7 @@ keyhint.blacklist: name: String default: [] desc: >- - Keychains that shouldn\'t be shown in the keyhint dialog. + Keychains that shouldn't be shown in the keyhint dialog. Globs are supported, so `;*` will blacklist all keychains starting with `;`. Use `*` to disable keyhints. @@ -1734,7 +1734,7 @@ fonts.monospace: desc: >- Default monospace fonts. - Whenever "monospace" is used in a font setting, it\'s replaced with the + Whenever "monospace" is used in a font setting, it's replaced with the fonts listed here. fonts.completion.entry: From 40f0f75ad5706aa150fc01078f265f31d61a2a79 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 24 Sep 2017 19:42:39 +0200 Subject: [PATCH 67/77] Improve error message for duplicate keys in config.py --- qutebrowser/config/configfiles.py | 6 +++++- tests/unit/config/test_configfiles.py | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index c9720a6d8..62cd6088c 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -212,7 +212,11 @@ class ConfigAPI: def bind(self, key, command, mode='normal', *, force=False): with self._handle_error('binding', key): - self._keyconfig.bind(key, command, mode=mode, force=force) + try: + 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'): with self._handle_error('unbinding', key): diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 6a58659fa..a2ed4099a 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -364,6 +364,15 @@ class TestConfigPy: "config.bind(',f', 'foo')") confpy.read() + def test_bind_duplicate_key(self, confpy): + """Make sure we get a nice error message on duplicate key bindings.""" + confpy.write("config.bind('H', 'message-info back')") + api = confpy.read(error=True) + error = api.errors[0] + + expected = "Duplicate key H - use force=True to override!" + assert str(error.exception) == expected + @pytest.mark.parametrize('line, key, mode', [ ('config.unbind("o")', 'o', 'normal'), ('config.unbind("y", mode="prompt")', 'y', 'prompt'), From d7273283ce00e23f22a8eabba3f467af81e42eb4 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 25 Sep 2017 06:55:17 +0200 Subject: [PATCH 68/77] Regenerate docs --- doc/help/settings.asciidoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 620e4467c..c600e820b 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -201,7 +201,7 @@ |<>|Timeout (in milliseconds) for partially typed key bindings. |<>|Enable Opera-like mouse rocker gestures. |<>|Enable Spatial Navigation. -|<>|Keychains that shouldn\'t be shown in the keyhint dialog. +|<>|Keychains that shouldn't be shown in the keyhint dialog. |<>|Time from pressing a key to seeing the keyhint dialog (ms). |<>|Time (in ms) to show messages in the statusbar for. |<>|Show messages in unfocused windows. @@ -1955,7 +1955,7 @@ Default: +pass:[true]+ [[content.xss_auditing]] === content.xss_auditing Whether load requests should be monitored for cross-site scripting attempts. -Suspicious scripts will be blocked and reported in the inspector\'s JavaScript console. Enabling this feature might have an impact on performance. +Suspicious scripts will be blocked and reported in the inspector's JavaScript console. Enabling this feature might have an impact on performance. Type: <> @@ -2144,7 +2144,7 @@ Default: +pass:[8pt monospace]+ [[fonts.monospace]] === fonts.monospace Default monospace fonts. -Whenever "monospace" is used in a font setting, it\'s replaced with the fonts listed here. +Whenever "monospace" is used in a font setting, it's replaced with the fonts listed here. Type: <> @@ -2535,7 +2535,7 @@ Default: +pass:[false]+ [[keyhint.blacklist]] === keyhint.blacklist -Keychains that shouldn\'t be shown in the keyhint dialog. +Keychains that shouldn't be shown in the keyhint dialog. Globs are supported, so `;*` will blacklist all keychains starting with `;`. Use `*` to disable keyhints. Type: <> From 8408d6ed9b735b51c3425f7c4cd3f164875999d8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 25 Sep 2017 06:56:33 +0200 Subject: [PATCH 69/77] Fix emacs syntax highlighting in configdata.yml --- qutebrowser/config/configdata.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index 642afb634..a3b0023b1 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -566,6 +566,8 @@ content.xss_auditing: JavaScript console. Enabling this feature might have an impact on performance. +# emacs: ' + ## completion completion.cmd_history_max_items: @@ -922,6 +924,8 @@ keyhint.blacklist: Globs are supported, so `;*` will blacklist all keychains starting with `;`. Use `*` to disable keyhints. +# emacs: ' + keyhint.delay: type: name: Int @@ -1737,6 +1741,8 @@ fonts.monospace: Whenever "monospace" is used in a font setting, it's replaced with the fonts listed here. +# emacs: ' + fonts.completion.entry: default: 8pt monospace type: Font From c7c198b9496e4a096ba326a6bf24ca5400459e3e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 25 Sep 2017 08:22:40 +0200 Subject: [PATCH 70/77] Stabilize hint test --- tests/end2end/features/hints.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 889e9e03e..8552586fa 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -362,6 +362,7 @@ Feature: Using hints And I set hints.mode to letter And I hint with args "--mode number all" And I press the key "s" + And I wait for "Filtering hints on 's'" in the log And I run :follow-hint 1 Then data/numbers/7.txt should be loaded From 54eb23eab162fffed2bf9e16c986f2a41263beb8 Mon Sep 17 00:00:00 2001 From: Panagiotis Ktistakis Date: Mon, 25 Sep 2017 20:54:28 +0300 Subject: [PATCH 71/77] Fix the link to faq.html in help page --- doc/help/index.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/help/index.asciidoc b/doc/help/index.asciidoc index 4edea719e..e90d472b5 100644 --- a/doc/help/index.asciidoc +++ b/doc/help/index.asciidoc @@ -7,7 +7,7 @@ Documentation The following help pages are currently available: * link:../quickstart.html[Quick start guide] -* link:../doc.html[Frequently asked questions] +* link:../faq.html[Frequently asked questions] * link:../changelog.html[Change Log] * link:commands.html[Documentation of commands] * link:configuring.html[Configuring qutebrowser] From 6aed6bca93ab910cc1ceb7adf540fd77d8410a75 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 25 Sep 2017 19:32:06 +0200 Subject: [PATCH 72/77] Make loading autoconfig.yml opt-in when a config.py exists This lets the user control the precedence those files should have, and also simplifies the code quite a bit. Fixes #2975 --- doc/help/configuring.asciidoc | 17 ++++---- qutebrowser/config/configexc.py | 6 +++ qutebrowser/config/configfiles.py | 34 ++++++++++----- qutebrowser/config/configinit.py | 41 +++++++----------- tests/unit/config/test_configexc.py | 7 ++++ tests/unit/config/test_configfiles.py | 60 +++++++++++---------------- tests/unit/config/test_configinit.py | 30 ++++++++------ 7 files changed, 103 insertions(+), 92 deletions(-) diff --git a/doc/help/configuring.asciidoc b/doc/help/configuring.asciidoc index b68e0bedc..afb6ec8e6 100644 --- a/doc/help/configuring.asciidoc +++ b/doc/help/configuring.asciidoc @@ -204,21 +204,22 @@ config.bind(',v', 'spawn mpv {url}') To suppress loading of any default keybindings, you can set `c.bindings.default = {}`. -Prevent loading `autoconfig.yml` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Loading `autoconfig.yml` +~~~~~~~~~~~~~~~~~~~~~~~~ -If you want all customization done via `:set`, `:bind` and `:unbind` to be -temporary, you can suppress loading `autoconfig.yml` in your `config.py` by -doing: +By default, all customization done via `:set`, `:bind` and `:unbind` is +temporary as soon as a `config.py` exists. The settings done that way are always +saved in the `autoconfig.yml` file, but you'll need to explicitly load it in +your `config.py` by doing: .config.py: [source,python] ---- -config.load_autoconfig = False +config.load_autoconfig() ---- -Note that the settings are still saved in `autoconfig.yml` that way, but then -not loaded on start. +If you do so at the top of your file, your `config.py` settings will take +precedence as they overwrite the settings done in `autoconfig.yml`. Importing other modules ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/qutebrowser/config/configexc.py b/qutebrowser/config/configexc.py index 4d283d21f..0d20bb09d 100644 --- a/qutebrowser/config/configexc.py +++ b/qutebrowser/config/configexc.py @@ -94,6 +94,12 @@ class ConfigErrorDesc: def __str__(self): return '{}: {}'.format(self.text, self.exception) + def with_text(self, text): + """Get a new ConfigErrorDesc with the given text appended.""" + return self.__class__(text='{} ({})'.format(self.text, text), + exception=self.exception, + traceback=self.traceback) + class ConfigFileErrors(Error): diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 62cd6088c..a017d025d 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -176,7 +176,6 @@ class ConfigAPI: Attributes: _config: The main Config object to use. _keyconfig: The KeyConfig object. - load_autoconfig: Whether autoconfig.yml should be loaded. errors: Errors which occurred while setting options. configdir: The qutebrowser config directory, as pathlib.Path. datadir: The qutebrowser data directory, as pathlib.Path. @@ -185,7 +184,6 @@ class ConfigAPI: def __init__(self, conf, keyconfig): self._config = conf self._keyconfig = keyconfig - self.load_autoconfig = True self.errors = [] self.configdir = pathlib.Path(standarddir.config()) self.datadir = pathlib.Path(standarddir.data()) @@ -194,6 +192,10 @@ class ConfigAPI: def _handle_error(self, action, name): try: yield + except configexc.ConfigFileErrors as e: + for err in e.errors: + new_err = err.with_text(e.basename) + self.errors.append(new_err) except configexc.Error as e: text = "While {} '{}'".format(action, name) self.errors.append(configexc.ConfigErrorDesc(text, e)) @@ -202,6 +204,10 @@ class ConfigAPI: """Do work which needs to be done after reading config.py.""" self._config.update_mutables() + def load_autoconfig(self): + with self._handle_error('reading', 'autoconfig.yml'): + read_autoconfig() + def get(self, name): with self._handle_error('getting', name): return self._config.get_obj(name) @@ -223,20 +229,15 @@ class ConfigAPI: self._keyconfig.unbind(key, mode=mode) -def read_config_py(filename=None, raising=False): +def read_config_py(filename, raising=False): """Read a config.py file. Arguments; + filename: The name of the file to read. raising: Raise exceptions happening in config.py. This is needed during tests to use pytest's inspection. """ api = ConfigAPI(config.instance, config.key_instance) - - if filename is None: - filename = os.path.join(standarddir.config(), 'config.py') - if not os.path.exists(filename): - return api - container = config.ConfigContainer(config.instance, configapi=api) basename = os.path.basename(filename) @@ -282,7 +283,20 @@ def read_config_py(filename=None, raising=False): exception=e, traceback=traceback.format_exc())) api.finalize() - return api + + if api.errors: + raise configexc.ConfigFileErrors('config.py', api.errors) + + +def read_autoconfig(): + """Read the autoconfig.yml file.""" + try: + config.instance.read_yaml() + except configexc.ConfigFileErrors as e: + raise # caught in outer block + except configexc.Error as e: + desc = configexc.ConfigErrorDesc("Error", e) + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) @contextlib.contextmanager diff --git a/qutebrowser/config/configinit.py b/qutebrowser/config/configinit.py index 27a37b5e9..a64438b83 100644 --- a/qutebrowser/config/configinit.py +++ b/qutebrowser/config/configinit.py @@ -19,18 +19,19 @@ """Initialization of the configuration.""" +import os.path import sys from PyQt5.QtWidgets import QMessageBox from qutebrowser.config import (config, configdata, configfiles, configtypes, configexc) -from qutebrowser.utils import objreg, qtutils, usertypes, log +from qutebrowser.utils import objreg, qtutils, usertypes, log, standarddir from qutebrowser.misc import earlyinit, msgbox, objects -# Errors which happened during init, so we can show a message box. -_init_errors = [] +# Error which happened during init, so we can show a message box. +_init_errors = None def early_init(args): @@ -52,29 +53,17 @@ def early_init(args): config.key_instance) objreg.register('config-commands', config_commands) - config_api = None + config_file = os.path.join(standarddir.config(), 'config.py') try: - config_api = configfiles.read_config_py() - # Raised here so we get the config_api back. - if config_api.errors: - raise configexc.ConfigFileErrors('config.py', config_api.errors) + if os.path.exists(config_file): + configfiles.read_config_py(config_file) + else: + configfiles.read_autoconfig() except configexc.ConfigFileErrors as e: - log.config.exception("Error while loading config.py") - _init_errors.append(e) - - try: - if getattr(config_api, 'load_autoconfig', True): - try: - config.instance.read_yaml() - except configexc.ConfigFileErrors as e: - raise # caught in outer block - except configexc.Error as e: - desc = configexc.ConfigErrorDesc("Error", e) - raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) - except configexc.ConfigFileErrors as e: - log.config.exception("Error while loading config.py") - _init_errors.append(e) + log.config.exception("Error while loading {}".format(e.basename)) + global _init_errors + _init_errors = e configfiles.init() @@ -109,14 +98,14 @@ def get_backend(args): def late_init(save_manager): """Initialize the rest of the config after the QApplication is created.""" global _init_errors - for err in _init_errors: + if _init_errors is not None: errbox = msgbox.msgbox(parent=None, title="Error while reading config", - text=err.to_html(), + text=_init_errors.to_html(), icon=QMessageBox.Warning, plain_text=False) errbox.exec_() - _init_errors = [] + _init_errors = None config.instance.init_save_manager(save_manager) configfiles.state.init_save_manager(save_manager) diff --git a/tests/unit/config/test_configexc.py b/tests/unit/config/test_configexc.py index 8eaa21f05..024bbb1d0 100644 --- a/tests/unit/config/test_configexc.py +++ b/tests/unit/config/test_configexc.py @@ -49,6 +49,13 @@ def test_duplicate_key_error(): assert str(e) == "Duplicate key asdf" +def test_desc_with_text(): + """Test ConfigErrorDesc.with_text.""" + old = configexc.ConfigErrorDesc("Error text", Exception("Exception text")) + new = old.with_text("additional text") + assert str(new) == 'Error text (additional text): Exception text' + + @pytest.fixture def errors(): """Get a ConfigFileErrors object.""" diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index a2ed4099a..7ce29484a 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -222,9 +222,15 @@ class ConfPy: def read(self, error=False): """Read the config.py via configfiles and check for errors.""" - api = configfiles.read_config_py(self.filename, raising=not error) - assert len(api.errors) == (1 if error else 0) - return api + if error: + with pytest.raises(configexc.ConfigFileErrors) as excinfo: + configfiles.read_config_py(self.filename) + errors = excinfo.value.errors + assert len(errors) == 1 + return errors[0] + else: + configfiles.read_config_py(self.filename, raising=True) + return None def write_qbmodule(self): self.write('import qbmodule', @@ -263,8 +269,7 @@ class TestConfigPyModules: confpy.write_qbmodule() qbmodulepy.write('def run(config):', ' 1/0') - api = confpy.read(error=True) - error = api.errors[0] + error = confpy.read(error=True) assert error.text == "Unhandled exception" assert isinstance(error.exception, ZeroDivisionError) @@ -277,8 +282,7 @@ class TestConfigPyModules: confpy.write('import foobar', 'foobar.run(config)') - api = confpy.read(error=True) - error = api.errors[0] + error = confpy.read(error=True) assert error.text == "Unhandled exception" assert isinstance(error.exception, ImportError) @@ -367,8 +371,7 @@ class TestConfigPy: def test_bind_duplicate_key(self, confpy): """Make sure we get a nice error message on duplicate key bindings.""" confpy.write("config.bind('H', 'message-info back')") - api = confpy.read(error=True) - error = api.errors[0] + error = confpy.read(error=True) expected = "Duplicate key H - use force=True to override!" assert str(error.exception) == expected @@ -390,17 +393,6 @@ class TestConfigPy: assert config.instance._values['aliases']['foo'] == 'message-info foo' assert config.instance._values['aliases']['bar'] == 'message-info bar' - def test_reading_default_location(self, config_tmpdir, data_tmpdir): - (config_tmpdir / 'config.py').write_text( - 'c.colors.hints.bg = "red"', 'utf-8') - configfiles.read_config_py() - assert config.instance._values['colors.hints.bg'] == 'red' - - def test_reading_missing_default_location(self, config_tmpdir, - data_tmpdir): - assert not (config_tmpdir / 'config.py').exists() - configfiles.read_config_py() # Should not crash - def test_oserror(self, tmpdir, data_tmpdir, config_tmpdir): with pytest.raises(configexc.ConfigFileErrors) as excinfo: configfiles.read_config_py(str(tmpdir / 'foo')) @@ -443,12 +435,9 @@ class TestConfigPy: assert " ^" in tblines def test_unhandled_exception(self, confpy): - confpy.write("config.load_autoconfig = False", "1/0") + confpy.write("1/0") + error = confpy.read(error=True) - api = confpy.read(error=True) - error = api.errors[0] - - assert not api.load_autoconfig assert error.text == "Unhandled exception" assert isinstance(error.exception, ZeroDivisionError) @@ -460,8 +449,7 @@ class TestConfigPy: def test_config_val(self, confpy): """Using config.val should not work in config.py files.""" confpy.write("config.val.colors.hints.bg = 'red'") - api = confpy.read(error=True) - error = api.errors[0] + error = confpy.read(error=True) assert error.text == "Unhandled exception" assert isinstance(error.exception, AttributeError) @@ -470,11 +458,9 @@ class TestConfigPy: @pytest.mark.parametrize('line', ["c.foo = 42", "config.set('foo', 42)"]) def test_config_error(self, confpy, line): - confpy.write(line, "config.load_autoconfig = False") - api = confpy.read(error=True) - error = api.errors[0] + confpy.write(line) + error = confpy.read(error=True) - assert not api.load_autoconfig assert error.text == "While setting 'foo'" assert isinstance(error.exception, configexc.NoOptionError) assert str(error.exception) == "No option 'foo'" @@ -482,16 +468,20 @@ class TestConfigPy: def test_multiple_errors(self, confpy): confpy.write("c.foo = 42", "config.set('foo', 42)", "1/0") - api = configfiles.read_config_py(confpy.filename) - assert len(api.errors) == 3 - for error in api.errors[:2]: + with pytest.raises(configexc.ConfigFileErrors) as excinfo: + configfiles.read_config_py(confpy.filename) + + errors = excinfo.value.errors + assert len(errors) == 3 + + for error in errors[:2]: assert error.text == "While setting 'foo'" assert isinstance(error.exception, configexc.NoOptionError) assert str(error.exception) == "No option 'foo'" assert error.traceback is None - error = api.errors[2] + error = errors[2] assert error.text == "Unhandled exception" assert isinstance(error.exception, ZeroDivisionError) assert error.traceback is not None diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index fef352584..d13624c66 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -38,7 +38,7 @@ def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, monkeypatch.setattr(config, 'instance', None) monkeypatch.setattr(config, 'key_instance', None) monkeypatch.setattr(config, 'change_filters', []) - monkeypatch.setattr(configinit, '_init_errors', []) + monkeypatch.setattr(configinit, '_init_errors', None) # Make sure we get no SSL warning monkeypatch.setattr(configinit.earlyinit, 'check_backend_ssl_support', lambda _backend: None) @@ -73,8 +73,8 @@ def test_early_init(init_patch, config_tmpdir, caplog, fake_args, if config_py: config_py_lines = ['c.colors.hints.bg = "red"'] - if not load_autoconfig: - config_py_lines.append('config.load_autoconfig = False') + if load_autoconfig: + config_py_lines.append('config.load_autoconfig()') if config_py == 'error': config_py_lines.append('c.foo = 42') config_py_file.write_text('\n'.join(config_py_lines), @@ -85,21 +85,25 @@ def test_early_init(init_patch, config_tmpdir, caplog, fake_args, # Check error messages expected_errors = [] - if config_py == 'error': - expected_errors.append( - "Errors occurred while reading config.py:\n" - " While setting 'foo': No option 'foo'") + if load_autoconfig or not config_py: - error = "Errors occurred while reading autoconfig.yml:\n" + suffix = ' (autoconfig.yml)' if config_py else '' if invalid_yaml == '42': - error += " While loading data: Toplevel object is not a dict" + error = ("While loading data{}: Toplevel object is not a dict" + .format(suffix)) expected_errors.append(error) elif invalid_yaml == 'wrong-type': - error += (" Error: Invalid value 'True' - expected a value of " - "type str but got bool.") + error = ("Error{}: Invalid value 'True' - expected a value of " + "type str but got bool.".format(suffix)) expected_errors.append(error) + if config_py == 'error': + expected_errors.append("While setting 'foo': No option 'foo'") + + if configinit._init_errors is None: + actual_errors = [] + else: + actual_errors = [str(err) for err in configinit._init_errors.errors] - actual_errors = [str(err) for err in configinit._init_errors] assert actual_errors == expected_errors # Make sure things have been init'ed @@ -134,7 +138,7 @@ def test_late_init(init_patch, monkeypatch, fake_save_manager, fake_args, if errors: err = configexc.ConfigErrorDesc("Error text", Exception("Exception")) errs = configexc.ConfigFileErrors("config.py", [err]) - monkeypatch.setattr(configinit, '_init_errors', [errs]) + monkeypatch.setattr(configinit, '_init_errors', errs) msgbox_mock = mocker.patch('qutebrowser.config.configinit.msgbox.msgbox', autospec=True) From 1086e31f2898f6ac85b8a561e2cc7f9c72f7623b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 25 Sep 2017 20:09:43 +0200 Subject: [PATCH 73/77] Split up configinit tests --- tests/unit/config/test_configinit.py | 172 ++++++++++++++++----------- 1 file changed, 104 insertions(+), 68 deletions(-) diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index d13624c66..7ec371a6d 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -49,86 +49,122 @@ def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, pass -@pytest.mark.parametrize('load_autoconfig', [True, False]) # noqa -@pytest.mark.parametrize('config_py', [True, 'error', False]) -@pytest.mark.parametrize('invalid_yaml', - ['42', 'unknown', 'wrong-type', False]) -# pylint: disable=too-many-branches -def test_early_init(init_patch, config_tmpdir, caplog, fake_args, - load_autoconfig, config_py, invalid_yaml): - # Prepare files - autoconfig_file = config_tmpdir / 'autoconfig.yml' - config_py_file = config_tmpdir / 'config.py' +class TestEarlyInit: - if invalid_yaml == '42': - text = '42' - elif invalid_yaml == 'unknown': - text = 'global:\n colors.foobar: magenta\n' - elif invalid_yaml == 'wrong-type': - text = 'global:\n tabs.position: true\n' - else: - assert not invalid_yaml - text = 'global:\n colors.hints.fg: magenta\n' - autoconfig_file.write_text(text, 'utf-8', ensure=True) + @pytest.mark.parametrize('config_py', [True, 'error', False]) + def test_config_py(self, init_patch, config_tmpdir, caplog, fake_args, + config_py): + """Test loading with only a config.py.""" + config_py_file = config_tmpdir / 'config.py' - if config_py: - config_py_lines = ['c.colors.hints.bg = "red"'] - if load_autoconfig: - config_py_lines.append('config.load_autoconfig()') + if config_py: + config_py_lines = ['c.colors.hints.bg = "red"'] + if config_py == 'error': + config_py_lines.append('c.foo = 42') + config_py_file.write_text('\n'.join(config_py_lines), + 'utf-8', ensure=True) + + with caplog.at_level(logging.ERROR): + configinit.early_init(fake_args) + + # Check error messages + expected_errors = [] if config_py == 'error': - config_py_lines.append('c.foo = 42') - config_py_file.write_text('\n'.join(config_py_lines), - 'utf-8', ensure=True) + expected_errors.append("While setting 'foo': No option 'foo'") - with caplog.at_level(logging.ERROR): - configinit.early_init(fake_args) + if configinit._init_errors is None: + actual_errors = [] + else: + actual_errors = [str(err) + for err in configinit._init_errors.errors] - # Check error messages - expected_errors = [] + assert actual_errors == expected_errors - if load_autoconfig or not config_py: - suffix = ' (autoconfig.yml)' if config_py else '' - if invalid_yaml == '42': - error = ("While loading data{}: Toplevel object is not a dict" - .format(suffix)) - expected_errors.append(error) - elif invalid_yaml == 'wrong-type': - error = ("Error{}: Invalid value 'True' - expected a value of " - "type str but got bool.".format(suffix)) - expected_errors.append(error) - if config_py == 'error': - expected_errors.append("While setting 'foo': No option 'foo'") + # Make sure things have been init'ed + objreg.get('config-commands') + assert isinstance(config.instance, config.Config) + assert isinstance(config.key_instance, config.KeyConfig) - if configinit._init_errors is None: - actual_errors = [] - else: - actual_errors = [str(err) for err in configinit._init_errors.errors] + # Check config values + if config_py: + assert config.instance._values == {'colors.hints.bg': 'red'} + else: + assert config.instance._values == {} - assert actual_errors == expected_errors + @pytest.mark.parametrize('load_autoconfig', [True, False]) + @pytest.mark.parametrize('config_py', [True, 'error', False]) + @pytest.mark.parametrize('invalid_yaml', ['42', 'unknown', 'wrong-type', + False]) + def test_autoconfig_yml(self, init_patch, config_tmpdir, caplog, fake_args, + load_autoconfig, config_py, invalid_yaml): + """Test interaction between config.py and autoconfig.yml.""" + # pylint: disable=too-many-locals,too-many-branches + # Prepare files + autoconfig_file = config_tmpdir / 'autoconfig.yml' + config_py_file = config_tmpdir / 'config.py' - # Make sure things have been init'ed - objreg.get('config-commands') - assert isinstance(config.instance, config.Config) - assert isinstance(config.key_instance, config.KeyConfig) - - # Check config values - if config_py and load_autoconfig and not invalid_yaml: - assert config.instance._values == { - 'colors.hints.bg': 'red', - 'colors.hints.fg': 'magenta', + yaml_text = { + '42': '42', + 'unknown': 'global:\n colors.foobar: magenta\n', + 'wrong-type': 'global:\n tabs.position: true\n', + False: 'global:\n colors.hints.fg: magenta\n', } - elif config_py: - assert config.instance._values == {'colors.hints.bg': 'red'} - elif invalid_yaml: - assert config.instance._values == {} - else: - assert config.instance._values == {'colors.hints.fg': 'magenta'} + autoconfig_file.write_text(yaml_text[invalid_yaml], 'utf-8', + ensure=True) + if config_py: + config_py_lines = ['c.colors.hints.bg = "red"'] + if load_autoconfig: + config_py_lines.append('config.load_autoconfig()') + if config_py == 'error': + config_py_lines.append('c.foo = 42') + config_py_file.write_text('\n'.join(config_py_lines), + 'utf-8', ensure=True) -def test_early_init_invalid_change_filter(init_patch, fake_args): - config.change_filter('foobar') - with pytest.raises(configexc.NoOptionError): - configinit.early_init(fake_args) + with caplog.at_level(logging.ERROR): + configinit.early_init(fake_args) + + # Check error messages + expected_errors = [] + + if load_autoconfig or not config_py: + suffix = ' (autoconfig.yml)' if config_py else '' + if invalid_yaml == '42': + error = ("While loading data{}: Toplevel object is not a dict" + .format(suffix)) + expected_errors.append(error) + elif invalid_yaml == 'wrong-type': + error = ("Error{}: Invalid value 'True' - expected a value of " + "type str but got bool.".format(suffix)) + expected_errors.append(error) + if config_py == 'error': + expected_errors.append("While setting 'foo': No option 'foo'") + + if configinit._init_errors is None: + actual_errors = [] + else: + actual_errors = [str(err) + for err in configinit._init_errors.errors] + + assert actual_errors == expected_errors + + # Check config values + if config_py and load_autoconfig and not invalid_yaml: + assert config.instance._values == { + 'colors.hints.bg': 'red', + 'colors.hints.fg': 'magenta', + } + elif config_py: + assert config.instance._values == {'colors.hints.bg': 'red'} + elif invalid_yaml: + assert config.instance._values == {} + else: + assert config.instance._values == {'colors.hints.fg': 'magenta'} + + def test_invalid_change_filter(self, init_patch, fake_args): + config.change_filter('foobar') + with pytest.raises(configexc.NoOptionError): + configinit.early_init(fake_args) @pytest.mark.parametrize('errors', [True, False]) From 38038df70379a7ae14e58bbf9641de0309ea26a7 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 25 Sep 2017 21:10:52 +0200 Subject: [PATCH 74/77] Compare objects with :set with multiple values --- qutebrowser/config/config.py | 9 ++++++--- tests/unit/config/test_config.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 8d93a2344..c7826aef2 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -275,14 +275,17 @@ class ConfigCommands: # Use the next valid value from values, or the first if the current # value does not appear in the list - old_value = self._config.get_str(option) + old_value = self._config.get_obj(option, mutable=False) + opt = self._config.get_opt(option) + values = [opt.typ.from_str(val) for val in values] + try: - idx = values.index(str(old_value)) + idx = values.index(old_value) idx = (idx + 1) % len(values) value = values[idx] except ValueError: value = values[0] - self._config.set_str(option, value, save_yaml=not temp) + self._config.set_obj(option, value, save_yaml=not temp) @contextlib.contextmanager def _handle_config_error(self): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 24c6814b5..949064bc9 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -439,6 +439,18 @@ class TestSetConfigCommand: assert config_stub.get(opt) == expected assert config_stub._yaml[opt] == expected + def test_cycling_different_representation(self, commands, config_stub): + """When using a different representation, cycling should work. + + For example, we use [foo] which is represented as ["foo"]. + """ + opt = 'qt_args' + config_stub.set_obj(opt, ['foo']) + commands.set(0, opt, '[foo]', '[bar]') + assert config_stub.get(opt) == ['bar'] + commands.set(0, opt, '[foo]', '[bar]') + assert config_stub.get(opt) == ['foo'] + class TestBindConfigCommand: From 38449e3e2bc87d01b3173ae2bbe0bc2792775f37 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 26 Sep 2017 06:41:55 +0200 Subject: [PATCH 75/77] Make sure the autoconfig.yml is saved periodically Fixes #2982 --- qutebrowser/config/configfiles.py | 11 +++++++---- qutebrowser/config/configinit.py | 1 + tests/unit/config/test_configfiles.py | 6 ++++-- tests/unit/config/test_configinit.py | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index a017d025d..7eba211f6 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -29,7 +29,7 @@ import configparser import contextlib import yaml -from PyQt5.QtCore import QSettings +from PyQt5.QtCore import pyqtSignal, QObject, QSettings import qutebrowser from qutebrowser.config import configexc, config, configdata @@ -72,7 +72,7 @@ class StateConfig(configparser.ConfigParser): self.write(f) -class YamlConfig: +class YamlConfig(QObject): """A config stored on disk as YAML file. @@ -81,8 +81,10 @@ class YamlConfig: """ VERSION = 1 + changed = pyqtSignal() - def __init__(self): + def __init__(self, parent=None): + super().__init__(parent) self._filename = os.path.join(standarddir.config(auto=True), 'autoconfig.yml') self._values = {} @@ -94,12 +96,13 @@ class YamlConfig: We do this outside of __init__ because the config gets created before the save_manager exists. """ - save_manager.add_saveable('yaml-config', self._save) + save_manager.add_saveable('yaml-config', self._save, self.changed) def __getitem__(self, name): return self._values[name] def __setitem__(self, name, value): + self.changed.emit() self._dirty = True self._values[name] = value diff --git a/qutebrowser/config/configinit.py b/qutebrowser/config/configinit.py index a64438b83..3ec98a293 100644 --- a/qutebrowser/config/configinit.py +++ b/qutebrowser/config/configinit.py @@ -43,6 +43,7 @@ def early_init(args): config.instance = config.Config(yaml_config=yaml_config) config.val = config.ConfigContainer(config.instance) config.key_instance = config.KeyConfig(config.instance) + yaml_config.setParent(config.instance) for cf in config.change_filters: cf.validate() diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 7ce29484a..d987d7442 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -127,7 +127,7 @@ class TestYaml: ('confirm_quit', True), ('confirm_quit', False), ]) - def test_changed(self, config_tmpdir, old_config, key, value): + def test_changed(self, qtbot, config_tmpdir, old_config, key, value): autoconfig = config_tmpdir / 'autoconfig.yml' if old_config is not None: autoconfig.write_text(old_config, 'utf-8') @@ -135,7 +135,9 @@ class TestYaml: yaml = configfiles.YamlConfig() yaml.load() - yaml[key] = value + with qtbot.wait_signal(yaml.changed): + yaml[key] = value + assert key in yaml assert yaml[key] == value diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index 7ec371a6d..1332499de 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -183,7 +183,7 @@ def test_late_init(init_patch, monkeypatch, fake_save_manager, fake_args, fake_save_manager.add_saveable.assert_any_call( 'state-config', unittest.mock.ANY) fake_save_manager.add_saveable.assert_any_call( - 'yaml-config', unittest.mock.ANY) + 'yaml-config', unittest.mock.ANY, unittest.mock.ANY) if errors: assert len(msgbox_mock.call_args_list) == 1 _call_posargs, call_kwargs = msgbox_mock.call_args_list[0] From 6e226c688552e19216416146c53bb661dfde48bd Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 26 Sep 2017 07:08:42 +0200 Subject: [PATCH 76/77] Add a recipes section to configuring.asciidoc Closes #2987 Closes #2969 Closes #3009 See #2975 --- doc/help/configuring.asciidoc | 103 ++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/doc/help/configuring.asciidoc b/doc/help/configuring.asciidoc index afb6ec8e6..62b10ebf5 100644 --- a/doc/help/configuring.asciidoc +++ b/doc/help/configuring.asciidoc @@ -262,3 +262,106 @@ qutebrowser tries to display errors which are easy to understand even for people who are not used to writing Python. If you see a config error which you find confusing or you think qutebrowser could handle better, please https://github.com/qutebrowser/qutebrowser/issues[open an issue]! + +Recipes +~~~~~~~ + +Reading a YAML file +^^^^^^^^^^^^^^^^^^^ + +To read a YAML config like this: + +.config.yml: +---- +tabs.position: left +tabs.show: switching +---- + +You can use: + +.config.py: +[source,python] +---- +import yaml + +with (config.configdir / 'config.yml').open() as f: + yaml_data = yaml.load(f) + +for k, v in yaml_data.items(): + config.set(k, v) +---- + +Reading a nested YAML file +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +To read a YAML file with nested values like this: + +.colors.yml: +---- +colors: + statusbar: + normal: + bg: lime + fg: black + url: + fg: red +---- + +You can use: + +.config.py: +[source,python] +---- +import yaml + +with (config.configdir / 'colors.yml').open() as f: + yaml_data = yaml.load(f) + +def dict_attrs(obj, path=''): + if isinstance(obj, dict): + for k, v in obj.items(): + yield from dict_attrs(v, '{}.{}'.format(path, k) if path else k) + else: + yield path, obj + +for k, v in dict_attrs(yaml_data): + config.set(k, v) +---- + +Note that this won't work for values which are dictionaries. + +Binding chained commands +^^^^^^^^^^^^^^^^^^^^^^^^ + +If you have a lot of chained comamnds you want to bind, you can write a helper +to do so: + +[source,python] +---- +def bind_chained(key, *commands, force=False): + config.bind(key, ' ;; '.join(commands), force=force) + +bind_chained('', 'clear-keychain', 'search', force=True) +---- + +Avoiding flake8 errors +^^^^^^^^^^^^^^^^^^^^^^ + +If you use an editor with flake8 integration which complains about `c` and `config` being undefined, you can use: + +[source,python] +---- +c = c # noqa: F821 +config = config # noqa: F821 +---- + +For type annotation support (note that those imports aren't guaranteed to be +stable across qutebrowser versions): + +[source,python] +---- +from qutebrowser.config.configfiles import ConfigAPI # noqa: F401 +from qutebrowser.config.config import ConfigContainer # noqa: F401 +config = config # type: ConfigAPI # noqa: F821 +c = c # type: ConfigContainer # noqa: F821 +---- From e766cf5ed117a0f270248d5ea5a653439eba8a94 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 26 Sep 2017 07:12:38 +0200 Subject: [PATCH 77/77] build_release: print artifacts if not releasing --- scripts/dev/build_release.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/dev/build_release.py b/scripts/dev/build_release.py index 0019752c2..55ee9a151 100755 --- a/scripts/dev/build_release.py +++ b/scripts/dev/build_release.py @@ -377,6 +377,11 @@ def main(): github_upload(artifacts, args.upload[0]) if upload_to_pypi: pypi_upload(artifacts) + else: + print() + scriptutils.print_title("Artifacts") + for artifact in artifacts: + print(artifact) if __name__ == '__main__':