Added pipe for multiline string (correctly indented), which makes them much more readable

[mercmobily]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	?
Deprecations?	no
Tests pass?	Nod tone
Fixed tickets	#17391
License	MIT
Doc PR	-

The PR to actually implement #17391.
Text:

I nearly gave up on this -- twice. Actually, three times.
Once I got to a point where I was happy with where I was... I realised that I had a version of Symfony that was too old! Having done all that work, I went ahead and re-applied the changes I had made.

NOTE: You absolutely have to document that multiline strings are NOT compatible with inline ones. So, if the parser is being used like this:

$yaml = Yaml::dump ( $user, 0, 4, 0, Yaml::DUMP_MULTI_LINE_AS_BLOCK );

Then the flag will basically be ignored, since everything will be inlined (and you cannot use multilne | pipe in inline fields.

About the PR... I tried the path of returning an array. However, it got messy very quickly. The main issue I had with that approach is that while the Dumper::dump is the "data preparator", the "Inline:dump" is the part that always does the formatting. Forcing Dumper to do data formatting quickly showed that the approach was not ideal.

The approach I took in the end was to add a parameter to Dumper::dump():

   public function dump($input, $inline = 0, $indent = 0, $absIndent = 0, $flags = 0)
   {

The key point of this is:

           $willBeInlined = $inline - 1 <= 0 || !is_array($value) || empty($value);

            $output .= sprintf('%s%s%s%s',
                $prefix,
                $isAHash ? Inline::dump($key, $flags, $absIndent).':' : '-',
                $willBeInlined ? ' ' : "\n",
                $this->dump($value, $inline - 1, $willBeInlined ? 0 : $indent + $this->indentation, $inline - 1 <= 0 ? 0 : $absIndent + $this->indentation, $flags)

Basically the problem I was having is that whenever a non-array was found, the field was going to be inlined -- and therefore, the tab was zeroed. I needed another tab indicator, something that would always be current and correct. So, I added one.

As I said, I am not a PHP person. So I had to do this without any debugging tools -- and without any real current knowledge of PHP.

I did my best to follow the coding styles... hopefully it will work!

Also, I didn't do any unit-tests. Since I don't know anything about unit testing with PHP, and I have already spent 3.5 hours on something I will never, ever use nor need... I was hoping you could 💃

Thank you!

Addendum: I couldn't actually work out when the function Inline::dumpArray() would ever, ever get called. I added the extra parameter to it. However, I do wonder...

Added pipe for multiline string, which makes them much more readabl

[stof]

the new argument must be placed after the flags, for BC reasons

[stof]

This cannot be accepted without tests covering the new feature.

[mercmobily]

Hi,

I don't know PHP.... How do I carry that value over without passing it as
argument in recursion?

An hint will suffice!

Will write tests once you approve at least what I am doing...

Merc.

[xabbuh]

Thank you for creating this pull request @mercmobily and sorry for the lack of feedback from my side so far. Actually, I think we should implement this feature a bit differently to better fit into the existing code. Would you mind to take a look at #17943 which I just created if it provides a solution for your needs?

[mercmobily]

Hi,

That indeed does the trick. Please accept my apologies if my code wasn't
acceptable -- as I mentioned, I don't really know PHP and had a bit of
grief following that codebase...

I hop I was at least small source of inspiration...

When will this code make it into the next release? (Assuming it will get
merged!)

Merc
ᐧ

On 27 February 2016 at 17:45, Christian Flothmann notifications@github.com
wrote:

Thank you for creating this pull request @mercmobily
https://github.com/mercmobily and sorry for the lack of feedback from
my side so far. Actually, I think we should implement this feature a bit
differently to better fit into the existing code. Would you mind to take a
look at #17943 #17943 which I
just created if it provides a solution for your needs?

—
Reply to this email directly or view it on GitHub
#17912 (comment).

[fabpot]

Closing in favor of #17943