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

use str_getcsv instead of explode in Delimited rule #62

Merged
merged 2 commits into from
May 9, 2023

Conversation

ragulka
Copy link
Contributor

@ragulka ragulka commented May 9, 2023

Using str_getcsv to convert the delimited list into an array allows escaping commas inside the list items.

A real-life example would be list of email address specifications, ie:
"Doe, John" <johndoe@example.com>, "Doe, Jane" <janedoe@example.com>.

With the current implementation, the rule will break this into:

[
  ""Doe",
  " John" <johndoe@example.com>",
  ""Doe",
  " Jane" <janedoe@example.com>",
]

With str_getcsv, we get the expected result:

[
  "Doe, John <johndoe@example.com>",
  "Doe, Jane <janedoe@example.com>",
]

@freekmurze
Copy link
Member

I'm a bit torn if this is a bug fix or a breaking change.

I'm going for breaking change as the validated input will be different. I would accept a PR that modifies the behaviour by calling considerCsvStrings(). When that function is called on the rule, use the str_getcsv function.

@ragulka
Copy link
Contributor Author

ragulka commented May 9, 2023

@freekmurze that makes sense and I can totally see how that might break some existing use-cases. I've updated the PR with 4178cb7, although I propose a slightly shorter flag name: asCsv. What do you think?

@freekmurze freekmurze merged commit 5819eb5 into spatie:main May 9, 2023
@freekmurze
Copy link
Member

Perfect, thanks!

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.

None yet

2 participants