diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index b72bb77ac..69a25e177 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -1961,7 +1961,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: <> diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 93956157b..17179b6ff 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1618,10 +1618,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') diff --git a/qutebrowser/browser/webengine/webengineelem.py b/qutebrowser/browser/webengine/webengineelem.py index c294bebef..fb528a93d 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,10 @@ class WebEngineElement(webelem.AbstractWebElement): def set_value(self, value): 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): if not self.is_editable(strict=True): raise webelem.Error("Element is not editable!") diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 54dd6519d..2a1eafc9e 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): + """Get the text caret position for the current element.""" + self._check_vanished() + pos = self._elem.evaluateJavaScript('this.selectionStart') + if pos is None: + return 0 + return int(pos) + def insert_text(self, text): self._check_vanished() if not self.is_editable(strict=True): diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index 39c8ec226..17e1c75b0 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -766,11 +766,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 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 diff --git a/qutebrowser/javascript/webelem.js b/qutebrowser/javascript/webelem.js index ce348c46b..cbd1b16d8 100644 --- a/qutebrowser/javascript/webelem.js +++ b/qutebrowser/javascript/webelem.js @@ -48,11 +48,26 @@ 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 (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 err; + } + } + var out = { "id": id, "value": elem.value, "outer_xml": elem.outerHTML, "rects": [], // Gets filled up later + "caret_position": caret_position, }; // https://github.com/qutebrowser/qutebrowser/issues/2569 diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 3686e028f..2be96e012 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=0): """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,9 @@ class ExternalEditor(QObject): return self._remove_file = True - self._start_editor() + + line, column = self._calc_line_and_column(text, caret_position) + self._start_editor(line=line, column=column) def edit_file(self, filename): """Edit the file with the given filename.""" @@ -129,13 +132,82 @@ 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: + 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) 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 _calc_line_and_column(self, text, 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 + 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 + + Return: + A (line, column) tuple of (int, int) + """ + line = text[:caret_position].count('\n') + 1 + column = caret_position - text[:caret_position].rfind('\n') + return line, column + + def _sub_placeholder(self, arg, line, column): + """Substitute a single placeholder. + + 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: + 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. + + Return: + The substituted placeholder or the original argument. + """ + replacements = { + '{}': self._filename, + '{file}': self._filename, + '{line}': str(line), + '{line0}': str(line-1), + '{column}': str(column), + '{column0}': str(column-1) + } + + for old, new in replacements.items(): + arg = arg.replace(old, new) + + return arg diff --git a/tests/unit/config/test_configcommands.py b/tests/unit/config/test_configcommands.py index c33bb54d4..0fd1b6680 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): diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 9b5e23263..36988a0a7 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -1815,9 +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': 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) @@ -1825,8 +1828,15 @@ class TestShellCommand: assert cmd.to_py(expected) == expected @pytest.mark.parametrize('kwargs, val', [ - ({'placeholder': '{}'}, '[foo, bar]'), - ({'placeholder': '{}'}, '[foo, "{", "}", bar'), + ({'placeholder': True}, '[foo, bar]'), + ({'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): diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index d97a39f7b..683b38841 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -177,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