An implementation of paste-retaining-outside-clones command

[vitalije]

This might not be the best possible name but it reflects an idea that came from Seth Jonson in another thread.

The idea is to clone all those nodes that are present in the outline outside copied tree. All nodes that are located only under the copied tree should not be cloned unless they have clones under the copied tree. In essence this is just what paste-node command does plus it keeps identity of all nodes that have clones elsewhere in the outline outside the copied tree.

Here is the code:

from xml.etree import ElementTree as ET
import leo.core.leoNodes as leoNodes

c.frame.log.clearTab('Log')
def paste_as_template(p):
    def newgnx():
        ni = g.app.nodeIndices
        t_s = ni.update()
        return f"{ni.userId}.{t_s}.{ni.lastIndex:d}"
    def skip_root(v):
        if v.gnx != root_gnx:
            yield v
            for ch in v.children:
                yield from skip_root(ch)
    def translate_gnx(gnx):
        if gnx in outside:
            return gnx
        return newgnx()
    gnx2v = c.fileCommands.gnxDict
    def getv(gnx):
        v = gnx2v.get(gnx)
        if v is None:
            return leoNodes.VNode(c, gnx), True
        return v, False
    def viter(pargnx, xv):
        chgnx = xv.attrib.get('t')
        b = bodies[chgnx]
        gnx = translation.get(chgnx)
        if gnx in seen:
            yield pargnx, gnx, heads[gnx], b
        else:
            seen.add(gnx)
            h = xv[0].text
            heads[gnx] = h
            yield pargnx, gnx, h, b
            for xch in xv[1:]:
                yield from viter(gnx, xch)
    def dumptree(): # just for debuging
        levs = {'':-1}
        for pgnx, gnx, h, b in viter('', xvnodes[0]):
            lev = levs[pgnx] + 1
            levs[gnx] = lev
            g.es(f'{"----" * lev}[{gnx}]{h}')
    def do_paste(vpar, index):
        vpargnx = vpar.gnx
        rows = viter(vpargnx, xvnodes[0])
        pgnx, gnx, h, b = next(rows)
        v, _ = getv(gnx)
        v.h = h
        v.b = b
        vpar.children.insert(index, v)
        v.parents.append(vpar)
        pasted = v
        for pgnx, gnx, h, b in rows:
            v, isNew = getv(gnx)
            if isNew:
                v.h = h
                v.b = b
            vpar = getv(pgnx)[0]
            vpar.children.append(v)
            v.parents.append(vpar)
        return pasted
    def undoHelper():
        v = vpar.children.pop(index)
        v.parents.remove(vpar)
        c.redraw(bunch.p)
    def redoHelper():
        vpar.children.insert(index, pasted)
        pasted.parents.append(vpar)
        c.redraw(newp)
    xroot = ET.fromstring(g.app.gui.getTextFromClipboard())
    #xroot = ET.fromstring(clipboard_text())
    xvnodes = xroot.find('vnodes')
    xtnodes = xroot.find('tnodes')
    bodies = {x.attrib['tx']:x.text for x in xtnodes}
    root_gnx = xvnodes[0].attrib.get('t')
    outside = {x.gnx:x for x in skip_root(c.hiddenRootNode)}
    translation = {x:translate_gnx(x) for x in bodies.keys()}
    seen = set()
    heads = {}
    dumptree()
    bunch = c.undoer.createCommonBunch(p)
    if p.isExpanded():
        vpar = p.v
        index = 0
        parStack = p.stack + [(p.v, p._childIndex)]
    else:
        parStack = p.stack
        vpar = p.stack[-1][0] if p.stack else c.hiddenRootNode
        index = p._childIndex + 1
    seen.clear()
    pasted = do_paste(vpar, index)
    newp = leoNodes.Position(pasted, index, parStack)
    bunch.undoHelper = undoHelper
    bunch.redoHelper = redoHelper
    bunch.undoType = 'paste-retaining-outside-clones'
    c.undoer.pushBead(bunch)
    c.redraw(newp)
