Open jonathan-mayer opened 6 days ago
JTaeuber
commented
4 days ago I've looked a little into this. The custom rule you pinned only seemed to work with markdownlint-cli2. The problems are for one, that we have two new config files. And the other thing is that there is no autofix for this rule. I've pushed a branch so you can see what I've concocted. Depending on what exactly we want to achieve this might require us writing something custom. ¯\_(ツ)_/¯
jonathan-mayer
commented
2 days ago I don't think any of the things you addressed are a real problem, i do want to see if i can get autofix to work but even that wouldn't be too bad. I do want to address one thing tough. In your branch you disabled a lot of rules which i think should not be disabled but just handled differently.
// inline-html
// yes we do sadly need to disable this rule, although if used improperly we should still address this in the review
// we could consider using the `allowed_elements` option (see https://github.com/DavidAnson/markdownlint/blob/main/doc/md033.md)
// this would "warn" us about this in reviews
// and should be fine since we only really use DocCardList and maybe the CaaS avatar
MD033: false
// fenced-code-language
// this is definitely necessary, its not hard to just write '```text' on code blocks with plain text in them
// and we should always set the corresponding language if it is more than plain text
MD040: false
// line-length
// while the 80char limit is hard, i think we should come up with a better solution to this instead of disabling it completely
// either we actually enforce this (which could be pretty annoying)
// or we increase it by a bit (maybe 100, and then go ahead and enforce that limit)
MD013: false
// changes as per: https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md
// > MD013:
// > line_length: 100
// single-title
// yes this is actually a problem with blog-style front matter in markdown
// but since it is so common to have yaml front matter they thought of a way to combat this issue.
// since we do specify the title field we should set `front_matter_title` to "title"
// but this would result in problems with us putting the h1 title in again for "offline" readability
// so i think we should just leave it empty so it will ignore the front matter completely
MD025: false
// changes as per: https://github.com/DavidAnson/markdownlint/blob/main/doc/md025.md
// > MD025:
// > front_matter_title: ""
// first-line-heading
// pretty sure its same here
MD041: false
// changes as per: https://github.com/DavidAnson/markdownlint/blob/main/doc/md041.md
// > MD041:
// > front_matter_title: ""
// ordered-list-marker
// this rule is kind of picky about block elements in a list
// but as far as https://github.com/DavidAnson/markdownlint/blob/main/doc/md029.md says
// this is fixable by indenting the block, which would be an easy enought fix
// but when i tried with the vscode extension of markdownlint
// it didn't seem to stop and prettier wanted to format it back
// we might have to just disable this and make sure to check it ourselves
MD029: false* i do think you just didn't want to deal with all the problems in that test branch but i think its was good to address this in this thread to further demonstrate the way we should implement this * also we should definitely comment the .markdownlint.yaml since it will otherwise become unreadable. * i also just spent almost a hour researching and writing all this so 🙃
Issue
Currently we were just hoping that people try to stick to markdown best practices or that we catch problems in the review.
Problem to solve
We should have a pre-commit check that runs markdown lint. Additionally we should use markdown lint title case as markdown lint doesn't check for title case.
Further details
Proposal
Who can address the issue
Other links/references
https://github.com/DavidAnson/markdownlint https://www.npmjs.com/package/markdownlint-rule-titlecase