gtg icon indicating copy to clipboard operation
gtg copied to clipboard

Renaming a tag does not reparent the children tags, creates a zombie duplicate

Open nekohayo opened this issue 4 years ago • 5 comments

I'm not sure if this is just a GTG logic bug, or if this is actually a libLarch treeview representation bug, or both, or something else entirely. But let's say you have a tag named "foo" with two children tags named "bar" and "baz":

  • Foo
    • Bar
    • Baz

You right-click on the "Foo" tag in the sidebar, "Edit tag", and rename it (or even just add a single character, let's say it is now "Food").

Incorrect result:

  • Foo
    • Bar
    • Baz
  • Food

Instead it should be:

  • Food
    • Bar
    • Baz

Another thing to note with the "incorrect" result above: if you then manually reparent the children by dragging them to "Food", you realize that "Foo" was effectively (correctly) detagged from the tasks but that it sems like you still need to manually delete "Foo" from the tags sidebar... but when you try to do that, the changes are not visually reflected until you restart the application (at which time the tag is indeed gone from the sidebar).

I was wondering if that might warrant a test case for liblarch's test suite too, but this only affects tags and not (I think?) tasks, so this might just be a GTG-specific UI thing, I'm not sure... maybe @ploum would have a guess as to what could be the "right" place to look at in this case.

Ideally would be nice to fix for the upcoming 0.4 because this kind of behavior is "scary" for users, but if it's somehow not doable easily enough then it could be considered a 0.5 item.

nekohayo avatar May 09 '20 16:05 nekohayo

Arguably, I don't know why renaming a tag would actually be a "create + delete" operation at all, vs just changing the tag name in the app's running backend + storage backend, and changing the text label in the sidebar's treeview; if we could avoid all that stuff, then there would be no deparenting/reparenting to deal with.

nekohayo avatar May 09 '20 16:05 nekohayo

This is how it was coded apparently. I managed to re-parent the tasks as a temporary solution, but I still can't delete the old one without getting a traceback.

diegogangl avatar May 11 '20 22:05 diegogangl

I manage to re-parent tasks, but the original task doesn't go away until you open a task. Also, renaming the new tag or opening one a task with the new throws a traceback.

Here's an abbreviated version of the code with my changes

            for task_id in tag.get_related_tasks():

                # Store old tag attributes
                color = tag.get_attribute("color")
                icon = tag.get_attribute("icon")

                my_task = self.get_task(task_id)
                my_task.rename_tag(oldname, newname)

            # Restore attributes on tag
            new_tag = self.get_tag(newname)

            if color:
                new_tag.set_attribute("color", color)

            if icon:
                new_tag.set_attribute("icon", icon)

            # Re-parent children
            for child in tag.get_children():
                print('t', new_tag)
                child_tag = self.get_tag(child)
                child_tag.set_parent(new_tag.get_id())

            self.remove_tag(oldname)

(My changes are from "re-parenting" down)

This is the traceback:

Traceback (most recent call last):
  File "/home/januz/code/gtg/GTG/gtk/editor/editor.py", line 733, in destruction
    self.save()
  File "/home/januz/code/gtg/GTG/gtk/editor/editor.py", line 681, in save
    self.task.sync()
  File "/home/januz/code/gtg/GTG/core/task.py", line 632, in sync
    self.modified()
  File "/home/januz/code/liblarch/src/liblarch/liblarch/treenode.py", line 52, in modified
    self.tree.modify_node(self.node_id, priority=priority)
  File "/home/januz/code/liblarch/src/liblarch/liblarch/tree.py", line 89, in modify_node
    self._queue.push(self._modify_node, node_id, priority=priority)
  File "/home/januz/code/liblarch/src/liblarch/liblarch/processqueue.py", line 67, in push
    func(*element[1:])
  File "/home/januz/code/liblarch/src/liblarch/liblarch/tree.py", line 261, in _modify_node
    self._callback('node-modified', node_id)
  File "/home/januz/code/liblarch/src/liblarch/liblarch/tree.py", line 79, in _callback
    func(node_id)
  File "/home/januz/code/liblarch/src/liblarch/liblarch/viewcount.py", line 86, in __modify
    displayed &= filt.is_displayed(nid)
  File "/home/januz/code/liblarch/src/liblarch/liblarch/filters_bank.py", line 42, in is_displayed
    value = self.func(task, parameters=self.dic)
  File "/home/januz/code/gtg/GTG/core/treefactory.py", line 142, in tag_filter
    return node.has_tags([tag])
  File "/home/januz/code/gtg/GTG/core/task.py", line 786, in has_tags
    toreturn = children_tag(tagname)
  File "/home/januz/code/gtg/GTG/core/task.py", line 770, in children_tag
    for tagc_name in tag.get_children():
AttributeError: 'NoneType' object has no attribute 'get_children'

It seems like the tag doesn't go away completely when we delete. GTG or liblarch still thinks it exists and tries to access it, but finds a None at some point. Sounds like the tasks or tags or one of those trees need to be updated manually, but I can't figure out what to call on what.

pinging @ploum, maybe you have some ideas on this?

diegogangl avatar May 12 '20 23:05 diegogangl

according to the trace, it looks like a bug in liblarch. It should be interesting to have the details of this trace to see when it becomes "None" but, basically, Liblarch should never pass along a "None" object.

Not sure that the trace is related to reparenting, it looks like it's only the deleting of the tags.

It might happen that there's some kind of odd recursion :

delete_tag -> update task that contain this tag -> task try to re-delete the tag

ploum avatar May 13 '20 12:05 ploum

Another remark: when this bug happens, even if you eventually manage to delete the tags from the app and its data, there will be some leftover tags/cruft in tags.xml...

nekohayo avatar Jun 05 '20 05:06 nekohayo