(updated the P.R description with code that actually makes sense 😅)

Ping @JosephSilber

I actually tried adding this functionality a while ago, but it didn't work out (the naming was quite confusing). I've been thinking of taking another shot at this, so thanks @redgluten for submitting this. Looks great 👍

Here are some thoughts:

1. We don't need a separate implementation for the eager collection. It can defer to the lazy collection. With code as involved as this, it's better not to have two implementations.

2. All methods on the lazy collection need to be tested to ensure they're actually lazy.

3. We always maintain the original collection's keys. This PR resets all keys.

4. Although Ruby doesn't do this, I think it makes sense to pass the whole chunk (thus far) to the callback. Also, we should pass the current key to the callback.

In short, what I'm proposing is something like this (haven't tested it):

public function chunkWhile(callable $callback)
{
    return new static(function () use ($callback) {
        $iterator = $this->getIterator();

        $chunk = new Collection();

        if ($iterator->valid()) {
            $chunk[$iterator->key()] = $iterator->current();

            $iterator->next();
        }

        while ($iterator->valid()) {
            if (! $callback($iterator->current(), $iterator->key(), $chunk)) {
                yield new static($chunk);

                $chunk = new Collection();
            }

            $chunk[$iterator->key()] = $iterator->current();

            $iterator->next();
        }

        if ($chunk->isNotEmpty()) {
            yield new static($chunk);
        }
    });
}

@redgluten if you want to submit a follow-up PR, go for it 👍

Otherwise I'll cook something up a little later, or tomorrow.

Yeah, let's make the proposed changes.

@JosephSilber Yeah I stumbled onto your PR while finishing up mine yesterday, seemed to me like this more general approach might be valuable.

Very cool that we can defer to lazy collections. 🔥

Your proposed changes make total sense. The only concern I see is that using the chunk makes the common operation of comparing the current and previous elements a bit more wordy unless I’m missing some trick?

It’s quite late on my side of the world to get a crack at it now feel free to make your changes if you have the time otherwise I’ll get back to it later on. 👍

@redgluten your original example would have to be converted to this:

$chunks = collect(str_split('AABBCCCD'))
    ->chunkWhile(fn ($char, $key, $chars) => $chars->last() === $char));

A tiny bit more verbose, but totally worth it for the additional flexibility.

Oh yeah at that point the chunk’s last element is the previous 😅 that’s what you get from reading code before going to bed late at night I guess. 🙃