From 27966c94a60b744592a6038351778589d9cc640d Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Tue, 13 Mar 2018 07:31:48 -0400
Subject: [PATCH] Fix up editor backup patch.

- Use qutebrowser-editor-backup as the backup file prefix
- Consistently use message.error instead of cmdexc
- Improve test coverage for the backup function
- Fix lint errors in the unit test code
---
 qutebrowser/browser/commands.py |  2 +-
 qutebrowser/misc/editor.py      |  9 +++++----
 tests/unit/misc/test_editor.py  | 25 ++++++++++++++++++++++---
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index fb447d6a9..0a30c7135 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -1668,8 +1668,8 @@ class CommandDispatcher:
             message.error('Edited element vanished')
             ed.backup()
         except webelem.Error as e:
+            message.error(str(e))
             ed.backup()
-            raise cmdexc.CommandError(str(e))
 
     @cmdutils.register(instance='command-dispatcher', maxsplit=0,
                        scope='window')
diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py
index d9024000b..473f67c3e 100644
--- a/qutebrowser/misc/editor.py
+++ b/qutebrowser/misc/editor.py
@@ -113,7 +113,7 @@ class ExternalEditor(QObject):
         if self._filename is not None:
             raise ValueError("Already editing a file!")
         try:
-            self._filename = self._create_tempfile(text)
+            self._filename = self._create_tempfile(text, 'qutebrowser-editor-')
         except OSError as e:
             message.error("Failed to create initial file: {}".format(e))
             return
@@ -128,19 +128,20 @@ class ExternalEditor(QObject):
         if not self._content:
             return
         try:
-            fname = self._create_tempfile(self._content)
+            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):
+    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='qutebrowser-editor-',
+                mode='w', prefix=prefix,
                 encoding=config.val.editor.encoding,
                 delete=False) as fobj:
                 # pylint: enable=bad-continuation
diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py
index caa0b42cf..095f00ef2 100644
--- a/tests/unit/misc/test_editor.py
+++ b/tests/unit/misc/test_editor.py
@@ -160,7 +160,7 @@ class TestFileHandling:
     def test_backup(self, qtbot, message_mock):
         editor = editormod.ExternalEditor(watch=True)
         editor.edit('foo')
-        with qtbot.wait_signal(editor.file_updated) as blocker:
+        with qtbot.wait_signal(editor.file_updated):
             _update_file(editor._filename, 'bar')
 
         editor.backup()
@@ -170,12 +170,31 @@ class TestFileHandling:
         assert msg.text.startswith(prefix)
         fname = msg.text[len(prefix):]
 
-        with qtbot.wait_signal(editor.editing_finished) as blocker:
+        with qtbot.wait_signal(editor.editing_finished):
             editor._proc.finished.emit(0, QProcess.NormalExit)
 
-        with open(fname, 'r') as f:
+        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(self, qtbot, message_mock, mocker):
+        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)
+        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'),