-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: documentation broken links in production. #4243
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
Conversation
These paths are using hardcoded absolute paths; this works in a local `hugo serve`, but will not work in `hugo server --baseURL <host>/distribution/`, which is how the actual production docs are deployed. Either you convert all of these to relative- which means hugo still can't flag broken links like these- or you use the rel tag to have hugo render it. The benefit of rel is it makes the content prefix movable, and hugo will break the build if you refer to something internal that doesn't exist. Either way, this fixes the broken documentation; storage links, about links, etc. For further commentary, see discussion thread in PR #4238. For auditing/repro, this change was generated via: ``` grep -r '(/' . -l | xargs -n1 sed -i -re 's:\(/([^)]+)\):({{< ref "/\1" >}})/:' ``` From there, a hugo build was ran to verify the changes didn't introduce broken intra-links. Signed-off-by: Brian Harring <ferringb@gmail.com>
|
@thaJeztah pardon to pull you across two PRs; I wound up poking at this and it was a few minutes rather than a shiteload of auditing. Basically all of your abspath intra site documentation links are broken; see the PR, you can verify each manually. This fixes it so they render correctly as long as --baseURL is set correctly (which you can test w/ either This change will fix the issue of content being at /distribution . I'd suggest double checking your github workflows to ensure it shoves a proper |
| | `filesystem` | Uses the local disk to store registry files. It is ideal for development and may be appropriate for some small-scale production applications. See the [driver's reference documentation]({{< ref "/storage-drivers/filesystem" >}})/. | | ||
| | `azure` | Uses Microsoft Azure Blob Storage. See the [driver's reference documentation]({{< ref "/storage-drivers/azure" >}})/. | | ||
| | `gcs` | Uses Google Cloud Storage. See the [driver's reference documentation]({{< ref "/storage-drivers/gcs" >}})/. | | ||
| | `s3` | Uses Amazon Simple Storage Service (S3) and compatible Storage Services. See the [driver's reference documentation]({{< ref "/storage-drivers/s3" >}})/. | | ||
|
|
||
| For testing only, you can use the [`inmemory` storage | ||
| driver](/storage-drivers/inmemory). | ||
| driver]({{< ref "/storage-drivers/inmemory" >}})/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm.. right, so the downside of this is that the MarkDown is now fully depending on Hugo to be rendered correctly, which quite limits how it can be read on GitHub, e.g. links in this block are no longer "valid" GFM, and no longer rendered as links;
Which is why I was considering if there's other ways to make it work (as I commented in #4238 (comment))
I'm not sure if that's possible though. I do recall we faced similar challenges long time ago when the original Docker documentation also used Hugo, and where we encountered some corner-cases (ISTR those were mostly related to _index.md files, which needed special handling that was missing in Hugo); a Docker maintainer at that time contributed some changes to Hugo to make that work (but I know some of that broke later, and was removed, so not sure what's still there);
Now, admitted, reading these docs (AND navigating them) on GitHub may be a "nice to have" if we consider the rendered docs to be the canonical place to read, but it is somewhat in line of expectation for these to be GitHub flavoured markdown; contributors and maintainers will be way more familiar with "regular" links, in addition to IDEs / editors that will assist (and validate) links with the assumption.
So if possible it would be great if that would work and if we could use regular / relative links to the .md files that work both in Hugo and in other settings.
(We should definitely consider having a link-checker in CI though, as these links currently being "silently" broken isn't great.)
@dvdksn @milosgajdos any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional note- I'd suggested looking into https://gohugo.io/content-management/urls/#relative-urls as something of a flagrant hack to try and avoid the manual relpathing or ref tag usage. That won't fix the issue here, although you may want to consider it if you're doing something like shipping a copy of the docs with the distribution package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could add a render hook for links that ensures we can write links that are compatible with both Hugo and GitHub. Something like:
layouts/_default/_markup/render-link.html
{{- if (strings.HasPrefix .Destination "http") -}}
<a href="{{ safe.URL .Destination }}" target="_blank">{{ safe.HTML .Text }}</a>
{{- else if (strings.HasSuffix .Destination ".md") -}}
<a href="{{ ref .Page .Destination | safe.URL }}">{{ safe.HTML .Text }}</a>
{{- else -}}
<a href="{{ safe.URL .Destination }}">{{ safe.HTML .Text }}</a>
{{- end -}}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My ideal solution would be:
- make sure GH links remain navigable using GH UI
- when deploying the links we should somehow "override" them during the build so they work in Hugo
Now, I'm not a hug expert unfortunately so I don't know how easy it'd be to do, but what @dvdksn describes in his last comment seeeems interesting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed an alternative solution in #4247
These paths are using hardcoded absolute paths; this works in a local
hugo serve, but will not work inhugo server --baseURL <host>/distribution/, which is how the actual production docs are deployed.Either you convert all of these to relative- which means hugo still can't flag broken links like these- or you use the rel tag to have hugo render it.
The benefit of rel is it makes the content prefix movable, and hugo will break the build if you refer to something internal that doesn't exist.
Either way, this fixes the broken documentation; storage links, about links, etc.
For further commentary, see discussion thread in PR #4238.
For auditing/repro, this change was generated via:
From there, a hugo build was ran to verify the changes didn't introduce broken intra-links.