epub: fix fatal errors while parsing EPUB files #1854
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 generating the EPUB file for the Elixir docs with this version, and reviewing the result with `epubcheck`, I got the following summary: ```console $ epubcheck doc/elixir/Elixir.epub --json elixir_docs.json (base) Check finished with errors Messages: 0 fatals / 141 errors / 0 warnings / 0 infos ``` If you compare the previous result with what we had on #1851 ``` Messages: 9 fatals / 425 errors / 0 warnings / 0 infos ``` you can see that now we don't have messages with `fatal` severity and we have reduced considerably the number of errors =) I manually checked the generated EPUB on Apple Books and the previous truncated sections are solved, I don't see the banner _Below is a rendering of the page up to the first error_ and also the links to anchor different anchor seems to work. Fixes: #1851
milmazz
commented
Jan 26, 2024
Comment on lines
+66
to
+67
| |> String.replace(~r{id="&+/\d+[^"]*}, &String.replace(&1, "&", "&")) | ||
| |> String.replace(~r{href="[^#"]*#&+/\d+[^"]*}, &String.replace(&1, "&", "&")) |
There was a problem hiding this comment.
I frowned a little with these nested String.replace. So, please let me now if you have any advice on how to improve this function.
There was a problem hiding this comment.
@wojtekmach I though we had already escaped those when generating the links. Maybe this is something (or an option) we can pass when autolinking? The id we can fix by escaping in the document itself.
|
|
||
| The following text includes a reference to an anchor that causes problems in EPUB documents. | ||
|
|
||
| To remove this anti-pattern, we can replace `&&/2`, `||/2`, and `!/1` by `and/2`, `or/2`, and `not/1` respectively. |
There was a problem hiding this comment.
Added this line to demonstrate that we're transforming the links to problematic anchors in EPUB files.
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.
After generating the EPUB file for the Elixir docs with this version, and reviewing the result with
epubcheck, I got the following summary:If you compare the previous result with what we had on #1851
you can see that now we don't have messages with
fatalseverity and we have reduced considerably the number oferrors=)I manually checked the generated EPUB on Apple Books and the previous truncated sections are fixed, I don't see the banner Below is a rendering of the page up to the first error and also the links to different anchors seems to work.
Fixes: #1851