feat: lint links and definitions

[aduh95]

Replacement for checkLinks.mjs from the nodejs/node repo. Adds a validation for self-reference links to not contain the name of the file.

[Trott]

LGTM once the necessary changes land in core

[nschonni]

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

[aduh95]

This rule seems like it's less nodejs specific that 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).

[aduh95]

Tests are now passing, I think this is now ready to land.
I'm not sure what is the semver-ness of this change, as it would be quite breaking for a repo that doesn't currently order md references.

[Trott]

Tests are now passing, I think this is now ready to land.
I'm not sure what is the semver-ness of this change, as it would be quite breaking for a repo that doesn't currently order md references.

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.

[SEWeiTung]

@Trott & @aduh95:
Sorry to interrupt：but it seems for mul-language translations in md such as nodejs/nodejs.org#3956, there're so many errors when testing... What does this change mainly do and what to do with so many errors for node.org?

[Trott]

@Trott & @aduh95:
Sorry to interrupt：but it seems for mul-language translations in md such as nodejs/nodejs.org#3956, there're so many errors when testing... What does this change mainly do and what to do with so many errors for node.org?

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 .md file are in ASCII order. This doesn't affect rendering and seems sensible for when the link lists get very long. However, obviously the "right" ordering in English is going to be different from the "right" ordering once all the link text is translated to another language. And that adds an unexpected burden to translators. And it's not even clear that they should be re-ordering the links. Maybe the "right" ordering is to keep it the same as in the document from which they are translating so that the link is in the expected place should it ever need to be updated based on an update from the original doc.

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.

[aduh95]

I suppose the solution could be to add a disable comment to the translated files:

<!--lint disable remark-lint:nodejs-links-->

[SEWeiTung]

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 :(

Notice, this is a test file only, we don't need to check its syntax. But when running "npm run test:lint:md", it still shows me a lot of errors in node.org forked project :(

So it is for all the other translated files:

And this is my command in node.org:

[SEWeiTung]

BTW: Is it possible to disable in the ".remarkrc"? I suspect whether it's not a published package, so this cannot be used here...?

[nschonni]

@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 [Translated text][english reference]

[nschonni]

It seems like there is a small case sensitivity issue with the current imlementation though. EX: from nodejs.org

locale\en\blog\wg\diag-wg-update-2017-02.md
       70:1-70:65  warning  Unordered reference ("CLI debugger" should be before "async_hooks")                                     nodejs-links  remark-lint
       73:1-73:63  warning  Unordered reference ("Diagnostics WG" should be before "diag-agenda")                                   nodejs-links  remark-lint
       75:1-75:89  warning  Unordered reference ("Inspector" should be before "Inspector API")                                      nodejs-links  remark-lint
       79:1-79:70  warning  Broken link                                                                                             nodejs-links  remark-lint
       79:1-79:70  warning  Unordered reference ("Node.js Foundation survey" should be before "node-report")                        nodejs-links  remark-lint
       81:1-81:66  warning  Unordered reference ("Open an issue" should be before "nodejs/node")                                    nodejs-links  remark-lint
       83:1-83:61  warning  Unordered reference ("Trace Controller" should be before "stability")                                   nodejs-links  remark-lint

Why should the capitalized "CLI" be before "async"

[Trott]

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.

[Trott]

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....

The following syntax is working for me:

<!--lint disable nodejs-links remark-lint-->