feat: lint links and definitions #188
Conversation
Replacement for checkLinks.mjs from the nodejs/node repo.
|
This rule seems like it's less nodejs specific than some of the others, so it might be worth having it as a standalone that is just used here |
If someone wants to publish it standalone on npm, nothing's stoping them, but I don't feel like doing it myself (at least, not for the time being). |
PR-URL: nodejs#39170 Refs: nodejs/remark-preset-lint-node#188 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
Refs: nodejs/remark-preset-lint-node#188 PR-URL: #39165 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Harshitha K P <[email protected]>
|
Tests are now passing, I think this is now ready to land. |
I believe we usually treat new rules as semver-minor and save semver-major for things like dropping support of an older version of Node.js. |
PR-URL: #39170 Refs: nodejs/remark-preset-lint-node#188 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
Refs: nodejs/remark-preset-lint-node#188 PR-URL: #39165 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Harshitha K P <[email protected]>
|
@Trott & @aduh95: |
I wonder if there's a way to disable this check for localized/translated files. It seems like it would create extra surprising work unnecessarily for translators. Or at least maybe as a partial measure, ignore links that start with (or contain) non-ASCII characters? That would at least address the problem for languages where the ordering will surely change (like Arabic). What do you think, @aduh95? @MaledongGit It does a few things, but the one that is probably causing all the issues for translations is that it checks to make sure that the links listed at the bottom of the If we can't figure out something to make it work for translations, I'd consider removing the ordering part of the rule entirely. I also wonder if we should add nodejs.org to our CI test for this repo so we catch these things earlier. Translations are likely to run into all sorts of issues that we probably won't see in the main repo. |
|
I suppose the solution could be to add a disable comment to the translated files: <!--lint disable remark-lint:nodejs-links--> |
Well, I copied some of your files into my node_module to have a test with.... However it doens't work properly :( And this is my command in node.org: |
|
BTW: Is it possible to disable in the ".remarkrc"? I suspect whether it's not a published package, so this cannot be used here...? |
|
@MaledongGit we already do https://github.com/nodejs/nodejs.org/blob/master/.remarkrc I think one thing might be to leave the English "references" at the bottom, as this is accepted |
|
It seems like there is a small case sensitivity issue with the current imlementation though. EX: from nodejs.org Why should the capitalized "CLI" be before "async" |
This was for localization, ironically. The idea was that instead of alphabetical order, it should be in code-point order, and capital letters all come before lowercase letters that way. The thinking was that "alphabetical order" might not make any sense and/or be hard to code for in some languages (like Chinese), whereas ordering by code-point value is straightforward. Of course, what we're discovering now is that maybe the thing to do wasn't to try to accommodate these languages but rather to exempt them from the check altogether. |
The following syntax is working for me: |
According to #3956, it seems we're ordering the links' titles with ASCII orders, however NOT all the docs (such as translations) and test\scripts\*.md, we DON'T need to cope with this rule, so we should disable it manually as a special case. Ref: 1. nodejs/remark-preset-lint-node#188. 2. nodejs/node#39170.
Refs: nodejs/remark-preset-lint-node#188 PR-URL: #39165 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Harshitha K P <[email protected]>
PR-URL: #39170 Refs: nodejs/remark-preset-lint-node#188 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
Replacement for checkLinks.mjs from the nodejs/node repo. Adds a validation for self-reference links to not contain the name of the file.