[Serializer] Add CsvEncoder tests for PHP 7.4 #32051
Conversation
src/Symfony/Component/Serializer/Tests/Encoder/CsvEncoderTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Tests/Encoder/CsvEncoderTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Tests/Encoder/CsvEncoderTest.php
Outdated
Show resolved
Hide resolved
|
cc @dunglas |
|
What should we do with this PR? |
|
we could merge as-is, maybe add the expected output using comments. Utimately this is broken in PHP: https://3v4l.org/reb5N however we may want to look at |
|
Oh, and here's the interesting part: as of 7.4 this seems fixed at the escape level: |
|
Im not sure what the point of
|
|
I'm not for merging a PR that proves a PHP bug... |
|
I agree with @nicolas-grekas |
|
I think the solution is to change the default escape char from after #32289, we can test targeting php 7.4 as well. I'll look into this 👍 |
|
Right, we cant use 'empty string' on older PHP versions :)
we always hit the issue if the last char in a cell value is the escaping character https://3v4l.org/3NIb6 (fixes trailing slash by using tilde as escape char) So i think the only way is to update the escape char in master for php 7.4 to an empty string. |
|
Why not 3.4? PHP 7.4 will be supported there too. |
|
What's the status here? |
|
@fabpot @nicolas-grekas all good 👍 |
|
note merging this in 4.3 requires a bit more work, let me know if you want a PR for that. |
|
Thank you @ro0NL. |
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
…0NL) This PR was merged into the 4.3 branch. Discussion ---------- [DI] Add CSV env var processor tests / support PHP 7.4 | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #... <!-- prefix each issue number with "Fix #", if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Similar as #32051 Commits ------- 82f3418 [DI] Add CSV env var processor tests
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
Some CSV encoder tests to show the broken behavior of a trailing slash. Spotted in #31867, not sure what to do with it :)