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: Move required heading structure into TOP004 rule directly #28759

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

MaoShizhong
Copy link
Contributor

@MaoShizhong MaoShizhong commented Sep 7, 2024

Because

We use different required heading structures for lesson and project files which are currently being handled by separate config files that extend the base markdownlint config. Because of this, the VSCode extension doesn't pick up any TOP004 errors. Because of this, people sometimes make unnecessary "fixes" because they ran the wrong lint script (e.g. lesson script for a project file).

By moving the heading structure definitions into the rule itself, we can make the rule determine which heading structure to use based on the file name, eliminating the need for separate configs and scripts.

This PR

  • Moves heading structure definitions into the TOP004 rule file
  • Deletes previous lesson/project extending config files
  • Removes redundant functions (carryovers from original MD043 rule) like ellipsify and handleCase
  • Provides better error message for missing heading error, otherwise no message would show for VSCode extension errors (the context field only comes up in the terminal report)
  • Adds/amends TOP004 test files to demonstrate different errors, including lesson/project structure difference
  • Replaces config-dependent linting scripts with simple scripts for linting, and fixing
  • Amends repo contributing guide and relevant custom rule doc files to account for changes
  • Removes custom config from markdownlint GH actions (default is .markdownlint-cli2.jsonc)

Issue

N/A

Additional information

Sorry for the markdownlint bomb recently, Eric!

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum contributing guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

No need to extract from separate custom config
Carryover from original rule - we always want to match case with ours
Detail field required for VSCode extension error message - context field
alone is insufficient.
Removed ellipsify method and unused params (carryover from original rule).
Separated lesson/project tests due to different required headings.
Separated different kinds of heading errors from lesson test file.
Default config is .markdownlint-cli2.jsonc. Contributing guide amended accordingly.
@MaoShizhong MaoShizhong added the Content: Markdownlint Involves anything related to the curriculum repo linter label Sep 7, 2024
@MaoShizhong MaoShizhong changed the title Markdownlint: TOP004 Markdownlint: Move required heading structure into TOP004 rule directly Sep 7, 2024
TOP005 also missing a few language exceptions
@MaoShizhong MaoShizhong removed the request for review from thatblindgeye October 3, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: Markdownlint Involves anything related to the curriculum repo linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant