Re: Invalid position error on redo position move

2019-08-21 Thread Brian Theado
Vitalije,

On Wed, Aug 21, 2019 at 5:41 PM vitalije  wrote:

> Well I didn't tried too hard to make it more readable. It can be improved.
>

Ok, after looking at it some more, I see that the vnode tree structure
involves only a list of children and a list of parents (one parent for each
clone). That's pretty straight forward and I added a few comments and it
seems plenty readable. I was able to change the code (without breaking it!)
so it creates the day node at the second position instead of the last
position. That is an important feature for me.


> But changing tree using vnodes has several benefits. First of all, vnodes
> are stable (immune to tree changes) - positions are not.
>

And I think this is what appeals the most to me.  With the position code, I
tried to factor out some common code, but it took two positions as input
(saved positions issue!) and for one of the cases I was using it, one of
the positions got invalidated. I suspect if I try to factor out some of the
vnode code in a similar way, it won't be as fragile as that.


> The second they work much faster because every change in the tree
> performed through positions are followed by a redraw and a lot of code that
> doesn't need to be executed until the complete tree change is done. You
> don't usually want to see intermediate versions of tree. You want to see
> the finished tree. When making changes using vnodes, nothing is redrawn
> until you explicitly call `c.redraw()`.
>

I took a quick look through the vnode code and only saw the redraw for .h
and .b changes, but not for the tree structure changes. Is that what you
are referring to or was there something else I missed?

 [...]

> The function make_snapshot creates a snapshot of data needed to recreate
> subtree structure. In the variant where no changes to headlines nor bodies
> are made, this function takes enough data to undo the changes. However if
> you need to keep record of headlines and bodies too, then you should add
> them too to snapshot. Of course  in that case you must change the function
> from_snapshot too.
>

Got it, thanks.

[...]

> This two functions can store and recreate the exact copy of the given
> vnode. If you want to make snapshot of the whole outline, pass the
> c.hiddenRootNode as the argument to the make_snapshot function. However, if
> you are certain that the only changes made are under one vnode (for example
> @ftlist node), then it is more efficient to store only subtree of @ftlist.
>

Thanks a lot for your help.

Brian

-- 
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 leo-editor+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/CAO5X8Cy_37DQ0ZXT%3D6Bs%2BO7q9-9B2g-s9hC9Vmfhn7HjXF-nWQ%40mail.gmail.com.


Re: Invalid position error on redo position move

2019-08-21 Thread vitalije



> I'm finding the vnode code harder to read, but if it works better then it 
> is better :-). However, is it better because you are the one who wrote it 
> instead of me :-), or because it is easier to avoid mistakes using vnodes? 
>

Well I didn't tried too hard to make it more readable. It can be improved. 
But changing tree using vnodes has several benefits. First of all, vnodes 
are stable (immune to tree changes) - positions are not. The second they 
work much faster because every change in the tree performed through 
positions are followed by a redraw and a lot of code that doesn't need to 
be executed until the complete tree change is done. You don't usually want 
to see intermediate versions of tree. You want to see the finished tree. 
When making changes using vnodes, nothing is redrawn until you explicitly 
call `c.redraw()`.

Iterating through every node is at least 2-3 times faster using vnodes than 
using position iterators.

 

> I'll take a close look at it and add the insert functionality I also had 
> in my code. Maybe that way I will if/how out how vnodes help avoid the 
> mistakes.
>
> About this comment:
> # if you need to keep track of headlines and boides too
> # yield v, tuple(v.children), tuple(v.parents), v.h, v.b
>
> The function make_snapshot creates a snapshot of data needed to recreate 
subtree structure. In the variant where no changes to headlines nor bodies 
are made, this function takes enough data to undo the changes. However if 
you need to keep record of headlines and bodies too, then you should add 
them too to snapshot. Of course  in that case you must change the function 
from_snapshot too. 
def from_snapshot(data):
for v, children, parents, h, b in data:
v.children[:] = children
v.parents[:] = parents
v.h = h
v.b = b

def make_snapshot(v0):
def it(v):
yield v, tuple(v.children), tuple(v.parents), v.h, v.b.b
for ch in v.children:
yield from it(ch)
return tuple(it(v0))

This two functions can store and recreate the exact copy of the given 
vnode. If you want to make snapshot of the whole outline, pass the 
c.hiddenRootNode as the argument to the make_snapshot function. However, if 
you are certain that the only changes made are under one vnode (for example 
@ftlist node), then it is more efficient to store only subtree of @ftlist.

HTH Vitalije

-- 
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 leo-editor+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/ff4fed4b-4833-4b58-a369-8d7dab2fb2a6%40googlegroups.com.


Re: Invalid position error on redo position move

2019-08-21 Thread Brian Theado
Thanks vitalije for noticing the position error. I wonder why it only gave
an error on redo and not also for the original operation.

Thanks for the vnode-based code. Based on a quick test, it seems better
than my position based code as I haven't seen any errors on undo/redo.

I'm finding the vnode code harder to read, but if it works better then it
is better :-). However, is it better because you are the one who wrote it
instead of me :-), or because it is easier to avoid mistakes using vnodes?
I'll take a close look at it and add the insert functionality I also had in
my code. Maybe that way I will if/how out how vnodes help avoid the
mistakes.

About this comment:
# if you need to keep track of headlines and boides too
# yield v, tuple(v.children), tuple(v.parents), v.h, v.b

Does that only mean if headlines and bodies are changing that the above
needs to be done?

On Wed, Aug 21, 2019, 1:35 PM vitalije  wrote:

> Well after `one_clone = one.clone()`, position `three` becomes invalid,
> because `one.clone()` inserts cloned node above the node `three`. If you
> add:
> three._childIndex += 1
>
>
> after `one_clone = one.clone()`, there will be no error after executing
> script.
>
> If I were you I would make all tree changes using v-nodes, and for making
> things undoable I would create my own undoHelper and redoHelper. Here is
> for example one of your scripts that we have discussed before:
>
> import time
> import leo.core.leoNodes as leoNodes
> def findFtlistAncestor(p):
> p = p.copy()
> while p and not p.h.startswith('@ftlist'):
> p = p.parent()
> return p
>
>
> def moveToTopAndCloneToAtDay(c, p):
> """
> Move the node identified by position p to the first child of
> an ancestor @ftlist node and also clone it to a sibling @day
> node with date matching today's date
> """
> ftlist = findFtlistAncestor(p)
> if not ftlist:
> g.es("Not in ftlist tree")
> return {}
> vft = ftlist.v
> before = make_snapshot(vft)
> v = p.v
> if vft in v.parents:
> i = vft.children.index(v)
> del vft.children[i]
> vft.children.insert(0, v)
> else:
> vft.children.insert(0, v)
> v.parents.append(vft)
> today = "@day " +  time.strftime("%Y-%m-%d", time.gmtime())
> for i, ch in enumerate(vft.children):
> if ch.h == today:
> break
> else:
> i = len(vft.children)
> ch = leoNodes.vnode(c)
> ch.h = today
> vft.children.append(ch)
> ch.parents.append(vft)
> if ch in v.parents:
> i = ch.children.index(v)
> del ch.children[i]
> ch.children.insert(0, v)
> else:
> ch.children.insert(0, v)
> v.parents.append(ch)
> return ftlist, before, make_snapshot(vft)
>
>
> def from_snapshot(data):
> for v, children, parents in data:
> v.children[:] = children
> v.parents[:] = parents
>
>
> def make_snapshot(v0):
> def it(v):
> yield v, tuple(v.children), tuple(v.parents)
> # if you need to keep track of headlines and boides too
> # yield v, tuple(v.children), tuple(v.parents), v.h, v.b
> for ch in v.children:
> yield from it(ch)
> return tuple(it(v0))
>
>
> def do_change():
> undo_data = c.undoer.createCommonBunch(p)
> undo_data.kind = 'my-special-operation'
> undo_data.undoType = 'my-special-operation'
> ftlist, before, after = moveToTopAndCloneToAtDay(c, p)
> undo_data.before = before
> undo_data.after = after
> undo_data.ftlist = ftlist
> undo_data.undoHelper = lambda:from_snapshot(undo_data.before) or c.
> redraw(undo_data.p)
> undo_data.redoHelper = lambda:from_snapshot(undo_data.after) or c.
> redraw(undo_data.ftlist.firstChild())
> c.undoer.pushBead(undo_data)
> c.redraw(ftlist.firstChild())
>
> do_change()
>
> If you make a script button of this code, you can try your action and its
> undo/redo functionality on some of the descendants of your @ftlist tree.
>
> HTH Vitalije
>
> --
> 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 leo-editor+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/leo-editor/20942315-145c-4dff-83ea-d0b6a1a39afe%40googlegroups.com
> 
> .
>

-- 
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 leo-editor+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/CAO5X8CwDc1PwdXbpWe0yZMuC9fSy_ZcsXP4W1U_srRzfeMJhhA%40mail.gmail.com.

Re: Invalid position error on redo position move

2019-08-21 Thread vitalije
Well after `one_clone = one.clone()`, position `three` becomes invalid, 
because `one.clone()` inserts cloned node above the node `three`. If you 
add:
three._childIndex += 1


after `one_clone = one.clone()`, there will be no error after executing 
script.

If I were you I would make all tree changes using v-nodes, and for making 
things undoable I would create my own undoHelper and redoHelper. Here is 
for example one of your scripts that we have discussed before:

import time
import leo.core.leoNodes as leoNodes
def findFtlistAncestor(p):
p = p.copy()
while p and not p.h.startswith('@ftlist'):
p = p.parent()
return p


def moveToTopAndCloneToAtDay(c, p):
"""
Move the node identified by position p to the first child of
an ancestor @ftlist node and also clone it to a sibling @day
node with date matching today's date
"""
ftlist = findFtlistAncestor(p)
if not ftlist:
g.es("Not in ftlist tree")
return {}
vft = ftlist.v
before = make_snapshot(vft)
v = p.v
if vft in v.parents:
i = vft.children.index(v)
del vft.children[i]
vft.children.insert(0, v)
else:
vft.children.insert(0, v)
v.parents.append(vft)
today = "@day " +  time.strftime("%Y-%m-%d", time.gmtime())
for i, ch in enumerate(vft.children):
if ch.h == today:
break
else:
i = len(vft.children)
ch = leoNodes.vnode(c)
ch.h = today
vft.children.append(ch)
ch.parents.append(vft)
if ch in v.parents:
i = ch.children.index(v)
del ch.children[i]
ch.children.insert(0, v)
else:
ch.children.insert(0, v)
v.parents.append(ch)
return ftlist, before, make_snapshot(vft)


def from_snapshot(data):
for v, children, parents in data:
v.children[:] = children
v.parents[:] = parents


def make_snapshot(v0):
def it(v):
yield v, tuple(v.children), tuple(v.parents)
# if you need to keep track of headlines and boides too
# yield v, tuple(v.children), tuple(v.parents), v.h, v.b
for ch in v.children:
yield from it(ch)
return tuple(it(v0))


def do_change():
undo_data = c.undoer.createCommonBunch(p)
undo_data.kind = 'my-special-operation'
undo_data.undoType = 'my-special-operation'
ftlist, before, after = moveToTopAndCloneToAtDay(c, p)
undo_data.before = before
undo_data.after = after
undo_data.ftlist = ftlist
undo_data.undoHelper = lambda:from_snapshot(undo_data.before) or c.
redraw(undo_data.p)
undo_data.redoHelper = lambda:from_snapshot(undo_data.after) or c.redraw
(undo_data.ftlist.firstChild())
c.undoer.pushBead(undo_data)
c.redraw(ftlist.firstChild())

do_change()

If you make a script button of this code, you can try your action and its 
undo/redo functionality on some of the descendants of your @ftlist tree.

HTH Vitalije

-- 
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 leo-editor+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/20942315-145c-4dff-83ea-d0b6a1a39afe%40googlegroups.com.


Re: Invalid position error on redo position move

2019-08-21 Thread Brian Theado
Thanks, vitalije. The else part of the 'if 1:' was my failed attempt to
further simplify the failing case.

The error is happening when the code does: insert, clone, move the clone to
the first child of node="two" (which already has one child named "three"),
undo, redo.

I though I might also duplicate it by skipping the insert and just cloning
the already existing node="one". But still I want to move the clone to the
first child of "two". Since the insert was skipped, one fewer moveToNext()
was needed in order to get the to new parent node="two".

The error given by the code as currently written is what I need to solve.
You can ignore the else clause it is just noise :-).

