Merge remote-tracking branch 'origin/pr/3704'

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.
     """
@@ -1640,7 +1639,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)
@@ -1655,7 +1654,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.
@@ -1668,8 +1667,10 @@ class CommandDispatcher:
             elem.set_value(text)
         except webelem.OrphanedError as e:
             message.error('Edited element vanished')
+            ed.backup()
         except webelem.Error as e:
-            raise cmdexc.CommandError(str(e))
+            message.error(str(e))
+            ed.backup()
 
     @cmdutils.register(instance='command-dispatcher', maxsplit=0,
                        scope='window')

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
+            self._filename = self._create_tempfile(text, 'qutebrowser-editor-')
-            # 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
         except OSError as e:
             message.error("Failed to create initial file: {}".format(e))
             return
@@ -134,6 +123,32 @@ 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,
+                                          'qutebrowser-editor-backup-')
+            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, prefix):
+        # 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=prefix,
+                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:

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

tests/unit/misc/test_editor.py

@@ -157,6 +157,45 @@ 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):
+            _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):
+            editor._proc.finished.emit(0, QProcess.NormalExit)
+
+        with open(fname, 'r', encoding='utf-8') as f:
+            assert f.read() == 'bar'
+
+    def test_backup_no_content(self, qtbot, message_mock):
+        editor = editormod.ExternalEditor(watch=True)
+        editor.edit('foo')
+        editor.backup()
+        # content has not changed, so no backup should be created
+        assert not message_mock.messages
+
+    def test_backup_error(self, qtbot, message_mock, mocker, caplog):
+        editor = editormod.ExternalEditor(watch=True)
+        editor.edit('foo')
+        with qtbot.wait_signal(editor.file_updated):
+            _update_file(editor._filename, 'bar')
+
+        mocker.patch('tempfile.NamedTemporaryFile', side_effect=OSError)
+        with caplog.at_level(logging.ERROR):
+            editor.backup()
+
+        msg = message_mock.getmsg(usertypes.MessageLevel.error)
+        assert msg.text.startswith('Failed to create editor backup:')
+
 
 @pytest.mark.parametrize('initial_text, edited_text', [
     ('', 'Hello'),