Ensure that a tree's node is refreshed right away when its label is changed #2713
Conversation
The previous change actually broke some of the tests in test_disabled.py -- well actually it flat out broke one and caused all the others to run *very* slowly. No clue why though. But thinking about this some more, it does feel like delaying the refresh of the node makes more sense.
| @@ -307,6 +307,7 @@ def set_label(self, label: TextType) -> None: | |||
| self._updates += 1 | |||
| text_label = self._tree.process_label(label) | |||
| self._label = text_label | |||
| self._tree.call_later(self._tree._refresh_node, self) | |||
There was a problem hiding this comment.
I never know whether I should use call_later, call_soon, or the other one.
How do you decide?
There was a problem hiding this comment.
Did you mean call_next rather than call_soon? I generally go with the rule that call_later is the one to go with unless it's obvious otherwise because jumping the queue should be an exceptional thing to do.
There was a problem hiding this comment.
That makes sense, thanks.
| @@ -307,6 +307,7 @@ def set_label(self, label: TextType) -> None: | |||
| self._updates += 1 | |||
| text_label = self._tree.process_label(label) | |||
| self._label = text_label | |||
| self._tree.call_later(self._tree._refresh_node, self) | |||
There was a problem hiding this comment.
Why can't you refresh right here?
There was a problem hiding this comment.
Calling refresh directly broke much of test_disabled.py in a really fun way (try it and see, it's one of the weirdest breaks I've seen so far); some tests taking an age to complete, one just flat out breaking. I suspect some sort of deadly embrace going on; seemed to happen as the Tree in the tests was being spun up.
See #2698.