From f43a5973705602793b256b6e39bfccc394e5f9f1 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Thu, 12 Oct 2017 22:42:40 +0200 Subject: [PATCH 01/39] Add `cursor_position` to `serialize_elem` output --- qutebrowser/javascript/webelem.js | 1 + 1 file changed, 1 insertion(+) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index ce348c46b..a37fcd6c8 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -53,6 +53,7 @@ window._qutebrowser.webelem = (function() { "value": elem.value, "outer_xml": elem.outerHTML, "rects": [], // Gets filled up later + "caret_position": elem.selectionStart, }; // https://github.com/qutebrowser/qutebrowser/issues/2569 From 67e41af8754a26cd84923285ac38f611deb215e2 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Thu, 12 Oct 2017 22:43:06 +0200 Subject: [PATCH 02/39] Add sanity check and accessor for `caret_position` --- qutebrowser/browser/webengine/webengineelem.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qutebrowser/browser/webengine/webengineelem.py b/qutebrowser/browser/webengine/webengineelem.py index c294bebef..522605bba 100644 --- a/qutebrowser/browser/webengine/webengineelem.py +++ b/qutebrowser/browser/webengine/webengineelem.py @@ -47,6 +47,7 @@ class WebEngineElement(webelem.AbstractWebElement): 'class_name': str, 'rects': list, 'attributes': dict, + 'caret_position': int, } assert set(js_dict.keys()).issubset(js_dict_types.keys()) for name, typ in js_dict_types.items(): @@ -132,6 +133,9 @@ class WebEngineElement(webelem.AbstractWebElement): def set_value(self, value): self._js_call('set_value', value) + def caret_position(self): + return self._js_dict.get('caret_position', 0) + def insert_text(self, text): if not self.is_editable(strict=True): raise webelem.Error("Element is not editable!") From cdf4f692518f4c35f23c359d3bad715ea030ae00 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Thu, 12 Oct 2017 22:43:31 +0200 Subject: [PATCH 03/39] Pass `caret_position` to editor's `edit()` --- qutebrowser/browser/commands.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 7a8825720..a8400954e 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1594,10 +1594,12 @@ class CommandDispatcher: return assert isinstance(text, str), text + caret_position = elem.caret_position() + ed = editor.ExternalEditor(self._tabbed_browser) ed.editing_finished.connect(functools.partial( self.on_editing_finished, elem)) - ed.edit(text) + ed.edit(text, caret_position) @cmdutils.register(instance='command-dispatcher', hide=True, scope='window') From 6425061b3ad18354e0c59d78d39003d90132f43a Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Thu, 12 Oct 2017 22:46:05 +0200 Subject: [PATCH 04/39] Substitute new `editor.command` placeholders Added placeholders are: * `{file}` has the same function as `{}` * `{line}` is the 1-based line number * `{column}` is the 1-based column number * `{line0}` like `{line}`, but 0-based * `{column0}` like `{column}` but 0-based --- qutebrowser/misc/editor.py | 62 +++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 3686e028f..54dd736d3 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -96,11 +96,12 @@ class ExternalEditor(QObject): def on_proc_error(self, _err): self._cleanup() - def edit(self, text): + def edit(self, text, caret_position): """Edit a given text. Args: text: The initial text to edit. + caret_position: The position of the caret in the text. """ if self._filename is not None: raise ValueError("Already editing a file!") @@ -121,7 +122,32 @@ class ExternalEditor(QObject): return self._remove_file = True - self._start_editor() + + # Here we calculate the line and column of the caret based on its + # position and the given text. + # + # NOTE: Both line and column are 1-based indexes, because that's what + # most editors use as line and column starting index. + # By "most" we mean at least vim, nvim, gvim, emacs, atom, sublimetext, + # notepad++, brackets, visual studio, QtCreator and so on. + # + # To find the line we just count how many newlines there are before + # the caret and add 1. + # + # To find the column we calculate the difference between the caret and + # the last newline before the caret. + # + # For example in the text `aaa\nbb|bbb` (| represents the caret): + # caret_position = 6 + # text[:caret_position] = `aaa\nbb` + # text[:caret_psotion].count('\n') = 1 + # caret_position - text[:caret_position].rfind('\n') = 3 + # + # Thus line, column = 2, 3, and the caret is indeed in the second + # line, third column + line = text[:caret_position].count('\n') + 1 + column = caret_position - text[:caret_position].rfind('\n') + self._start_editor(line=line, column=column) def edit_file(self, filename): """Edit the file with the given filename.""" @@ -129,13 +155,39 @@ class ExternalEditor(QObject): self._remove_file = False self._start_editor() - def _start_editor(self): - """Start the editor with the file opened as self._filename.""" + def _start_editor(self, line=1, column=1): + """Start the editor with the file opened as self._filename. + + Args: + caret_position: The position of the caret in the text. + """ self._proc = guiprocess.GUIProcess(what='editor', parent=self) self._proc.finished.connect(self.on_proc_closed) self._proc.error.connect(self.on_proc_error) editor = config.val.editor.command executable = editor[0] - args = [arg.replace('{}', self._filename) for arg in editor[1:]] + + args = [self._sub_placeholder(arg, line, column) for arg in editor[1:]] log.procs.debug("Calling \"{}\" with args {}".format(executable, args)) self._proc.start(executable, args) + + def _sub_placeholder(self, possible_placeholder, line, column): + """Substitute a single placeholder. + + The input to this function is not guaranteed to be a valid or known + placeholder. In this case the return value is the unchanged input. + + Args: + possible_placeholder: an argument of editor.command. + + Return: + The substituted placeholder or the original argument + """ + sub = possible_placeholder\ + .replace('{}', self._filename)\ + .replace('{file}', self._filename)\ + .replace('{line}', str(line))\ + .replace('{line0}', str(line-1))\ + .replace('{column}', str(column))\ + .replace('{column0}', str(column-1)) + return sub From ad9ac2191bc1a06a8dbbcdf8e3c24ca6d684554d Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Thu, 12 Oct 2017 23:48:49 +0200 Subject: [PATCH 05/39] Also accept `{file}` placeholder --- qutebrowser/config/configtypes.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index da77401ac..203c4e573 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -1341,9 +1341,12 @@ class ShellCommand(List): if not value: return value - if self.placeholder and '{}' not in ' '.join(value): + if (self.placeholder and + '{}' not in ' '.join(value) and + '{file}' not in ' '.join(value)): raise configexc.ValidationError(value, "needs to contain a " - "{}-placeholder.") + "{}-placeholder or a " + "{file}-placeholder.") return value From 440740d30bf84a242c255ad987a3364f9071e069 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Fri, 13 Oct 2017 20:40:08 +0200 Subject: [PATCH 06/39] Don't crash when opening editor under webkit --- qutebrowser/browser/webkit/webkitelem.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 54dd6519d..e130e4623 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -126,6 +126,14 @@ class WebKitElement(webelem.AbstractWebElement): value = javascript.string_escape(value) self._elem.evaluateJavaScript("this.value='{}'".format(value)) + def caret_position(self): + self._check_vanished() + pos = self._elem.evaluateJavaScript('this.selectionStart') + assert isinstance(pos, (int, float, type(None))) + if pos is None: + return 0 + return int(pos) + def insert_text(self, text): self._check_vanished() if not self.is_editable(strict=True): From b3445bc35a628fecefc51a6f490f705854df5c78 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 17 Oct 2017 22:08:54 +0200 Subject: [PATCH 07/39] Add default value for `caret_position` --- 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 54dd736d3..80df9b66f 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -96,7 +96,7 @@ class ExternalEditor(QObject): def on_proc_error(self, _err): self._cleanup() - def edit(self, text, caret_position): + def edit(self, text, caret_position=0): """Edit a given text. Args: From e508224a46dea915388668c18d0da15a1b0dff4e Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 17 Oct 2017 22:35:01 +0200 Subject: [PATCH 08/39] Avoid the use of chained `replace`s --- qutebrowser/misc/editor.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 80df9b66f..69a83ffb5 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -174,20 +174,30 @@ class ExternalEditor(QObject): def _sub_placeholder(self, possible_placeholder, line, column): """Substitute a single placeholder. - The input to this function is not guaranteed to be a valid or known - placeholder. In this case the return value is the unchanged input. + If the `possible_placeholder` input to this function is valid it will + be substituted with the appropriate value, otherwise it will be left + unchanged. Args: possible_placeholder: an argument of editor.command. + line: the previously-calculated line number for the text caret + column: the previously-calculated column number for the text caret Return: The substituted placeholder or the original argument """ - sub = possible_placeholder\ - .replace('{}', self._filename)\ - .replace('{file}', self._filename)\ - .replace('{line}', str(line))\ - .replace('{line0}', str(line-1))\ - .replace('{column}', str(column))\ - .replace('{column0}', str(column-1)) - return sub + replacements = { + '{}': self._filename, + '{file}': self._filename, + '{line}': str(line), + '{line0}': str(line-1), + '{column}': str(column), + '{column0}': str(column-1) + } + + # The for loop would create a copy anyway, this should be more legible + ph = possible_placeholder + for old, new in replacements.items(): + ph = ph.replace(old, new) + + return ph From 233e72fef190dfd872843ac0d1efcc65dec0ee57 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 17 Oct 2017 22:38:11 +0200 Subject: [PATCH 09/39] Adjust docstring --- qutebrowser/misc/editor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 69a83ffb5..3c7522568 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -159,7 +159,8 @@ class ExternalEditor(QObject): """Start the editor with the file opened as self._filename. Args: - caret_position: The position of the caret in the text. + line: the line number to pass to the editor + column: the column number to pass to the editor """ self._proc = guiprocess.GUIProcess(what='editor', parent=self) self._proc.finished.connect(self.on_proc_closed) From f71053609256f543ca50092560ca4a48a84d1026 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 17 Oct 2017 22:48:43 +0200 Subject: [PATCH 10/39] Move line and column calculation to own function --- qutebrowser/misc/editor.py | 58 ++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 3c7522568..d3d04ee59 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -123,30 +123,7 @@ class ExternalEditor(QObject): self._remove_file = True - # Here we calculate the line and column of the caret based on its - # position and the given text. - # - # NOTE: Both line and column are 1-based indexes, because that's what - # most editors use as line and column starting index. - # By "most" we mean at least vim, nvim, gvim, emacs, atom, sublimetext, - # notepad++, brackets, visual studio, QtCreator and so on. - # - # To find the line we just count how many newlines there are before - # the caret and add 1. - # - # To find the column we calculate the difference between the caret and - # the last newline before the caret. - # - # For example in the text `aaa\nbb|bbb` (| represents the caret): - # caret_position = 6 - # text[:caret_position] = `aaa\nbb` - # text[:caret_psotion].count('\n') = 1 - # caret_position - text[:caret_position].rfind('\n') = 3 - # - # Thus line, column = 2, 3, and the caret is indeed in the second - # line, third column - line = text[:caret_position].count('\n') + 1 - column = caret_position - text[:caret_position].rfind('\n') + line, column = self._calc_line_and_column(text, caret_position) self._start_editor(line=line, column=column) def edit_file(self, filename): @@ -172,6 +149,39 @@ class ExternalEditor(QObject): log.procs.debug("Calling \"{}\" with args {}".format(executable, args)) self._proc.start(executable, args) + def _calc_line_and_column(self, text, caret_position): + """Calculate line and column numbers given a text and caret position + + Args: + text: the text for which the numbers must be calculated + caret_position: the position of the caret in the text + + Return: + A (line, column) tuple of (int, int) + """ + # Both line and column are 1-based indexes, because that's what most + # editors use as line and column starting index. By "most" we mean at + # least vim, nvim, gvim, emacs, atom, sublimetext, notepad++, brackets, + # visual studio, QtCreator and so on. + # + # To find the line we just count how many newlines there are before the + # caret and add 1. + # + # To find the column we calculate the difference between the caret and + # the last newline before the caret. + # + # For example in the text `aaa\nbb|bbb` (| represents the caret): + # caret_position = 6 + # text[:caret_position] = `aaa\nbb` + # text[:caret_psotion].count('\n') = 1 + # caret_position - text[:caret_position].rfind('\n') = 3 + # + # Thus line, column = 2, 3, and the caret is indeed in the second + # line, third column + line = text[:caret_position].count('\n') + 1 + column = caret_position - text[:caret_position].rfind('\n') + return (line, column) + def _sub_placeholder(self, possible_placeholder, line, column): """Substitute a single placeholder. From 06b990c0d16ef840652843c9ad00304419637686 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 17 Oct 2017 23:03:42 +0200 Subject: [PATCH 11/39] Add ShellCommand tests for {file} --- tests/unit/config/test_configtypes.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 9b5e23263..d3a058935 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1818,6 +1818,9 @@ class TestShellCommand: ({'placeholder': '{}'}, '[foo, "{}", bar]', ['foo', '{}', 'bar']), ({'placeholder': '{}'}, '["foo{}bar"]', ['foo{}bar']), ({'placeholder': '{}'}, '[foo, "bar {}"]', ['foo', 'bar {}']), + ({'placeholder': '{file}'}, '[f, "{file}", b]', ['f', '{file}', 'b']), + ({'placeholder': '{file}'}, '["f{file}b"]', ['f{file}b']), + ({'placeholder': '{file}'}, '[f, "b {file}"]', ['f', 'b {file}']), ]) def test_valid(self, klass, kwargs, val, expected): cmd = klass(**kwargs) @@ -1827,6 +1830,8 @@ class TestShellCommand: @pytest.mark.parametrize('kwargs, val', [ ({'placeholder': '{}'}, '[foo, bar]'), ({'placeholder': '{}'}, '[foo, "{", "}", bar'), + ({'placeholder': '{file}'}, '[foo, bar]'), + ({'placeholder': '{file}'}, '[foo, "{fi", "le}", bar'), ]) def test_from_str_invalid(self, klass, kwargs, val): with pytest.raises(configexc.ValidationError): From 6f1b9b79848c85a2262c2f811ad5797a3e076dd2 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 17 Oct 2017 23:19:10 +0200 Subject: [PATCH 12/39] Add tests for line & column calculation --- tests/unit/misc/test_editor.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index d97a39f7b..75f0dd59c 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -157,6 +157,24 @@ class TestFileHandling: editor.edit("") +class TestLineColumnCalculation: + + """Test calculation for line and column given text and caret_position""" + + @pytest.mark.parametrize('text, caret_position, result', [ + ('', 0, (1, 1)), + ('a', 0, (1, 1)), + ('a\nb', 1, (1, 2)), + ('a\nb', 2, (2, 1)), + ('a\nb', 3, (2, 2)), + ('a\nbb\nccc', 4, (2, 3)), + ('a\nbb\nccc', 5, (3, 1)), + ('a\nbb\nccc', 8, (3, 4)), + ]) + def test_calculation(self, editor, text, caret_position, result): + assert editor._calc_line_and_column(text, caret_position) == result + + @pytest.mark.parametrize('initial_text, edited_text', [ ('', 'Hello'), ('Hello', 'World'), From addccd749236f1d43fb1cec25f64a986641cc2e2 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 20:19:09 +0200 Subject: [PATCH 13/39] Move comment to docstring and fix typo --- qutebrowser/misc/editor.py | 39 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index d3d04ee59..e4fc3bd35 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -152,6 +152,26 @@ class ExternalEditor(QObject): def _calc_line_and_column(self, text, caret_position): """Calculate line and column numbers given a text and caret position + Both line and column are 1-based indexes, because that's what most + editors use as line and column starting index. By "most" we mean at + least vim, nvim, gvim, emacs, atom, sublimetext, notepad++, brackets, + visual studio, QtCreator and so on. + + To find the line we just count how many newlines there are before the + caret and add 1. + + To find the column we calculate the difference between the caret and + the last newline before the caret. + + For example in the text `aaa\nbb|bbb` (| represents the caret): + caret_position = 6 + text[:caret_position] = `aaa\nbb` + text[:caret_position].count('\n') = 1 + caret_position - text[:caret_position].rfind('\n') = 3 + + Thus line, column = 2, 3, and the caret is indeed in the second + line, third column + Args: text: the text for which the numbers must be calculated caret_position: the position of the caret in the text @@ -159,25 +179,6 @@ class ExternalEditor(QObject): Return: A (line, column) tuple of (int, int) """ - # Both line and column are 1-based indexes, because that's what most - # editors use as line and column starting index. By "most" we mean at - # least vim, nvim, gvim, emacs, atom, sublimetext, notepad++, brackets, - # visual studio, QtCreator and so on. - # - # To find the line we just count how many newlines there are before the - # caret and add 1. - # - # To find the column we calculate the difference between the caret and - # the last newline before the caret. - # - # For example in the text `aaa\nbb|bbb` (| represents the caret): - # caret_position = 6 - # text[:caret_position] = `aaa\nbb` - # text[:caret_psotion].count('\n') = 1 - # caret_position - text[:caret_position].rfind('\n') = 3 - # - # Thus line, column = 2, 3, and the caret is indeed in the second - # line, third column line = text[:caret_position].count('\n') + 1 column = caret_position - text[:caret_position].rfind('\n') return (line, column) From 7907840ead0341d9afc2b9730ce99d8511de5160 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 20:19:39 +0200 Subject: [PATCH 14/39] Add full stops --- qutebrowser/misc/editor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index e4fc3bd35..85c552c8c 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -192,11 +192,11 @@ class ExternalEditor(QObject): Args: possible_placeholder: an argument of editor.command. - line: the previously-calculated line number for the text caret - column: the previously-calculated column number for the text caret + line: the previously-calculated line number for the text caret. + column: the previously-calculated column number for the text caret. Return: - The substituted placeholder or the original argument + The substituted placeholder or the original argument. """ replacements = { '{}': self._filename, From cf04219f790b2cde982774d728ddc85258fb4551 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 20:20:05 +0200 Subject: [PATCH 15/39] Rename `possible_placeholder` to `arg` --- qutebrowser/misc/editor.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 85c552c8c..bbc4fe25e 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -183,15 +183,15 @@ class ExternalEditor(QObject): column = caret_position - text[:caret_position].rfind('\n') return (line, column) - def _sub_placeholder(self, possible_placeholder, line, column): + def _sub_placeholder(self, arg, line, column): """Substitute a single placeholder. - If the `possible_placeholder` input to this function is valid it will + If the `arg` input to this function is a valid placeholder it will be substituted with the appropriate value, otherwise it will be left unchanged. Args: - possible_placeholder: an argument of editor.command. + arg: an argument of editor.command. line: the previously-calculated line number for the text caret. column: the previously-calculated column number for the text caret. @@ -207,9 +207,7 @@ class ExternalEditor(QObject): '{column0}': str(column-1) } - # The for loop would create a copy anyway, this should be more legible - ph = possible_placeholder for old, new in replacements.items(): - ph = ph.replace(old, new) + arg = arg.replace(old, new) - return ph + return arg From 0d7a557396a74690cdfcf208c1fd7ed580d04161 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 20:22:32 +0200 Subject: [PATCH 16/39] Fix configtypes tests so that placeholder is True --- tests/unit/config/test_configtypes.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index d3a058935..7b77ef8f3 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1815,12 +1815,12 @@ class TestShellCommand: @pytest.mark.parametrize('kwargs, val, expected', [ ({}, '[foobar]', ['foobar']), - ({'placeholder': '{}'}, '[foo, "{}", bar]', ['foo', '{}', 'bar']), - ({'placeholder': '{}'}, '["foo{}bar"]', ['foo{}bar']), - ({'placeholder': '{}'}, '[foo, "bar {}"]', ['foo', 'bar {}']), - ({'placeholder': '{file}'}, '[f, "{file}", b]', ['f', '{file}', 'b']), - ({'placeholder': '{file}'}, '["f{file}b"]', ['f{file}b']), - ({'placeholder': '{file}'}, '[f, "b {file}"]', ['f', 'b {file}']), + ({'placeholder': True}, '[foo, "{}", bar]', ['foo', '{}', 'bar']), + ({'placeholder': True}, '["foo{}bar"]', ['foo{}bar']), + ({'placeholder': True}, '[foo, "bar {}"]', ['foo', 'bar {}']), + ({'placeholder': True}, '[f, "{file}", b]', ['f', '{file}', 'b']), + ({'placeholder': True}, '["f{file}b"]', ['f{file}b']), + ({'placeholder': True}, '[f, "b {file}"]', ['f', 'b {file}']), ]) def test_valid(self, klass, kwargs, val, expected): cmd = klass(**kwargs) @@ -1828,10 +1828,10 @@ class TestShellCommand: assert cmd.to_py(expected) == expected @pytest.mark.parametrize('kwargs, val', [ - ({'placeholder': '{}'}, '[foo, bar]'), - ({'placeholder': '{}'}, '[foo, "{", "}", bar'), - ({'placeholder': '{file}'}, '[foo, bar]'), - ({'placeholder': '{file}'}, '[foo, "{fi", "le}", bar'), + ({'placeholder': True}, '[foo, bar]'), + ({'placeholder': True}, '[foo, "{", "}", bar'), + ({'placeholder': True}, '[foo, bar]'), + ({'placeholder': True}, '[foo, "{fi", "le}", bar'), ]) def test_from_str_invalid(self, klass, kwargs, val): with pytest.raises(configexc.ValidationError): From 937d0d068847dfcafe41f18c593a35c95767da0a Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 20:30:16 +0200 Subject: [PATCH 17/39] Add some more tests --- tests/unit/config/test_configtypes.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 7b77ef8f3..36988a0a7 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1832,6 +1832,11 @@ class TestShellCommand: ({'placeholder': True}, '[foo, "{", "}", bar'), ({'placeholder': True}, '[foo, bar]'), ({'placeholder': True}, '[foo, "{fi", "le}", bar'), + + # Like valid ones but with wrong placeholder + ({'placeholder': True}, '[f, "{wrong}", b]'), + ({'placeholder': True}, '["f{wrong}b"]'), + ({'placeholder': True}, '[f, "b {wrong}"]'), ]) def test_from_str_invalid(self, klass, kwargs, val): with pytest.raises(configexc.ValidationError): From 9b177ae8e7fe8927088ac7cc8d1ed20f63f8d21f Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 20:33:14 +0200 Subject: [PATCH 18/39] Remove single-function test class (move test out) --- tests/unit/misc/test_editor.py | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index 75f0dd59c..62abe7461 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -157,24 +157,6 @@ class TestFileHandling: editor.edit("") -class TestLineColumnCalculation: - - """Test calculation for line and column given text and caret_position""" - - @pytest.mark.parametrize('text, caret_position, result', [ - ('', 0, (1, 1)), - ('a', 0, (1, 1)), - ('a\nb', 1, (1, 2)), - ('a\nb', 2, (2, 1)), - ('a\nb', 3, (2, 2)), - ('a\nbb\nccc', 4, (2, 3)), - ('a\nbb\nccc', 5, (3, 1)), - ('a\nbb\nccc', 8, (3, 4)), - ]) - def test_calculation(self, editor, text, caret_position, result): - assert editor._calc_line_and_column(text, caret_position) == result - - @pytest.mark.parametrize('initial_text, edited_text', [ ('', 'Hello'), ('Hello', 'World'), @@ -195,3 +177,18 @@ def test_modify(qtbot, editor, initial_text, edited_text): editor._proc.finished.emit(0, QProcess.NormalExit) assert blocker.args == [edited_text] + + +@pytest.mark.parametrize('text, caret_position, result', [ + ('', 0, (1, 1)), + ('a', 0, (1, 1)), + ('a\nb', 1, (1, 2)), + ('a\nb', 2, (2, 1)), + ('a\nb', 3, (2, 2)), + ('a\nbb\nccc', 4, (2, 3)), + ('a\nbb\nccc', 5, (3, 1)), + ('a\nbb\nccc', 8, (3, 4)), +]) +def test_calculation(editor, text, caret_position, result): + """Test calculation for line and column given text and caret_position""" + assert editor._calc_line_and_column(text, caret_position) == result From 04365262036ef40cca9f4a5ab7e7d7d832a15258 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 21:08:22 +0200 Subject: [PATCH 19/39] Change default editor command --- qutebrowser/config/configdata.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index 9f47a7d70..1d6dd601a 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -746,11 +746,11 @@ editor.command: type: name: ShellCommand placeholder: true - default: ["gvim", "-f", "{}"] + default: ["gvim", "-f", "{file}", "-c", "normal {line}G{column0}l"] desc: >- The editor (and arguments) to use for the `open-editor` command. - `{}` gets replaced by the filename of the file to be edited. + `{file}` gets replaced by the filename of the file to be edited. editor.encoding: type: Encoding From 9613deb89da6462300dcd1e245aabb2d5ebe0a34 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 18 Oct 2017 21:20:05 +0200 Subject: [PATCH 20/39] Document new `editor.command` placeholders --- doc/help/settings.asciidoc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index d5511dff0..31aff3740 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -1913,7 +1913,14 @@ Default: +pass:[-1]+ [[editor.command]] === editor.command The editor (and arguments) to use for the `open-editor` command. -`{}` gets replaced by the filename of the file to be edited. +Several placeholders are supported. Placeholders are substituted by the +respective value when executing the command. + +`{file}` gets replaced by the filename of the file to be edited. +`{line}` gets replaced by the line in which the caret is found in the text. +`{column}` gets replaced by the column in which the caret is found in the text. +`{line0}` same as `{line}`, but starting from index 0 +`{column0}` same as `{column}`, but starting from index 0 Type: <> From 8b91a74aefd3799445ec82f99f9cfe4dc406c94a Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Sun, 22 Oct 2017 20:02:06 +0200 Subject: [PATCH 21/39] Fix broken test after default config change --- tests/unit/config/test_configcommands.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/config/test_configcommands.py b/tests/unit/config/test_configcommands.py index 1841e6260..a44327bd2 100644 --- a/tests/unit/config/test_configcommands.py +++ b/tests/unit/config/test_configcommands.py @@ -60,8 +60,9 @@ class TestSet: @pytest.mark.parametrize('option, old_value, inp, new_value', [ ('url.auto_search', 'naive', 'dns', 'dns'), # https://github.com/qutebrowser/qutebrowser/issues/2962 - ('editor.command', ['gvim', '-f', '{}'], '[emacs, "{}"]', - ['emacs', '{}']), + ('editor.command', + ['gvim', '-f', '{file}', '-c', 'normal {line}G{column0}l'], + '[emacs, "{}"]', ['emacs', '{}']), ]) def test_set_simple(self, monkeypatch, commands, config_stub, temp, option, old_value, inp, new_value): From 96bbdb19e6e678924c412ff530b1b715c73c97ea Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Sun, 22 Oct 2017 20:02:32 +0200 Subject: [PATCH 22/39] Add missing docstrings --- qutebrowser/browser/webengine/webengineelem.py | 1 + qutebrowser/browser/webkit/webkitelem.py | 1 + 2 files changed, 2 insertions(+) diff --git a/qutebrowser/browser/webengine/webengineelem.py b/qutebrowser/browser/webengine/webengineelem.py index 522605bba..fe8541e77 100644 --- a/qutebrowser/browser/webengine/webengineelem.py +++ b/qutebrowser/browser/webengine/webengineelem.py @@ -134,6 +134,7 @@ class WebEngineElement(webelem.AbstractWebElement): self._js_call('set_value', value) def caret_position(self): + """Get the text caret position for the current element""" return self._js_dict.get('caret_position', 0) def insert_text(self, text): diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index e130e4623..4ddaf5e51 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -127,6 +127,7 @@ class WebKitElement(webelem.AbstractWebElement): self._elem.evaluateJavaScript("this.value='{}'".format(value)) def caret_position(self): + """Get the text caret position for the current element""" self._check_vanished() pos = self._elem.evaluateJavaScript('this.selectionStart') assert isinstance(pos, (int, float, type(None))) From 1f521da134ad1e3f4888379c3711f96d83773e20 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Sun, 22 Oct 2017 20:03:46 +0200 Subject: [PATCH 23/39] Add missing full-stops --- doc/help/settings.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 31aff3740..991596f3d 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -1919,8 +1919,8 @@ respective value when executing the command. `{file}` gets replaced by the filename of the file to be edited. `{line}` gets replaced by the line in which the caret is found in the text. `{column}` gets replaced by the column in which the caret is found in the text. -`{line0}` same as `{line}`, but starting from index 0 -`{column0}` same as `{column}`, but starting from index 0 +`{line0}` same as `{line}`, but starting from index 0. +`{column0}` same as `{column}`, but starting from index 0. Type: <> From f195b7e4d24851035a7c1cf127cd4f52195b4f9f Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 25 Oct 2017 21:18:53 +0200 Subject: [PATCH 24/39] Fix flake8 failures --- qutebrowser/browser/webengine/webengineelem.py | 2 +- qutebrowser/browser/webkit/webkitelem.py | 2 +- qutebrowser/misc/editor.py | 2 +- tests/unit/misc/test_editor.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/webengine/webengineelem.py b/qutebrowser/browser/webengine/webengineelem.py index fe8541e77..fb528a93d 100644 --- a/qutebrowser/browser/webengine/webengineelem.py +++ b/qutebrowser/browser/webengine/webengineelem.py @@ -134,7 +134,7 @@ class WebEngineElement(webelem.AbstractWebElement): self._js_call('set_value', value) def caret_position(self): - """Get the text caret position for the current element""" + """Get the text caret position for the current element.""" return self._js_dict.get('caret_position', 0) def insert_text(self, text): diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 4ddaf5e51..86c701234 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -127,7 +127,7 @@ class WebKitElement(webelem.AbstractWebElement): self._elem.evaluateJavaScript("this.value='{}'".format(value)) def caret_position(self): - """Get the text caret position for the current element""" + """Get the text caret position for the current element.""" self._check_vanished() pos = self._elem.evaluateJavaScript('this.selectionStart') assert isinstance(pos, (int, float, type(None))) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index bbc4fe25e..2fd1d8597 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -150,7 +150,7 @@ class ExternalEditor(QObject): self._proc.start(executable, args) def _calc_line_and_column(self, text, caret_position): - """Calculate line and column numbers given a text and caret position + r"""Calculate line and column numbers given a text and caret position. Both line and column are 1-based indexes, because that's what most editors use as line and column starting index. By "most" we mean at diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index 62abe7461..683b38841 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -190,5 +190,5 @@ def test_modify(qtbot, editor, initial_text, edited_text): ('a\nbb\nccc', 8, (3, 4)), ]) def test_calculation(editor, text, caret_position, result): - """Test calculation for line and column given text and caret_position""" + """Test calculation for line and column given text and caret_position.""" assert editor._calc_line_and_column(text, caret_position) == result From 3fd7fb3e14854306a801c9caa9bb2a89be150718 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 25 Oct 2017 22:38:44 +0200 Subject: [PATCH 25/39] Do not assume elem.selectionStart exists --- qutebrowser/javascript/webelem.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index a37fcd6c8..7f17a1209 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -48,12 +48,20 @@ window._qutebrowser.webelem = (function() { var id = elements.length; elements[id] = elem; + // InvalidStateError will be thrown if elem doesn't have selectionStart + var caret_position = 0; + try { + caret_position = elem.selectionStart; + } catch (e) { + // nothing to do, caret_position is already 0 + } + var out = { "id": id, "value": elem.value, "outer_xml": elem.outerHTML, "rects": [], // Gets filled up later - "caret_position": elem.selectionStart, + "caret_position": caret_position, }; // https://github.com/qutebrowser/qutebrowser/issues/2569 From ae2dad7d188ed1c7f43bf99bd99166a56b0e2f42 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 25 Oct 2017 22:43:17 +0200 Subject: [PATCH 26/39] Only catch the correct exception --- qutebrowser/javascript/webelem.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 7f17a1209..06f56bf1b 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -53,7 +53,9 @@ window._qutebrowser.webelem = (function() { try { caret_position = elem.selectionStart; } catch (e) { - // nothing to do, caret_position is already 0 + if (e instanceof DOMException && e.name === "InvalidStateError") { + // nothing to do, caret_position is already 0 + } } var out = { From ff7edf79e7ec5bbb6923420b99a7e90d527115c8 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Wed, 25 Oct 2017 22:53:54 +0200 Subject: [PATCH 27/39] Rethrow exception if we can't handle it --- qutebrowser/javascript/webelem.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 06f56bf1b..283f772b5 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -55,6 +55,9 @@ window._qutebrowser.webelem = (function() { } catch (e) { if (e instanceof DOMException && e.name === "InvalidStateError") { // nothing to do, caret_position is already 0 + } else { + // not the droid we're looking for + throw e } } From 2947b75ab9ce0f1d2852d0f602225414de0767a5 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Fri, 27 Oct 2017 19:52:10 +0200 Subject: [PATCH 28/39] Make eslint happy --- qutebrowser/javascript/webelem.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 283f772b5..0a23ad543 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -52,12 +52,15 @@ window._qutebrowser.webelem = (function() { var caret_position = 0; try { caret_position = elem.selectionStart; - } catch (e) { - if (e instanceof DOMException && e.name === "InvalidStateError") { + } catch (err) { + if ( + err instanceof DOMException && + err.name === "InvalidStateError" + ) { // nothing to do, caret_position is already 0 } else { // not the droid we're looking for - throw e + throw err; } } From dc26808a941910ab41176f8f393de0b912241598 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 31 Oct 2017 14:41:36 +0100 Subject: [PATCH 29/39] Fix setting names in FAQ --- doc/faq.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/faq.asciidoc b/doc/faq.asciidoc index 75fe5ee8c..2091b9e0b 100644 --- a/doc/faq.asciidoc +++ b/doc/faq.asciidoc @@ -152,7 +152,7 @@ For QtWebEngine: `:set spellcheck.languages "['en-US', 'pl-PL']"` How do I use Tor with qutebrowser?:: - Start tor on your machine, and do `:set network proxy socks://localhost:9050/` + Start tor on your machine, and do `:set content.proxy socks://localhost:9050/` in qutebrowser. Note this won't give you the same amount of fingerprinting protection that the Tor Browser does, but it's useful to be able to access `.onion` sites. @@ -162,7 +162,7 @@ Why does J move to the next (right) tab, and K to the previous (left) one?:: and qutebrowser's keybindings are designed to be compatible with dwb's. The rationale behind it is that J is "down" in vim, and K is "up", which corresponds nicely to "next"/"previous". It also makes much more sense with - vertical tabs (e.g. `:set tabs position left`). + vertical tabs (e.g. `:set tabs.position left`). What's the difference between insert and passthrough mode?:: They are quite similar, but insert mode has some bindings (like `Ctrl-e` to From 370405c0ed5fb15dec7f59bca31b35657ee363dd Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 31 Oct 2017 23:20:13 +0100 Subject: [PATCH 30/39] Remove assert --- qutebrowser/browser/webkit/webkitelem.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 86c701234..2a1eafc9e 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -130,7 +130,6 @@ class WebKitElement(webelem.AbstractWebElement): """Get the text caret position for the current element.""" self._check_vanished() pos = self._elem.evaluateJavaScript('this.selectionStart') - assert isinstance(pos, (int, float, type(None))) if pos is None: return 0 return int(pos) From bc0c877b8729d3f155292881f0171e77f2f0441d Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 31 Oct 2017 23:21:37 +0100 Subject: [PATCH 31/39] Formatting --- qutebrowser/javascript/webelem.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index 0a23ad543..cbd1b16d8 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -53,10 +53,8 @@ window._qutebrowser.webelem = (function() { try { caret_position = elem.selectionStart; } catch (err) { - if ( - err instanceof DOMException && - err.name === "InvalidStateError" - ) { + if (err instanceof DOMException && + err.name === "InvalidStateError") { // nothing to do, caret_position is already 0 } else { // not the droid we're looking for From 24231ae4059e272dfeb42d4ccdc55cbaa8704ad7 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 31 Oct 2017 23:22:17 +0100 Subject: [PATCH 32/39] Remove unnecessary parens --- 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 2fd1d8597..2be96e012 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -181,7 +181,7 @@ class ExternalEditor(QObject): """ line = text[:caret_position].count('\n') + 1 column = caret_position - text[:caret_position].rfind('\n') - return (line, column) + return line, column def _sub_placeholder(self, arg, line, column): """Substitute a single placeholder. From cb7e6ab02d22995bff0728ee5ee78820221f2d2e Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Tue, 31 Oct 2017 19:06:39 -0400 Subject: [PATCH 33/39] Abort pinned tab prompt if tab is destroyed Closes #3223 --- qutebrowser/mainwindow/tabbedbrowser.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py index 48803f1c8..5c3b10148 100644 --- a/qutebrowser/mainwindow/tabbedbrowser.py +++ b/qutebrowser/mainwindow/tabbedbrowser.py @@ -259,13 +259,14 @@ class TabbedBrowser(tabwidget.TabWidget): def tab_close_prompt_if_pinned(self, tab, force, yes_action): """Helper method for tab_close. - If tab is pinned, prompt. If everything is good, run yes_action. + If tab is pinned, prompt. If not, run yes_action. + If tab is destroyed, abort question. """ if tab.data.pinned and not force: message.confirm_async( title='Pinned Tab', text="Are you sure you want to close a pinned tab?", - yes_action=yes_action, default=False) + yes_action=yes_action, default=False, abort_on=[tab.destroyed]) else: yes_action() From 385337eb90966706960c618b75c9e6854fea1bcf Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 1 Nov 2017 09:24:57 +0100 Subject: [PATCH 34/39] Use lts version of NopeJS Looks like npm doesn't work with Node v9: https://github.com/nodejs/node/issues/16649 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 18d1dd416..65d917d73 100644 --- a/.travis.yml +++ b/.travis.yml @@ -51,7 +51,7 @@ matrix: env: TESTENV=eslint language: node_js python: null - node_js: node + node_js: "lts/*" fast_finish: true cache: From bb54a954fe58c45d4fb8e2a25833ecc1d0f422f2 Mon Sep 17 00:00:00 2001 From: Gyorgy Orban Date: Mon, 23 Oct 2017 11:22:56 +0200 Subject: [PATCH 35/39] use subprocess run The usage of subprocess.run is recommended since python 3.5. Popen, check_call, call and check_output calls were replaced with run. --- qutebrowser/app.py | 4 +- qutebrowser/utils/version.py | 10 +++-- scripts/asciidoc2html.py | 10 ++--- scripts/dev/build_release.py | 39 ++++++++++--------- scripts/dev/check_coverage.py | 8 ++-- scripts/dev/check_doc_changes.py | 5 ++- scripts/dev/gen_resources.py | 2 +- scripts/dev/get_coredumpctl_traces.py | 20 +++++----- scripts/dev/misc_checks.py | 3 +- scripts/dev/recompile_requirements.py | 8 ++-- scripts/dev/run_profile.py | 11 +++--- scripts/dev/run_pylint_on_tests.py | 2 +- scripts/dev/segfault_test.py | 10 ++--- scripts/dev/src2asciidoc.py | 6 +-- scripts/link_pyqt.py | 6 ++- scripts/setupcommon.py | 10 +++-- tests/end2end/features/test_qutescheme_bdd.py | 6 +-- tests/end2end/test_invocations.py | 7 ++-- tests/unit/misc/test_checkpyver.py | 7 ++-- tests/unit/utils/test_standarddir.py | 5 ++- tests/unit/utils/test_version.py | 17 ++++---- 21 files changed, 105 insertions(+), 91 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 2ed579f61..02eb9bf18 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -660,9 +660,9 @@ class Quitter: try: args, cwd = self._get_restart_args(pages, session, override_args) if cwd is None: - subprocess.Popen(args) + subprocess.run(args) else: - subprocess.Popen(args, cwd=cwd) + subprocess.run(args, cwd=cwd) except OSError: log.destroy.exception("Failed to restart") return False diff --git a/qutebrowser/utils/version.py b/qutebrowser/utils/version.py index bc04c7524..547185623 100644 --- a/qutebrowser/utils/version.py +++ b/qutebrowser/utils/version.py @@ -151,12 +151,14 @@ def _git_str_subprocess(gitpath): return None try: # https://stackoverflow.com/questions/21017300/21017394#21017394 - commit_hash = subprocess.check_output( + commit_hash = subprocess.run( ['git', 'describe', '--match=NeVeRmAtCh', '--always', '--dirty'], - cwd=gitpath).decode('UTF-8').strip() - date = subprocess.check_output( + cwd=gitpath, check=True, + stdout=subprocess.PIPE).stdout.decode('UTF-8').strip() + date = subprocess.run( ['git', 'show', '-s', '--format=%ci', 'HEAD'], - cwd=gitpath).decode('UTF-8').strip() + cwd=gitpath, check=True, + stdout=subprocess.PIPE).stdout.decode('UTF-8').strip() return '{} ({})'.format(commit_hash, date) except (subprocess.CalledProcessError, OSError): return None diff --git a/scripts/asciidoc2html.py b/scripts/asciidoc2html.py index 51deb009f..4e9c7544b 100755 --- a/scripts/asciidoc2html.py +++ b/scripts/asciidoc2html.py @@ -224,16 +224,16 @@ class AsciiDoc: return self._args.asciidoc try: - subprocess.call(['asciidoc'], stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL) + subprocess.run(['asciidoc'], stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL) except OSError: pass else: return ['asciidoc'] try: - subprocess.call(['asciidoc.py'], stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL) + subprocess.run(['asciidoc.py'], stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL) except OSError: pass else: @@ -258,7 +258,7 @@ class AsciiDoc: try: env = os.environ.copy() env['HOME'] = self._homedir - subprocess.check_call(cmdline, env=env) + subprocess.run(cmdline, check=True, env=env) except (subprocess.CalledProcessError, OSError) as e: self._failed = True utils.print_col(str(e), 'red') diff --git a/scripts/dev/build_release.py b/scripts/dev/build_release.py index 0dc45f2e2..65d1d651b 100755 --- a/scripts/dev/build_release.py +++ b/scripts/dev/build_release.py @@ -50,7 +50,7 @@ def call_script(name, *args, python=sys.executable): python: The python interpreter to use. """ path = os.path.join(os.path.dirname(__file__), os.pardir, name) - subprocess.check_call([python, path] + list(args)) + subprocess.run([python, path] + list(args), check=True) def call_tox(toxenv, *args, python=sys.executable): @@ -64,9 +64,9 @@ def call_tox(toxenv, *args, python=sys.executable): env = os.environ.copy() env['PYTHON'] = python env['PATH'] = os.environ['PATH'] + os.pathsep + os.path.dirname(python) - subprocess.check_call( + subprocess.run( [sys.executable, '-m', 'tox', '-vv', '-e', toxenv] + list(args), - env=env) + env=env, check=True) def run_asciidoc2html(args): @@ -89,8 +89,9 @@ def _maybe_remove(path): def smoke_test(executable): """Try starting the given qutebrowser executable.""" - subprocess.check_call([executable, '--no-err-windows', '--nowindow', - '--temp-basedir', 'about:blank', ':later 500 quit']) + subprocess.run([executable, '--no-err-windows', '--nowindow', + '--temp-basedir', 'about:blank', ':later 500 quit'], + check=True) def patch_mac_app(): @@ -178,7 +179,7 @@ def build_mac(): utils.print_title("Patching .app") patch_mac_app() utils.print_title("Building .dmg") - subprocess.check_call(['make', '-f', 'scripts/dev/Makefile-dmg']) + subprocess.run(['make', '-f', 'scripts/dev/Makefile-dmg'], check=True) dmg_name = 'qutebrowser-{}.dmg'.format(qutebrowser.__version__) os.rename('qutebrowser.dmg', dmg_name) @@ -187,14 +188,14 @@ def build_mac(): try: with tempfile.TemporaryDirectory() as tmpdir: - subprocess.check_call(['hdiutil', 'attach', dmg_name, - '-mountpoint', tmpdir]) + subprocess.run(['hdiutil', 'attach', dmg_name, + '-mountpoint', tmpdir], check=True) try: binary = os.path.join(tmpdir, 'qutebrowser.app', 'Contents', 'MacOS', 'qutebrowser') smoke_test(binary) finally: - subprocess.call(['hdiutil', 'detach', tmpdir]) + subprocess.run(['hdiutil', 'detach', tmpdir]) except PermissionError as e: print("Failed to remove tempdir: {}".format(e)) @@ -242,13 +243,13 @@ def build_windows(): patch_windows(out_64) utils.print_title("Building installers") - subprocess.check_call(['makensis.exe', - '/DVERSION={}'.format(qutebrowser.__version__), - 'misc/qutebrowser.nsi']) - subprocess.check_call(['makensis.exe', - '/DX64', - '/DVERSION={}'.format(qutebrowser.__version__), - 'misc/qutebrowser.nsi']) + subprocess.run(['makensis.exe', + '/DVERSION={}'.format(qutebrowser.__version__), + 'misc/qutebrowser.nsi'], check=True) + subprocess.run(['makensis.exe', + '/DX64', + '/DVERSION={}'.format(qutebrowser.__version__), + 'misc/qutebrowser.nsi'], check=True) name_32 = 'qutebrowser-{}-win32.exe'.format(qutebrowser.__version__) name_64 = 'qutebrowser-{}-amd64.exe'.format(qutebrowser.__version__) @@ -292,12 +293,12 @@ def build_sdist(): _maybe_remove('dist') - subprocess.check_call([sys.executable, 'setup.py', 'sdist']) + subprocess.run([sys.executable, 'setup.py', 'sdist'], check=True) dist_files = os.listdir(os.path.abspath('dist')) assert len(dist_files) == 1 dist_file = os.path.join('dist', dist_files[0]) - subprocess.check_call(['gpg', '--detach-sign', '-a', dist_file]) + subprocess.run(['gpg', '--detach-sign', '-a', dist_file], check=True) tar = tarfile.open(dist_file) by_ext = collections.defaultdict(list) @@ -366,7 +367,7 @@ def github_upload(artifacts, tag): def pypi_upload(artifacts): """Upload the given artifacts to PyPI using twine.""" filenames = [a[0] for a in artifacts] - subprocess.check_call(['twine', 'upload'] + filenames) + subprocess.run(['twine', 'upload'] + filenames, check=True) def main(): diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index ba4e7a7a7..f2c9dd853 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -285,8 +285,8 @@ def main_check(): print(msg.text) print() filters = ','.join('qutebrowser/' + msg.filename for msg in messages) - subprocess.check_call([sys.executable, '-m', 'coverage', 'report', - '--show-missing', '--include', filters]) + subprocess.run([sys.executable, '-m', 'coverage', 'report', + '--show-missing', '--include', filters], check=True) print() print("To debug this, run 'tox -e py36-pyqt59-cov' " "(or py35-pyqt59-cov) locally and check htmlcov/index.html") @@ -312,9 +312,9 @@ def main_check_all(): for test_file, src_file in PERFECT_FILES: if test_file is None: continue - subprocess.check_call( + subprocess.run( [sys.executable, '-m', 'pytest', '--cov', 'qutebrowser', - '--cov-report', 'xml', test_file]) + '--cov-report', 'xml', test_file], check=True) with open('coverage.xml', encoding='utf-8') as f: messages = check(f, [(test_file, src_file)]) os.remove('coverage.xml') diff --git a/scripts/dev/check_doc_changes.py b/scripts/dev/check_doc_changes.py index 7fc993283..1cc2a90f9 100755 --- a/scripts/dev/check_doc_changes.py +++ b/scripts/dev/check_doc_changes.py @@ -24,7 +24,8 @@ import sys import subprocess import os -code = subprocess.call(['git', '--no-pager', 'diff', '--exit-code', '--stat']) +code = subprocess.run(['git', '--no-pager', 'diff', + '--exit-code', '--stat']).returncode if os.environ.get('TRAVIS_PULL_REQUEST', 'false') != 'false': if code != 0: @@ -42,6 +43,6 @@ if code != 0: if 'TRAVIS' in os.environ: print() print("travis_fold:start:gitdiff") - subprocess.call(['git', '--no-pager', 'diff']) + subprocess.run(['git', '--no-pager', 'diff']) print("travis_fold:end:gitdiff") sys.exit(code) diff --git a/scripts/dev/gen_resources.py b/scripts/dev/gen_resources.py index 12bf5b97c..cbfc69b6f 100644 --- a/scripts/dev/gen_resources.py +++ b/scripts/dev/gen_resources.py @@ -23,4 +23,4 @@ import subprocess with open('qutebrowser/resources.py', 'w', encoding='utf-8') as f: - subprocess.check_call(['pyrcc5', 'qutebrowser.rcc'], stdout=f) + subprocess.run(['pyrcc5', 'qutebrowser.rcc'], stdout=f, check=True) diff --git a/scripts/dev/get_coredumpctl_traces.py b/scripts/dev/get_coredumpctl_traces.py index 6d1a41a11..711bc52c0 100644 --- a/scripts/dev/get_coredumpctl_traces.py +++ b/scripts/dev/get_coredumpctl_traces.py @@ -84,7 +84,8 @@ def parse_coredumpctl_line(line): def get_info(pid): """Get and parse "coredumpctl info" output for the given PID.""" data = {} - output = subprocess.check_output(['coredumpctl', 'info', str(pid)]) + output = subprocess.run(['coredumpctl', 'info', str(pid)], check=True, + stdout=subprocess.PIPE).stdout output = output.decode('utf-8') for line in output.split('\n'): if not line.strip(): @@ -117,12 +118,12 @@ def dump_infos_gdb(parsed): """Dump all needed infos for the given crash using gdb.""" with tempfile.TemporaryDirectory() as tempdir: coredump = os.path.join(tempdir, 'dump') - subprocess.check_call(['coredumpctl', 'dump', '-o', coredump, - str(parsed.pid)]) - subprocess.check_call(['gdb', parsed.exe, coredump, - '-ex', 'info threads', - '-ex', 'thread apply all bt full', - '-ex', 'quit']) + subprocess.run(['coredumpctl', 'dump', '-o', coredump, + str(parsed.pid)], check=True) + subprocess.run(['gdb', parsed.exe, coredump, + '-ex', 'info threads', + '-ex', 'thread apply all bt full', + '-ex', 'quit'], check=True) def dump_infos(parsed): @@ -143,7 +144,7 @@ def check_prerequisites(): """Check if coredumpctl/gdb are installed.""" for binary in ['coredumpctl', 'gdb']: try: - subprocess.check_call([binary, '--version']) + subprocess.run([binary, '--version'], check=True) except FileNotFoundError: print("{} is needed to run this script!".format(binary), file=sys.stderr) @@ -158,7 +159,8 @@ def main(): action='store_true') args = parser.parse_args() - coredumps = subprocess.check_output(['coredumpctl', 'list']) + coredumps = subprocess.run(['coredumpctl', 'list'], check=True, + stdout=subprocess.PIPE).stdout lines = coredumps.decode('utf-8').split('\n') for line in lines[1:]: if not line.strip(): diff --git a/scripts/dev/misc_checks.py b/scripts/dev/misc_checks.py index 1bb263d15..e1517391d 100644 --- a/scripts/dev/misc_checks.py +++ b/scripts/dev/misc_checks.py @@ -62,7 +62,8 @@ def check_git(): print() return False untracked = [] - gitst = subprocess.check_output(['git', 'status', '--porcelain']) + gitst = subprocess.run(['git', 'status', '--porcelain'], check=True, + stdout=subprocess.PIPE).stdout gitst = gitst.decode('UTF-8').strip() for line in gitst.splitlines(): s, name = line.split(maxsplit=1) diff --git a/scripts/dev/recompile_requirements.py b/scripts/dev/recompile_requirements.py index ddc85dfec..4718ca7ab 100644 --- a/scripts/dev/recompile_requirements.py +++ b/scripts/dev/recompile_requirements.py @@ -116,9 +116,11 @@ def main(): with tempfile.TemporaryDirectory() as tmpdir: pip_bin = os.path.join(tmpdir, 'bin', 'pip') - subprocess.check_call(['virtualenv', tmpdir]) - subprocess.check_call([pip_bin, 'install', '-r', filename]) - reqs = subprocess.check_output([pip_bin, 'freeze']).decode('utf-8') + subprocess.run(['virtualenv', tmpdir], check=True) + subprocess.run([pip_bin, 'install', '-r', filename], check=True) + reqs = subprocess.run([pip_bin, 'freeze'], check=True, + stdout=subprocess.PIPE + ).stdout.decode('utf-8') with open(filename, 'r', encoding='utf-8') as f: comments = read_comments(f) diff --git a/scripts/dev/run_profile.py b/scripts/dev/run_profile.py index 31fe539aa..2247bd842 100755 --- a/scripts/dev/run_profile.py +++ b/scripts/dev/run_profile.py @@ -76,14 +76,15 @@ def main(): pass elif args.profile_tool == 'gprof2dot': # yep, shell=True. I know what I'm doing. - subprocess.call('gprof2dot -f pstats {} | dot -Tpng | feh -F -'.format( - shlex.quote(profilefile)), shell=True) + subprocess.run( + 'gprof2dot -f pstats {} | dot -Tpng | feh -F -'.format( + shlex.quote(profilefile)), shell=True) elif args.profile_tool == 'kcachegrind': callgraphfile = os.path.join(tempdir, 'callgraph') - subprocess.call(['pyprof2calltree', '-k', '-i', profilefile, - '-o', callgraphfile]) + subprocess.run(['pyprof2calltree', '-k', '-i', profilefile, + '-o', callgraphfile]) elif args.profile_tool == 'snakeviz': - subprocess.call(['snakeviz', profilefile]) + subprocess.run(['snakeviz', profilefile]) shutil.rmtree(tempdir) diff --git a/scripts/dev/run_pylint_on_tests.py b/scripts/dev/run_pylint_on_tests.py index e6263692b..1f99aa362 100644 --- a/scripts/dev/run_pylint_on_tests.py +++ b/scripts/dev/run_pylint_on_tests.py @@ -73,7 +73,7 @@ def main(): env = os.environ.copy() env['PYTHONPATH'] = os.pathsep.join(pythonpath) - ret = subprocess.call(['pylint'] + args, env=env) + ret = subprocess.run(['pylint'] + args, env=env).returncode return ret diff --git a/scripts/dev/segfault_test.py b/scripts/dev/segfault_test.py index c5e7c106f..642bb837d 100755 --- a/scripts/dev/segfault_test.py +++ b/scripts/dev/segfault_test.py @@ -92,22 +92,22 @@ def main(): utils.print_bold("==== {} ====".format(page)) if test_harfbuzz: print("With system harfbuzz:") - ret = subprocess.call([sys.executable, '-c', SCRIPT, page]) + ret = subprocess.run([sys.executable, '-c', SCRIPT, page]).returncode print_ret(ret) retvals.append(ret) if test_harfbuzz: print("With QT_HARFBUZZ=old:") env = dict(os.environ) env['QT_HARFBUZZ'] = 'old' - ret = subprocess.call([sys.executable, '-c', SCRIPT, page], - env=env) + ret = subprocess.run([sys.executable, '-c', SCRIPT, page], + env=env).returncode print_ret(ret) retvals.append(ret) print("With QT_HARFBUZZ=new:") env = dict(os.environ) env['QT_HARFBUZZ'] = 'new' - ret = subprocess.call([sys.executable, '-c', SCRIPT, page], - env=env) + ret = subprocess.run([sys.executable, '-c', SCRIPT, page], + env=env).returncode print_ret(ret) retvals.append(ret) if all(r == 0 for r in retvals): diff --git a/scripts/dev/src2asciidoc.py b/scripts/dev/src2asciidoc.py index a112b60d4..a58791297 100755 --- a/scripts/dev/src2asciidoc.py +++ b/scripts/dev/src2asciidoc.py @@ -529,9 +529,9 @@ def regenerate_cheatsheet(): ] for filename, x, y in files: - subprocess.check_call(['inkscape', '-e', filename, '-b', 'white', - '-w', str(x), '-h', str(y), - 'misc/cheatsheet.svg']) + subprocess.run(['inkscape', '-e', filename, '-b', 'white', + '-w', str(x), '-h', str(y), + 'misc/cheatsheet.svg'], check=True) def main(): diff --git a/scripts/link_pyqt.py b/scripts/link_pyqt.py index a7de598cd..57eeb9138 100644 --- a/scripts/link_pyqt.py +++ b/scripts/link_pyqt.py @@ -46,12 +46,14 @@ def run_py(executable, *code): f.write('\n'.join(code)) cmd = [executable, filename] try: - ret = subprocess.check_output(cmd, universal_newlines=True) + ret = subprocess.run(cmd, universal_newlines=True, check=True, + stdout=subprocess.PIPE).stdout finally: os.remove(filename) else: cmd = [executable, '-c', '\n'.join(code)] - ret = subprocess.check_output(cmd, universal_newlines=True) + ret = subprocess.run(cmd, universal_newlines=True, check=True, + stdout=subprocess.PIPE).stdout return ret.rstrip() diff --git a/scripts/setupcommon.py b/scripts/setupcommon.py index 1f85b225f..1832ead86 100644 --- a/scripts/setupcommon.py +++ b/scripts/setupcommon.py @@ -51,12 +51,14 @@ def _git_str(): return None try: # https://stackoverflow.com/questions/21017300/21017394#21017394 - commit_hash = subprocess.check_output( + commit_hash = subprocess.run( ['git', 'describe', '--match=NeVeRmAtCh', '--always', '--dirty'], - cwd=BASEDIR).decode('UTF-8').strip() - date = subprocess.check_output( + cwd=BASEDIR, check=True, + stdout=subprocess.PIPE).stdout.decode('UTF-8').strip() + date = subprocess.run( ['git', 'show', '-s', '--format=%ci', 'HEAD'], - cwd=BASEDIR).decode('UTF-8').strip() + cwd=BASEDIR, check=True, + stdout=subprocess.PIPE).stdout.decode('UTF-8').strip() return '{} ({})'.format(commit_hash, date) except (subprocess.CalledProcessError, OSError): return None diff --git a/tests/end2end/features/test_qutescheme_bdd.py b/tests/end2end/features/test_qutescheme_bdd.py index 14bdc5cb3..1269d10ed 100644 --- a/tests/end2end/features/test_qutescheme_bdd.py +++ b/tests/end2end/features/test_qutescheme_bdd.py @@ -47,10 +47,10 @@ def update_documentation(): return try: - subprocess.call(['asciidoc'], stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL) + subprocess.run(['asciidoc'], stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL) except OSError: pytest.skip("Docs outdated and asciidoc unavailable!") update_script = os.path.join(script_path, 'asciidoc2html.py') - subprocess.call([sys.executable, update_script]) + subprocess.run([sys.executable, update_script]) diff --git a/tests/end2end/test_invocations.py b/tests/end2end/test_invocations.py index 1a91be2e0..ef97d5317 100644 --- a/tests/end2end/test_invocations.py +++ b/tests/end2end/test_invocations.py @@ -259,14 +259,13 @@ def test_command_on_start(request, quteproc_new): def test_launching_with_python2(): try: - proc = subprocess.Popen(['python2', '-m', 'qutebrowser', - '--no-err-windows'], stderr=subprocess.PIPE) + proc = subprocess.run(['python2', '-m', 'qutebrowser', + '--no-err-windows'], stderr=subprocess.PIPE) except FileNotFoundError: pytest.skip("python2 not found") - _stdout, stderr = proc.communicate() assert proc.returncode == 1 error = "At least Python 3.5 is required to run qutebrowser" - assert stderr.decode('ascii').startswith(error) + assert proc.stderr.decode('ascii').startswith(error) def test_initial_private_browsing(request, quteproc_new): diff --git a/tests/unit/misc/test_checkpyver.py b/tests/unit/misc/test_checkpyver.py index c5fa83a48..6487eb263 100644 --- a/tests/unit/misc/test_checkpyver.py +++ b/tests/unit/misc/test_checkpyver.py @@ -36,15 +36,14 @@ TEXT = (r"At least Python 3.5 is required to run qutebrowser, but it's " def test_python2(): """Run checkpyver with python 2.""" try: - proc = subprocess.Popen( + proc = subprocess.run( ['python2', checkpyver.__file__, '--no-err-windows'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = proc.communicate() except FileNotFoundError: pytest.skip("python2 not found") - assert not stdout - stderr = stderr.decode('utf-8') + assert not proc.stdout + stderr = proc.stderr.decode('utf-8') assert re.match(TEXT, stderr), stderr assert proc.returncode == 1 diff --git a/tests/unit/utils/test_standarddir.py b/tests/unit/utils/test_standarddir.py index c0f63d410..5a3f74a66 100644 --- a/tests/unit/utils/test_standarddir.py +++ b/tests/unit/utils/test_standarddir.py @@ -555,8 +555,9 @@ def test_no_qapplication(qapp, tmpdir): pyfile = tmpdir / 'sub.py' pyfile.write_text(textwrap.dedent(sub_code), encoding='ascii') - output = subprocess.check_output([sys.executable, str(pyfile)] + sys.path, - universal_newlines=True) + output = subprocess.run([sys.executable, str(pyfile)] + sys.path, + universal_newlines=True, + check=True, stdout=subprocess.PIPE).stdout sub_locations = json.loads(output) standarddir._init_dirs() diff --git a/tests/unit/utils/test_version.py b/tests/unit/utils/test_version.py index 9db7603e3..9e5f455ef 100644 --- a/tests/unit/utils/test_version.py +++ b/tests/unit/utils/test_version.py @@ -299,8 +299,8 @@ class TestGitStr: def _has_git(): """Check if git is installed.""" try: - subprocess.check_call(['git', '--version'], stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL) + subprocess.run(['git', '--version'], stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, check=True) except (OSError, subprocess.CalledProcessError): return False else: @@ -337,12 +337,13 @@ class TestGitStrSubprocess: # If we don't call this with shell=True it might fail under # some environments on Windows... # http://bugs.python.org/issue24493 - subprocess.check_call( + subprocess.run( 'git -C "{}" {}'.format(tmpdir, ' '.join(args)), - env=env, shell=True) + env=env, check=True, shell=True) else: - subprocess.check_call( - ['git', '-C', str(tmpdir)] + list(args), env=env) + subprocess.run( + ['git', '-C', str(tmpdir)] + list(args), + check=True, env=env) (tmpdir / 'file').write_text("Hello World!", encoding='utf-8') _git('init') @@ -368,14 +369,14 @@ class TestGitStrSubprocess: subprocess.CalledProcessError(1, 'foobar') ]) def test_exception(self, exc, mocker, tmpdir): - """Test with subprocess.check_output raising an exception. + """Test with subprocess.run raising an exception. Args: exc: The exception to raise. """ m = mocker.patch('qutebrowser.utils.version.os') m.path.isdir.return_value = True - mocker.patch('qutebrowser.utils.version.subprocess.check_output', + mocker.patch('qutebrowser.utils.version.subprocess.run', side_effect=exc) ret = version._git_str_subprocess(str(tmpdir)) assert ret is None From 1c39715267c9dd242a06fe5478b1e9cbe831c1f6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 1 Nov 2017 22:36:59 +0100 Subject: [PATCH 36/39] Clarify qute://configdiff/old title --- qutebrowser/config/configdiff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/config/configdiff.py b/qutebrowser/config/configdiff.py index 3f07daba1..020be2c8c 100644 --- a/qutebrowser/config/configdiff.py +++ b/qutebrowser/config/configdiff.py @@ -755,6 +755,6 @@ def get_diff(): lexer = pygments.lexers.DiffLexer() formatter = pygments.formatters.HtmlFormatter( full=True, linenos='table', - title='Config diff') + title='Diffing pre-1.0 default config with pre-1.0 modified config') # pylint: enable=no-member return pygments.highlight(conf_diff + key_diff, lexer, formatter) From 4498141c8b2159c50d785f508c0e4103c6403c14 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 2 Nov 2017 09:15:41 +0100 Subject: [PATCH 37/39] Update changelog --- doc/changelog.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index f4e31d5e8..e4ab9a4d9 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -76,6 +76,8 @@ Fixed ~~~~~ - Handle accessing a locked sqlite database gracefully +- Abort pinned tab dialogs properly when a tab is closed e.g. by closing a + window. v1.0.2 ------ From 4a1cdef88720bd05852e4d893dd9b6b641e80df5 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 2 Nov 2017 11:03:19 +0100 Subject: [PATCH 38/39] Fix indent --- scripts/dev/build_release.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/dev/build_release.py b/scripts/dev/build_release.py index 65d1d651b..e12ff5c93 100755 --- a/scripts/dev/build_release.py +++ b/scripts/dev/build_release.py @@ -90,7 +90,7 @@ def _maybe_remove(path): def smoke_test(executable): """Try starting the given qutebrowser executable.""" subprocess.run([executable, '--no-err-windows', '--nowindow', - '--temp-basedir', 'about:blank', ':later 500 quit'], + '--temp-basedir', 'about:blank', ':later 500 quit'], check=True) From c0eae5d4e48dbfe620677fbcfefb34fb2326dc1b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 2 Nov 2017 11:35:40 +0100 Subject: [PATCH 39/39] Update changelog --- doc/changelog.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index e4ab9a4d9..36079535c 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -34,6 +34,8 @@ Added widget. - `:edit-url` now handles the `--private` and `--related` flags, which have the same effect they have with `:open`. +- New `{line}` and `{column}` replacements for `editor.command` to position the + cursor correctly. Changed ~~~~~~~