diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index d7bda59b5..a23f283b3 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -60,6 +60,7 @@ Fixed ~~~~~ - Fixed crash when doing `:`, another corner-case introduced in v0.8.0 +- Fixed `:open-editor` (``) on Windows v0.8.2 ------ diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index bcc07129a..c72841f30 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -35,8 +35,8 @@ class ExternalEditor(QObject): Attributes: _text: The current text before the editor is opened. - _oshandle: The OS level handle to the tmpfile. - _filehandle: The file handle to the tmpfile. + _file: The file handle as tempfile.NamedTemporaryFile. Note that this + handle will be closed after the initial file has been created. _proc: The GUIProcess of the editor. _win_id: The window ID the ExternalEditor is associated with. """ @@ -46,20 +46,18 @@ class ExternalEditor(QObject): def __init__(self, win_id, parent=None): super().__init__(parent) self._text = None - self._oshandle = None - self._filename = None + self._file = None self._proc = None self._win_id = win_id def _cleanup(self): """Clean up temporary files after the editor closed.""" - if self._oshandle is None or self._filename is None: + if self._file is None: # Could not create initial file. return try: - os.close(self._oshandle) if self._proc.exit_status() != QProcess.CrashExit: - os.remove(self._filename) + os.remove(self._file.name) except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's # executed async. @@ -82,7 +80,7 @@ class ExternalEditor(QObject): return encoding = config.get('general', 'editor-encoding') try: - with open(self._filename, 'r', encoding=encoding) as f: + with open(self._file.name, 'r', encoding=encoding) as f: text = f.read() except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's @@ -108,13 +106,18 @@ class ExternalEditor(QObject): if self._text is not None: raise ValueError("Already editing a file!") self._text = text + encoding = config.get('general', 'editor-encoding') try: - self._oshandle, self._filename = tempfile.mkstemp( - text=True, prefix='qutebrowser-editor-') - if text: - encoding = config.get('general', 'editor-encoding') - with open(self._filename, 'w', encoding=encoding) as f: - f.write(text) + # Close while the external process is running, as otherwise systems + # with exclusive write access (e.g. Windows) may fail to update + # the file from the external editor, see + # https://github.com/The-Compiler/qutebrowser/issues/1767 + with tempfile.NamedTemporaryFile( + mode='w', prefix='qutebrowser-editor-', encoding=encoding, + delete=False) as fobj: + if text: + fobj.write(text) + self._file = fobj except OSError as e: message.error(self._win_id, "Failed to create initial file: " "{}".format(e)) @@ -125,6 +128,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) for arg in editor[1:]] + args = [arg.replace('{}', self._file.name) 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 f9a5d4370..26828ee12 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -69,14 +69,14 @@ class TestArg: config_stub.data['general']['editor'] = ['bin', 'foo', '{}', 'bar'] editor.edit("") editor._proc._proc.start.assert_called_with( - "bin", ["foo", editor._filename, "bar"]) + "bin", ["foo", editor._file.name, "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"]) + "bin", ["foo" + editor._file.name, "bar"]) class TestFileHandling: @@ -86,7 +86,7 @@ class TestFileHandling: def test_ok(self, editor): """Test file handling when closing with an exit status == 0.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) assert os.path.basename(filename).startswith('qutebrowser-editor-') editor._proc.finished.emit(0, QProcess.NormalExit) @@ -95,7 +95,7 @@ class TestFileHandling: def test_error(self, editor): """Test file handling when closing with an exit status != 0.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) editor._proc._proc.exitStatus = mock.Mock( @@ -109,7 +109,7 @@ class TestFileHandling: def test_crash(self, editor): """Test file handling when closing with a crash.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) editor._proc._proc.exitStatus = mock.Mock( @@ -125,7 +125,7 @@ class TestFileHandling: def test_unreadable(self, message_mock, editor): """Test file handling when closing with an unreadable file.""" editor.edit("") - filename = editor._filename + filename = editor._file.name assert os.path.exists(filename) os.chmod(filename, 0o077) editor._proc.finished.emit(0, QProcess.NormalExit) @@ -160,10 +160,10 @@ def test_modify(editor, initial_text, edited_text): """Test if inputs get modified correctly.""" editor.edit(initial_text) - with open(editor._filename, 'r', encoding='utf-8') as f: + with open(editor._file.name, 'r', encoding='utf-8') as f: assert f.read() == initial_text - with open(editor._filename, 'w', encoding='utf-8') as f: + with open(editor._file.name, 'w', encoding='utf-8') as f: f.write(edited_text) editor._proc.finished.emit(0, QProcess.NormalExit)