Fenced code block with list fails
Closed this issue · 19 comments
Given the following fenced code block,
1. Foo
```
+ bar
```blackfriday renders:
<ol>\n<li><p>Foo</p>\n\n<p>```</p>\n\n<ul>\n<li>bar\n```</li>\n</ul></li>\n</ol>\n
I expect it to render:
<ol>\n<li><p>Foo</p>\n\n<pre><code>+ bar\n</code></pre></li>\n</ol>\n
It appears to occur when the first line of the code block matches a list item pattern (ie. -, +, *, or 1.). If there's a blank line or any non-list item patterned content on the first line, the code fencing works properly.
Here's a test case for TestFenceCodeBlockWithinList:
"1. Foo\n\n ```\n + bar\n ```\n",
`<ol>
<li><p>Foo</p>
<pre><code>+ bar
</code></pre></li>
</ol>
`,
I haven't been able to figure out where it's breaking down.
I encountered this issue with Jenkins console output, which contains xtrace lines (i.e. shell commands are logged to stdout prefixed by + followed by a space).
Notably, the + character was not on the first line. I've taken the liberty to include the whole thing below.
Started by user anonymous
[Pipeline] Allocate node : Start
Running on master in /var/jenkins_home/jobs/simple-pipeline/workspace
[Pipeline] node {
[Pipeline] sh
[workspace] Running shell script
+ echo
echo "Hello from script"
[Pipeline] load: Loaded script: script.groovy
[Pipeline] load {
[Pipeline] echo
Hello from script
[Pipeline] } //load
[Pipeline] } //node
[Pipeline] Allocate node : End
[Pipeline] End of Pipeline
Finished: SUCCESS
Confirmed, this is a bug.
I encountered this problem today due to a line in a code section that starts with ": ".
Disclaimer: this morning I knew nothing about the go programming language.
Nonetheless I looked into it a little. I built a test case in TestFenceCodeBlockWithinList very similar to the one @moorereason posted.
--- FAIL: TestFencedCodeBlockWithinList (0.00s)
block_test.go:65:
Input ["* Foo\n\n ```\n : bar\n ```\n"]
Expected["<ul>\n<li><p>Foo</p>\n\n<pre><code>: bar\n</code></pre></li>\n</ul>\n"]
Actual ["<ul>\n<li><p>Foo</p>\n\n<p>```</p>\n\n<p>: bar\n```</p></li>\n</ul>\n"]
It seems to happen during secondPass, when parsing out lists. I think due to the "list-like" start of the line, it considers the line starting with the offending symbol as another list item. What then happens is that data.len is set in such a way, that
if beg == 0 || beg >= len(data)
in func (p *parser) fencedCode(out *bytes.Buffer, data []byte, doRender bool) int becomes true due to both beg and len(data) being 4.
I faced this issue today. The issue occurs specifically when there is Markdown list syntax in a fenced code block within a Markdown list. The parser tries to interpret the hyphens in the code fence to list in HTML.
Looking forward to the fix.
This is very frustrating when trying to display YAML files in code blocks inside of numbered lists. Our use case is Lab steps which require sequential numbers, code blocks, and to display YAML files.
Workaround I found in other bugs which reference this issue is to use 8 spaces instead of code fences.
This issue was reported almost 2 years ago; Is there a plan to fix this?
I hit this too, here's a minimal repro:
1. First item
```yaml
ports:
foo: bar
```
2. Second item
```yaml
ports:
- port: 80
```
3. Third item
Both code fences are indented by 4 spaces. The first YAML block under "First item" renders just fine, however the one under "Second item" doesn't, and just prints the raw code fence syntax (picture below). I was able to determine that it's happening when the code block contains an array like above.
@client9 Thanks for the work. I won't be able to test it though, till Hugo moves to Blackfriday v2.
Oh good point this could be backported to v1
Yeah a backport to v1 would be great for Hugo. I'm happy to put together a PR for it today unless you would like to do it @client9
@tfogo made a start of it on https://github.com/client9/blackfriday
I get some failures in the tests, but this might mean the test just need adjusting. V1 might work differently.
existing test still pass -- only the new tests have some failures.
--- FAIL: TestFencedCodeBlockWithinList (0.00s)
block_test.go:64:
Input ["* Foo\n\n ```\n bar\n\n qux\n ```\n"]
Expected["<ul>\n<li><p>Foo</p>\n\n<pre><code>bar\n\nqux\n</code></pre></li>\n</ul>\n"]
Actual ["<ul>\n<li>Foo\n\n<code>\nbar\n\nqux\n</code></li>\n</ul>\n"]
--- FAIL: TestListWithMalformedFencedCodeBlock (0.00s)
block_test.go:64:
Input ["1. one\n\n ```\n code\n\n2. two\n"]
Expected["<ol>\n<li>one\n```\ncode\n2. two</li>\n</ol>\n"]
Actual ["<ol>\n<li>one\n\n```\ncode\n\n2. two</li>\n</ol>\n"]
block_test.go:64:
Input ["1. one\n\n ```\n - code\n\n2. two\n"]
Expected["<ol>\n<li>one\n```\n- code\n2. two</li>\n</ol>\n"]
Actual ["<ol>\n<li>one\n\n```\n- code\n\n2. two</li>\n</ol>\n"]
FAIL
FAIL github.com/client9/blackfriday 16.318s
Hi @client9 - sorry for the late reply.
I looked into this. Looks like TestFencedCodeBlockWithinList is a test which is in v1 but not v2. It's testing a fenced code block if there is one item in a list. Looks like this has caught an edge case with your fix where the output is different from what is expected.
I believe this input:
* Foo
```
Hello
```
should produce this output:
<ul>
<li><p>Foo</p>
<pre><code>Hello
</code></pre></li>
</ul>However it is producing this:
<ul>
<li>Foo
<code>
Hello
</code></li>
</ul>It looks like this is because the LIST_ITEM_CONTAINS_BLOCK flag is never set so the list item is run through the inline parser instead of the block parser. See here: https://github.com/client9/blackfriday/blob/d68ad860fb6ced29d8a9448e3da5aa8ad7efa145/block.go#L1279
I've opened a PR to your repo to set the LIST_ITEM_CONTAINS_BLOCK flag if there is a fenced code block in a list item.
As for the TestListWithMalformedFencedCodeBlock test, as @rtfb mentioned in #372 this is undefined behavior and it should be okay as long as it doesn't cut off content. Looks like it's just outputting more newlines so in my PR I just change the expected output of the test.
PR here: https://github.com/client9/blackfriday/pull/2
@client9 if it looks good to you, I think we should submit it to @rtfb for approval. There may be some edge case I'm not thinking of here where this isn't the solution we want.
Also, we probably want to add this test case and fix to v2. This test fails on v2. But I'll wait to hear from you and @rtfb if you think this is the right fix.
This should now be fixed in both v1 and v2.
This should now be fixed in both v1 and v2.
Just a quick check on what you meant by "should": are we good to close it, or should we still doublecheck whether it helped?
I've confirmed that it's fixed in master and v2. 🎉
Awesome! Thanks for everyone involved!