Skip to content

chore: avoid formatting 3rd-party license files#11867

Merged
jcfranco merged 3 commits intodevfrom
jcfranco/switch-to-markdownlint-cli2
Apr 5, 2025
Merged

chore: avoid formatting 3rd-party license files#11867
jcfranco merged 3 commits intodevfrom
jcfranco/switch-to-markdownlint-cli2

Conversation

@jcfranco
Copy link
Copy Markdown
Member

@jcfranco jcfranco commented Apr 4, 2025

Related Issue: N/A

Summary

Stems from #11853 (review)

This ensures that the markdownlint ignore list is used consistently across linting tasks.

This was done by primarily switching from markdownlint-cli to markdownlint-cli2, which builds on the former and simplifies the setup.

For context, the root .markdownlintignore wasn't picked up by markdownlint-cli because it must be present in the cwd. This was doable, but it meant duplicating the ignore file across packages.

Notes

  • renames lint-staged configs and uses .mjs
  • tested with both lint-staged and NPM scripts
  • --dot is the default
  • .markdownlintignore and --ignore-path was removed in favor of config options (ignores and gitignore, respectively), which will be applied consistently
  • moves the contents of .markdownlintrc to .markdownlint-cli2.mjs

@github-actions github-actions Bot added the chore Issues with changes that don't modify src or test files. label Apr 4, 2025
@benelan
Copy link
Copy Markdown
Contributor

benelan commented Apr 4, 2025

Is #11853 the only reason for switching? I don't think markdownlint formats html in markdown files. Those formatting issues should be resolved by #11858

@jcfranco
Copy link
Copy Markdown
Member Author

jcfranco commented Apr 4, 2025

I don't think markdownlint formats html in markdown files.

Not sure I follow, but this addresses the markdownlint ignore file not working properly and unintentionally formatting 3rd-party license text. I should've finished updating the PR description before posting in #11853, my bad! 😅

@jcfranco jcfranco changed the title build(deps): switch to markdownlint-cli2 chore: avoid formatting 3rd-party license files Apr 4, 2025
Copy link
Copy Markdown
Contributor

@benelan benelan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I have one suggestion but otherwise LGTM!

Not sure I follow, but this addresses the markdownlint ignore file not working properly and unintentionally formatting 3rd-party license text.

Woops that was my bad. I somehow misread the thread in #11853 as being about a formatting issue I noticed in the HTML table of contributors. I was just talking to myself in #11853 (comment), nothing to see here!

Comment thread .markdownlint-cli2.mjs Outdated
export default {
config: {
$schema:
"https://raw.githubusercontent.com/DavidAnson/markdownlint-cli2/main/schema/markdownlint-cli2-config-schema.json",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSON Schema won't work in a JavaScript file, and since there aren't types, we won't get any completion like this. If there isn't a reason this needs to be mjs, can you change it to json?

{
  "$schema": "https://raw.githubusercontent.com/DavidAnson/markdownlint-cli2/main/schema/markdownlint-cli2-config-schema.json",
  "config": {
    "first-line-heading": false,
    "no-inline-html": false,
    "line-length": false,
    "code-block-style": {
      "style": "fenced"
    },
    "code-fence-style": {
      "style": "backtick"
    },
    "heading-style": {
      "style": "atx"
    },
    "no-duplicate-heading": {
      "siblings_only": true
    },
    "emphasis-style": {
      "style": "asterisk"
    },
    "strong-style": {
      "style": "asterisk"
    },
    "ul-style": {
      "style": "dash"
    }
  },
  "gitignore": true,
  "ignores": ["**/THIRD-PARTY-LICENSES.md"]
}

That way the options have completion and hover documentation.

Copy link
Copy Markdown
Member Author

@jcfranco jcfranco Apr 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! I think I went with mjs early on when exploring options, in case I needed to extend from a base config. The schema came from the previous config. I'll make the change, thanks!

Copy link
Copy Markdown
Member Author

@jcfranco jcfranco Apr 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll actually need to use jsonc since markdownlint-cli2 doesn't support the full range of options with json. I'll create a follow-up issue for including this extension in other linting configs/scripts.

@benelan benelan added the skip visual snapshots Pull requests that do not need visual regression testing. label Apr 5, 2025
@jcfranco jcfranco merged commit 6d5c3ac into dev Apr 5, 2025
10 checks passed
@jcfranco jcfranco deleted the jcfranco/switch-to-markdownlint-cli2 branch April 5, 2025 05:06
@github-actions github-actions Bot added this to the 2025-04-29 - Apr Milestone milestone Apr 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Issues with changes that don't modify src or test files. skip visual snapshots Pull requests that do not need visual regression testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants