Skip to content
This repository was archived by the owner on Apr 21, 2026. It is now read-only.

Fixes #4743: Arr::path() to return associative array when wildcard used#352

Open
tommcdo wants to merge 2 commits into
kohana:3.3/developfrom
tommcdo:3.3/feature/4743-enhance-arr-path
Open

Fixes #4743: Arr::path() to return associative array when wildcard used#352
tommcdo wants to merge 2 commits into
kohana:3.3/developfrom
tommcdo:3.3/feature/4743-enhance-arr-path

Conversation

@tommcdo

@tommcdo tommcdo commented May 15, 2013

Copy link
Copy Markdown

@enov

enov commented Nov 21, 2014

Copy link
Copy Markdown
Contributor

This is a well written bug fix, with tests. However, existing code relying on the numeric sequential keys of the output of this function will break, if we apply this fix.

@acoulton could you kindly comment what to do in this case?

@acoulton

Copy link
Copy Markdown
Member

Tricky. Since it changes the response, I think it would have to go to 3.4 with a clear note in the upgrade guide.

It will only be if end-user code is specifically looking at the keys, I think, that it's an issue - they'll still come out in the same sequence as far as I can see? It's probably quite an edge case, but still not safe for 3.3.

@tommcdo if you disagree we'd welcome your opinion.

@tommcdo

tommcdo commented Nov 21, 2014

Copy link
Copy Markdown
Author

I think it's quite unlikely that users are currently relying on the numerical keys with the current implementation, since they're quite frankly not useful at all. However, this is not strictly a bug fix, so it's perfectly reasonable to hold off until 3.4.

@enov

enov commented Nov 22, 2014

Copy link
Copy Markdown
Contributor

However, this is not strictly a bug fix, so it's perfectly reasonable to hold off until 3.4.

@tommcdo could you please PR this to 3.4/develop branch.

@neo22s

neo22s commented Mar 21, 2016

Copy link
Copy Markdown
Member

What we do with this? to 4.0.0? Looks good to me.

@neo22s neo22s added this to the 4.0.0 milestone Mar 21, 2016
@acoulton

Copy link
Copy Markdown
Member

I suggest a contrib manually merges this PR to the 4.0 branch and adds to upgrade notes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants