From e9128ebb2aae6983965e2a7901577e81fdb1a2fd Mon Sep 17 00:00:00 2001 From: Oliver Caldwell Date: Fri, 22 Jan 2016 20:55:37 +0000 Subject: [PATCH 1/8] Relax editor templating I tried to set my editor to `termite -e "vim -f {}"`, termite being a pretty cool and light terminal I use within my i3wm Arch linux box. So when I open my editor I want it to launch a terminal with Vim inside instead of GVim for various reasons. The validation rejected this at first because it was looking for '{}' inside ['foo', 'bar', 'baz {}'], essentially. So I need it to look inside the sub-strings, not just the list. Then after validation I need to perform the '{}' replacement inside the sub-string too, not just replacing the whole string. --- qutebrowser/config/configtypes.py | 2 +- qutebrowser/misc/editor.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index 177f55456..65ea745c1 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -1094,7 +1094,7 @@ class ShellCommand(BaseType): shlex.split(value) except ValueError as e: raise configexc.ValidationError(value, str(e)) - if self.placeholder and '{}' not in self.transform(value): + if self.placeholder and '{}' not in value: raise configexc.ValidationError(value, "needs to contain a " "{}-placeholder.") diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 2fa9791e9..d844cdc80 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -124,6 +124,6 @@ class ExternalEditor(QObject): self._proc.error.connect(self.on_proc_error) editor = config.get('general', 'editor') executable = editor[0] - args = [self._filename if arg == '{}' else arg for arg in editor[1:]] + args = [arg.replace('{}', self._filename) if '{}' in arg else arg for arg in editor[1:]] log.procs.debug("Calling \"{}\" with args {}".format(executable, args)) self._proc.start(executable, args) From 84c44f33957c692db7492c1ce164043dccc2ecaa Mon Sep 17 00:00:00 2001 From: Oliver Caldwell Date: Sun, 31 Jan 2016 22:15:18 +0000 Subject: [PATCH 2/8] Remove invalid test parameter --- tests/unit/misc/test_editor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index a005c265e..d97cb6657 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -57,7 +57,7 @@ class TestArg: editor: The ExternalEditor instance to test. """ - @pytest.mark.parametrize('args', [[], ['foo', 'bar'], ['foo{}bar']]) + @pytest.mark.parametrize('args', [[], ['foo'], ['foo', 'bar']]) def test_start_no_placeholder(self, config_stub, editor, args): """Test starting editor without arguments.""" config_stub.data['general']['editor'] = ['bin'] + args From a9a42e0a990737dbd70e246803764f819c691405 Mon Sep 17 00:00:00 2001 From: Oliver Caldwell Date: Sun, 31 Jan 2016 22:36:58 +0000 Subject: [PATCH 3/8] Removed invalid placeholder test --- tests/unit/config/test_configtypes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 34f4329fc..7bb505578 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1586,7 +1586,6 @@ class TestShellCommand: @pytest.mark.parametrize('kwargs, val', [ ({}, ''), - ({'placeholder': '{}'}, 'foo{} bar'), ({'placeholder': '{}'}, 'foo bar'), ({}, 'foo"'), # not splittable with shlex ]) From 4cd7d193f1506a1d0a943c575750d589300d0690 Mon Sep 17 00:00:00 2001 From: Oliver Caldwell Date: Sun, 31 Jan 2016 22:56:11 +0000 Subject: [PATCH 4/8] Simplify arg placeholder replacement --- qutebrowser/misc/editor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index d844cdc80..143eb0a9c 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -124,6 +124,6 @@ class ExternalEditor(QObject): self._proc.error.connect(self.on_proc_error) editor = config.get('general', 'editor') executable = editor[0] - args = [arg.replace('{}', self._filename) if '{}' in arg else arg for arg in editor[1:]] + args = [arg.replace('{}', self._filename) for arg in editor[1:]] log.procs.debug("Calling \"{}\" with args {}".format(executable, args)) self._proc.start(executable, args) From 5474f902ddf87028d92a5ac5bf1d2844c179b3fe Mon Sep 17 00:00:00 2001 From: Oliver Caldwell Date: Mon, 1 Feb 2016 21:36:24 +0000 Subject: [PATCH 5/8] Add more tests for new editor config --- tests/unit/config/test_configtypes.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index da4fef8fc..3996e03b5 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1580,6 +1580,8 @@ class TestShellCommand: ({'none_ok': True}, ''), ({}, 'foobar'), ({'placeholder': '{}'}, 'foo {} bar'), + ({'placeholder': '{}'}, 'foo{}bar'), + ({'placeholder': '{}'}, 'foo "bar {}"') ]) def test_validate_valid(self, klass, kwargs, val): klass(**kwargs).validate(val) @@ -1587,6 +1589,7 @@ class TestShellCommand: @pytest.mark.parametrize('kwargs, val', [ ({}, ''), ({'placeholder': '{}'}, 'foo bar'), + ({'placeholder': '{}'}, 'foo { } bar') ({}, 'foo"'), # not splittable with shlex ]) def test_validate_invalid(self, klass, kwargs, val): From a617bc3ef4a1ea22bbe1eba2f83936d4c5dbbf66 Mon Sep 17 00:00:00 2001 From: Oliver Caldwell Date: Mon, 1 Feb 2016 21:41:14 +0000 Subject: [PATCH 6/8] Add missing commas I haven't written Python in quite a long time :| --- tests/unit/config/test_configtypes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 3996e03b5..3cdde6b95 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1581,7 +1581,7 @@ class TestShellCommand: ({}, 'foobar'), ({'placeholder': '{}'}, 'foo {} bar'), ({'placeholder': '{}'}, 'foo{}bar'), - ({'placeholder': '{}'}, 'foo "bar {}"') + ({'placeholder': '{}'}, 'foo "bar {}"'), ]) def test_validate_valid(self, klass, kwargs, val): klass(**kwargs).validate(val) @@ -1589,7 +1589,7 @@ class TestShellCommand: @pytest.mark.parametrize('kwargs, val', [ ({}, ''), ({'placeholder': '{}'}, 'foo bar'), - ({'placeholder': '{}'}, 'foo { } bar') + ({'placeholder': '{}'}, 'foo { } bar'), ({}, 'foo"'), # not splittable with shlex ]) def test_validate_invalid(self, klass, kwargs, val): From bc012cbaedf144980491dc66d670efa0f80876b4 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 2 Feb 2016 06:50:05 +0100 Subject: [PATCH 7/8] Regenerate authors. --- README.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/README.asciidoc b/README.asciidoc index 43b6b6fca..01b8cce80 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -162,6 +162,7 @@ Contributors, sorted by the number of commits in descending order: * John ShaggyTwoDope Jenkins * Peter Vilim * Tarcisio Fedrizzi +* Oliver Caldwell * Jonas Schürmann * Panagiotis Ktistakis * Jimmy From 37bb26bdd52e37c994ab43e32f3ea52fa3e1362d Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 2 Feb 2016 06:53:12 +0100 Subject: [PATCH 8/8] Add a test in test_editor.py. --- tests/unit/misc/test_editor.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index d97cb6657..dbc4c51ed 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -71,6 +71,13 @@ class TestArg: editor._proc._proc.start.assert_called_with( "bin", ["foo", editor._filename, "bar"]) + def test_placeholder_inline(self, config_stub, editor): + """Test starting editor with placeholder arg inside of another arg.""" + config_stub.data['general']['editor'] = ['bin', 'foo{}', 'bar'] + editor.edit("") + editor._proc._proc.start.assert_called_with( + "bin", ["foo" + editor._filename, "bar"]) + class TestFileHandling: