Save `edit` result only when tempfile was modified

mtimes of tempfile prior to editing and after leaving the editor are now
compared before changing anything in todolist. If tempfile wasn't
modified, todolist (and todo file) is left untouched and user gets
appropriate error message.

test/TestEditCommand.py

@@ -45,24 +45,30 @@ class EditCommandTest(CommandTest):
 
         self.todolist = TodoList(todos)
 
+    @mock.patch('topydo.commands.EditCommand._is_edited')
+    @mock.patch('topydo.commands.EditCommand.EditCommand._todos_from_temp')
     @mock.patch('topydo.commands.EditCommand.EditCommand._open_in_editor')
-    def test_edit1(self, mock_open_in_editor):
+    def test_edit01(self, mock_open_in_editor, mock_todos_from_temp, mock_is_edited):
         """ Preserve dependencies after editing. """
         mock_open_in_editor.return_value = 0
+        mock_todos_from_temp.return_value = [Todo('Foo id:1')]
+        mock_is_edited.return_value = True
 
         command = EditCommand(["1"], self.todolist, self.out, self.error, None)
         command.execute()
 
-        self.assertTrue(self.todolist.is_dirty())
         self.assertEqual(self.errors, "")
+        self.assertTrue(self.todolist.is_dirty())
         self.assertEqual(self.todolist.print_todos(), u("Bar p:1 @test\nBaz @test\nFo\u00f3B\u0105\u017a\nFoo id:1"))
 
+    @mock.patch('topydo.commands.EditCommand._is_edited')
     @mock.patch('topydo.commands.EditCommand.EditCommand._todos_from_temp')
     @mock.patch('topydo.commands.EditCommand.EditCommand._open_in_editor')
-    def test_edit2(self, mock_open_in_editor, mock_todos_from_temp):
+    def test_edit02(self, mock_open_in_editor, mock_todos_from_temp, mock_is_edited):
         """ Edit some todo. """
         mock_open_in_editor.return_value = 0
         mock_todos_from_temp.return_value = [Todo('Lazy Cat')]
+        mock_is_edited.return_value = True
 
         command = EditCommand(["Bar"], self.todolist, self.out, self.error,
                               None)
@@ -72,7 +78,7 @@ class EditCommandTest(CommandTest):
         self.assertEqual(self.errors, "")
         self.assertEqual(self.todolist.print_todos(), u("Foo id:1\nBaz @test\nFo\u00f3B\u0105\u017a\nLazy Cat"))
 
-    def test_edit3(self):
+    def test_edit03(self):
         """ Throw an error after invalid todo number given as argument. """
         command = EditCommand(["FooBar"], self.todolist, self.out, self.error,
                               None)
@@ -81,7 +87,7 @@ class EditCommandTest(CommandTest):
         self.assertFalse(self.todolist.is_dirty())
         self.assertEqual(self.errors, "Invalid todo number given.\n")
 
-    def test_edit4(self):
+    def test_edit04(self):
         """ Throw an error with pointing invalid argument. """
         command = EditCommand(["Bar", "5"], self.todolist, self.out,
                               self.error, None)
@@ -90,12 +96,14 @@ class EditCommandTest(CommandTest):
         self.assertFalse(self.todolist.is_dirty())
         self.assertEqual(self.errors, "Invalid todo number given: 5.\n")
 
+    @mock.patch('topydo.commands.EditCommand._is_edited')
     @mock.patch('topydo.commands.EditCommand.EditCommand._todos_from_temp')
     @mock.patch('topydo.commands.EditCommand.EditCommand._open_in_editor')
-    def test_edit5(self, mock_open_in_editor, mock_todos_from_temp):
+    def test_edit05(self, mock_open_in_editor, mock_todos_from_temp, mock_is_edited):
         """ Don't let to delete todos acidentally while editing. """
         mock_open_in_editor.return_value = 0
         mock_todos_from_temp.return_value = [Todo('Only one line')]
+        mock_is_edited.return_value = True
 
         command = EditCommand(["1", "Bar"], self.todolist, self.out,
                               self.error, None)
@@ -105,7 +113,7 @@ class EditCommandTest(CommandTest):
         self.assertEqual(self.errors, "Number of edited todos is not equal to number of supplied todo IDs.\n")
         self.assertEqual(self.todolist.print_todos(), u("Foo id:1\nBar p:1 @test\nBaz @test\nFo\u00f3B\u0105\u017a"))
 
-    def test_edit6(self):
+    def test_edit06(self):
         """
         Throw an error with invalid argument containing special characters.
         """
@@ -117,12 +125,14 @@ class EditCommandTest(CommandTest):
         self.assertEqual(self.errors,
                          u("Invalid todo number given: Fo\u00d3B\u0105r.\n"))
 
+    @mock.patch('topydo.commands.EditCommand._is_edited')
     @mock.patch('topydo.commands.EditCommand.EditCommand._todos_from_temp')
     @mock.patch('topydo.commands.EditCommand.EditCommand._open_in_editor')
