-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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
Conversation
*/ | ||
public function testStringOffsetCastError() | ||
{ | ||
Inline::parse('{this, is not, yaml}'); |
There was a problem hiding this comment.
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}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, updated
91e38be
to
7520f7b
Compare
@@ -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)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
👍 |
Thank you @nicolas-grekas. |
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
Related to #20335 on 3.2.