Skip to content
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

Block parsing for fenced code blocks with syntax highlighting in lists regression. #495

Closed
dmitshur opened this issue Oct 8, 2018 · 5 comments · Fixed by #521
Closed
Labels

Comments

@dmitshur
Copy link
Collaborator

dmitshur commented Oct 8, 2018

I've just discovered PR #476 is causing the following regression in v1. /cc @tfogo @client9

Given this Markdown input:

### Codeblock within list

-	list entry 1

	```
	if (i == 5)
	    break;
	```

-	list entry 2

### Stuff that comes later

stuff here

Everything works as expected:

image

However, if one uses a fenced code block with syntax highlighting, everything that comes after the fenced code block ends up being a part of "list entry 1":

### Codeblock within list

-	list entry 1

	```C
	if (i == 5)
	    break;
	```

-	list entry 2

### Stuff that comes later

stuff here

image

I.e., it parses as if the input was:

### Codeblock within list

-	list entry 1

	```C
	if (i == 5)
	    break;
	```

	-	list entry 2

	### Stuff that comes later

	stuff here
@dmitshur
Copy link
Collaborator Author

dmitshur commented Oct 8, 2018

I have a lead. Modifying the code as follows seems to fix the issue, at least for that one instance of the problematic input:

-_, marker := isFenceLine(chunk, nil, codeBlockMarker, false)
+_, marker := isFenceLine(chunk, new(string), codeBlockMarker, false)

I.e., passing a non-nil info *string parameter to isFenceLine.

@SamWhited
Copy link

SamWhited commented Oct 11, 2018

I came here to report a similar issue which sounds like it might have the same cause, right now if a fenced code block immediately follows a list even if it's not indented it becomes part of the list. The following patch adds a test that can be used to demonstrate the behavior (the expected result might have wrong newlines, I just guessed):

diff --git a/block_test.go b/block_test.go
index 326c3110ce70..4db7d24156e2 100644
--- a/block_test.go
+++ b/block_test.go
@@ -1135,6 +1135,9 @@ func TestFencedCodeBlock(t *testing.T) {

                "```\n[]:()\n[]:)\n[]:(\n[]:x\n[]:testing\n[:testing\n\n[]:\nlinebreak\n[]()\n\n[]:\n[]()\n```",
                "<pre><code>[]:()\n[]:)\n[]:(\n[]:x\n[]:testing\n[:testing\n\n[]:\nlinebreak\n[]()\n\n[]:\n[]()\n</code></pre>\n",                                                                                                                      
+
+               "- test\n\n\n``` go\nfunc foo() bool {\n\treturn true;\n}\n```\n",
+               "<ul>\n<li>test</li>\n<ul>\n\n<pre><code class=\"language-go\">func foo() bool {\n\treturn true;\n}\n</code></pre>\n",                                                                                                                  
        }
        doTestsBlock(t, tests, FencedCode)
 }

EDIT: The above is against the v2 branch, but just noticed that this has the v1 label. Not sure if it's the same or not.

@TeaWeb
Copy link

TeaWeb commented Dec 18, 2018

+1 the same issue here

@rtfb rtfb closed this as completed in #521 Jan 24, 2019
rtfb pushed a commit that referenced this issue Jan 24, 2019
This attempts to fix #495 and #485.

Note the test cases which were added at the bottom of the list. The first added test case was passing even before the changes, but the second was not.
@dmitshur
Copy link
Collaborator Author

Thank you very much for resolving this issue, @aignas and @rtfb!

You've also indirectly resolved shurcooL/markdownfmt#44.

dmitshur added a commit to shurcooL/markdownfmt that referenced this issue Mar 24, 2019
The intention is to make it so that it's possible to import and use
this package in module mode (in addition to the well-supported
GOPATH mode), and have it build successfully. That doesn't happen
automatically because the latest version of the module that provides
the github.com/russross/blackfriday package (the v1 API) is an
v2.0.0+incompatible version that provides the v2 API, which is not
compatible.

Use the latest pseudo-version of blackfriday v1 API. Can't use the
latest tagged release version v1.5.2 because it doesn't include an
important bug fix for russross/blackfriday#495.

Only specify the blackfriday version in go.mod; others are unspecified
to avoid the added burden of maintaining and updating them. The latest
version, whichever they are, are fully supported, just like in GOPATH
mode.

Fixes #47
arvindsv added a commit to arvindsv/docs.go.cd that referenced this issue Jun 17, 2019
dmitshur added a commit to shurcooL/markdownfmt that referenced this issue Jan 17, 2021
There is now a released version of blackfriday v1 that includes the
bug fix for russross/blackfriday#495 that
our tests rely on. Start using it instead of an older pseudo-version.

Updates #47.
Updates russross/blackfriday#587.
dmitshur added a commit to shurcooL/markdownfmt that referenced this issue Jan 17, 2021
There is now a released version of blackfriday v1 that includes the
bug fix for russross/blackfriday#495 that
our tests rely on. Start using it instead of an older pseudo-version.

Updates #47.
Updates russross/blackfriday#587.
willdollman pushed a commit to willdollman/blackfriday that referenced this issue Feb 27, 2021
This attempts to fix russross#495 and russross#485.

Note the test cases which were added at the bottom of the list. The first added test case was passing even before the changes, but the second was not.
@simonasGit
Copy link

#715 Same issue on V2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants