diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 524588b54..3686e028f 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -35,8 +35,9 @@ class ExternalEditor(QObject): Attributes: _text: The current text before the editor is opened. - _file: The file handle as tempfile.NamedTemporaryFile. Note that this - handle will be closed after the initial file has been created. + _filename: The name of the file to be edited. + _remove_file: Whether the file should be removed when the editor is + closed. _proc: The GUIProcess of the editor. """ @@ -44,18 +45,20 @@ class ExternalEditor(QObject): def __init__(self, parent=None): super().__init__(parent) - self._text = None - self._file = None + self._filename = None self._proc = None + self._remove_file = None def _cleanup(self): """Clean up temporary files after the editor closed.""" - if self._file is None: + assert self._remove_file is not None + if self._filename is None or not self._remove_file: # Could not create initial file. return + try: if self._proc.exit_status() != QProcess.CrashExit: - os.remove(self._file.name) + os.remove(self._filename) except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's # executed async. @@ -77,7 +80,7 @@ class ExternalEditor(QObject): return encoding = config.val.editor.encoding try: - with open(self._file.name, 'r', encoding=encoding) as f: + with open(self._filename, 'r', encoding=encoding) as f: text = f.read() except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's @@ -99,9 +102,8 @@ class ExternalEditor(QObject): Args: text: The initial text to edit. """ - if self._text is not None: + if self._filename is not None: raise ValueError("Already editing a file!") - self._text = text try: # Close while the external process is running, as otherwise systems # with exclusive write access (e.g. Windows) may fail to update @@ -113,15 +115,27 @@ class ExternalEditor(QObject): delete=False) as fobj: if text: fobj.write(text) - self._file = fobj + self._filename = fobj.name except OSError as e: message.error("Failed to create initial file: {}".format(e)) return + + self._remove_file = True + self._start_editor() + + def edit_file(self, filename): + """Edit the file with the given filename.""" + self._filename = filename + self._remove_file = False + self._start_editor() + + def _start_editor(self): + """Start the editor with the file opened as self._filename.""" 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._file.name) 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) diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index 9ca4fdcbb..e6902852e 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -59,14 +59,14 @@ class TestArg: config_stub.val.editor.command = ['bin', 'foo', '{}', 'bar'] editor.edit("") editor._proc._proc.start.assert_called_with( - "bin", ["foo", editor._file.name, "bar"]) + "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.val.editor.command = ['bin', 'foo{}', 'bar'] editor.edit("") editor._proc._proc.start.assert_called_with( - "bin", ["foo" + editor._file.name, "bar"]) + "bin", ["foo" + editor._filename, "bar"]) class TestFileHandling: @@ -76,16 +76,26 @@ class TestFileHandling: def test_ok(self, editor): """Test file handling when closing with an exit status == 0.""" editor.edit("") - filename = editor._file.name + filename = editor._filename assert os.path.exists(filename) assert os.path.basename(filename).startswith('qutebrowser-editor-') editor._proc.finished.emit(0, QProcess.NormalExit) assert not os.path.exists(filename) + def test_existing_file(self, editor, tmpdir): + """Test editing an existing file.""" + path = tmpdir / 'foo.txt' + path.ensure() + + editor.edit_file(str(path)) + editor._proc.finished.emit(0, QProcess.NormalExit) + + assert path.exists() + def test_error(self, editor): """Test file handling when closing with an exit status != 0.""" editor.edit("") - filename = editor._file.name + filename = editor._filename assert os.path.exists(filename) editor._proc._proc.exitStatus = mock.Mock( @@ -99,7 +109,7 @@ class TestFileHandling: def test_crash(self, editor): """Test file handling when closing with a crash.""" editor.edit("") - filename = editor._file.name + filename = editor._filename assert os.path.exists(filename) editor._proc._proc.exitStatus = mock.Mock( @@ -114,7 +124,7 @@ class TestFileHandling: def test_unreadable(self, message_mock, editor, caplog): """Test file handling when closing with an unreadable file.""" editor.edit("") - filename = editor._file.name + filename = editor._filename assert os.path.exists(filename) os.chmod(filename, 0o077) if os.access(filename, os.R_OK): @@ -160,10 +170,10 @@ def test_modify(editor, initial_text, edited_text): """Test if inputs get modified correctly.""" editor.edit(initial_text) - with open(editor._file.name, 'r', encoding='utf-8') as f: + with open(editor._filename, 'r', encoding='utf-8') as f: assert f.read() == initial_text - with open(editor._file.name, 'w', encoding='utf-8') as f: + with open(editor._filename, 'w', encoding='utf-8') as f: f.write(edited_text) editor._proc.finished.emit(0, QProcess.NormalExit)