Skip to content

Fix naive handling of relative links when proofing #75

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

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

SeanTAllen
Copy link
Contributor

@SeanTAllen SeanTAllen commented Feb 26, 2024

This commit fixes naive handling of relative links when proofing. The problem here was the assumption that a relative link of foo.html that was coming from a file at bar/fubar.md in the filesystem would always appear at bar/foo.html.

That is not always true. If directory_urls was on, then it wasn't true. Thus, #46.

It was also not true when using relative links between blog posts when using the mkdocs material blog plugin.

This commit changes the relative url lookup logic to be more robust and in the process make the proofer work with both the mkdocs material blog plugin and with directory_urls on.

It does this by storing additional information in the lookup files that we use to find information about pages. We can now look up a file by its src_uri entry, for example blog/posts/index.md and then use that to get the dest_uri like blog/12/10/some-post/ and then use that dest_uri value to robustly handle figuring out where a relative link is pointing.

Closes #46
Closes #70

@SeanTAllen SeanTAllen force-pushed the intra-blog-links branch 8 times, most recently from 6b1e373 to bb16cc7 Compare February 26, 2024 21:45
@SeanTAllen SeanTAllen marked this pull request as draft February 26, 2024 23:47
@SeanTAllen SeanTAllen marked this pull request as ready for review February 26, 2024 23:52
@SeanTAllen
Copy link
Contributor Author

There appears to be a bit more than needs to be done to this. It works on our site, but not the integration tests. Will look into.

This commit fixes naive handling of relative links when proofing. The
problem here was the assumption that a relative link of foo.html that
was coming from a file at bar/fubar.md in the filesystem would always
appear at bar/foo.html.

That is not always true. If directory_urls was on, then it wasn't true.
Thus, manuzhang#46.

It was also not true when using relative links between blog posts when
using the mkdocs material blog plugin.

This commit changes the relative url lookup logic to be more robust and
in the process make the proofer work with both the mkdocs material blog
plugin and with directory_urls on.

It does this by storing additional information in the lookup files that
we use to find information about pages. We can now look up a file by its
src_uri entry, for example blog/posts/index.md and then use that to get
the dest_uri like blog/12/10/some-post/ and then use that dest_uri value
to robustly handle figuring out where a relative link is pointing.

Closes manuzhang#46
Closes manuzhang#70
@@ -27,6 +27,6 @@ But allows valid anchors such as

## Image Link absolute/relative

<a href="../assets/hello-world.drawio.svg">![test](../assets/hello-world.drawio.svg)</a>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needed to be changed as it isn't a valid test with directory urls on. mkdocs won't change the link in the raw href to be correct. only those in markdown.

@@ -8,7 +8,7 @@ you can either link to a local file without or with an anchor.
* [Main Page](../index.md)
* [Sub-Page](./page2.md)
* <figure markdown>
<a href="../assets/hello-world.drawio.svg">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needed to be changed as it isn't a valid test with directory urls on. mkdocs won't change the link in the raw href to be correct. only those in markdown.

@manuzhang manuzhang merged commit 5776fea into manuzhang:main Feb 27, 2024
@manuzhang
Copy link
Owner

LGTM. Merged. Nice work!

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.

Incorrect broken links reported disabling the implicit use_directory_urls renders plugin useless for most
2 participants