Skip to content

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

$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