Merge branch 'Kingdread-fix-windows-editor'

CHANGELOG.asciidoc

@@ -60,6 +60,7 @@ Fixed
 ~~~~~
 
 - Fixed crash when doing `:<space><enter>`, another corner-case introduced in v0.8.0
+- Fixed `:open-editor` (`<Ctrl-e>`) on Windows
 
 v0.8.2
 ------

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.
+        _file: The file handle as tempfile.NamedTemporaryFile. Note that this
-        _filehandle: The file handle to the tmpfile.
+               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._file = None
-        self._filename = 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
-        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:
+        try:
-                    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)

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)