-
Notifications
You must be signed in to change notification settings - Fork 63
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
<!-- eslint-disable --> does not work in the v1.0.0-beta.6 #69
Comments
@vsemozhetbyt thanks for reporting these!
|
@btmills Use cases examples for docs: REPL sessions logs that are not valid JS but need syntax highlighting; big Object listings that erroneously parsed as code blocks by ESLint (wrapping them in parentheses make them less readable). It would be very handy to have an option to disable eslint-plugin-markdown for such fragments: code fragments in docs do have some specificity. |
@vsemozhetbyt that makes sense, thanks for explaining. As a first step, I tracked down what was causing the behavior you observed in beta.4. It turns out that the Markdown parser in use at the time wasn't picking up code fences unless they were preceded by blank lines. I've added a test in #72 to make sure that doesn't happen again.
Since your disable comments and code fences were on adjacent lines, the processor wasn't even getting those code blocks back from the parser. I'm glad that bug was fixed in a beta! 😅 Now, as for supporting your use case. I'd like to keep the meaning of
|
@btmills I like it) Let's see what others think. |
Me too. |
👍 |
I just submitted a pull request to implement |
@btmills Thank you! Looking for landing and the new version to update the plugin in the Node.js. |
|
@btmills Thank you! |
An issue affecting Node.js source has been fixed in eslint-plugin-markdown so we don't need to pin it to beta-4 anymore. Refs: eslint/markdown#69
* Remove pinning of eslint-plugin-markdown An issue affecting Node.js source has been fixed in eslint-plugin-markdown so we don't need to pin it to beta-4 anymore. Refs: eslint/markdown#69 * Update eslint-plugin-markdown up to 1.0.0-beta.7 * Fix docs for [email protected] PR-URL: #14047 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
* Remove pinning of eslint-plugin-markdown An issue affecting Node.js source has been fixed in eslint-plugin-markdown so we don't need to pin it to beta-4 anymore. Refs: eslint/markdown#69 * Update eslint-plugin-markdown up to 1.0.0-beta.7 * Fix docs for [email protected] PR-URL: #14047 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
* Remove pinning of eslint-plugin-markdown An issue affecting Node.js source has been fixed in eslint-plugin-markdown so we don't need to pin it to beta-4 anymore. Refs: eslint/markdown#69 * Update eslint-plugin-markdown up to 1.0.0-beta.7 * Fix docs for [email protected] PR-URL: #14047 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
eslint-plugin-markdown is being installed @next to get a bugfix for eslint/markdown#69 but that bugfix is in 1.0.0. Go back to installing @latest rather than @next.
eslint-plugin-markdown is being installed @next to get a bugfix for eslint/markdown#69 but that bugfix is in 1.0.0. Go back to installing @latest rather than @next. PR-URL: nodejs#26345 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
eslint-plugin-markdown is being installed @next to get a bugfix for eslint/markdown#69 but that bugfix is in 1.0.0. Go back to installing @latest rather than @next. PR-URL: #26345 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1. I've found an error on the v1.0.0-beta.4:
<!-- eslint-disable some-rule -->
completely disables parsing the whole fragment and even the next fragment:test.md
### Heading Text ` ``js console.log('a') ` `` ` ``js console.log('b') ` `` ` ``js console.log('c') ` ``
Error log:
test.md
with comments:Error log (
semi
errors for the fragments 1 and 2 are not reported; fragment 2 is ignored):2. So I've tried to update to v1.0.0-beta.6 and make a PR for Node.js. v1.0.0-beta.6 seems to fix this issue, but it has a new breaking one: code blocks with
<!-- eslint-disable -->
are parsed and throws if parsing errors found:test.md
:Error log:
7:18 error Parsing error: Unexpected token bang
The text was updated successfully, but these errors were encountered: