Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add seek($array, $offset, $whence) #645

Closed
wants to merge 5 commits into from

Conversation

7 participants
@datibbaw
Copy link
Contributor

datibbaw commented Apr 17, 2014

This fixes bug 31375 .

It introduces seek($array, $offset [, $whence = SEEK_CUR) to perform a seek on an array. For example:

$a = [1, 2, 3];
seek($a, 1); // current = 2
seek($a, 1); // current = 3
seek($a, -1, SEEK_END); // current = 2
@bwoebi

This comment has been minimized.

Copy link
Contributor

bwoebi commented Apr 18, 2014

Actually you could just remove these superfluous fetches from the HashTable:

// for next(), similar code for reset()

    while (offset > 0 && zend_hash_move_forward(array) == SUCCESS) {
        --offset;
    }
@datibbaw

This comment has been minimized.

Copy link
Contributor Author

datibbaw commented Apr 18, 2014

@bwoebi Good point, I made that patch some time ago and didn't reevaluate the code :)

@datibbaw datibbaw changed the title Allow offsets in end() and reset() Add seek($array, $offset, $whence) Apr 23, 2014

@ghost

This comment has been minimized.

Copy link

ghost commented Mar 7, 2015

Can one of the admins verify this patch?

@datibbaw

This comment has been minimized.

Copy link
Contributor Author

datibbaw commented Mar 8, 2015

so far it's had a lukewarm reception in internals, which is why I didn't merge it yet. I'll bring it up again

@marcioAlmada

This comment has been minimized.

Copy link
Contributor

marcioAlmada commented Aug 3, 2015

This is useful

👍

@marcioAlmada

This comment has been minimized.

Copy link
Contributor

marcioAlmada commented Aug 3, 2015

I meant the SEEK_SET variation, more specifically. I don't really care for moving according to an offset. IMMO it makes more sense to make SEEK_SET the default behavior.

@Onion-Shen

This comment has been minimized.

Copy link

Onion-Shen commented Aug 3, 2015

i am a foreniger , i am not understand what you say.sorry

At 2015-08-03 14:48:51, "Márcio Almada" notifications@github.com wrote:

I meant the SEEK_SET variation, more specifically. I don't really care for moving according to an offset. IMMO it makes more sense to make SEEK_SET the default behavior.


Reply to this email directly or view it on GitHub.

@nikic

This comment has been minimized.

Copy link
Member

nikic commented Sep 22, 2016

@krakjoe

This comment has been minimized.

Copy link
Member

krakjoe commented Jan 5, 2017

Since this PR is against a security fix only branch, and since it has merge conflicts, and would need to be different for a supported branch, I'm closing this PR.

I've read the internals discussions and the feature seems some what contentious, so please take this action as encouragement to open a clean PR accompanied by an RFC and fresh internals discussion, so that we may gather consensus and move forward with the feature.

@krakjoe krakjoe closed this Jan 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.