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

Chore: Reproduce nested blocks failure (refs #181) #185

Closed
wants to merge 2 commits into from
Closed

Conversation

btmills
Copy link
Member

@btmills btmills commented Mar 30, 2021

@JounQin I put together a reproduction test tonight to make sure I fully understand the problem, and I'm sharing it here in case it helps as you're working on #183.

In the next commit, I'm going to need to pass in other options besides
`fix`.
The current implementation assumes the `blocks` module-level variable
will only be used once per physical file. If the file contains a nested
Markdown code block, the processor will be recursively run on the code
block's virtual file.
@btmills btmills added this to Triaging in Triage Mar 30, 2021
@JounQin
Copy link
Contributor

JounQin commented Mar 30, 2021

Thanks, I was thinking that has been validated via https://github.com/eslint/eslint-plugin-markdown/pull/183/files#diff-8c591ed3533957b84298776fda1d6d11b2da563b3222e471b4725fe248c0e294

I'll add this test case in that PR.

@btmills
Copy link
Member Author

btmills commented Mar 30, 2021

That fixture was the right .md to reproduce the issue, but by itself it wasn't being executed because no describe()/it() test was using it. If you were to add that fixture by itself without implementing the blocks cache fix yet and run the tests, it wouldn't fail as hoped.

Here I wrote a describe()/it() test that will be executed as part of the test run to prove that 1) preprocess is extracting nested blocks, 2) ESLint is finding the expected lint messages in those nested blocks, and 3) the two layers of postprocess calls map those lint messages back to the correct line in test.md.

The test would function just as well using a file in tests/fixtures/test.md instead of creating the file text with [...].join("\n");, but I personally find it easier to see what the test is doing when the file text and assertions are right next to each other.

@JounQin
Copy link
Contributor

JounQin commented Mar 30, 2021

@btmills No worry, I've added this test case at #183 as you suggested.

@btmills
Copy link
Member Author

btmills commented Mar 30, 2021

Closing as this has been integrated into #183, which contains the fix.

@btmills btmills closed this Mar 30, 2021
Triage automation moved this from Triaging to Complete Mar 30, 2021
@btmills btmills deleted the issue181 branch March 30, 2021 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Triage
Complete
Development

Successfully merging this pull request may close these issues.

None yet

2 participants