-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DIC] Add a split env var processor #31867
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
Conversation
f38ab86
to
44461c1
Compare
I think you should use the |
This feature should go in |
Agree with @nicolas-grekas , you can already use the |
44461c1
to
e9fea7c
Compare
@OskarStark base branch updated ;) |
@Orkin can you please give us some more insights, why you cannot/wont use the Thank you |
i realized there can be different behavior :) https://3v4l.org/gOCPR that's escaping support out-of-the box 😅 but perhaps that works counter-wise for @Orkin for the
Perhaps SF should fix it? |
not sure if #32007 is related, as it fixes the encoding of For DI it's If we decide to unescape the escape char, both DI + Serializer should do that yes. (Unless the escape char is https://bugs.php.net/bug.php?id=55413 is the bug report. |
Hi, @OskarStark @nicolas-grekas first of all because I didn't know the csv processor. For my usecase it's for memcached server list retrieve via environment variable so for my need it's easier to split by something. I don't know as much env var processor so I don't have any ideas if it's possible and if not what kind of format it can be used. But in the future it can be a improvement. And last thing, I don't want my string to be escaped because it can be url or something that need to stay in the same format |
In this case it makes sense to me to not use the csv processor and got with the new explode one. Also from a DX perspective, having csv:MEMCACHED_HOSTS... looks weird |
Id prefer |
Sounds good to me, but I think it’s more common to use explode kn the php context, also because of split is an old PHP 5.3 deprecated method https://www.php.net/manual/en/function.split.php But definitely better then using csv here 👍🏻 |
that's the thing, explode sounds PHP specific whereas in SF DI config it should be more language agnostic IMHO. Agree
i think having both |
It didn’t know and find the |
maybe i tend to believe people associate "CSV" with a collection of rows
|
Let’s go with split then |
e9fea7c
to
475d11b
Compare
Everything is up to date ;) |
Please update the PR title |
Not sure we need yet another processor. Here, I'm sure some will want to have another separator than So, I'm 👎 for this addition. |
Closing as explained, thanks for proposing. |
This PR was squashed before being merged into the 3.4 branch (closes #32051). Discussion ---------- [Serializer] Add CsvEncoder tests for PHP 7.4 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? |no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Some CSV encoder tests to show the broken behavior of a trailing slash. Spotted in #31867, not sure what to do with it :) Commits ------- 760354d [Serializer] Add CsvEncoder tests for PHP 7.4
This PR was squashed before being merged into the 3.4 branch (closes symfony#32051). Discussion ---------- [Serializer] Add CsvEncoder tests for PHP 7.4 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? |no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Some CSV encoder tests to show the broken behavior of a trailing slash. Spotted in symfony#31867, not sure what to do with it :) Commits ------- 760354d [Serializer] Add CsvEncoder tests for PHP 7.4
This adds a new
explode
processor that will explode astring
to aPHP array