Merge pull request #59 from mruwek/edit-mtime-fix

Save `edit` result only when tempfile was modified

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