editor: fix external editor on windows

On windows, only one process can open a file in write mode at once. We
didn't close the handle we got (self._oshandle) before _cleanup, which
means that we had the file open the whole time, which means that the
external editor couldn't write back the changes.

This patch closes the file while the external editor is running and only
opens it once the editor is closed. We re-opened the file anyway, so
this shouldn't be a huge change. Additionally, tempfile.NamedTemporaryFile
is used instead of mkstemp, as we don't have to deal with os-level file
handles that way.
This commit is contained in:
Daniel Schadt 2016-08-06 19:59:56 +02:00
parent 778ccad39f
commit f23a28079c

View File

@ -35,8 +35,8 @@ class ExternalEditor(QObject):
Attributes: Attributes:
_text: The current text before the editor is opened. _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. _proc: The GUIProcess of the editor.
_win_id: The window ID the ExternalEditor is associated with. _win_id: The window ID the ExternalEditor is associated with.
""" """
@ -46,20 +46,18 @@ class ExternalEditor(QObject):
def __init__(self, win_id, parent=None): def __init__(self, win_id, parent=None):
super().__init__(parent) super().__init__(parent)
self._text = None self._text = None
self._oshandle = None self._file = None
self._filename = None
self._proc = None self._proc = None
self._win_id = win_id self._win_id = win_id
def _cleanup(self): def _cleanup(self):
"""Clean up temporary files after the editor closed.""" """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. # Could not create initial file.
return return
try: try:
os.close(self._oshandle)
if self._proc.exit_status() != QProcess.CrashExit: if self._proc.exit_status() != QProcess.CrashExit:
os.remove(self._filename) os.unlink(self._file.name)
except OSError as e: except OSError as e:
# NOTE: Do not replace this with "raise CommandError" as it's # NOTE: Do not replace this with "raise CommandError" as it's
# executed async. # executed async.
@ -82,7 +80,7 @@ class ExternalEditor(QObject):
return return
encoding = config.get('general', 'editor-encoding') encoding = config.get('general', 'editor-encoding')
try: try:
with open(self._filename, 'r', encoding=encoding) as f: with open(self._file.name, 'r', encoding=encoding) as f:
text = f.read() text = f.read()
except OSError as e: except OSError as e:
# NOTE: Do not replace this with "raise CommandError" as it's # NOTE: Do not replace this with "raise CommandError" as it's
@ -108,13 +106,18 @@ class ExternalEditor(QObject):
if self._text is not None: if self._text is not None:
raise ValueError("Already editing a file!") raise ValueError("Already editing a file!")
self._text = text self._text = text
encoding = config.get('general', 'editor-encoding')
try: try:
self._oshandle, self._filename = tempfile.mkstemp( # Close while the external process is running, as otherwise systems
text=True, prefix='qutebrowser-editor-') # with exclusive write access (e.g. Windows) may fail to update
if text: # the file from the external editor, see
encoding = config.get('general', 'editor-encoding') # https://github.com/The-Compiler/qutebrowser/issues/1767
with open(self._filename, 'w', encoding=encoding) as f: with tempfile.NamedTemporaryFile(
f.write(text) mode='w', prefix='qutebrowser-editor-', encoding=encoding,
delete=False) as fobj:
if text:
fobj.write(text)
self._file = fobj
except OSError as e: except OSError as e:
message.error(self._win_id, "Failed to create initial file: " message.error(self._win_id, "Failed to create initial file: "
"{}".format(e)) "{}".format(e))
@ -125,6 +128,6 @@ class ExternalEditor(QObject):
self._proc.error.connect(self.on_proc_error) self._proc.error.connect(self.on_proc_error)
editor = config.get('general', 'editor') editor = config.get('general', 'editor')
executable = editor[0] 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)) log.procs.debug("Calling \"{}\" with args {}".format(executable, args))
self._proc.start(executable, args) self._proc.start(executable, args)