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

Appearance settings

[Yaml] Clean some messages + add test case #20388

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 2, 2016

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 2.7
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Related to #20335 on 3.2.

*/
public function testStringOffsetCastError()
{
Inline::parse('{this, is not, yaml}');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is valid yaml, to not confuse people what about {this, is, not, supported}?

Copy link
Member Author

Choose a reason for hiding this comment

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

right, updated

@@ -249,7 +249,7 @@ public static function parseScalar($scalar, $delimiters = null, $stringDelimiter
$output = $match[1];
$i += strlen($output);
} else {
throw new ParseException(sprintf('Malformed inline YAML string (%s).', $scalar));
throw new ParseException(sprintf('Malformed inline YAML string: %s.', $scalar));
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

this avoids confusion, especially in case the YAML string contains unbalanced braces

Copy link
Member Author

@nicolas-grekas nicolas-grekas Nov 2, 2016

Choose a reason for hiding this comment

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

also for consistency: some messages had brackets+final dot around, others not.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, messages without brackets were changed in the past precisely for the reason I gave. Looks like messages added in newer branches were not updated after the branch merge

@xabbuh
Copy link
Member

xabbuh commented Nov 2, 2016

👍

@fabpot
Copy link
Member

fabpot commented Nov 2, 2016

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 7520f7b into symfony:2.7 Nov 2, 2016
fabpot added a commit that referenced this pull request Nov 2, 2016
This PR was merged into the 2.7 branch.

Discussion
----------

[Yaml] Clean some messages + add test case

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Related to #20335 on 3.2.

Commits
-------

7520f7b [Yaml] Clean some messages + add test case
@nicolas-grekas nicolas-grekas deleted the yaml-clean branch November 3, 2016 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.