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

markdownlint-disable/enable is the wrong pattern; use disable/restore instead #276

Closed
DavidAnson opened this issue Aug 13, 2020 · 7 comments · Fixed by #278
Closed

markdownlint-disable/enable is the wrong pattern; use disable/restore instead #276

DavidAnson opened this issue Aug 13, 2020 · 7 comments · Fixed by #278

Comments

@DavidAnson
Copy link
Contributor

DavidAnson commented Aug 13, 2020

  • all-contributors-cli version: Latest
  • node version: N/A
  • npm (or yarn) version: N/A

Relevant code or config

What you did:

Investigated an issue reported by @bnb where markdownlint rules were being ignored in a specific file that was generated by this project.

What happened:

The pattern produced by this project of <!-- markdownlint-disable --> then <!-- markdownlint-enable --> in the two files referenced above has the effect of (re-)enabling all markdownlint rules. In cases where the relevant project/file/user has disabled some (ex: line-length), they are undesirably enabled for the remainder of the file. Instead, use <!-- markdownlint-restore --> to restore the previously-enabled rules.

There's a discussion and example here: https://github.com/DavidAnson/markdownlint#configuration

Reproduction repository:

Any repository that uses the pattern above.

Problem description:

See above.

Suggested solution:

See above.

@Berkmann18
Copy link
Member

I'm not quite sure where the issue is, this disable/enable comment section is there to prevent a previously raised issue where MarkdownLint would complain about the table not respecting rules while still allowing it to do its job as far as the rest of the file (e.g. README.md) is concerned.

@bnb
Copy link

bnb commented Aug 13, 2020

@Berkmann18 the problem is that enable turns on all rules ignoring the users' initial config. In my case, I had a rule I was ignoring that was triggered after the all-contributors block, and because enable is used rather than restore, my config was ignored and the rules were reset to defaults which was throwing errors rather than restoring my rules.

@Berkmann18
Copy link
Member

@bnb That makes sense. A PR would be welcome for this.

@DavidAnson
Copy link
Contributor Author

Here’s a visual demonstration:

What it does

What it should do

@DavidAnson
Copy link
Contributor Author

@bnb I probably won't get to this anytime soon. If you want to send the PR, please go ahead. The change is trivial, but I don't know how easy/hard it will be to add tests, etc..

@DavidAnson
Copy link
Contributor Author

I decided to YOLO a PR. All checks pass. Here you go: #278

@all-contributors-release-bot
Copy link
Member

🎉 This issue has been resolved in version 6.17.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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