search

Michael-F-Bryan / mdbook-linkcheck

A backend for `mdbook` which will check your links for you.
https://michael-f-bryan.github.io/mdbook-linkcheck/
MIT License
145 stars 29 forks source link

Check files that aren't in SUMMARY but are linked in other pages OR warn about files not in SUMMARY #32

Closed ericonr closed 4 years ago

ericonr commented 4 years ago

Hi there!

Per https://github.com/rust-lang/mdBook/issues/830 and https://github.com/rust-lang/mdBook/issues/1232, mdBook doesn't include any file that isn't linked in the summary, even though they are linked in other files. This could lead to unexpected issues, so I believe it'd be nice to get warnings about that. However, this seems to be a bit of "undefined behavior", so to speak, so it's not clear what the correct behaviour would be.

Therefore, I propose another solution: add a configuration option for warning the user that they haven't included a file in SUMMARY. By default it should be off, but can be turned on. That way, it's a useful warning both now that the mdBook issues are being discussed, and later for verifying the SUMMARY file.

Is that possible in the current code structure?

Michael-F-Bryan commented 4 years ago

Is that possible in the current code structure?

I believe so.

I was working on switching over to the linkcheck crate for doing linkchecking, and this sort of "ignore links not in SUMMARY.md" logic should slot right into the validator's Context::should_ignore() hook.

Alternatively, we could update the Options used when checking filesystem items to have a is_valid: impl Fn(&Path) -> bool property that defaults to calling path.exists() (or just letting canonicalize() detect when a file doesn't exist).

That way you leave it with the default if you just want to check that the file exists, but if you want to add extra logic (i.e. path.exists() && summary.contains(path)), you can do that too.

mdBook doesn't include any file that isn't linked in the summary, even though they are linked in other files

I think this is the correct behaviour, and how mdbook-linkcheck would implement it.

The SUMMARY.md file is kinda like a contents page that declares which files are in the document. So a link that points to a file not in the document is, almost by definition, broken.

Michael-F-Bryan commented 4 years ago

Amusingly enough, my integration tests started failing immediately after implementing this because the test fixture containing all "valid" links forgot to include a file in SUMMARY.md.

ericonr commented 4 years ago

Sorry for missing your responses. Thanks for the quick implementation, that's super cool! Will surely help us to have it all in the same package! And glad it helped with making test cases better too :p

ericonr commented 4 years ago

And damn, thanks for putting it in a release! Certainly helps with packaging :D