Skip to content

Conversation

@stevebauman
Copy link
Contributor

@stevebauman stevebauman commented Nov 28, 2022

@davidjr82
Copy link
Contributor

davidjr82 commented Nov 28, 2022

hi @stevebauman,

this PR covers the multidimensional case my PR didn't cover, that's great!

My only doubt here is that the ignored values will apply to any of the nested values of the multidimensional array, right? For example, you can not ignore only "John Doe" in the key "value" of your example in #826 .

Thinking on how to improve it, right now ignored is just a collection of values. If we add the concept of key/value we would be able to ignore only when the combination of both happens, that is useful for multidimensional arrays.

But anyway, your PR improves the behaviour that has the package right now, so looks very good IMO.

@stevebauman
Copy link
Contributor Author

Hey @davidjr82, thanks for reviewing the PR, much appreciated 🙏

My only doubt here is that the ignored values will apply to any of the nested values of the multidimensional array, right?

You're correct -- if any array value is contained in the ignored collection, then it will be filtered out.

I agree with your improvement. We could tackle that in a following PR so that the bug can be patched and then the additional feature could be placed into branch and release.

@freekmurze freekmurze merged commit 14a6802 into spatie:main Dec 2, 2022
@freekmurze
Copy link
Member

Thank you!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v5.1.0] BC Break -- array_diff_assoc vs array_diff -- Array to string conversion

3 participants