Zipper deleteLeft, deleteRight and deleteC do not do what the docs say they do (and look broken)

[Chris Marshall]

The documentation of these methods on Zipper is at odds with the implementations. For example:

  /**
   * Deletes the element at focus and moves the focus to the right. If there is no element on the right,
   * focus is moved to the left.
   */
  def deleteRight: Option[Zipper[A]]

OK, excellent!

scala> zipper[Int](Stream.empty, 0, Stream(1, 2, 3 ,4)) findNext (_ == 4)

res25: Option[scalaz.Zipper[Int]] = Some(<zipper>)

scala> .flatMap(_.deleteRight)

res27: Option[scalaz.Zipper[Int]] = None

Errr, what? I would expect to now be looking at this:

zipper(Stream(2, 1, 0), 3, Stream.empty)

The same holds true for deleteLeft and deleteC. 

deleteRight looks like this

rights match {
    case Stream.Empty => None
    case r #:: rs     => Some(lefts match {
      case Stream.Empty => zipper(Stream.Empty, r, rs)
      case l #:: ls     => zipper(ls, l, rights)
    })
  }

But should look like this (if it is to match the docs). 

rights match {
        case r #:: rs     => Some(zipper(lefts, r, rs))
        case Stream.Empty => lefts match {
          case l #:: ls     => some(zipper(ls, l, Stream.empty))
          case Stream.Empty => None
        }
      }

This seems broken in the scalaz-seven branch also

Chris

[Runar Bjarnason]

That does indeed look inconsistent. Can you send a pull request?

> --
> You received this message because you are subscribed to the Google Groups
> "scalaz" group.
> To post to this group, send email to sca...@googlegroups.com.
> To unsubscribe from this group, send email to
> scalaz+un...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/scalaz?hl=en.

[Chris Marshall]

I'll see what I can get done this weekend; I'm a complete beginner to SBT and GIT though, so it will be fun!

Chris

[Chris Marshall]

I'm a bit stuck. I've made the code changes locally and I'm trying to initiate a pull-request. Except git tells me "Oops, there's already a pull request from "oxbowlakes:master", which indeed there is [1]. It asks me if I want to update the commit range, which presumably I do, but offers no help on what range I might wish to choose or even what a commit range is. 

Can I "park" the original pull-request and create a new one with just the latest changes? If so, how?

Chris

[1] https://github.com/scalaz/scalaz/pull/29

[Jason Zaugg]

On Sat, Feb 25, 2012 at 5:14 PM, Chris Marshall <oxbow...@gmail.com> wrote:
> I'm a bit stuck. I've made the code changes locally and I'm trying to
> initiate a pull-request. Except git tells me "Oops, there's already a pull
> request from "oxbowlakes:master", which indeed there is [1]. It asks me if I
> want to update the commit range, which presumably I do, but offers no help
> on what range I might wish to choose or even what a commit range is.
>
> Can I "park" the original pull-request and create a new one with just the
> latest changes? If so, how?

Make a branch dedicated to each pull request.

-jason

[Chris Marshall]

I'd appreciate if you could walk me through this as it's exactly what I tried to do. I created a branch called fixzipper and committed my changes to it. But then I couldn't navigate to this branch on the website; so I had to merge back into master and push the changes in order to try and get a pull request.

So, if I create a new branch fixzipper2:

git checkout -b fixzipper

What do I do now? 

Chris

These suggestions (http://stackoverflow.com/questions/5256021/submitting-github-pull-request-for-only-latest-commit) just leave me with errors as well.

Chris

[Chris Marshall]

I guess I want to understand *exactly* how I do this:

  "pushed three commits to a topic branch in his fork"

What does this mean? I have a local branch (on my machine) with the relevant changes. How do I get this branch pushed to the origin?

Chris

[Chris Marshall]

I've probably ballsed this up massively. I now have a pull request here (https://github.com/scalaz/scalaz/pull/76) and it contains 4 commits. Only the last two are applicable to the fix at hand (they are clearly labelled). Is it possible for you guys to selectively pull in only the pertinent commits?

Apologies for cretinousness

Chris

[Chris Marshall]

I've added an extra commit which adds a bunch of tests to ZipperSpec covering this functionality

Chris