Manage empty array for serializer #510
Conversation
|
A test is needed to prevent regressions |
I was looking for it but I don't know how to manage tests files. Look at the test classes for yml, I don't know how to put the expected result to assert. Any clue? |
|
Well, there is a |
soullivaneuh
force-pushed
the
yaml-empty-array
branch
from
138d266 to
8c6f1c9
Compare
soullivaneuh
changed the title
|
Ok so I have to manage all file formats. Will work on it. Actually, I got hard to understand phpunit errors. |
|
Well, I got this error on master too. Not related. For me, this PR is ready to merge. ping @schmittjoh |
|
ping @schmittjoh Could you please take a look? This issue is quite annoying, a quick patch is needed IMHO. |
|
👍 |
| @@ -111,6 +111,11 @@ public function visitArray($data, array $type, Context $context) | |||
| ->rtrim(false) | |||
| ->writeln(' {}') | |||
| ; | |||
| } elseif (0 === count($data)) { | |||
There was a problem hiding this comment.
Can you change this to empty($data) if we specifically check for 0.
There was a problem hiding this comment.
I'm not sure empty($data) is the good way. We check for an empty array.
All another empty values should have ~ and not [].
There was a problem hiding this comment.
Oh I say something wrong, we are on visitArray. Will modify it.
soullivaneuh
force-pushed
the
yaml-empty-array
branch
from
8c6f1c9 to
08e7f48
Compare
soullivaneuh
force-pushed
the
yaml-empty-array
branch
from
08e7f48 to
a6443ad
Compare
|
@schmittjoh Review corrected. |
Manage empty array for serializer
|
How does this PR fix #124? I don't see it. |
|
Yes, that seems unrelated, but seems to have been auto-closed. |
|
@Tobion @schmittjoh My bad, I was thinking it was related. |
Fixes #183.
Fixes #124.
Actually, empty array would produce this:
This is not correct. Furthermore, if you parse it with JMS or Symfony Yaml component, you will got
nullthat is not the expected return.With this PR, you will have this: