Fix relative URLs when serving the specification with a custom baseURL
#1984
Conversation
|
Leaving as a draft for now, it turns out that it creates broken links in places (see CI). All the broken links are declared as only a fragment. For some of them, it's good that they are broken, because the need at least the spec path, otherwise it means that they are also broken in the generated OpenAPI definitions. But other fragments should just work, like the ones inside info boxes. |
|
Alright the issues have been fixed, but I am leaving this as a draft for now until the docsy PR is merged. So the good news is that now some links that would have been broken in the OpenAPI definitions are also broken in the rendered spec, and we detect them in CI thanks to htmltest. |
Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
Helps Hugo to generate the correct links for files in /static. Signed-off-by: Kévin Commaille <[email protected]>
It is deprecated and messes with relative links when used with some baseURLs. Signed-off-by: Kévin Commaille <[email protected]>
Now that relative links are fixed, having it enabled is not a problem anymore. Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
Otherwise the `relref` in the changelogs does not work. Version 0.124.1 is the last version of Hugo to work with the current version of the docsy theme. Signed-off-by: Kévin Commaille <[email protected]>
This causes issues for links as the method doesn't have the right context to resolve them. The content is already markdownified because of the `%` syntax of the shortcode that calls it, and it has the right context. Signed-off-by: Kévin Commaille <[email protected]>
Links that contain only a fragment are not resolved by Hugo so the final relative URL is wrong. They also cause problems for the OpenAPI definitions because these links are not edited in the dump-openapi script so they end up broken. Signed-off-by: Kévin Commaille <[email protected]>
zecakeh
force-pushed
the
no-canonifyurls
branch
from
7dbb166
to
a493acb
Compare
There was a problem hiding this comment.
This LGTM otherwise, thank you for your work!
Fixes the causes behind #1964, but the artifacts that were already generated for v1.11 and v1.12 still need to be fixed somehow.
Would manual find/replace surgery be tractable, or do we need to figure out how to regenerate? We can also discuss this further on a new issue to avoid bumping a merged PR.
| @@ -2,4 +2,4 @@ module github.com/matrix-org/matrix-spec | |||
|
|
|||
| go 1.12 | |||
|
|
|||
| require github.com/matrix-org/docsy v0.0.0-20231227121313-d55dbaf3aaaf // indirect | |||
| require github.com/matrix-org/docsy v0.0.0-20241030165433-a14705b7e3ca // indirect | |||
There was a problem hiding this comment.
I suppose this should be updated to matrix-org/docsy@ec7b98e now?
There was a problem hiding this comment.
I was planning to upgrade docsy in a separate PR because it brings visual changes that might need to be discussed.
anoadragon453
dismissed
their stale review
meant to hit the "comment" button, but approval is close!
There must be something like 30 invalid links in those artifacts so it is doable manually. Running htmltest detects all the issues. But it should also be simple to regenerate, we just need to use Hugo 0.117.0, like the CI that generates the "non-historical" spec-artifact. We can discuss this in the original issue, I already left a few comments there: #1964. |
Allows to only serve the file that we actually use. Signed-off-by: Kévin Commaille <[email protected]>
Requires matrix-org/docsy#4.Using
canonifyURLsand abaseURLthat is not/at the same time makes.RelPermalinkandrelrefreturn relative URLs where the first path segment is duplicated, e.g./v1.11/v1.11/rest/of/the/path.canonifyURLsis deprecated so let's get rid of it and allow Hugo to compute the relative URLs itself, because it keeps track of the resources and knows where they will be served.This can be reviewed commit by commit.
Since the PR CI uses
/as abaseURL, I guess the only way to test that this works before it is deployed is to generate the spec locally with a custombaseURLlike/v1.11and make sure that there are no URLs where it is duplicated.Closes #1957.
Fixes the causes behind #1964, but the artifacts that were already generated for v1.11 and v1.12 still need to be fixed somehow.
Pull Request Checklist
Preview: https://pr1984--matrix-spec-previews.netlify.app