-    def test_edit7(self, mock_open_in_editor, mock_todos_from_temp):
+    def test_edit07(self, mock_open_in_editor, mock_todos_from_temp, mock_is_edited):
         """ Edit todo with special characters. """
         mock_open_in_editor.return_value = 0
         mock_todos_from_temp.return_value = [Todo('Lazy Cat')]
+        mock_is_edited.return_value = True
 
         command = EditCommand([u("Fo\u00f3B\u0105\u017a")], self.todolist,
                               self.out, self.error, None)
@@ -133,13 +143,32 @@ class EditCommandTest(CommandTest):
         self.assertEqual(self.todolist.print_todos(),
                          u("Foo id:1\nBar p:1 @test\nBaz @test\nLazy Cat"))
 
+    @mock.patch('topydo.commands.EditCommand._is_edited')
     @mock.patch('topydo.commands.EditCommand.EditCommand._todos_from_temp')
     @mock.patch('topydo.commands.EditCommand.EditCommand._open_in_editor')
-    def test_edit_expr(self, mock_open_in_editor, mock_todos_from_temp):
+    def test_edit08(self, mock_open_in_editor, mock_todos_from_temp, mock_is_edited):
+        """ Don't perform write if tempfile is unchanged """
+        mock_open_in_editor.return_value = 0
+        mock_todos_from_temp.return_value = [Todo('Only one line')]
+        mock_is_edited.return_value = False
+
+        command = EditCommand(["1", "Bar"], self.todolist, self.out,
+                              self.error, None)
+        command.execute()
+
+        self.assertFalse(self.todolist.is_dirty())
+        self.assertEqual(self.errors, "Editing aborted. Nothing to do.\n")
+        self.assertEqual(self.todolist.print_todos(), u("Foo id:1\nBar p:1 @test\nBaz @test\nFo\u00f3B\u0105\u017a"))
+
+    @mock.patch('topydo.commands.EditCommand._is_edited')
+    @mock.patch('topydo.commands.EditCommand.EditCommand._todos_from_temp')
+    @mock.patch('topydo.commands.EditCommand.EditCommand._open_in_editor')
+    def test_edit_expr(self, mock_open_in_editor, mock_todos_from_temp, mock_is_edited):
         """ Edit todos matching expression. """
         mock_open_in_editor.return_value = 0
         mock_todos_from_temp.return_value = [Todo('Lazy Cat'),
                                              Todo('Lazy Dog')]
+        mock_is_edited.return_value = True
 
         command = EditCommand(["-e", "@test"], self.todolist, self.out,
                               self.error, None)
@@ -147,8 +176,8 @@ class EditCommandTest(CommandTest):
 
         expected = u("|  3| Lazy Cat\n|  4| Lazy Dog\n")
 
-        self.assertTrue(self.todolist.is_dirty())
         self.assertEqual(self.errors, "")
+        self.assertTrue(self.todolist.is_dirty())
         self.assertEqual(self.output, expected)
         self.assertEqual(self.todolist.print_todos(), u("Foo id:1\nFo\u00f3B\u0105\u017a\nLazy Cat\nLazy Dog"))
 

topydo/commands/EditCommand.py

@@ -33,6 +33,11 @@ DEFAULT_EDITOR = 'vi'
 # cannot use super() inside the class itself
 BASE_TODOLIST = lambda tl: super(TodoList, tl)
 
+def _get_file_mtime(p_file):
+    return os.stat(p_file.name).st_mtime
+
+def _is_edited(p_orig_mtime, p_file):
+    return p_orig_mtime < _get_file_mtime(p_file)
 
 class EditCommand(MultiCommand):
     def __init__(self, p_args, p_todolist, p_output, p_error, p_input):
@@ -105,19 +110,24 @@ class EditCommand(MultiCommand):
         self.printer.add_filter(PrettyPrinterNumbers(self.todolist))
 
         temp_todos = self._todos_to_temp()
+        orig_mtime = _get_file_mtime(temp_todos)
 
         if not self._open_in_editor(temp_todos.name):
             new_todos = self._todos_from_temp(temp_todos)
-            if len(new_todos) == len(self.todos):
-                for todo in self.todos:
-                    BASE_TODOLIST(self.todolist).delete(todo)
 
-                for todo in new_todos:
-                    self.todolist.add_todo(todo)
-                    self.out(self.printer.print_todo(todo))
+            if _is_edited(orig_mtime, temp_todos):
+                if len(new_todos) == len(self.todos):
+                    for todo in self.todos:
+                        BASE_TODOLIST(self.todolist).delete(todo)
+
+                    for todo in new_todos:
+                        self.todolist.add_todo(todo)
+                        self.out(self.printer.print_todo(todo))
+                else:
+                    self.error('Number of edited todos is not equal to '
+                            'number of supplied todo IDs.')
             else:
-                self.error('Number of edited todos is not equal to '
-                           'number of supplied todo IDs.')
+                self.error('Editing aborted. Nothing to do.')
         else:
             self.error(self.usage())