-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
enclose YAML strings containing % with quotes #4030
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
@@ -499,7 +499,7 @@ Full default Configuration | |||
collect: true | |||
only_exceptions: false | |||
only_master_requests: false | |||
dsn: file:%kernel.cache_dir%/profiler | |||
dsn: "file:%kernel.cache_dir%/profiler" |
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.
to be exact, YAML 1.1 has an issue only when %
starts the values, so this change is not necessary
A quick search on the documentation give me this:
The last one in the JSON example is caused by the three dots in the file I think. It looks like everytime the syntax highlighter fails, it sets the code dark on bright. Was this the case before the new version of the website? @javiereguiluz any inputs on this? |
@iamdto when the syntax highlighter fails, the code was not highlighted either previously (this is not related to the website itself, but to the Python tool used to render the doc). However, the background was still black in the old website IIRC. |
Yes, exactly, that's what I meant. Even when syntax highlighting failed the code was still bright on black. |
Indeed, the old style rendered it like a |
@@ -119,15 +119,15 @@ or this: | |||
|
|||
The output will look like this: | |||
|
|||
.. code-block:: text | |||
.. code-block:: 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.
In this file the issue is not about YAML but more about the first JSON example (see the rendered version on the site). In fact in this part we're talking about the output of the command above, so I think we should let it be a text code block.
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.
Given that this is the output of the command, I agree we should keep it highlited as text, not as yaml (and we should not add the quotes which are not part of the output).
Highlighting the YAML could be confusing if some user then expect the command output to be highlighted
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.
That's true. I didn't look carefully enough. This change is now reverted.
For the JSON example: It's not rendered properly because of the ...
the JSON is no longer valid. Though I'm not sure how to work with that.
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.
Well, we don't have that much solutions:
- We fix the syntax of all JSON examples in the documentation (though it could happen on other languages too).
- We had back the light on dark theme for the default syntax highlighting.
- We keep it like this.
@javiereguiluz What do you think about the default code block style? |
The xml code containing the three dots in a tag (e.g. |
Pygment uses the YAML 1.1 standard to parse YAML code being highlighted. A string value cannot start with the percent sign or the @ character in YAML 1.1 without being quoted. To highlight the YAML code blocks properly, strings starting with the percent sign are now enclosed in double quotes.
@wouterj Thanks for the hint. Most of the code blocks you found have been fixed in the meantime in the |
@xabbuh I've added this bug to the symfony.com backlog. It will be probably fixed soon because it can be easily fixed. Thank you all for reporting this error! |
@javiereguiluz Thank you! |
The ellipses were used as placeholders in XML element tags. Thus, the XML wasn't well-formed and therefore would have never been rendered properly.
This error has been fixed and it will be online after the next deploy. |
👍 |
Very good changes, looks great to me :). Thanks Christian! |
This PR was merged into the 2.3 branch. Discussion ---------- enclose YAML strings containing % with quotes | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | all | Fixed tickets | #1886, #4029 Thanks @iamdto, did you spot some other occurences in the docs? Commits ------- c1c4bc8 remove "..." from XML element tags dbc02a2 quote YAML strings starting with % or @ characters
Thanks @iamdto, did you spot some other occurences in the docs?