ENB: Bugs in Leo's outline-paste commands
Edward K. Ream
This Engineering Notebook post discusses subtle problems with Leo's paste-outline commands: paste-node, paste-retaining-clones, and paste-as-template. A new unit test (in PR #3473) revealed these problems. They may be ancient.
This post will be of interest only to hard-core Leo devs. Feel free to ignore it. Note: Remember that I'll be discussing code in the new (6.7.5) codebase.
A subtle bug
Dumps in a new unit test reveal that c.recompute_all_parents fails:
def recompute_all_parents(self) -> None:
"""
Recompute all v.parents arrays using neither positions
nor v.parents ivars.
"""
c = self
root_v = c.hiddenRootNode
# Clear all v.parents arrays.
root_v.parents = []
for v in c.alt_all_unique_nodes():
v.parents = []
# Loop invariant: Visit each *parent* vnode *once*.
# Child vnodes may be visited more than once.
# Visit the hidden root.
for child in root_v.children:
child.parents.append(root_v)
# Visit all other parents.
for parent in c.alt_all_unique_nodes():
for child in parent.children:
child.parents.append(parent)
How can this "obviously correct" code fail? What's going on??
Aha! c.alt_all_unique_nodes() can yield nodes with different ids than those in root_v.children. In other words, two distinct VNodes may share the same gnx. Big oops.
The cause of the bug
Leo calls c.reassignNonClonedIndices after calling c.unarchive_to_vnode. But unarchive_to_vnode itself creates (or patches) parent/child links.
In short: unarchive_to_vnode must reallocate gnxs.
fc.gnxDict
Lurking behind all this code is the infamous fc.gnxDict dictionary. Keys are gnxs; values are the corresponding VNodes. c.recreateGnxDict supposedly recreates this dict. Various read/write/paste helpers save and restore this dict for impossible-to-remember reasons.
I would love to remove this dict, but doing so probably won't work. Because of undo, Leo never deallocates VNodes! Instead, I am leaning towards replacing fc.gnxDict with, say, c.all_gnxs_dict. Leo will:
- only add keys to this dict.
- never delete keys.
- never save/restore this dict.
Only a detailed examination of Leo's entire codebase will show whether this plan is feasible.
Summary
Let this post be a warning for anyone foolish enough to assert that some code is "obviously correct."
It's impossible to say whether the bugs discussed here exist in Leo's existing codebase without adding new unit tests to devel. I'm not going to do that. It's too late to change Leo 6.7.4.
All calls to c.recreateGnxDict seem dubious. PR #3473 is an opportunity to re-engineer how Leo remembers existing VNodes.
All three paste-outline commands and their undo/redo helpers must likely confront these issues explicitly. "Clever" solutions will likely fail.
Edward
Edward K. Ream
> Lurking behind all this code is the infamous fc.gnxDict dictionary. Keys are gnxs; values are the corresponding VNodes. c.recreateGnxDict supposedly recreates this dict. Various read/write/paste helpers save and restore this dict for impossible-to-remember reasons.