Skip to content

feature(config): adding config.ignore to replace metadata#2

Closed
rodneyrehm wants to merge 1 commit intoDavidAnson:masterfrom
rodneyrehm:feature/ignore-frontmatter
Closed

feature(config): adding config.ignore to replace metadata#2
rodneyrehm wants to merge 1 commit intoDavidAnson:masterfrom
rodneyrehm:feature/ignore-frontmatter

Conversation

@rodneyrehm
Copy link
Copy Markdown
Contributor

My markdown files contain frontmatter to configure the documentation build system. That code is erroneously detected as a headline and subsequently fails MD002 and MD003. (github displays the frontmatter as a table). Should you wish to parse the the frontmatter properly, the gray-matter package might be a good place to start.

This PR adds the config option ignore with which any part of a markdown file can be replaced by line breaks (to preserve the line-markings of the actual tests) before the markdown file is linted. Because frontmatter seems to be used by a couple of systems (Jekyll and metalsmith being the ones I use), I added the regular expression to your code directly and acitvating it if config.ignore === 'frontmatter' is passed in.

Cheers!

@DavidAnson
Copy link
Copy Markdown
Owner

Wow, thank you very much for doing this, Rodney!

I don't use front matter myself, but am familiar with the idea. The approach you've taken seems quite reasonable at first glance. :) Please give me a few days to reflect on this some more and finish up the change I am already in the middle of.

In the meanwhile, let me share the two thoughts I have so far:

  • Rather than disable blank line detection in order to support front matter, what about removing the front matter before processing the text and then adjusting line numbers for any errors to compensate? This should be pretty easy, would stay out of the way of all other rules, and seems consistent with how front matter is actually handled. (As an aside, it will avoid problems with upcoming rule MD041 as well.)
  • If that is done, the config option could become simply ignoreFrontMatter:true and there would be no need to support Function or RegExp (which I would want to test more aggressively anyway).

What do you think?

PS - It looks like there were some CI/coverage issues with the PR - but they seem pretty minor.

@rodneyrehm
Copy link
Copy Markdown
Contributor Author

Rather than disable blank line detection in order to support front matter, what about removing the front matter before processing the text and then adjusting line numbers for any errors to compensate?

Yes! That's the approach I would've preferred. I just did the minimum amount of change to get it working (for me) quickly, sorry…

the config option could become simply ignoreFrontMatter:true and there would be no need to support Function or RegExp

Which would've solved exactly the problem of frontmatter, without allowing other / similar problems being taken care of by the tool's userbase. I'm not aware of any metadata conventions besides frontmatter, but then I just learned of frontmatter yesterday, so I guess that doesn't say much.

What do you think?

Agree with both points. Will you take care of it (and simply close this PR), or do I need to find some time to fix the PR?

@DavidAnson
Copy link
Copy Markdown
Owner

It will be a couple of days before I have a chance to return to this. If you find some time before then to update the pull request, that's great! If not, I'll follow through with the ideas above. However it works out is fine with me. :)

@rodneyrehm
Copy link
Copy Markdown
Contributor Author

I'm spread thin at the moment. I'd appreciate you taking care of it whenever you can - for now I have my fix that allows me to work somehow :)

@DavidAnson
Copy link
Copy Markdown
Owner

No problem. :)

@DavidAnson
Copy link
Copy Markdown
Owner

I had a bit of time to play around here and settled on an approach I like that's somewhere in the middle of what you did and what I proposed. :) I should be able to push it in the next couple of days.

@rodneyrehm
Copy link
Copy Markdown
Contributor Author

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants