Permalink
Browse files

Fix issue #24: navigating up (as one would during a map on a preorder…

… traversal of a tree of sufficient depth) would forget that mutations had occurred.
  • Loading branch information...
1 parent ddc3c59 commit 75dac639befe3600154fd2a16007e9f7b037803e @frankshearar committed Feb 25, 2012
Showing with 58 additions and 33 deletions.
  1. +27 −33 lib/zipr/zipper.rb
  2. +31 −0 test/zipper_test.rb
View
@@ -155,11 +155,7 @@ def root
if context.root? then
@value
else
- if context.changed? then
- __changed_up.__changed_root
- else
- up.root
- end
+ up.root
end
end
@@ -345,37 +341,20 @@ def safe_up
if context.root? then
Left.new(ZipperError.new(:up_at_root, self))
else
- Right.new(new_zipper(context.parent_nodes.last,
- context.path))
- end
- end
-
- # An internal method. Zip up a mutated structure.
- def __changed_root
- if context.root? then
- @value
- else
- __changed_up.__changed_root
+ if (context.changed?) then
+ # Once we have a mutation, we must create new nodes. We replace
+ # the contexts with copies+changed=true versions to remember that
+ # we've changed something.
+ Right.new(new_zipper(mknode(context.parent_nodes.last,
+ context.left_nodes + [value] + context.right_nodes),
+ context.path.copy_as_changed))
+ else
+ Right.new(new_zipper(context.parent_nodes.last,
+ context.path))
+ end
end
end
- def __changed_up
- __safe_changed_up.either(->z{z},
- ->e{raise ZipperNavigationError.new(e.error)})
- end
-
- # An internal method. The zipper uses this recursion to record in the call
- # stack that the structure has changed.
- def __safe_changed_up
- if context.root? then
- Left.new(ZipperError.new(:up_at_root, self))
- else
- Right.new(new_zipper(mknode(context.parent_nodes.last,
- context.left_nodes + [value] + context.right_nodes),
- context.path))
- end
- end
-
def new_zipper(value, context)
Zipper.new(value, context, @branch, @children, @mknode)
end
@@ -523,8 +502,14 @@ def enqueue_unmarked_children(zipper)
# A one-hole context in some arbitrary hierarchical structure
class Context
attr_reader :left_nodes
+ # The path taken through the structure to this context. A nested sequence of
+ # Contexts terminating in an innermost RootContext representing a sequence
+ # of edits/navigations.
attr_reader :path
attr_reader :right_nodes
+ # A sequence of nodes representing the path from the entry point/root node
+ # of the structure to this context. parent_nodes.first is the parent of the
+ # hole, parent_nodes[1] the grandparent, and so on.
attr_reader :parent_nodes
def self.root_context
@@ -556,6 +541,10 @@ def changed?
@changed
end
+ def copy_as_changed
+ self.class.new(path, parent_nodes, left_nodes, right_nodes, true)
+ end
+
def end_of_traversal?
false
end
@@ -585,6 +574,11 @@ def initialize(unused, parent_nodes, left_nodes, right_nodes, changed)
super(self, parent_nodes, left_nodes, right_nodes, changed)
end
+ def copy_as_changed
+ # Root contexts mark the start of a navigation; you can't change them.
+ self
+ end
+
def root?
true
end
View
@@ -97,6 +97,18 @@ module Zipr
}
end
+ it "should have up preserve mutations to child nodes" do
+ z = Node.new(1, [Leaf.new(1)]).zipper
+ new_t = z.down.replace(Leaf.new(2)).up.root
+ new_t.should == Node.new(1, [Leaf.new(2)])
+ end
+
+ it "should have up preserve mutations to deep child nodes" do
+ z = Node.new(1, [Node.new(2, [Leaf.new(3)])]).zipper
+ new_t = z.down.down.replace(Leaf.new(4)).up.up.root
+ new_t.should == Node.new(1, [Node.new(2, [Leaf.new(4)])])
+ end
+
it "should have left fail on a leftmost child" do
t = Node.new(1, [Leaf.new(1)])
loc = t.zipper.down
@@ -580,4 +592,23 @@ module Zipr
answers.should == [1, 2, 3, 4, 5, 6, 7]
end
end
+
+ describe Context do
+ describe :copy_as_changed do
+ it "should copy everything except changed" do
+ c = Context.new(:path, :parent_nodes, :left_nodes, :right_nodes, :changed)
+ c_new = c.copy_as_changed
+ c_new.path.equal?(c.path).should be_true
+ c_new.parent_nodes.equal?(c.parent_nodes).should be_true
+ c_new.left_nodes.equal?(c.left_nodes).should be_true
+ c_new.right_nodes.equal?(c.right_nodes).should be_true
+ c_new.should be_changed
+ end
+
+ it "should return a RootContext unchanged" do
+ c = Context.root_context
+ c.equal?(c.copy_as_changed).should be_true
+ end
+ end
+ end
end

0 comments on commit 75dac63

Please sign in to comment.