content fix: MD037 space inside emphasis

Opening as draft, as this depends on https://github.com/mdn/content/pull/20113 for the other part of the flagged issues

Aug 30 '22 05:08 nschonni

Preview URLs

(comment last updated: 2023-05-17 04:04:20)

Aug 30 '22 05:08 github-actions[bot]

@teoli2003 there is no special handling for HTML in Markdownlint, and the recommendation is to use the disable directives when you run into edge cases

Some of those tables could probably be converted to Markdown, and those disable directives would go away with the conversion. That's why I left them at the top of the table

Aug 30 '22 16:08 nschonni

Rebased now that the license cleanup PR landed

Aug 31 '22 04:08 nschonni

I still believe it is a bug in Markdownlint. It should not flag Markdown errors in the content of <table> or <math> as Markdown syntax is forbidden in these elements.

I don't think it is a good idea to add 100s of magic comments to work around such a bug.

Aug 31 '22 05:08 teoli2003

I don't think it is a good idea to add 100s of magic comments to work around such a bug.

There are 5 disables in this one, and I think the other PR that I abandoned had 40. The comments are there for when this situations happen. The same will occur with the Prettier formatting

Aug 31 '22 14:08 nschonni

There is no consensus to add Markdown or Prettier magic comments on the pages. For the moment, we add/activate rules that don't need these.

Additional question: Is MD037 auto-fixable? The Markdownlint workflow is informative: it tells authors and reviewers that the Markdownlint fixing workflow, running once a day, will fix something but doesn't force authors or reviewers to act (that's why the annotation are all warnings).

Sep 05 '22 11:09 teoli2003

Yes, this rule is fixable https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md#md037---spaces-inside-emphasis-markers and the docs also talk about why this one is a functional issue. Spaces around the emphasis marker mean Markdown no longer treats the block as emphasised

Sep 05 '22 15:09 nschonni

@Josh-Cena I'm not inclinced to change this because you and @teoli2003 don't like disable comments. You appear to be the only ones that keep raising this on these PRs

Nov 09 '22 00:11 nschonni

We seem to be the only ones reviewing these PRs anyway 😄 We can definitely talk with others about this and how we should proceed, but in general I don't like how this rule introduces far more disable comments than valid fixes. Even in a codebase where disable comments are acceptable, it seems unnecessary if a rule fixes two bugs while requiring another 10 disable comments. It's better if we can just get the fixes in.

Nov 09 '22 00:11 Josh-Cena

This pull request has merge conflicts that must be resolved before it can be merged.

Apr 24 '23 05:04 github-actions[bot]

@Josh-Cena , as discussed, I've removed the enabling of MD037 and the associated directives, and kept the other changes in this PR. Do you want to take a look?

May 17 '23 04:05 wbamberg

@wbamberg if you wanted those changes, you should have created a new PR, not hijacked mine

May 17 '23 04:05 nschonni