Zipper unselect is incorrect after multiple operations

[eed3si9n]

I am assuming Zippers are supposed to behave like a normal collection. Let me know if this is not the case.

steps

1. run this from $ sbt console:

   import com.codecommit.antixml._
   val foo = <parent><a/><b/><c/><d/><e/></parent>.convert
   val notA = foo \ * filter {
     case Elem(p, name, a, s, c) => name != "a"
     case _ => false }
   notA.unselect
   val notAOrB = foo \ * filter {
     case Elem(p, name, a, s, c) => name != "a"
     case _ => false } filter {
     case Elem(p, name, a, s, c) => name != "b"
     case _ => false }
   notAOrB.unselect
   

2. this produces the following outputs:

   foo: com.codecommit.antixml.Elem = <parent><a/><b/><c/><d/><e/></parent>
   notA: com.codecommit.antixml.Zipper[com.codecommit.antixml.Node] = <b/><c/><d/><e/>
   res0: com.codecommit.antixml.Zipper[com.codecommit.antixml.Node] = <parent><b/><c/><d/><e/></parent>
   notAOrB: com.codecommit.antixml.Zipper[com.codecommit.antixml.Node] = <c/><d/><e/>
   res1: com.codecommit.antixml.Zipper[com.codecommit.antixml.Node] = <parent><a/><c/><d/><e/></parent>
   

problem

1. The final res1 includes <a/> I filtered in the earlier filter.

expectations

1. Filtering can be chained multiple times:

   res1: com.codecommit.antixml.Zipper[com.codecommit.antixml.Node] = <parent><c/><d/><e/></parent>
   

note

This is the implementation of filter:

    override def filter(f: A => Boolean): Zipper[A] = collect {
      case e if f(e) => e
    }

collect is equally short, so the problem is likely in the flatMap.
I have an impl for some more context-aware Zipper operations, but it's not working because of this.

[eed3si9n]

Here's from REPL (I temporarily added printMap):

scala> (books filter (books(1) !=)) printMap
Vector(Some((0,2,<function2>,Map(0 -> Set(0), 1 -> Set(), 2 -> Set(1)))))

scala> (books filter (books(1) !=) filter (books(0) !=)) printMap
Vector(Some((0,1,<function2>,Map(0 -> Set(), 2 -> Set(0)))))

[josharnold52]

Sorry for any duplicated work; I was trying to fix this as an exercise to help me understand Zipper and came up with a fix before I saw your commits. That said, I don't think your fix is correct. For an example that fails, try:

    val foo = <parent><a/><b/><c/></parent>.convert
    val elems = foo \ *
    val shouldBeEmpty = (foo \ *) filter (elems(0) == ) filter (elems(0) !=)
    val result = shouldBeEmpty.unselect

The result should just be

    <parent/> 

but your Zipper yields

  <parent><c/></parent>

The basic problem (I think) is the keys in childMap are supposed to be indexes into the parent rather than indexes into "this" group.

Anyway, the commit in my previous comment works for the above example as well as your original case.

[eed3si9n]

@josharnold52 I was trying as an exercise myself, and it's great that you've put in the work and also found my mistake.

The basic problem (I think) is the keys in childMap are supposed to be indexes into the parent rather than indexes into "this" group.

I kind of had similar thoughts, but it was hazy. Your description of the problem makes sense.

[djspiewak]

That'll teach me to actually read once in a while…

I didn't see this whole thread, and so I marked this issue as closed when Josh's fix hasn't actually been merged yet. @josharnold52, could you create a pull request for your commit? It's just to make sure there's a paper trail for code usage rights.

[josharnold52]

Sure, I've crated the pull request.