Support character based line wrapping (#649) #657

gmischler · 2023-01-01T23:41:55Z

e.g. Fixes #649

Checklist:

The GitHub pipeline is OK (green),
meaning that both pylint (static code analyzer) and black (code formatter) are happy with the changes of this PR.
A unit test is covering the code added / modified by this PR
This PR is ready to be merged
In case of a new feature, docstrings have been added, with also some documentation in the docs/ folder
A mention of the change is present in CHANGELOG.md

multi_cell() and write() now have a wrapmode argument, which can be "WORD" or "CHAR".

"WORD" is the default and works as before.
With "CHAR" all whitespace gets treated as normal characters and wrapping occurs at the first character that doesn't fit anymore. Trailing spaces on a line get removed to enable correct justification.

By submitting this pull request, I confirm that my contribution is made under the terms of the GNU LGPL 3.0 license.

codecov · 2023-01-02T00:17:47Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (1093622) 93.68% compared to head (102752c) 93.71%.

❗ Current head 102752c differs from pull request most recent head f479430. Consider uploading reports for the commit f479430 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #657      +/-   ##
==========================================
+ Coverage   93.68%   93.71%   +0.02%     
==========================================
  Files          26       26              
  Lines        6923     6949      +26     
  Branches     1235     1240       +5     
==========================================
+ Hits         6486     6512      +26     
  Misses        260      260              
  Partials      177      177

Impacted Files	Coverage Δ
fpdf/enums.py	`100.00% <100.00%> (ø)`
fpdf/fpdf.py	`91.71% <100.00%> (+<0.01%)`	⬆️
fpdf/line_break.py	`99.60% <100.00%> (+0.03%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Lucas-C

Good job @gmischler!

Lucas-C · 2023-01-02T16:07:25Z

test/image/test_load_image.py

@@ -42,7 +42,7 @@ def test_load_invalid_base64_data():


 # ensure memory usage does not get too high - this value depends on Python version:
-@memunit.assert_lt_mb(141)
+@memunit.assert_lt_mb(200)


Please do not bump this above 150MB, it's not necessary

What's the point?
The demand will rise with every new test added, and that seems unlikely to get fixed in pytest any time soon.
Is there any real disadvantage of adding some extra headroom here?

I'm still not sure this is due to pytest... Investigation is ongoing in #641

The point is that keeping those thresholds allows me to keep track of how fast / what causes memory usage increases.
So far they helped me to link the growing number of tests with this memory usage, but this could be spurious-correlation...
As long as the increasing memory usage problem is not solved, I'd prefer to keep those checks a low as possible, in hope this will help pinpoint the origin of the issue

Lucas-C · 2023-01-02T16:25:06Z

docs/Text.md

-A manual break happens any time the \\n character is met,
-Upon method exit, the current position is left near the end of the text, ready for the next call to continue without a gap, potentially with a different font or size set. Returns a boolean indicating if page break was triggered.
+space or soft-hyphen character (in word wrap mode) or at the current position (in
+character break mode), and text continues from the left margin.


Similarly to the "Right-to-Left & Arabic Script workaround" section above,
maybe an explicit suggestion could be made of using wrapmode='CHAR' for langages like chinese or japanese that do not separate words with spaces?

The whole section is about limitations (and their workarounds) right now, which this really isn't.
But we can turn it into a general typography and language specifics section.

But we can turn it into a general typography and language specifics section.

Yes, this would be great!

CHANGELOG.md

Lucas-C · 2023-01-02T20:24:08Z

You will need to rebase and fix some minor merge conflicts in CHANGELOG.md I think

I'm going to wait for this PR and then I'll publish a new release

Lucas-C · 2023-01-09T09:52:21Z

Hi @gmischler!

Do you think you will have time to add the final touches to this PR in the close future?
No pressure, but I'm willing to handle them if you don't have time for this, in order to perform a new release after that 😊

gmischler · 2023-01-11T13:31:50Z

Do you think you will have time to add the final touches to this PR in the close future?

Sorry, I'm out of the country until end of next week. Will get to it after that.

Lucas-C · 2023-01-11T16:30:51Z

OK, no worries, it can wait 😊
Thank you for your answer

Lucas-C · 2023-02-24T14:05:28Z

Hi @gmischler!

Do you plan to continue this PR in the future? 😊
This would be a nice addition!

gmischler · 2023-02-24T16:05:01Z

Do you plan to continue this PR in the future?

Yes I do. Actually, I was about to get back to it in short order...

gmischler · 2023-02-24T21:57:11Z

As far as I am concerned, this is ready now.

Interesting that we're also running into a timeout here now:

2023-02-24T21:48:21.6096581Z +++++++++++++++++++++++++++++++++++ Timeout ++++++++++++++++++++++++++++++++++++
2023-02-24T21:48:21.6096765Z test/test_perfs.py::test_intense_image_rendering

I have made it a habit of excluding test_perfs locally for months now, because it very regularly fails to complete in time...

Lucas-C · 2023-02-24T22:39:36Z

As far as I am concerned, this is ready now.

Great! 😊
Could you please rebase your branch, so that the https://github.com/PyFPDF/fpdf2/pull/657/files diff only displays your changes?

Interesting that we're also running into a timeout here now:
2023-02-24T21:48:21.6096581Z +++++++++++++++++++++++++++++++++++ Timeout ++++++++++++++++++++++++++++++++++++
2023-02-24T21:48:21.6096765Z test/test_perfs.py::test_intense_image_rendering 
I have made it a habit of excluding test_perfs locally for months now, because it very regularly fails to complete in time...

Could you try removing PYTHONMALLOCSTATS from .github/workflows/continuous-integration-workflow.yml,
as I did in https://github.com/PyFPDF/fpdf2/pull/703/files#diff-59c82b60a237174fa32aa0d97791d34cd322b4f4d7a70cf9d26274e0d1d6945c?
This was an experiment from 3 days ago (#704), but it turned out to slow down tests and to expand the logs too much.

Apart from that and from the memory caps, test/test_perfs.py has been passing fine in the GitHub Actions pipeline for the past months, including in the latest PR #701, so there may be a reason if it stopped passing.

gmischler · 2023-02-27T13:02:27Z

Could you please rebase your branch, so that the https://github.com/PyFPDF/fpdf2/pull/657/files diff only displays your changes?

My branch is up to date and only shows my own changes over there. No idea why ist shows so many other changes here.

Could you try removing PYTHONMALLOCSTATS from .github/workflows/continuous-integration-workflow.yml,

That seems to have done the trick.

Lucas-C · 2023-02-27T14:27:19Z

My branch is up to date and only shows my own changes over there. No idea why ist shows so many other changes here.

This may be because there were "Merge commits" performed on your master branch:
master...gmischler:fpdf2:WrapByChar

If you don't mind, a simple solution could be to squash all the commits in this PR into a single commit 😊

Co-authored-by: Lucas Cimon <925560+Lucas-C@users.noreply.github.com>

gmischler · 2023-02-28T13:48:56Z

Ok, after the recent changes to master, a new rebase seems to have done the trick.

@Lucas-C , as far as I can tell it's now ready to merge.

Lucas-C · 2023-02-28T16:58:02Z

Merged!

Thank you @gmischler 😊

gmischler requested a review from Lucas-C as a code owner January 1, 2023 23:41

Lucas-C approved these changes Jan 2, 2023

View reviewed changes

Lucas-C changed the title ~~Support character based line warpping (#649)~~ Support character based line wrapping (#649) Jan 2, 2023

Lucas-C mentioned this pull request Feb 27, 2023

add ICC profiles to PDFs #709

Merged

5 tasks

Lucas-C mentioned this pull request Feb 27, 2023

Font fallback #669

Merged

5 tasks

gmischler and others added 6 commits February 28, 2023 14:26

Support character based line warpping (py-pdf#649)

8a0df52

bump memory for image tests (pytest likely to remain broken)

900dfc9

Update CHANGELOG.md

9599be5

Co-authored-by: Lucas Cimon <925560+Lucas-C@users.noreply.github.com>

testing weird edge cases

22537ef

check for edge case

7f307c3

doc: Typography and Language Specific Concepts

1bb75f5

gmischler force-pushed the WrapByChar branch from d109b52 to 1bb75f5 Compare February 28, 2023 13:31

fix changelog

f479430

Lucas-C approved these changes Feb 28, 2023

View reviewed changes

Lucas-C merged commit ca2f8b0 into py-pdf:master Feb 28, 2023

gmischler deleted the WrapByChar branch March 1, 2023 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support character based line wrapping (#649) #657

Support character based line wrapping (#649) #657

gmischler commented Jan 1, 2023

codecov bot commented Jan 2, 2023 •

edited

Loading

Lucas-C left a comment

Lucas-C Jan 2, 2023

gmischler Jan 2, 2023

Lucas-C Jan 2, 2023

Lucas-C Jan 2, 2023

gmischler Jan 2, 2023

Lucas-C Jan 2, 2023

Lucas-C commented Jan 2, 2023

Lucas-C commented Jan 9, 2023

gmischler commented Jan 11, 2023

Lucas-C commented Jan 11, 2023

Lucas-C commented Feb 24, 2023

gmischler commented Feb 24, 2023

gmischler commented Feb 24, 2023

Lucas-C commented Feb 24, 2023 •

edited

Loading

gmischler commented Feb 27, 2023

Lucas-C commented Feb 27, 2023

gmischler commented Feb 28, 2023

Lucas-C commented Feb 28, 2023

Support character based line wrapping (#649) #657

Support character based line wrapping (#649) #657

Conversation

gmischler commented Jan 1, 2023

codecov bot commented Jan 2, 2023 • edited Loading

Codecov Report

Lucas-C left a comment

Choose a reason for hiding this comment

Lucas-C Jan 2, 2023

Choose a reason for hiding this comment

gmischler Jan 2, 2023

Choose a reason for hiding this comment

Lucas-C Jan 2, 2023

Choose a reason for hiding this comment

Lucas-C Jan 2, 2023

Choose a reason for hiding this comment

gmischler Jan 2, 2023

Choose a reason for hiding this comment

Lucas-C Jan 2, 2023

Choose a reason for hiding this comment

Lucas-C commented Jan 2, 2023

Lucas-C commented Jan 9, 2023

gmischler commented Jan 11, 2023

Lucas-C commented Jan 11, 2023

Lucas-C commented Feb 24, 2023

gmischler commented Feb 24, 2023

gmischler commented Feb 24, 2023

Lucas-C commented Feb 24, 2023 • edited Loading

gmischler commented Feb 27, 2023

Lucas-C commented Feb 27, 2023

gmischler commented Feb 28, 2023

Lucas-C commented Feb 28, 2023

codecov bot commented Jan 2, 2023 •

edited

Loading

Lucas-C commented Feb 24, 2023 •

edited

Loading