On Wed, Apr 27, 2016 at 8:10 PM, Edward K. Ream <[email protected]>
wrote:
Rev dae5c2f fixed a subtle bug in cff command.
>
> The heart of the original code was:
>
> for p in clones:
> p2 = p.clone()
> p2.moveToLastChildOf(found)
>
> where the 'found' node is the top-level node created by the cff command.
>
[snip]
>
This is likely a bug in a low-level position method called by
p.moveToLastChildOf.
No. The low-level code isn't buggy, so trying to "fix" it would surely
introduce serious bugs into Leo's fundamental code.
This post explains in detail what the real problem is, namely saved
positions that low-level code has *no way* to update correctly. As in the
cff command, it's not always possible to avoid saved positions, but there
are workarounds discussed here.
This post will be of interest to Leo's core developers, and to script
writers who write scripts that alter Leo's outline. Everyone else can stop
reading now.
*Avoidable saved positions*
To repeat, here is the original, buggy, code:
for p in clones:
p2 = p.clone()
p2.moveToLastChildOf(found)
The 'found' node is a saved position. Found was created as the last
top-level node, so one solution (untested) would be the following:
for p in clones:
p2 = p.clone()
p2.moveToLastChildOf(c.lastTopLevel())
If p is a top-level position, the call to p.clone(), will change the
position of the last top-level node, so the code should recompute it rather
than use the saved/cached 'found' position.
*Important*: there is *nothing* that Leo's low-level methods can do to
handle this situation. p.clone() knows *nothing* about the 'found'
position, so it can't adjust it. Otoh, p.moveToLastChildOf(found)* does*
know about found, and so is *usually* justified in adjusting it.
*Outline structure was never corrupted*
Even with the buggy code, the outline was updated correctly. This is
because p2.moveToLastChildOf(found) doesn't use found._childIndex, the only
part of the 'found' position that was ever incorrect. Indeed, only found.v
and found.parent() are needed for moveToLastChildOf to work correctly.
To repeat, the only bug in the original code was that Leo could not select
the 'found' position in some cases. This suggests that yet another fix
would be to leave the original code alone and simply do this at the very
end:
c.selectPosition(c.lastTopLevel())
I don't much like this solution, as it seems to ignore what's really going
on.
*Other workarounds*
p.moveToLastChildOf just calls p.moveToNthChildOf, which is:
def moveToNthChildOf(self, parent, n):
p = self
parent._adjustPositionBeforeUnlink(p)
p._unlink()
p._linkAsNthChild(parent, n)
return p
So yet another solution (untested) is likely to be:
for p in clones:
p2 = p.clone()
p2._unlink()
n = found.numberOfChildren()
p2._linkAsNthChild(found, n, adjust=False)
*Harder problems*
Despite these complexities, the cff command is relatively simple because
in the end the outline doesn't change, except for the found node and its
contents. Things get much stickier if a script *permanently *inserts or
deletes nodes. In these situations, script writers should be aware of the
following methods:
- c.createNodeHierarchy(heads, parent=None, forcecreate=False)
- c.deletePositionsInList(aList, callback=None)
Edward
--
You received this message because you are subscribed to the Google Groups
"leo-editor" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/leo-editor.
For more options, visit https://groups.google.com/d/optout.