paste_as_template(p)

The command is undoable.

Vitalije

[vitalije]

It doesn't paste unknownAttributes but it would be trivial to add this functionality.

Vitalije

[Differance]

Glad I could help here in some manner!  Even if I'm still stymied about what the difference is in what you're trying to solve versus what I'm trying to solve.  :-)

I gather that you're not trying to do the "make local/internal clones with a new vnode" thing I focused on.  Rather, you're figuring a way to identify which nodes to clone and how.  Within that phrasing I don't really quite get where you're coming from yet.  But don't mind me, I'll pipe up at some point with maybe more of a clue!  :-)

Seth

[Edward K. Ream]

On Thu, May 28, 2020 at 5:28 AM vitalije <vita...@gmail.com> wrote:

This might not be the best possible name but it reflects an idea that came from Seth Jonson in another thread.

Thanks for this. I'll take a look. Imo, copy-node followed by paste-as-template is even better than an atomic duplicate-template command.

I thought I had a simple way earlier today, but it failed as before. Messing with gnx's is tricky. I'm glad you found a way.

Edward

[Edward K. Ream]

On Thursday, May 28, 2020 at 12:33:30 PM UTC-5, Edward K. Ream wrote:

> Thanks for this. I'll take a look.

This is great stuff. The code is difficult, and it seems to work.

To create the paste-as-template command, create the following in commands/commanderOutlineCommands.py:

@g.commander_command('paste-as-template')
def pasteAsTemplate(self, event=None):
    c = self
    c.frame.log.clearTab('Log')
    paste_as_template(c, c.p)

A bug: In the do_paste function, change in two places:

v.h = h
v.b = b

To:

v.h = h or ''
v.b = b or ''

One more thing. I realized earlier today that the NodeIndices class should have a computeNewIndex method, exactly the same as your gnx function.  I'll add this to devel soon.

Next steps

You definitely should get full credit for this code. How do you want to incorporate it into Leo?

I suggest creating your own branch, and add the wrapper code shown above.  I've stopped working on the template branch.

Edward

[Edward K. Ream]

On Thursday, May 28, 2020 at 1:06:39 PM UTC-5, Edward K. Ream wrote:

Rev 5ec08e8 in devel adds NodeIndices.computeNewIndex. You don't have to use it, but imo there is no reason not to.

I've updated the title and second comment of #1593. Vitalije, I plan no further work until after you commit your code.

Edward

[vitalije]

Done at a48cdaa67f367b30.

I've added some comments to make code easier to understand.

The function skip_root:

def skip_root(v, root_gnx):

    if v.gnx != root_gnx:
        yield v
        for ch in v.children:

            yield from skip_root(ch, root_gnx)
outside = { x.gnx for x in skip_root(c.hiddenRootNode, root_gnx) }

yields v nodes in the outline order skipping the node with gnx equal to root_gnx. This is used to gather all gnxes of all nodes located outside the copied tree.

Once we have calculated `outside`, we can make a gnx translation table. This table will translate gnxes from clipboard to the actual gnx that will be used for v nodes.

def translate_gnx(gnx):
    if gnx in outside:
        return

gnx
    return g.app.nodeIndices.computeNewIndex()

translation = { x: translate_gnx(x) for x in bodies.keys() }

New gnx values are allocated for each node that is not in `outside` set.

The core of the paste operation is a generator function `viter`:

def viter(parent_gnx, xv):

    chgnx = xv.attrib.get('t')
    b = bodies[chgnx]
    gnx = translation.get(chgnx)
    if gnx in seen:

        yield parent_gnx, gnx, heads[gnx], b
    else:

        seen.add(gnx)
        h = xv[0].text
        heads[gnx] =

h
        yield parent_gnx, gnx, h, b
        for xch in xv[1:]:
            yield from viter(gnx, xch)

Variables heads, bodies and seen are defined in the parent scope. Heads and bodies are dicts, and seen is a set. (While I was writing this I'vre realized there was a bug in pasteAsTemplate. If outside clones have children, their children used to be duplicated. The fix was to initialize `seen` set to `outside` set.)

Generator yields tuples of four elements (parent_gnx, child_gnx, headline, body). Descendant nodes are iterated only once. This tuple allows us to create or use already existent nodes and link them. This is done in the `do_paste` function:

def do_paste(vpar, index):
    '''
    pastes a new node as a child of vpar at given index
    '''
    vpargnx = vpar.gnx
    # the first node is inserted at the given index
    # and the rest are just appended at parents children
    # to achieve this we first create a generator object

    rows = viter(vpargnx, xvnodes[0])

    # then we just take first tuple

    pgnx, gnx, h, b = next(rows)

    # create vnode
    v, _ = getv(gnx)

    v.h = h
    v.b =

b

    # and finally insert it at the given index
    vpar.children.insert(index, v)
    v.parents.append(vpar)

    pasted = v # remember the first node as a return value

    # now we iterate the rest of tuples

    for pgnx, gnx, h, b in rows:

        # get or create a child `v`

        v, isNew = getv(gnx)
        if isNew:

            v.h = h
            v.b =

b

        # get parent node `vpar`
        vpar = getv(pgnx)[0]

        # and link them
        vpar.children.append(v)
        v.parents.append(vpar)

    return pasted

gnx2v = c.fileCommands.gnxDict

def getv(gnx):
    v = gnx2v.get(gnx)
    if v is None:
        return leoNodes.VNode(c, gnx), True
    return v, False

The rest of code in the c.pasteAsTemplate is just for making this operation undoable.

While writing this explanations I've discovered a few bugs and fixed them. The latest version is 027e147eb.

Now I would like to point out some problems I encountered while writing this code and suggestions how to improve Leo. First of all I've realized that running experiments during development can demage the outline. I've learned to postpone all v-node manipulations as late as possilbe. Using this aproach has lead me to more testable code as well. For example: it would be natural to use FastRead to read clipboard, but it was almost imposible to reuse anything from this class, because it messes up with gnxDict. FastRead is too tied to the rest of Leo. Therefore I had to reimplement this functionallity from scratch. FastRead should at least have a method to turn xml into ElementTree.Element, and a generator to generate tuples (level, gnx, h, b, ua) or (parent_gnx, gnx, h, b, ua). If it had such methods, I would certainly use them in this command. Furthermore this methods would be easy to test and they would be completely decoupled from anything else in Leo. They would provide functionallity without messing any other part of Leo. This would make them easy to use without fear that they could change something behind your back. Let's just remember a bug with the empty outline due to the gnx collisions. Segundo Bob has written a great issue report, provided scripts to reproduce the bug and yet we couldn't find the bug for several years. Unable to find the bug I was very certain that this bug was caused by some low level OS thing outside Python. But the bug was just in messed up gnxDict which FastRead relies on.

When I looked at FastAtRead's code recently I was thouroughly disappointed with its current shape. For the testing purpose, some new kwargs are added which again makes this code harder to read, harder to understand, and also slower than it used to be.

I see here the fundamentaly wrong decesion to change inner code of methods in order to make some tests against it. This kind of tests are useless because they test not the production code but the different one. There are many places in Leo that have `if g.unitTesting` guard. At some places they just add some logging but there are places where this guard is not so benign, where it changes the actual meaning of the method. I remember how the experimental code I wrote before, whch was the inspiration for FastAtRead, used to be testable and decoupled from everything else inside Leo. It doesn't have to be this way.

For example viter generator function yields tuples of strings. It is very easy to test it, to debug, to print the results and it won't spoil anything inside Leo ivars. Its usage is harmless. I think both FastRead and FastAtRead should do the same. Instead of actually creating vnodes and manipulating them, they should generate necessary data for node creation and linkage. Top level methods like readWithElementTree should use this generator methods to actually create vnodes. FastAtRead.read_into_root should also use some generator of tuples consisting of simple data types (strings, ints, booleans) and just use those tuples to actually create and link vnodes. That way it won't be necessary to add any kwargs for testing. It would be trivial to test these generators and compare the values from those tuples with expected ones. And if the generators work correctly than we can be pretty sure that top level functions will work correctly too.

Again while writing this I've just realized that I forgot about unknown attributes in paste-as-template.

Vitalije

[vitalije]

Something is wrong with google groups code formatting. My code examples in the previous message have some line breaks that I didn't write. When I tried to edit that message, those line breaks disappeared and now they are again present.

Vitalije

[vitalije]

I am glad to report that I've just found FastRead.scanTnodes method that I could use in paste-as-template. This method is a perfect example of what I was writing about: it provides functionality without disturbing anything else by returning an ordinary pair of dicts. Easy to use, easy to test.

Vitalije

[vitalije]

Done at c9e92f5574cd.

This version handles unknown attributes using FastRead.scanTnodes method. It would be nicer if this method was a function because it really doesn't need neither c nor gnxDict. If it were just a plain function inside leoGlobals, I could use it as:

bodies, uas = g.scanTnodes(xtnodes)

instead of much uglier:

import leo.core.leoFileCommands as leoFileCommands
bodies, uas = leoFileCommands.FastRead(c, {}).scanTnodes(xtnodes)

Vitalije

[Edward K. Ream]

On Fri, May 29, 2020 at 7:07 AM vitalije <vita...@gmail.com> wrote:

Something is wrong with google groups code formatting. My code examples in the previous message have some line breaks that I didn't write. When I tried to edit that message, those line breaks disappeared and now they are again present.

I have sometimes noticed similar problems. One workaround is pasting code to Leo, then copying from Leo. That seems to strip formatting characters that confuse matters.

Edward

[Edward K. Ream]

On Fri, May 29, 2020 at 7:25 AM vitalije <vita...@gmail.com> wrote:

Done at c9e92f5574cd.

This version handles unknown attributes using FastRead.scanTnodes method. It would be nicer if this method was a function...

Feel free to make the change if you like.

There is an ongoing tension between using methods and functions. My preferred style now includes top-level functions for general use. This sometimes avoids having to use subclasses.  The node "leoAst.py: top-level" in leoAst.py is a substantial.

Edward

[Edward K. Ream]

On Fri, May 29, 2020 at 7:01 AM vitalije <vita...@gmail.com> wrote:

Done at a48cdaa67f367b30. [in devel]

I've added some comments to make code easier to understand.

Many thanks for all this work! Feel free to close #1593 when you complete work.

The node "c_oc.pasteAsTemplate" defines the paste-as-template command. It's reasonable to do the work directly in devel, because the only new code is the new command.

I have just deleted the template branch. There is nothing of use there now.

Edward

[Edward K. Ream]

On Fri, May 29, 2020 at 7:01 AM vitalije <vita...@gmail.com> wrote:

> Now I would like to point out some problems I encountered while writing this code and suggestions how to improve Leo.

...

>it would be natural to use FastRead to read clipboard, but it was almost imposible to reuse anything from this class, because it messes up with gnxDict. FastRead is too tied to the rest of Leo.

I agree.

> When I looked at FastAtRead's code recently I was thoroughly disappointed with its current shape.

Yes. The various dependencies you mention make it much too hard to use, as I discovered in my own attempts at paste-as-template.

> I see here the fundamentally wrong decision to change inner code of methods in order to make some tests against it. This kind of tests are useless because they test not the production code but the different one.

I agree.

> There are many places in Leo that have `if g.unitTesting` guard. At some places they just add some logging but there are places where this guard is not so benign, where it changes the actual meaning of the method. I remember how the experimental code I wrote before, whch was the inspiration for FastAtRead, used to be testable and decoupled from everything else inside Leo. It doesn't have to be this way.

Feel free to change it. I trust you.

Edward