Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
mercmobily wants to merge4 commits intosymfony:masterfrommercmobily:master

Conversation

@mercmobily
Copy link

QA
Bug fix?no
New feature?yes
BC breaks??
Deprecations?no
Tests pass?Nod tone
Fixed tickets#17391
LicenseMIT
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 theDumper::dump is the "data preparator", the "Inline:dump" is the part that always does the formatting. ForcingDumper to do data formatting quickly showed that the approach was not ideal.

The approach I took in the end was to add a parameter toDumper::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 neededanother tab indicator, something that wouldalways 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 functionInline::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

* @return string The YAML representation of the PHP value
*/
publicfunctiondump($input,$inline =0,$indent =0,$flags =0)
publicfunctiondump($input,$inline =0,$indent =0,$absIndent =0,$flags =0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

@stof
Copy link
Member

This cannot be accepted without tests covering the new feature.

@mercmobily
Copy link
Author

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
Copy link
Member

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?

@xabbuhxabbuh added the Yaml labelFeb 27, 2016
@mercmobily
Copy link
Author

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 Flothmannnotifications@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
Copy link
Member

Closing in favor of#17943

@fabpotfabpot closed thisFeb 29, 2016
fabpot added a commit that referenced this pull requestMar 1, 2016
…cks (xabbuh)This PR was merged into the 3.1-dev branch.Discussion----------[Yaml] option to dump multi line strings as scalar blocks| Q             | A| ------------- | ---| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#16236,#16604,#17912,#17391| License       | MIT| Doc PR        | TODOCommits-------eff6902 option to dump multi line strings as scalar blocks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mercmobily@stof@xabbuh@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp