ENH: Create /Font if not exists (#2670) #2671
Conversation
Updates the _writer.py to create the /DR and /Font dictionaries, and add a font (Helvetica) if they don't exist. This enables filling out PDF forms.
|
Thanks for the PR. Besides of the above remarks, could you please add a corresponding test as well? |
- Added test - Cleaned up code (simplified and fixed variable name)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2671 +/- ##
=======================================
Coverage 95.00% 95.01%
=======================================
Files 50 50
Lines 8356 8362 +6
Branches 1673 1674 +1
=======================================
+ Hits 7939 7945 +6
Misses 259 259
Partials 158 158 ☔ View full report in Codecov by Sentry. |
I updated the SubType TrueType. I see some research saying that Type1 fonts are seeing declining support. I didn't see any change in functionality, but figured it would be good to make the change regardless.
| font_entry = DictionaryObject({ | ||
| NameObject("/Type"): NameObject("/Font"), | ||
| NameObject("/Subtype"): NameObject("/TrueType"), | ||
| NameObject("/BaseFont"): NameObject("/Helvetica"), |
There was a problem hiding this comment.
Couldn't we make the font name configurable through a constant or method parameter?
There was a problem hiding this comment.
Helvetica is the default font that should be embedded in viewers
| @@ -44,6 +44,29 @@ | |||
| SAMPLE_ROOT = Path(PROJECT_ROOT) / "sample-files" | |||
| GHOSTSCRIPT_BINARY = shutil.which("gs") | |||
|
|
|||
| def test_fill_form_without_font(pdf_file_path): | |||
There was a problem hiding this comment.
Please do not add new tests at the top. Additionally: Where can I see that the new functionality is checked? Shouldn't we test with your example file as well which would previously raise an exception?
|
I've proposed an alternative PR #2677 |
Updates the _writer.py to create the /DR and /Font dictionaries, and add a font (Helvetica) if they don't exist. This enables filling out PDF forms.
Response to issue #2670