Commit c3ef22af authored by Bram Schoenmakers's avatar Bram Schoenmakers

Fix crash when completing/deleting items after an edit.

The crash occurs when using text-based identifiers and performing an edit on
selected items (instead of the whole todo.txt file).

If you edit the text of a child item (having a p tag), then the item is deleted
using the TodoListBase.delete() instead of TodoList.delete(). But
this messes up the dependency administration, still thinking that the removed
todo item is still in the list. When you call the children of any todo item
(part of the deletion/completion step), it will return a stale Todo item, whose
number() can not be found anymore in the TodoListBase. That makes it crash.
parent 076e9852
...@@ -415,6 +415,30 @@ class TodoListDependencyTester(TopydoTest): ...@@ -415,6 +415,30 @@ class TodoListDependencyTester(TopydoTest):
self.assertTrue(self.todolist.dirty) self.assertTrue(self.todolist.dirty)
self.assertTrue(self.todolist.todo_by_dep_id('99')) self.assertTrue(self.todolist.todo_by_dep_id('99'))
def test_delete01(self):
""" Check that dependency tags are cleaned up. """
todo = self.todolist.todo(4)
self.todolist.delete(todo, p_leave_tags=False)
self.assertTrue(self.todolist.is_dirty())
self.assertEqual(self.todolist.todo(3).source(), "Baz p:1")
def test_delete02(self):
""" Check that dependency tags are left when requested. """
todo = self.todolist.todo(4)
self.todolist.delete(todo, p_leave_tags=True)
self.assertTrue(self.todolist.is_dirty())
self.assertEqual(self.todolist.todo(3).source(), "Baz p:1 id:2")
def test_delete03(self):
""" Check that dependency tags are left when requested. """
todo = self.todolist.todo(3)
self.todolist.delete(todo, p_leave_tags=True)
self.assertTrue(self.todolist.is_dirty())
self.assertEqual(self.todolist.todo(3).source(), "Buzz p:2")
class TodoListCleanDependencyTester(TopydoTest): class TodoListCleanDependencyTester(TopydoTest):
""" """
......
...@@ -27,10 +27,6 @@ from topydo.lib.TodoList import TodoList ...@@ -27,10 +27,6 @@ from topydo.lib.TodoList import TodoList
# the true and only editor # the true and only editor
DEFAULT_EDITOR = 'vi' DEFAULT_EDITOR = 'vi'
# Access the base class of the TodoList instance kept inside EditCommand. We
# cannot use super() inside the class itself
BASE_TODOLIST = lambda tl: super(TodoList, tl)
def _get_file_mtime(p_file): def _get_file_mtime(p_file):
return os.stat(p_file.name).st_mtime return os.stat(p_file.name).st_mtime
...@@ -115,7 +111,7 @@ class EditCommand(MultiCommand): ...@@ -115,7 +111,7 @@ class EditCommand(MultiCommand):
if _is_edited(orig_mtime, temp_todos): if _is_edited(orig_mtime, temp_todos):
for todo in self.todos: for todo in self.todos:
BASE_TODOLIST(self.todolist).delete(todo) self.todolist.delete(todo, p_leave_tags=True)
for todo in new_todos: for todo in new_todos:
self.todolist.add_todo(todo) self.todolist.add_todo(todo)
......
...@@ -124,18 +124,18 @@ class TodoList(TodoListBase): ...@@ -124,18 +124,18 @@ class TodoList(TodoListBase):
if self._initialized: if self._initialized:
self._register_todo(todo) self._register_todo(todo)
def delete(self, p_todo): def delete(self, p_todo, p_leave_tags=False):
""" Deletes a todo item from the list. """ """ Deletes a todo item from the list. """
try: try:
number = self._todos.index(p_todo) number = self._todos.index(p_todo)
if p_todo.has_tag('id'): if p_todo.has_tag('id'):
for child in self.children(p_todo): for child in self.children(p_todo):
self.remove_dependency(p_todo, child) self.remove_dependency(p_todo, child, p_leave_tags)
if p_todo.has_tag('p'): if p_todo.has_tag('p'):
for parent in self.parents(p_todo): for parent in self.parents(p_todo):
self.remove_dependency(parent, p_todo) self.remove_dependency(parent, p_todo, p_leave_tags)
del self._todos[number] del self._todos[number]
self._update_todo_ids() self._update_todo_ids()
...@@ -206,20 +206,22 @@ class TodoList(TodoListBase): ...@@ -206,20 +206,22 @@ class TodoList(TodoListBase):
self.dirty = True self.dirty = True
@_needs_dependencies @_needs_dependencies
def remove_dependency(self, p_from_todo, p_to_todo): def remove_dependency(self, p_from_todo, p_to_todo, p_leave_tags=False):
""" Removes a dependency between two todos. """ """ Removes a dependency between two todos. """
dep_id = p_from_todo.tag_value('id') dep_id = p_from_todo.tag_value('id')
if dep_id: if dep_id:
p_to_todo.remove_tag('p', dep_id)
self._depgraph.remove_edge(hash(p_from_todo), hash(p_to_todo)) self._depgraph.remove_edge(hash(p_from_todo), hash(p_to_todo))
self.dirty = True
# clean dangling dependency tags
if dep_id and not p_leave_tags:
p_to_todo.remove_tag('p', dep_id)
if not self.children(p_from_todo, True): if not self.children(p_from_todo, True):
p_from_todo.remove_tag('id') p_from_todo.remove_tag('id')
del self._parentdict[dep_id] del self._parentdict[dep_id]
self.dirty = True
@_needs_dependencies @_needs_dependencies
def parents(self, p_todo, p_only_direct=False): def parents(self, p_todo, p_only_direct=False):
""" """
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment