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.

Sign up for GitHub

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

Jump to bottom

Template and fonts fixes #843

Merged
merged 4 commits into from Mar 28, 2020
Merged

Template and fonts fixes #843

merged 4 commits into from Mar 28, 2020

Conversation

pshchelo
Copy link
Contributor

Fixes issue #601 and failures to load fonts

@pshchelo
Copy link
Contributor Author

Updated commit message in the second commit with exact error

@akrabat
Copy link
Member

akrabat commented Feb 10, 2020

Thanks @pshchelo.

Are you able to create a test that fails without the changes applied and then passes when they are?

@pshchelo
Copy link
Contributor Author

@akrabat haven't looked at how tests are done in the repo yet, but will try to come up with something

@pshchelo
Copy link
Contributor Author

Came out with a test for #601 , working on font loading one.

@akrabat what is your preferred process re git history? Should I add tests to each corresponding commit, or create a separate commits with tests and place them before the commits with fixes in the PR so that you can checkout only tests commits and see that they are failing? Travis will then check that they are working with fixes..

@akrabat
Copy link
Member

akrabat commented Feb 11, 2020

Separate commits please. Order would be convenient, but not strictly necessary.

@pshchelo
Test for issue 601 and font loading fallback
45a8749 
sphinx conf.py set to use cover page and non-existing cover template,
will fail to load the default fallback template (issue rst2pdf#601).

also add two dummy non-font TTF files to fail some tests that are
loading fonts from the input folder.
This in fact is a bit random due to how os.walk/os.listdir works,
(see https://docs.python.org/3/library/os.html#os.listdir).
To maximize chances of failure one dummy ttf file name starts with 0,
and another with Z (not an ideal and stable solution,
but is the best I could quickly think of).
@pshchelo
Fix pdf cover fallback
f103174 
Fixes issue rst2pdf#601

seems this logging code line was copy-pasted from createpdf.py,
and now fallback fails as it can not output log message
as the object has no 'custom_resource' attribute
@pshchelo
Fix exception handling when loading fonts
722ebc5 
currently TTFError exceptions raised in loadFonts() result in

  UnboundLocalError: local variable 'font' referenced before assignment

on the next line as the execution continues further.

Just skip to next font after producing the warning log.
@pshchelo
Copy link
Contributor Author

tests added, the one for font loading may not be ideal (as the order of files in os.walk is not guaranteed), but is the best I could quickly think of

@pshchelo
Copy link
Contributor Author

pshchelo commented Mar 5, 2020

@akrabat plz review

I bet the issue with font loading can affect multiple users.
For example, on my Ubuntu 18.04 system there are several installed by default fonts that font loading fails on (log when running with this patch):

[WARNING] findfonts.py:81 Error processing /usr/share/fonts/opentype/noto/NotoSansCJK-Regular.ttc                                                                                                                    
[WARNING] findfonts.py:81 Error processing /usr/share/fonts/opentype/noto/NotoSansCJK-Bold.ttc                                                                                                                       
[WARNING] findfonts.py:81 Error processing /usr/share/fonts/opentype/noto/NotoSerifCJK-Bold.ttc                                                                                                                      
[WARNING] findfonts.py:81 Error processing /usr/share/fonts/opentype/noto/NotoSerifCJK-Regular.ttc                                                                                                                   
[WARNING] findfonts.py:81 Error processing /usr/share/fonts/truetype/noto/NotoColorEmoji.ttf

Depending on the order in which Python's os.walk finds fonts this can completely break PDF build w/o this patch if such offending fonts are loaded early enough in the list of discovered fonts, producing PDF of zero size.

@akrabat akrabat requested a review from lornajane March 10, 2020 12:30
@akrabat
Set the date on the cover page for the 601 test
8525cd2 
The generated PDF has to be consisted each time, so we set the date
printed on the cover page to a specific date rather than using today's
date.
@akrabat akrabat closed this in 4084bfe Mar 28, 2020
@akrabat akrabat merged commit 4084bfe into rst2pdf:master Mar 28, 2020
@pshchelo pshchelo deleted the template-and-fonts-fixes branch March 29, 2020 17:59
@akrabat akrabat added this to the 1.0 milestone Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akrabat akrabat akrabat approved these changes

@lornajane lornajane Awaiting requested review from lornajane

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.97
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pshchelo @akrabat