[Yaml] respect inline level when dumping objects as maps #22409
Conversation
| Q | A |
|---|---|
| Branch? | 3.2 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #22392 |
| License | MIT |
| Doc PR |
|
Please also add a test with an empty ArrayObject or stdClass, to ensure it keeps being dumped as |
|
done |
|
Hi, I'm not sure is the best approach to solve the issue. // currently fails
public function testDumpingArrayObjectInstancesWithNumericKeysRespectsInlineLevel()
{
$deep = new \ArrayObject(array('d', 'e'));
$inner = new \ArrayObject(array('b', 'c', $deep));
$outer = new \ArrayObject(array('a', $inner));
$yaml = $this->dumper->dump($outer, 2, 0, Yaml::DUMP_OBJECT_AS_MAP);
$expected = <<<YAML
0: a
1:
0: b
1: c
2: { 0: d, 1: e }
YAML;
$this->assertEquals($expected, $yaml);
}
// This works with 3.1 branch
public function testDumpingArrayObjectInstancesWithNumericKeysInlined()
{
$deep = new \ArrayObject(array('d', 'e'));
$inner = new \ArrayObject(array('b', 'c', $deep));
$outer = new \ArrayObject(array('a', $inner));
$yaml = $this->dumper->dump($outer, 0, 0, Yaml::DUMP_OBJECT_AS_MAP);
$expected = <<<YAML
{ 0: a, 1: { 0: b, 1: c, 2: { 0: d, 1: e } } }
YAML;
$this->assertEquals($expected, $yaml);
}and the strategy (casting array object ot array) will make really hard (if not impossible) to solve this case and with the current strategy, https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Yaml/Inline.php#L176 is never reached since |
|
I will also try to find a solution |
|
Status: Needs work |
|
Here is my version of the fix #22419 |
|
Question: why #21471 is considered as a new feature? IMO should be a bugfix and included in the 3.2 branch. The only feature in #21471 should be this that allows to dump an empty arrayobject (or a stdclass) as |
e52ff8e to
fe515b3
Compare
This relies on adding a new constant for the configuration flag => this is a new feature for the component |
|
@stof i see, thanks |
|
@xabbuh if this is the candidate PR to be merged, do you mind adding: public function testDumpingArrayObjectInstancesWithNumericKeysRespectsInlineLevel()
{
$deep = new \ArrayObject(array('d', 'e'));
$inner = new \ArrayObject(array('b', 'c', $deep));
$outer = new \ArrayObject(array('a', $inner));
$yaml = $this->dumper->dump($outer, 2, 0, Yaml::DUMP_OBJECT_AS_MAP);
$expected = <<<YAML
0: a
1:
0: b
1: c
2: { 0: d, 1: e }
YAML;
$this->assertEquals($expected, $yaml);
}? |
| $willBeInlined = $inline - 1 <= 0 || !is_array($value) || empty($value); | ||
| $dumpObjectAsInlineMap = true; | ||
|
|
||
| if (Yaml::DUMP_OBJECT_AS_MAP & $flags && ($value instanceof \ArrayObject || $value instanceof \stdClass)) { |
There was a problem hiding this comment.
@xabbuh Should this code be put in a private static method? is identical to what we have on line 86
|
Did not know |
|
@goetas it does not pass the |
|
@stof Is so beautiful to learn things about PHP every day after so many years |
|
See https://3v4l.org/tu1Z0 for the proof btw |
|
Do this PR need still work? For me is 👍 |
|
Thank you @xabbuh. |
…goetas, xabbuh) This PR was merged into the 3.2 branch. Discussion ---------- [Yaml] respect inline level when dumping objects as maps | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22392 | License | MIT | Doc PR | Commits ------- 3cca48c respect inline level when dumping objects as maps 4f5c149 Test case for not in-lined map-objects