Attempt to fix broken image links in Discovery book #142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
After switching to relative URLs, linkchecker still encountered some broken links. This commit fixes those broken links and now the only broken links detected are in print.html itself. I will work on fixing those in a later commit. I also renamed some URLs from .html extensions to .md extensions since they get renamed to .html by mdbook and it allows GitHub renderings of these pages to end up going to the correct place when you click on the links. They work in VSCode preview as well.
rust-lang/mdBook#789 causes mdbook to generate print.html content with relative URLS that are still relative to the individual HTML files and doesn't correct for the fact that print.html isn't in the same folder. This commit will verify that the relative URLs are correct in all of the HTML output except for print.html I am assuming that people would like to see the images again in the Discovery book website as they click through the various chapters, even if the printable version is still broken. If the PR with this commit is merged, an issue should be created to track the fact that this change should be reverted once mdbook handles relative links correctly in its generaton of print.html
|
I guess Travis doesn't really check much. Is it bors that runs the actual checking part of ci/script.sh? If so, who is allowed to ask it to just try a test pass and not push up to master? Thanks! |
|
@adamgreen Yes, you can do bors try |
tryBuild succeeded |
|
@therealprof Thanks! Seems like it worked around the print.html failures. That makes me happy! |
therealprof
approved these changes
Oct 31, 2018
There was a problem hiding this comment.
Looks like a good and comprehensive approach to me. Let's try this. Thanks a lot @adamgreen.
|
bors r+ |
bors bot
added a commit
that referenced
this pull request
Oct 31, 2018
142: Attempt to fix broken image links in Discovery book r=therealprof a=adamgreen This PR pools together PRs #130 and #131 from @ChouzArt and adds 2 more commits that I created to correct other failing links missed by the first 2 and to modify the Travis verification steps to skip the checking of print.html - @ChouzArt has previously pointed out that rust-lang/mdBook#789 causes mdbook to generate an incorrect print.html file when relative links are used in the book content. Skipping verification of print.html during Travis runs means that we will still have a bad image links in print.html but the main Discovery book pages will have working images again and they will show up here on GitHub as well. I realize that this isn't a perfect solution but I thought that people might find it better than the current situation. The main reason I am creating this PR is to: * See if it will pass Travis. * Get people's feedback on this **half** solution. If we decide to take this PR then there are a few other things that need to be done: * Create a new issue to track the fact that we should revert this ignoring of print.html during link checking once the mdbook issue is resolved. * Make a similar change to the [Bookself version of the Travis script](https://github.com/rust-embedded/bookshelf/blob/master/ci/script.sh). I should also point out that running a `sed` script on print.html can fix most of the link issues that it contains so that the images will at least show up in a printout. Something like: ```console cp book/print.html print.html sed 's: src="\.\./assets/: src="assets/:g' <print.html >book/print.html rm print.html ``` Co-authored-by: Carlos <gsolracchz@gmail.com> Co-authored-by: Adam Green <adamgreen@users.noreply.github.com>
Build succeeded |
njmartin10
pushed a commit
to njmartin10/book
that referenced
this pull request
Nov 10, 2018
68: canonicalize URLs r=therealprof a=japaric as per rust-embedded#55 closes rust-embedded#55 this PR also ignores print.html when `linkcheck`-ing as done in rust-embedded/discovery#142 Co-authored-by: Jorge Aparicio <jorge@japaric.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR pools together PRs #130 and #131 from @ChouzArt and adds 2 more commits that I created to correct other failing links missed by the first 2 and to modify the Travis verification steps to skip the checking of print.html - @ChouzArt has previously pointed out that rust-lang/mdBook#789 causes mdbook to generate an incorrect print.html file when relative links are used in the book content. Skipping verification of print.html during Travis runs means that we will still have a bad image links in print.html but the main Discovery book pages will have working images again and they will show up here on GitHub as well. I realize that this isn't a perfect solution but I thought that people might find it better than the current situation.
The main reason I am creating this PR is to:
If we decide to take this PR then there are a few other things that need to be done:
I should also point out that running a
sedscript on print.html can fix most of the link issues that it contains so that the images will at least show up in a printout. Something like: