Fortran-FOSS-Programmers / ford

Automatically generates FORtran Documentation from comments within the code.
https://forddocs.readthedocs.io
GNU General Public License v3.0
405 stars 131 forks source link

Try to ensure all internal links are relative #593

Closed ZedThree closed 8 months ago

ZedThree commented 10 months ago

This seems to avoid more problems than optionally having links be absolute.

Fixes #581

amorison commented 9 months ago

Hi, thanks for this. Unfortunately, this doesn't seem to be enough. It seems that links are now properly resolved (as in, the target has the correct path), but most of them are still absolute for me. In particular, the ones in the root index.html to the various pages of the project and links to types, procedures, etc, making it impossible to serve the generated documentation with a web server. The static pages tree is also fully flattened. Do you also have that issue? Otherwise I'll come up with a way to reproduce it.

ZedThree commented 9 months ago

Ah yes, ok, I think that should be an easy fix. I didn't manage to get a decent test for checking for absolute paths.

amorison commented 9 months ago

I'm thinking, wouldn't it be much simpler to forgo absolute paths altogether, and use absolute URLs for internal link (i.e. /page/something.html instead of ../../../page/something.html)? This would mean the documentation has to be seen through a web server, but I don't think it's much of an issue. Running python3 -m http.server isn't too much to ask to the target audience of ford, and the advantages far outweigh this inconvenience. With absolute internal links, no absolute path will ever creep in the documentation and link resolution will be much simpler. There is also no risk that the local documentation looks alright when opening it locally, but is actually filled with absolute paths and therefore cannot be served. I think most static website generators work like this rather than bother with absolute paths and then making those relative (which is quite brittle and much harder to get right than absolute URLs).

ZedThree commented 9 months ago

Yes, I had considered this previously, and decided against it as Sphinx seems to prefer to always use relative links and so doesn't require using a web server. It would definitely simplify things, at the cost of an extra step to view pages locally.

ZedThree commented 8 months ago

Finally got round to finishing this off. Instead of trying to fix each link individually, we now just replace absolute paths in links when rendering each page. This should (hopefully!) now capture everything in one go basically.

amorison commented 8 months ago

Thanks a lot for this. This is indeed a much more robust strategy. It seems to be working, with one exception: links inside images are still absolute on the home page. I have ![cover image](|media|/image.png) in my root markdown file that generates <img alt="cover image" src="/absolute/path/to/media/image.png"/>. Other links on the home page are made relative, and links to images on other pages are also made relative, so this might be a weird corner case.

As an additional note, there seems to be a significant performance hit with this strategy. Without search and graph features, it took about 10 s to generate the documentation of our project before this change (similar times on ford 6), and it takes now about 1 minute (5 s in parsing, 0 s in processing, 49 s to write files). This is the documentation without the graph and search features (that add another 3 minutes or so to the build time). I would much rather have a slow code that produces the expected result than a fast code that produces something we can't use, so I don't think this is a blocking problem, but I thought this is worth mentioning here anyway.

EDIT: the search bar also sends me to /absolute/path/to/search.html

ZedThree commented 8 months ago

links inside images are still absolute on the home page

I think that will be fixed by #608 as the alias expansion will be done earlier, and then caught by the markdown relative-link processor.

it takes now about 1 minute (5 s in parsing, 0 s in processing, 49 s to write files).

Hmm, I was a bit worried about that, and that's much worse than I was expecting from my limited tests. I might try a different tack and see if that fares better. At least we have this implementation to fall back to.

amorison commented 8 months ago

Mmm, I rebased the aliases-in-tables branch on top of fix-absolute-paths in my local clone, but this didn't solve my issue with the image on the home page.

Also in case you haven't seen the edit in my previous comment, the search bar (from anywhere) sends me to /absolute/path/to/search.html instead of a relative path to search.html.

ZedThree commented 8 months ago

Replaced by #609