-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Markdown: fix handling of code fences appearing on the same line #5606
Conversation
spec/std/markdown/markdown_spec.cr
Outdated
@@ -70,6 +70,8 @@ describe Markdown do | |||
assert_render "```crystal\nHello\nWorld\n```", "<pre><code class='language-crystal'>Hello\nWorld</code></pre>" | |||
assert_render "Hello\n```\nWorld\n```", "<p>Hello</p>\n\n<pre><code>World</code></pre>" | |||
assert_render "```\n---\n```", "<pre><code>---</code></pre>" | |||
assert_render "````\n---\n````", "<pre><code>---</code></pre>" | |||
assert_render "```invisible man```", "<p><code></code><code>invisible man</code><code></code></p>" |
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 is having 2 empty code block around the actual code correct?
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.
Because there's no inline code fence, so it's an empty inline code block (``
), an inline code block with the "invisible man" content and finally another empty inline 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.
Wow, thank you! I had the same doubt.
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.
It's not so much about what's correct, just that the previous result was much, much worse (see first message). And typing like this is apparently quite common.
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.
According to this example of the spec: http://spec.commonmark.org/0.12/#example-95
There should just be one <code>
block, but as you said, the old behavior was worst, maybe it's ok to do this in another pr.
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.
Code spans can be delimited by any number of backticks, the only requirement is that start and end delimiter have equal length. So ```invisible man```
should really only produce <p><code>invisble man</code></p>
. See CommonMark spec: Code spans
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.
Inline code blocks are not affected by this PR and are outside the scope.
e.g. this is the same before and after.
require "markdown"; p Markdown.to_html("a ```b```")'
"<p>a <code></code><code>b</code><code></code></p>"
I added a TODO for the spec.
In conformance with http://spec.commonmark.org/0.12/#fenced-code-blocks To avoid detecting this whole line as the language of the code: ```invisible man``` Expected: "<p><code></code><code>invisible man</code><code></code></p>" got: "<pre><code class='language-invisible man```'></code></pre>" Also delete an unused method
Ping |
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.
I would prefer to go full way and have the TODO removed here, but if another contributor things it's fine as is I'm fine with it too to solve the immediate issue.
This fixes the described issue. Let's do a separate PR for the full dive for more improvements 👍 |
In conformance with http://spec.commonmark.org/0.12/#fenced-code-blocks
To avoid detecting this whole line as the language of the code:
Also delete an unused method