feat(mdlint): add rule TOP009 to ensure lesson overview items start with a capital letter and end with a period
Adds a new rule to markdown lint.
Lesson overview items should end with a period and start with a capital letter
Issue
Closes https://github.com/TheOdinProject/curriculum/issues/27625#issuecomment-2112275087
Additional Information
Pull Request Requirements
- [x] I have thoroughly read and understand The Odin Project curriculum contributing guide
- [x] The title of this PR follows the
location of change: brief description of changeformat, e.g.Intro to HTML and CSS lesson: Fix link text - [x] The
Becausesection summarizes the reason for this PR - [x] The
This PRsection has a bullet point list describing the changes in this PR - [x] If this PR addresses an open issue, it is linked in the
Issuesection - [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
- [x] If any lesson files are included in this PR, they follow the Layout Style Guide
The checks aren't running due to a merge conflict. You'll need to rename your rule and relevant files/dirs to use
TOP009(including the markdownlint config) because #27915 added what's nowTOP008. That PR also adjusted the markdownlint config, so that should be resolved once you resolve the merge conflicts.In terms of testing your rule, you'll need to make sure your test files include all of the different scenarios to test, and you can run the lint script on it locally. Once you're happy with that, then a new push to here (after resolving conflicts) will trigger the actions with your local linting files.
Thanks for letting me know! I've fixed the merge conflicts. Now I just need to fix the code so that it works
A heads up that the merge has wiped out your original doc file, so you'll want to restore it but under the TOP009 name of course
A heads up that the merge has wiped out your original doc file, so you'll want to restore it but under the TOP009 name of course
Yay! I got it working
A heads up that the merge has wiped out your original doc file, so you'll want to restore it but under the TOP009 name of course
Thankfully we have git for that :D
Question - my understanding from the original issue was that this rule was meant to prevent LO items being questions, as they instead should be statements of what the lesson will cover. But this rule tries to enforce the opposite - is this intended? I see that the doc file implies the rule is supposed to prevent questions (LO items must begin capitalised and end with a .)
Question - my understanding from the original issue was that this rule was meant to prevent LO items being questions, as they instead should be statements of what the lesson will cover. But this rule tries to enforce the opposite - is this intended?
Oh, this was just a mistake on my part. I just fixed it!
Actually, seems like there are still some mistakes. Sorry let me test it a bit more.
Question - my understanding from the original issue was that this rule was meant to prevent LO items being questions, as they instead should be statements of what the lesson will cover. But this rule tries to enforce the opposite - is this intended? I see that the doc file implies the rule is supposed to prevent questions (LO items must begin capitalised and end with a
.)
The tests are passing as expected now. I just accidentally searched for the 2nd character instead of the 1st character. .at(1). I have been writing code in another language where the array indexes start with 1 so I didn't think twice about this
For the changes that would be better made on my computer than via the github online editor, I'll add them in a few weeks time since I have exams at the moment.
For the changes that would be better made on my computer than via the github online editor, I'll add them in a few weeks time since I have exams at the moment.
Not a problem at all - there's no rush for this rule for sure, and even if there somehow was, your exams are more important so please do focus on those first and foremost! Whatever changes are needed for this PR, you can certainly sort them out when you're done and have the time for this, since this is all voluntary. We have a TOP010 rule in review as well, but we can always merge that first if appropriate then just resolve the merge conflict in the custom rules array when you get back to this later.
Not sure what's going on here, I've removed the comment but that didn't seem to work
used to work just fine before
@MaoShizhong Thank you! The PR is not ready for review yet as the tests are not passing. I will let you know when they are.
Had a think about this further.
I think we should add a non-script-fixable error if an LO item ends with a
?that reports something like "Lesson overview items must be statements, not questions."If I wrote
- What are linters?then the current code would only complain about the?and simply fixing (script or manual) to- What are linters.will not make sense, yet the linter will be happy.I think questions should be flagged as such and not made script-fixable, as that requires changing the sentence wording itself e.g. to
- Describe what linters are.Then in that situation, we can flag and script-fix capitalisation/end punctuation errors. Those errors must only flag if the question error doesn't.
I agree with you. I think introducing fixers only makes sense when they are beneficial in every case.
I don't think it makes sense for there to be a punctuation auto fixer in the first place. For example, while Describe what linters are -> Describe what linters are. makes sense, What are linters -> What are linters. would not make as much sense because the sentence itself is phrased as a question, only proper capitalisation fixer makes sense to me describe what linters are -> Describe what linters are, what do you think
I think I'm with you on that, where the nuances and complexity of a fixer catching all cases isn't really that beneficial given the example you provided.
I would probably think then we just need the one general error reporting something like
"Lesson overview items must be statements, not questions, and must begin with a capital letter and end with a period."
No need for actual/expected, just provide the LO item itself in the context property.
That should flag for questions (ends in ?), as well as uncapitalised/un-period-ended LO items. Then we leave it to the user to manually fix, since it's context-dependent.
I think I'm with you on that, where the nuances and complexity of a fixer catching all cases isn't really that beneficial given the example you provided.
I would probably think then we just need the one general error reporting something like "Lesson overview items must be statements, not questions, and must begin with a capital letter and end with a period." No need for actual/expected, just provide the LO item itself in the
contextproperty.That should flag for questions (ends in ?), as well as uncapitalised/un-period-ended LO items. Then we leave it to the user to manually fix, since it's context-dependent.
I made the lint rules behave like your suggestions:
- Single warning message
Lesson overview items must be statements, not questions, and must begin with a capital letter and end with a period.