Actually, inspired by your response, I found a way to duplicate the error
even without the insert. So now it is just clone, move, undo, redo. Here is
the new subtree xml:



http://leoeditor.com/namespaces/leo-python-editor/1.1;
>


redo bug (use ctrl-b)
one
two
three
four




@language python

# Code written to assume this descendent structure
# one
# two
# three
#   four
parent = p; u = c.undoer

one = parent.copy().moveToFirstChild()
three = one.copy().moveToNext().moveToNext()

undoData = u.beforeCloneNode(one)
one_clone = one.clone()
u.afterCloneNode(one_clone, "clone node", undoData)

undoData = u.beforeMoveNode(one_clone)
one_clone.moveToFirstChildOf(three)
u.afterMoveNode(one_clone, "move node to top", undoData)

# Maybe the next step is to set a breakpoint in the "invalid position"
# and trace it from there.
c.executeMinibufferCommand('undo')
c.executeMinibufferCommand('redo')







On Wed, Aug 21, 2019 at 8:15 AM vitalije  wrote:

> In the first case after `if 1:`
> you have called two times child.copy().moveToNext().moveToNext(), while in
> the else case you have called it only once.
> I have tried with one call in both cases and it works without error.
> Vitalije
>
> --
> 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 leo-editor+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/leo-editor/fced3afd-898a-493c-8b31-c3e5dbff7adb%40googlegroups.com
> 
> .
>

-- 
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 leo-editor+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/CAO5X8Cw40djUZCsRqc5xz0ZfOkqhpG2Fa6z2Q3i7bJVre2JTAw%40mail.gmail.com.


Re: Invalid position error on redo position move

2019-08-21 Thread vitalije
In the first case after `if 1:`
you have called two times child.copy().moveToNext().moveToNext(), while in 
the else case you have called it only once.
I have tried with one call in both cases and it works without error.
Vitalije

-- 
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 leo-editor+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/fced3afd-898a-493c-8b31-c3e5dbff7adb%40googlegroups.com.


Invalid position error on redo position move

2019-08-21 Thread Brian Theado
In the code I've been writing, there is one particular case which gives an
"invalid position" error when I redo it. I've written some simplified code
which replicates the scenario (even removing the grouped undo allows the
issue to duplicate).

The code performs 3 individually undoable operations: insert, clone, then
move. The error comes after undoing the move and then redoing that same
move.

Just sharing in case someone sees something obviously wrong. Otherwise I'll
be digging deeper sometime this week.

Procedure:

   1. Copy the below xml
   2. open a new leo outline
   3. Ctrl-shift-v to paste the nodes
   4. Ctrl-b to execute

Error looks like this:

setCurrentPosition Invalid position: 'newHeadline', root: 'NewHeadline'
select,selectHelper,select_new_node,set_body_text_after_select
setCurrentPosition Invalid position: 'newHeadline', root: 'NewHeadline'
new_cmd_wrapper,redo,redoMove,selectPosition



http://leoeditor.com/namespaces/leo-python-editor/1.1;
>


redo bug (use ctrl-b)
one
two
three




@language python

# Code written to assume this descendent structure
# one
# two
#   three
parent = p; u = c.undoer

# Will the issue still reproduce without the insert?
if 1:
undoData = u.beforeInsertNode(parent)
child =parent.insertAsNthChild(0)
u.afterInsertNode(child, "insert as first child", undoData)
sibling =  child.copy().moveToNext().moveToNext()
else:
# Nope. The issue doesn't duplicate with this
child = parent.copy().moveToFirstChild()
sibling = child.copy().moveToNext()

undoData = u.beforeCloneNode(child)
clone = child.clone()
u.afterCloneNode(clone, "clone node", undoData)

undoData = u.beforeMoveNode(clone)
clone.moveToFirstChildOf(sibling)
u.afterMoveNode(clone, "move node to top", undoData)

# Maybe the next step is to set a breakpoint in the "invalid position"
# and trace it from there.
c.executeMinibufferCommand('undo')
c.executeMinibufferCommand('redo')






-- 
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 leo-editor+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/CAO5X8Cx15dS8oktjUBQ%3DTk8rRTqxTYPR7jhOfcVxa%3DhZnL_4CA%40mail.gmail.com.