Refactor ExternalEditor to be able to edit an existing file

This commit is contained in:
Florian Bruhin 2017-10-03 18:19:09 +02:00
parent 8edaad51c3
commit 555930791f
2 changed files with 43 additions and 19 deletions

View File

@ -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)

View File

@ -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)