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, which makes them much more readable#17391

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
fabpot wants to merge1 commit intosymfony:masterfromfabpot:moved_yaml_15

Conversation

@fabpot
Copy link
Member

This PR was submitted on the symfony/yaml read-only repository by@mercmobily and moved automatically to the main Symfony repository (closessymfony/yaml#15).

I made this little change because I am converting a LOT of data into Yaml from a database, and I wanted to make sure that the data as as user-friendly as possible.

I asked this on SO:http://stackoverflow.com/questions/34805558/phps-yaml-deciding-how-to-generate-multiline-strings

Then I discovered your wonderful YAML module, which makes much better YAML than PHP's (at least in terms of user readibility) AND it was reallllyyyy easy to hack to add this simple feature (this tells you something about the quality of your code).

Disclaimer: I don't even know PHP. I am a nodeJS boy. But, I think the patch works, and I think it's worthwhile having.

Note: I am not sure how it should deal with\n so that it's actually compatible with any Windows etc. If you feel this is a good addition, let me know and I will try and make it more generic...

@fabpot
Copy link
MemberAuthor

ping@xabbuh

@xabbuh
Copy link
Member

@mercmobily Can you give an example of how this makes your use case easier? I am not sure I understand what you mean.

@xabbuhxabbuh added the Yaml labelJan 15, 2016
@mercmobily
Copy link

Well, it depends whether you want to end up with something like thiis:

profile_name: 'African Music Circles'profile_email: noemai1@noemail.comareas: 'workshops/music workshops,bands/drumming bands'profile_short_intro: ''profile_intro: |  African Music Circles is a Perth based company that delivers:  -Corporate team building experiences  -African music entertainment  -African drumming and dance classes  -Interactive facilitated drum circles  At African Music Circles - people come not to 'learn to drum'- but to have fun playing together and to create a drum song. The facilitator incorporates musical interventions to help the group become unified musically. The quality of the music that is made is less on the expertise of the players and more on the quality of the relationship with each other in that musical event.  Corporate Team Building Activities:  Discover the journey to percussion orchestra with a drum for each participant. Though a dynamic and exciting session, your teams energy levels will be raised, stress and thoughts cleared away and creativity and the imagination will be ignited. Our corporate sessions brings people together in celebration, transending all boundaries of race, religion, age, gender and social position. Its fun, easy and best of all makes people feel good about themselves.  During the events our facilitators ensure fun and excitement are the driving factors. Your team can use their body, voice or percussion instrument to engage with the session or simply be enthralled with the melodic, exhilarating sounds of the West African Drums.. African Music Circles can cater for teams of 10- 10,000.profile_special_talents: '

OR like this:

profile_name: 'African Music Circles'profile_city: Perthareas: 'workshops/music workshops,bands/drumming bands'profile_short_intro: ''profile_intro: "African Music Circles is a Perth based company that delivers:\r\n\r\n-Corporate team building experiences\r\n-African music entertainment\r\n-African drumming and dance classes\r\n-Interactive facilitated drum circles\r\n\r\nAt African Music Circles - people come not to 'learn to drum'- but to have fun playing together and to create a drum song. The facilitator incorporates musical interventions to help the group become unified musically. The quality of the music that is made is less on the expertise of the players and more on the quality of the relationship with each other in that musical event.\r\n\r\nCorporate Team Building Activities:\r\nDiscover the journey to percussion orchestra with a drum for each participant. Though a dynamic and exciting session, your teams energy levels will be raised, stress and thoughts cleared away and creativity and the imagination will be ignited. Our corporate sessions brings people together in celebration, transending all boundaries of race, religion, age, gender and social position. Its fun, easy and best of all makes people feel good about themselves.\r\n\r\nDuring the events our facilitators ensure fun and excitement are the driving factors. Your team can use their body, voice or percussion instrument to engage with the session or simply be enthralled with the melodic, exhilarating sounds of the West African Drums.. African Music Circles can cater for teams of 10- 10,000.\r\n"profile_special_talents: ''

Which one would you rather edit by hand with a text editor?
(When you answer, please keep in mind that YAML ismeant to be easy to edit byhumans...

Merc.

@mercmobily
Copy link

(I hope it makes sense... I realise that if you are strictly interested in "formally compliant" YaML then you might see this patch as useless. However, to be true to the spirit of YaML...

@xabbuhxabbuh mentioned this pull requestJan 25, 2016
@xabbuh
Copy link
Member

Sorry for the delay since my last comment here@mercmobily. I can see why you want to be able to dump strings more readable. With#17578 we will now (starting in Symfony 3.1) be able to customise the dumped YAML string to options that are passed to the dumper as a bit field. Would you like to create a new pull request (as you cannot push to@fabpot's fork) and introduce a new option in the dumper that would allow people to switch to the new way to dump strings?

@mercmobily
Copy link

Three questions!

  1. Right now we only haveconst DUMP_OBJECT = 1;. What should I call the new bit, and what value should I give it? If I am understanding it right, 2 would work (so that you could have DUMP_OBJECT | DUMP_PIPEMULTILINE) right?

  2. Is DUMP_PIPEMULTILINE a name you like? (I suck at making up names)

  3. My patch has a huge problem: the text MUST be indented by 2 spaces on TOP OF the attribute's indentation. So, at the moment my patch sucks and it only really works for 1st level attributes (since I always indent with 4 spaces). How do I find out the current indentation within the switch() statement in Inline.php? I looked and looked but I couldn't work this one out

  4. Bonus questions... so which repo should I create the PR against?

Thanks!

@xabbuh
Copy link
Member

  1. For the moment, you can use2 as the value (you will probably need to change that and also rebase as we will introduce other flags too).
  2. What about something likeDUMP_MULTI_LINE_AS_BLOCK?
  3. I think you need to handle that outside theInline class afterInline:dump() has been called (inside theDumper class you have the needed information.
  4. All pull requests must be opened at this repository.

@mercmobily
Copy link

Hi,

  1. OK

  2. OK

  3. Can I be a pain and ask you to be a little more specific? I looked at
    the code and couldn't find it. Any hints of locations/important bits would
    be fantastic!

  4. OK

Please note that I no longer need this for myself but will submit the patch
for the common good!

Merc.
On 9 Feb 2016 3:47 pm, "Christian Flothmann"notifications@github.com
wrote:

For the moment, you can use 2 as the value (you will probably need to
change that and also rebase as we will introduce other flags too).
2.

What about something like DUMP_MULTI_LINE_AS_BLOCK?
3.

I think you need to handle that outside the Inline class after
Inline:dump() has been called (inside the Dumper class you have the
needed information.
4.

All pull requests must be opened at this repository.


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

@xabbuh
Copy link
Member

One option I see is to return the lines of the block as an array inInline::dump() and inDumper::dump() indent each line with the required number of additional space characters.

@mercmobily
Copy link

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.

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!

@mercmobily
Copy link

I just realised I need to create a new PR on my own fork to actually submit a PR.
Please close this PR in favour of#17912

@fabpotfabpot closed thisFeb 24, 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.

4 participants

@fabpot@xabbuh@mercmobily@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp