Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to ensure all internal links are relative #593

Closed
wants to merge 9 commits into from

Conversation

ZedThree
Copy link
Member

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

Fixes #581

Fixes #581

Relative paths _should_ always work, whether viewing docs as a file or
via a web server, either locally or online
* master:
  Handle entities that might not have be correlated in resolving links
@amorison
Copy link

amorison commented Nov 16, 2023

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
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

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
Copy link
Member Author

ZedThree commented Jan 2, 2024

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
Copy link

amorison commented Jan 2, 2024

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
Copy link
Member Author

ZedThree commented Jan 3, 2024

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
Copy link

amorison commented Jan 3, 2024

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
Copy link
Member Author

ZedThree commented Jan 4, 2024

Replaced by #609

@ZedThree ZedThree closed this Jan 4, 2024
@ZedThree ZedThree deleted the fix-absolute-paths branch January 4, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

|page|, |media|, and |url| are absolute instead of relative
2 participants