Support linking to anchors in directories

[remarkjs]

Support linking to anchors in directories

[loilo]

Oh okay, I think I got it. So instead of internally resolving the directory to the readme file as I suggested, we'd do it the other way around and internally copy over the found anchors from each readme to its corresponding directory?

[loilo]

Sounds good. I'll take a look and attach a PR if I manage to wrap my head around it. 😉

[loilo]

Alright, potential case of YAGNI detected. 😁

[loilo]

By the way, here is the markdown file that I added to my branch for future tests of this feature. All links containing the term "invalid" should produce some kind of warning, all others should pass.

(The link to the project root at the end of the file might be a bit odd, but it's there to prevent regression of a currently present bug: Linking to the project root at some point leads to effectively calling path.relative(baseFolder, baseFolder), which results in the empty string. That empty string is in turn used as the path for a vfile, which causes a runtime error.)

[wooorm]

This won’t be trivial, but, may be doable, and is useful!

• Here, new files are added on the CLI:

  remark-validate-links/lib/find/find.js

  Line 79 in f613df5

  fileSet.add(vfile({cwd: file.cwd, path: path.relative(file.cwd, fp)}))

• Here, files are accessed. We could use fs.stats instead. Stats has a method .isDirectory that could be used to find out that something is a folder and that we need to find out which readme file is probably uses:

  remark-validate-links/lib/check/check-files.js

  Lines 29 to 35 in 13a54a5

  	function checkIfExists(filePath) {
  	fs.access(filePath, fs.F_OK, onaccess)
  	function onaccess(err) {
  	var noEntry = err && err.code === 'ENOENT'
  	landmarks[filePath] = {'': !noEntry}

• The file extensions to support for readmes are documented in github/markup. For the file name, /^readme$/i should be fine. On the CLI, we use:

  https://github.com/remarkjs/remark/blob/24cd52d6075966e9113524f002c0cb37133f1a37/packages/remark-cli/cli.js#L5

• Maybe you are right about older-file-first! That would be difficult. I was thinking that maybe they do alpha-sort, and prefer the first? That suggests why .asciidoc is preferred here. I would say to ignore all non-markdown extensions entirely. This is a markdown project. People using it will probably only use Markdown.

• What may be hard is that we now have URLs to things (/path/to/folder#heading) and (/path/to/folder/readme.md#heading) that are populated from different URLs: (/path/to/folder/readme.md#heading). Maybe a solution is, for every /^readme.md$/i file, to also expose URLs to its folder?

[loilo]

I was thinking that maybe they do alpha-sort, and prefer the first?

I just tested again, and you're absolutely correct. Either my memory trolled me or GitHub changed this.

I would say to ignore all non-markdown extensions entirely.

Definitely. But GitHub still allows you to have a README.md and a readme.md in the same folder. However, README.md is always preferred, so apparently alphasort it is.

What may be hard is that we now have URLs to things (/path/to/folder#heading) and (/path/to/folder/readme.md#heading) that are populated from different URLs: ( /path/to/folder/readme.md#heading). Maybe a solution is, for every /^readme.md$/i file, to also expose URLs to its folder?

Sorry, couldn't really follow you on that. Would you mind to rephrase or elaborate on this? 🙂

[wooorm]

👍

We process every file to find outgoing links and defined links.
I think that when generating defined links (/path/to/folder/readme.md#heading) it makes sense to set defined links for the folder if the file matches a supported readme filename (/path/to/folder#heading).

These places are I believe here and here

[wooorm]

Yes! So, paths to folders currently fail, we fix them, by loading a correct readme.
For every file that is processed, that has a valid readme name, expose both links from that file, but also links from its directory!

This solves the case where links are mixed: both to a readme, and to its folder. Only need to process that file once.

[loilo]

One more concern I just came up with: If we want to detect a readme file in a folder, we'd have to read all files in the directory and match them against the readme pattern. This might become a performance concern with really big folders.

Do you think it would be worth it to check for for the most common readme.md and README.md explicitely, and only then fall back to scanning the directory?

[wooorm]

Hmm, Readme.md is also likely.
I don’t know, the list becomes relatively long that I’m worried it wouldn’t add much of a performance improvement.

One potential approach is fs.Dir for streaming directory entries. It’s able to stop reading when a match is found. However, it doesn’t return results in a particular order, so that’s annoying.

🤷‍♂️ I don’t know! Maybe just go with the simplest approach and see if there are bottlenecks later? We can always fix it in the future.

[loilo]

Ok, I'm a bit stuck and need a little heads-up: Given I don't want to do a major program flow change to the plugin, I'll have to detect readme files inside the find function.

Is it fine to do it there using the fs module?

I'm asking because I don't know what the browser support story is for this module and how it relates to vfile.

[wooorm]

I would suggest using adding a resolve file, with a resolve.browser.js alternative that is a “noop” like find-repo.browser.js.

I think actual fs access can go in check-files.js, which already has a simplified version for browsers?

[loilo]

That's pretty much where I'm stuck.

My latest approach was roughly like "inject the file set into the check-files.js and add the readme vfile from there". This works in part as the added file will actually go through a find call, however that call only comes when the previous check & validation cycle is already finished (which is too late, obviously).

To be honest, I'm not sure whether it's a good idea for me to finish up this issue. I'm not too confident working with Node streams and not at all familiar with how remark works overall — I'll probably cause you more work by live-learning these things and then delivering a mediocre solution than I would if I left the implementation to you. 🙈

[wooorm]

🤗 alright, thanks for working on it! And for helping figure out a solution. I'm away for the weekend so can't code on it now. But I'll get to it soon!

[loilo]

No hurry, thanks for offering all these great packages in the first place. 😁

Looking forward to how you're going to approach this, I'll probably learn more about remark from the resulting code diff than from the last hours of reading the existing code. 😉