From 38bb3673db2215e0d88f56673b2c25893e286132 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 12 Mar 2018 08:34:50 -0400 Subject: [PATCH] Preserve a backup if editor callback fails. Currently the editor deletes its temp file whenever editing is finished. With this patch, the file will not be deleted if the editor callback encounters an exception. One example is if the tab containing the edited element is closed. The editor errors with "Edited element vanished", but with this patch it will also print "Backup at ..." so the user does not lose their work. Resolves #1596. Supersedes #3641, using the cleaner approach started in #1677. --- qutebrowser/browser/commands.py | 7 +++-- qutebrowser/misc/editor.py | 40 ++++++++++++++++++--------- tests/end2end/features/editor.feature | 1 + tests/unit/misc/test_editor.py | 19 +++++++++++++ 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index b84bde7a8..fb447d6a9 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -53,7 +53,6 @@ class CommandDispatcher: cmdutils.register() decorators are run, currentWidget() will return None. Attributes: - _editor: The ExternalEditor object. _win_id: The window ID the CommandDispatcher is associated with. _tabbed_browser: The TabbedBrowser used. """ @@ -1639,7 +1638,7 @@ class CommandDispatcher: ed = editor.ExternalEditor(watch=True, parent=self._tabbed_browser) ed.file_updated.connect(functools.partial( - self.on_file_updated, elem)) + self.on_file_updated, ed, elem)) ed.editing_finished.connect(lambda: mainwindow.raise_window( objreg.last_focused_window(), alert=False)) ed.edit(text, caret_position) @@ -1654,7 +1653,7 @@ class CommandDispatcher: tab = self._current_widget() tab.elements.find_focused(self._open_editor_cb) - def on_file_updated(self, elem, text): + def on_file_updated(self, ed, elem, text): """Write the editor text into the form field and clean up tempfile. Callback for GUIProcess when the edited text was updated. @@ -1667,7 +1666,9 @@ class CommandDispatcher: elem.set_value(text) except webelem.OrphanedError as e: message.error('Edited element vanished') + ed.backup() except webelem.Error as e: + ed.backup() raise cmdexc.CommandError(str(e)) @cmdutils.register(instance='command-dispatcher', maxsplit=0, diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 154660001..d9024000b 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -42,6 +42,7 @@ class ExternalEditor(QObject): _proc: The GUIProcess of the editor. _watcher: A QFileSystemWatcher to watch the edited file for changes. Only set if watch=True. + _content: The last-saved text of the editor. Signals: file_updated: The text in the edited file was updated. @@ -112,19 +113,7 @@ class ExternalEditor(QObject): if self._filename is not None: raise ValueError("Already editing a file!") try: - # 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/qutebrowser/qutebrowser/issues/1767 - with tempfile.NamedTemporaryFile( - # pylint: disable=bad-continuation - mode='w', prefix='qutebrowser-editor-', - encoding=config.val.editor.encoding, - delete=False) as fobj: - # pylint: enable=bad-continuation - if text: - fobj.write(text) - self._filename = fobj.name + self._filename = self._create_tempfile(text) except OSError as e: message.error("Failed to create initial file: {}".format(e)) return @@ -134,6 +123,31 @@ class ExternalEditor(QObject): line, column = self._calc_line_and_column(text, caret_position) self._start_editor(line=line, column=column) + def backup(self): + """Create a backup if the content has changed from the original.""" + if not self._content: + return + try: + fname = self._create_tempfile(self._content) + message.info('Editor backup at {}'.format(fname)) + except OSError as e: + message.error('Failed to create editor backup: {}'.format(e)) + + def _create_tempfile(self, 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/qutebrowser/qutebrowser/issues/1767 + with tempfile.NamedTemporaryFile( + # pylint: disable=bad-continuation + mode='w', prefix='qutebrowser-editor-', + encoding=config.val.editor.encoding, + delete=False) as fobj: + # pylint: enable=bad-continuation + if text: + fobj.write(text) + return fobj.name + @pyqtSlot(str) def _on_file_changed(self, path): try: diff --git a/tests/end2end/features/editor.feature b/tests/end2end/features/editor.feature index 391e88749..33535856c 100644 --- a/tests/end2end/features/editor.feature +++ b/tests/end2end/features/editor.feature @@ -128,6 +128,7 @@ Feature: Opening external editors And I run :tab-close And I kill the waiting editor Then the error "Edited element vanished" should be shown + And the message "Editor backup at *" should be shown # Could not get signals working on Windows @posix diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index 8cf778d89..caa0b42cf 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -157,6 +157,25 @@ class TestFileHandling: with pytest.raises(ValueError): editor.edit("") + def test_backup(self, qtbot, message_mock): + editor = editormod.ExternalEditor(watch=True) + editor.edit('foo') + with qtbot.wait_signal(editor.file_updated) as blocker: + _update_file(editor._filename, 'bar') + + editor.backup() + + msg = message_mock.getmsg(usertypes.MessageLevel.info) + prefix = 'Editor backup at ' + assert msg.text.startswith(prefix) + fname = msg.text[len(prefix):] + + with qtbot.wait_signal(editor.editing_finished) as blocker: + editor._proc.finished.emit(0, QProcess.NormalExit) + + with open(fname, 'r') as f: + assert f.read() == 'bar' + @pytest.mark.parametrize('initial_text, edited_text', [ ('', 'Hello'),