Undo redo

[jayantj]

Initial implementation of undo-redo for treema. It seems to work fine for most actions I tried in the treema (deleting, editing, adding nodes, changing node types).

I'm not sure about the code, it's not very clean, and probably can be more efficient as well. @sderickson It'd be great to have some feedback about stuff I might have missed, and anything else really.

If this is fine, I'll begin implementing it for the level editor. I use chrome console to debug and test it out, adding keyboard shortcuts in treema itself didn't make much sense. For scratch.html, just call -

1. treema.undo() for undo
2. treema.redo() for redo

[sderickson]

Looking good! I got it to work well, there are just a few things to do:

1. Two tests are broken. With the dev server running, go to http://localhost:9090/tests.html and you'll see a couple of them no longer work.
2. There are no tests for undo/redo. You should add your own file to the /test folder with some Jasmine tests. Basically anything you did on the command line to make sure treema did what you expected it to do. I would imagine you'd run various 'set', 'insert' and 'delete' commands on a treema, then use undo/redo and see if the root data value is correct. Some ideas would be:
   1. Does it do the basics (set a value, unset a value, undo/redo, not change anything when you're "out" of actions).
   2. Does it handle nested values.
   3. Does it handle a long, complicated string of actions.
3. Add keyboard shortcut support. It is a bit confusing, the way it's set up, so I went ahead and put it together. Go ahead and add to TreemaNode:

onZPressed call to onKeyDown:

  onKeyDown: (e) ->
    @onEscapePressed(e) if e.which is 27
    @onTabPressed(e) if e.which is 9
    @onLeftArrowPressed(e) if e.which is 37
    @onUpArrowPressed(e) if e.which is 38
    @onRightArrowPressed(e) if e.which is 39
    @onDownArrowPressed(e) if e.which is 40
    @onEnterPressed(e) if e.which is 13
    @onNPressed(e) if e.which is 78
    @onSpacePressed(e) if e.which is 32
    @onTPressed(e) if e.which is 84
    @onFPressed(e) if e.which is 70
    @onZPressed(e) if e.which is 90
    @onDeletePressed(e) if e.which in [8, 46] and not e.heldDown

the function itself:

  onZPressed: (e) ->
    if e.ctrlKey or e.metaKey
      if e.shiftKey
        @getRoot().redo()
      else
        @getRoot().undo()

[jayantj]

I've added the keyboard shortcuts, and fixed the broken tests. I'll add additional tests by tomorrow, hopefully

[jayantj]

A couple of things with tracking data changes made through set, insert and delete:

1. There's some rather hack-y code in the set method for saving the path, I'm not sure of how else it can be done.
2. I'm not sure how to go about it with delete, some help would be great.

[sderickson]

Hmm, something's still a bit wonky about undo/redo. I tested it out on the Overview page for Treema, with the first demonstration one on top, and it seems to break when I switch the product type between 'donut' and 'pastry'. Maybe the system isn't set up for enum type properties?

[sderickson]

Re finding a better way for set, insert and delete, it might be best to break these functions out into regular functions which call recursive functions, so you can have logic that runs at the beginning and the end but never in the middle. Does that make sense?

Re the delete function, I've marked the two places where properties are removed. For the first one you can just use the current node's path, for the other one (which is when you delete a node that isn't currently opened to) you'll need to combine the current node's path plus the rest of the path to construct the delete action object.

  delete: (path) ->
    path = @normalizePath(path)
    return @remove() if path.length is 0 # HERE
    return @digDeeper(path, 'delete', false, []) if @childrenTreemas?

    data = @data
    for seg, i in path
      seg = @normalizeKey(seg, data)
      if path.length is i+1
        if $.isArray(data) then data.splice(seg, 1) else delete data[seg]
        # HERE
        @refreshDisplay()
        return true
      else
        data = data[seg]
        return false if data is undefined

[jayantj]

I've taken care of the stuff about the enum nodes, deletion of multiple nodes, and the constructor.name thing, and a few other bugs I found. I added some initial tests too, though I ran into some problems with insert and delete, so those are commented out for now.

As far as I understand, set, insert and delete form a sort of recursive loop along with digDeeper, with each of them calling the other till the appropriate node is reached. I'm not sure how they could be converted into simple recursive functions.