8000 [Yaml] Improve newline handling in folded scalar blocks by teohhanhui · Pull Request #15482 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Yaml] Improve newline handling in folded scalar blocks #15482

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Sep 30, 2015
Merged

[Yaml] Improve newline handling in folded scalar blocks #15482

merged 1 commit into from
Sep 30, 2015

Conversation

teohhanhui
Copy link
Contributor
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #15477
License MIT
Doc PR N/A

@javiereguiluz
Copy link
Member

@teohhanhui I'm glad that after the discussion in #15477 issue you have created the pull request according to the format required by Symfony.

We have a bot called "fabbot" which reviews the submitted pull requests for basic errors. Although the bot is reporting an error for this pull request, we can safely ignore it because it's related to some feature needed for the unit tests.

So everything is ready to be reviewed by our team and ultimately merge it in Symfony. Let's wait for the revision. Thanks!

@teohhanhui
Copy link
Contributor Author

@javiereguiluz No worries. Actually, I'm not new to Symfony and the contributing process.

@teohhanhui teohhanhui force-pushed the yaml_folded_linebreak branch 2 times, most recently from 3fc7774 to 5b9a602 Compare August 7, 2015 07:23
@stof
Copy link
Member
stof commented Aug 7, 2015

@javiereguiluz we should make fabbot ignore the Yaml test fixtures to avoid reporting this issue here (or we should refactor the way we pass Yaml test fixtures)

stof
stof reviewed Aug 7, 2015
View reviewed changes
src/Symfony/Component/Yaml/Parser.php
@@ -20,7 +20,7 @@
*/
class Parser
{
const FOLDED_SCALAR_PATTERN = '(?P<separator>\||>)(?P<modifiers>\+|\-|\d+|\+\d+|\-\d+|\d+\+|\d+\-)?(?P<comments> +#.*)?';
const BLOCK_SCALAR_PATTERN = '(?P<separator>\||>)(?P<modifiers>\+|\-|\d+|\+\d+|\-\d+|\d+\+|\d+\-)?(?P<comments> +#.*)?';
Copy link
Member

Choose a reason for hiding this comment

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

renaming a constant is a BC break

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you suggest then? Can I recreate it like this?

const FOLDED_SCALAR_PATTERN = self::BLOCK_SCALAR_PATTERN;

@xabbuh xabbuh added the Yaml label Aug 7, 2015
@teohhanhui teohhanhui force-pushed the yaml_folded_linebreak branch from 5b9a602 to 3070586 Compare August 10, 2015 04:27
@teohhanhui teohhanhui force-pushed the yaml_folded_linebreak branch from 3070586 to 73366d5 Compare August 11, 2015 05:02
@teohhanhui
Copy link
Contributor Author

PHP7 build is failing on Travis... Does it have anything to do with my code?

@jakzal
Copy link
Contributor
jakzal commented Aug 11, 2015

Tests for the HttpFoundation fail, so it doesn't seem to be an issue related to your PR @teohhanhui.

@teohhanhui
Copy link
Contributor Author

Are we good on this?

@teohhanhui
Copy link
Contributor Author

ping @stof

@teohhanhui
Copy link
Contributor Author

Any changes required before this will get merged?

@stof
Copy link
Member
stof commented Sep 2, 2015

👍

1 similar comment
@jakzal
Copy link
Contributor
jakzal commented Sep 29, 2015

👍

@fabpot
8000 Copy link
Member
fabpot commented Sep 30, 2015

Thank you @teohhanhui.

@fabpot fabpot merged commit 73366d5 into symfony:2.3 Sep 30, 2015
fabpot added a commit that referenced this pull request Sep 30, 2015
@fabpot
bug #15482 [Yaml] Improve newline handling in folded scalar blocks (t…
c0ff4bf 
…eohhanhui)

This PR was merged into the 2.3 branch.

Discussion
----------

[Yaml] Improve newline handling in folded scalar blocks

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #15477
| License       | MIT
| Doc PR        | N/A

Commits
-------

73366d5 [Yaml] Improve newline handling in folded scalar blocks
This was referenced Oct 27, 2015
Closed
Merged
Merged
@teohhanhui teohhanhui deleted the yaml_folded_linebreak branch February 15, 2016 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Needs Review Yaml
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@teohhanhui @javiereguiluz @stof @jakzal @fabpot @xabbuh @carsonbot
0