search

elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.53k stars 297 forks source link

The `website/tools/check-rellinks.py` program has a surprising quirk regarding links that end in a slash #1749

Closed krader1961 closed 5 months ago

krader1961 commented 5 months ago

I was cleaning up my change to add a learn/useful-customizations.html page before opening a pull-request and was pulling my hair out because the make check-rellinks build target was failing thusly despite the link being valid and working in the resulting web page when traversed via my browser:

Found broken links:
learn/useful-customizations.html
    ../ref/edit.html/#edit:histlist:start

After more than an hour of debugging I finally realized it was because of the trailing slash. I have no objection to requiring such links not include a trailing slash but it would be nice if the tool checking those links provided a more useful explanation for why the link is "broken" since, in this instance, it is valid. Alternatively, augment the website/tools/check-rellinks.py program to allow a trailing slash in such URLs.

krader1961 commented 5 months ago

The fix I'm going to submit as a pull-request will result in this output for my mistaken reference that should not have included a trailing slash:

WARNING: A trailing slash implies you are referring to ../ref/edit.html/index.html
WARNING: This is not what you want and the trailing slash should be removed
Found broken links:
learn/useful-customizations.html
    ../ref/edit.html/#edit:histlist:start
xiaq commented 5 months ago

This isn't really a quirk in the check-rellinks script. Trailing slashes may still lead the browser to the correct file, but they are significant for resolving relative links. Your browser can load file:///.../ref/edit.html/, but all the relative links on it will be wrong.

This is true for duplicate internal slashes too. For example, if you go to file:///.../ref//edit.html, a relative link like ../learn/ resolves to file:///.../ref/learn/ instead of the correct file:///.../learn/.