curriculum icon indicating copy to clipboard operation
curriculum copied to clipboard

Feature: Automatic markdown tool

Open nik-rev opened this issue 1 year ago • 3 comments

Introduction

I'm excited to share that I've been working on automatic tool that will fix be able to solve ~95% of markdown-lint issues in every file in the curriculum with a single command.

To test-drive this tool, simply fork this PR, git checkout feature/markdown-checker your local machine and type the following command (while in the curriculum repo):

node ./markdowntool/markdowntool.js .

The above command will fix almost every single lint issues in the entire course at once. You can also specify any single .md lesson, or an any other directory containing .md files.

Features

  • api.js makes it easy to make changes to default section content
  • Automatically inserts blank lines above and below code blocks, unordered lists, ordered lists and headings
  • Changes all code blocks to be written with ``` instead of ~~~
  • Changes ordered lists to be 1, 1, 1 instead of 1, 2, 3
  • Changes unordered lists to use - instead of * for each point
  • Removes all spaces after list markers and leaves only 1, eg from 1. some text to 1. some text
  • Unindents lists that have been wrongfully indented
  • Changes inline html like <span class='additional-resources-link' href='#some-section'>additional resource</span> to use markdown syntax [additional resource](#some-section)
  • Correct heading text: make headings properly capitalised, eg from Additional Resources to Additional resources and change Learning Outcomes to Lesson overview
  • Automatically adjusts the section content to be correct, for example in Additional resources to have the This section contains... text and corrects the text if it already exists
  • Formats the end and start of files to follow the right format, eg by adding a blank line at the end if it doesn't exist
  • Automatically replace all __ and _ with asterisks ** and * for bold and italic text.
  • Automatically removes repeating blank lines.
  • Automatically removes trailing spaces from every line
  • Changes bold headings to h5, eg from **#Heading** to ##### Heading

Example

I've also made a PR that gives an example of the tool being used

Notes

  • Some linter issues that are impossible to automatically fix are ignored
    • Descriptive link text
    • Dollar signs to show output

This PR

  • Adds a new CLI tool that will automatically make a .md file or directory containing .md files align with the style guide

Pull Request Requirements

  • [x] I have thoroughly read and understand The Odin Project Contributing Guide
  • [x] If any lesson files are included in this PR, they follow the Layout Style Guide
  • [x] 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
  • [x] The Because section summarizes the reason for this PR
  • [x] The This PR section has a bullet point list describing the changes in this PR
  • [x] If this PR addresses an open issue, it is linked in the Issue section
  • [x] If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly

nik-rev avatar Feb 24 '24 14:02 nik-rev

First of all I just want to say impressive work on this!

Something that we weren't sure we wanted to add to the repo when we first added the markdown linter is actually the markdownlint-cli2 dependency that can be ran with a --fix flag similar to what you've done here (custom rules can also be configured with a fixInfo object according to the docs, though we haven't implemented any of that for our custom rules). If we were to implement a way to auto-fix a bunch of these issues at once locally, it may be more advantageous to use the tools from the linter itself.

That said, though, do you feel there's something more to benefit by implementing our own fixers like you have here vs installing the markdownlint-cli2 as a dependency?

thatblindgeye avatar Feb 28 '24 13:02 thatblindgeye

First of all I just want to say impressive work on this!

Something that we weren't sure we wanted to add to the repo when we first added the markdown linter is actually the markdownlint-cli2 dependency that can be ran with a --fix flag similar to what you've done here (custom rules can also be configured with a fixInfo object according to the docs, though we haven't implemented any of that for our custom rules). If we were to implement a way to auto-fix a bunch of these issues at once locally, it may be more advantageous to use the tools from the linter itself.

That said, though, do you feel there's something more to benefit by implementing our own fixers like you have here vs installing the markdownlint-cli2 as a dependency?

Yeah, in particular some TOP related issues such as sections having default content would be really useful. For example if the layout guide were to change in the future it would be way easier to update the content.

Some other fixes that would benefit I think would be the inline HTML for example. I'm not actually sure if markdown-cli automatically fixes that issue

But, I don't think all of these changes are necessary to implement as our own solution, if markdown-cli can automatically fix some issues. I wanted to give myself a challenge so I decided to implement all fixes, despite knowing that some of them can be automatically fixed with available tools.

And given the fact that the CLI tool can be setup with custom rules, if that is the case, then introducing our own fix may not be necessary. If so, then it's fine. If it's better to use outside tools.

I wanted some practice with node so this just felt like the perfect task, so I made this PR without first making an issue to ask if this is necessary. Just saying that incase this PR gets closed I don't want anyone to feel like I've done this unnecessary work! I really don't mind.

nik-rev avatar Feb 28 '24 13:02 nik-rev

Yeah one downside to the markdownlint tool is that some rules don't offer fixes. We could just do a copy+paste the code for the rule from markdownlint as a "custom" rule that would allow us to provide a fixer. Though I think some rules make sense to not have a fixer. Inline HTML, for example, may not always be clear cut whether an HTML tag should be removed, replaced, or added to the allowable array. In that case we may not really want to automatically fix it, but bring attention to it to decide based on the context.

I'll leave this open for now in case any other maintainer wants to voice thoughts, but also want to discuss with the team as to whether we ultimately do want to implement a means to automatically fix issues and if we want to go forward with installing markdownlint-cli2 as a dependency.

Like I said, this is some impressive work and I'd urge you to keep this branch on your fork (not that I think you would just go and delete this branch or anything, but just to reiterate I guess 😆).

thatblindgeye avatar Feb 29 '24 13:02 thatblindgeye

@thatblindgeye Are this and #27449 close-able given markdownlint-cli2 is officially a repo dependency with fixers?

Regarding additional custom rules for fixers, the current built-in and custom fixes should cover almost everything here that would make sense to have fixers for. The following rules might benefit from having fixers:

  • Fixing ol prefix to use lazy numbering - surprisingly MD029 (source code for rule) does not have a fixer. I guess that kind of makes sense given the multiple options available. But since we're sticking with lazy 1s, we could probably override MD029 with a custom rule enforcing 1s with a fixer.
  • Default section content - specifically the KC/AR sections' opening paragraph only, to replace the text with the desired text. The fixinfo can ignore the unordered list part of it.

mao-sz avatar Apr 21 '24 17:04 mao-sz

Yeah these can be closed. If we'd want we could open an issue for a custom lazy numbering rule.

I started tinkering locally with a fixer for the default content so maybe I'll get back to that to get something up.

thatblindgeye avatar Apr 22 '24 11:04 thatblindgeye

A custom lazy numbering with 1s rule with a fixer should be pretty simple to implement, so I'll open an issue and see if anyone fancies playing around with markdownlint :+1:

mao-sz avatar Apr 22 '24 11:04 mao-sz