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

[docs] Fix broken image links in docs #7710

Merged
merged 2 commits into from
Oct 18, 2024
Merged

[docs] Fix broken image links in docs #7710

merged 2 commits into from
Oct 18, 2024

Conversation

Ivecia
Copy link
Contributor

@Ivecia Ivecia commented Oct 16, 2024

Image links are broken in github preview.

@mikeurbach
Copy link
Contributor

The issue here is we use the markdown to generate the website circt.llvm.org, so the "broken" links are really correct, for the website generator. But they don't render in GitHub previews of the markdown. There are multiple issues / PRs that have been opened about this before, and no one has found a way to write down the links in a way that works reliably in the website generator and previews correctly on GitHub.

This version should work for both, but it does bake in assumptions about the domain where the website is hosted. If this works on the generated website, I think it would be a reasonable tradeoff, so people browsing the docs on GitHub actually see the images.

@Ivecia
Copy link
Contributor Author

Ivecia commented Oct 17, 2024

For now, I think it's the only way for browsing docs on both Github and website.

On Github, the image source links will be added directly behind the repo url, e.g. adding /includes/img/dialects.svg to https://github.com/llvm/circt/blob/main, which will be https://github.com/llvm/circt/blob/main/includes/img/dialects.svg, while the image url should be https://github.com/llvm/circt/blob/main/docs/includes/img/dialects.svg.

On website, the image source links will be added directly behind the root of website, e.g. adding /includes/img/dialects.svg to https://circt.llvm.org, which will be https://circt.llvm.org/includes/img/dialects.svg, which is correct. (as docs be the root of website? or maybe move all images to includes?)

I wonder if one can move images in include folder to docs/include, keeping the same path between Github and website, which may solve this problem.

By the way, you can find the same way written in Scheduling.md.

![Dump of example instance](https://circt.llvm.org/includes/img/sched-instance.svg)

Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

Yeah, I think this is reasonable, and I guess the Scheduling docs decided to do this. I do wonder if it's possible to rearrange the directory structure so relative paths work on both github and the website, since the would be (mildly) preferable. This is something no one spent any time on...

I'm fine to merge this, since as long as we remain on circt.llvm.org, the URLs should be valid on both the website itself and github.

@Ivecia
Copy link
Contributor Author

Ivecia commented Oct 18, 2024

Thanks for your approval. Could anyone help me to merge this? Cause I don't have write access to this repo. Thanks in advance!

@mikeurbach mikeurbach merged commit 0ae52c1 into llvm:main Oct 18, 2024
4 checks passed
@mikeurbach
Copy link
Contributor

Done, thanks for sending this PR!

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.

2 participants