Fix NBSP support. Closes: #834. #851

rysson · 2023-07-11T06:33:25Z

Fix NBSP, it's not-break but varible-width space. Fixes #834

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

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

rysson · 2023-07-11T06:34:02Z

I need to add tests and changelog.
I'm not sure if docs need be updated.

Lucas-C · 2023-07-11T08:36:02Z

I need to add tests and changelog. I'm not sure if docs need be updated.

You can just add a mention of your change in CHANGELOG.md, that would be enough 😊

rysson · 2023-07-11T20:39:40Z

I don't speak English well, is it enough?

Fixed:

variable-width non-breaking space (NBSP) support issue #834

gmischler · 2023-07-12T05:23:57Z

I'm not sure if docs need be updated.

It might be helpful to mention this eg. in the paragraph about Character or Word Based Line Wrapping. After the first sentence, you could add something like this:

Non-breaking spaces (\U00a0) do not trigger a word wrap, but are otherwise treated exactly as a normal space character.

With a test to actually verify that, this is a very nice little fix!
👍

Lucas-C · 2023-07-12T12:06:57Z

Based on the GitHub Actions execution logs, it seems that your PR is breaking a few existing unit tests:

FAILED test/fonts/test_charmap.py...
FAILED test/html/test_html.py::test_html_whitespace_handling

You can try running them on your computer with this command:

pytest -vv test/fonts/test_charmap.py test/html/test_html.py

Related doc: https://pyfpdf.github.io/fpdf2/Development.html

gmischler · 2023-07-23T17:37:17Z

docs/Text.md

@@ -32,8 +32,9 @@ fixed_text = get_display(reshape(some_text))
 ```

 ### Character or Word Based Line Wrapping
-By default, `multi_line()` and `write()` will wrap lines based on words, using space characters and soft hyphens as seperators.
-For languages like Chinese and Japanese, that don't usually seperate their words, character based wrapping is more appropriate.
+By default, `multi_line()` and `write()` will wrap lines based on words, using space characters and soft hyphens as separators.


On that occasion, you could also fix an old mistake, and change the incorrect multi_line() in that sentence to multi_cell(). 😉

Lucas-C · 2023-08-01T22:49:27Z

Hi @rysson!

Do you still want to finish this PR? 😊

Otherwise, maybe another contributor could take over

rysson · 2023-08-02T20:23:18Z

@Lucas-C it depends... I'm on holidays now. Then another contributor could finish it faster.
I'll back about Aug, 16.

Lucas-C · 2023-08-03T18:02:33Z

@Lucas-C it depends... I'm on holidays now. Then another contributor could finish it faster. I'll back about Aug, 16.

No worries, this can definitively wait until the end of August if you wish to finish this! 😊

I'm just going to perform a new release in the meantime.

rysson · 2023-09-02T12:09:42Z

I'm back and I trying to finish this PR.
Everything going wrong, eg.

oscrypto failed (Openssl version not detected in current oscrypto version (pypi) wbond/oscrypto#78)
pytest -vv test/fonts/test_charmap.py test/html/test_html.py needs fontTools, maybe it should be in test/requirements.txt?

I'm kidding, nothing to spot me :-P

OK. I've made some fixes.

@Lucas-C – about unitttest, test_html_whitespace_handling is OK IMHO, test_charmap.py looks the same, but files differs a bit. Could you look?

Lucas-C · 2023-09-04T11:27:45Z

Hi @rysson

Thank you for getting back to this 😊👍

I haven't reviewed all your changes, but only based on the ones in CHANGELOG.md, I think you have rebase issues:
your branch / Pull Request contains many changes in this file that make no sense in this context.

Could you please fix this?
If you cannot manage to properly rebase your branch,
it may be worth creating a new "clean" branch with your patch,
and open a new Pull Request.

Lucas-C · 2023-10-09T16:18:44Z

Hi @rysson

Have you been able to get back to this?

Do you need with the rebase or with anything else?

rysson · 2023-10-10T08:06:25Z

Hi @rysson
Have you been able to get back to this?

Oh, I've forgot, I'm very sorry.
Live it too short a little, too many tasks ;-)

Ok, I've used cherry-pick for patch and I've generate same test PDF.
BUT, I have a few more test failed:

test/html/test_html.py::test_html_features FAILED
test/html/test_html.py::test_html_customize_ul FAILED
test/html/test_html.py::test_html_description FAILED
test/html/test_html.py::test_html_HTMLMixin_deprecation_warning FAILED

It's seems, that space (20) is replaced with nbsp (a0), for example:

E             +  b'BT 31.18 779.94 Td (          description details ) Tj ET',
E             -  b'BT 31.18 779.94 Td <a0a0a0a0a0a0a0a0a0a06465736372697074696f6e2064657461696c'

Could you look into those test and tell me if I should generate those PDF too?
Path replace a0 to 20 then I suggest to generate PDF again.

rysson · 2023-10-10T08:40:04Z

OK, I generated PDF, they look good IMHO.

I have another test failed:

        with time_execution() as duration:
            build_pdf_with_big_images()
>       assert duration.seconds > 0.3
E       assert 0.2659818779939087 > 0.3
E        +  where 0.2659818779939087 = namespace(seconds=0.2659818779939087).seconds

Maybe build server be a little bit slower and test passes.

Lucas-C · 2023-10-10T11:32:16Z

OK, I generated PDF, they look good IMHO.

I have another test failed:

        with time_execution() as duration:
            build_pdf_with_big_images()
>       assert duration.seconds > 0.3
E       assert 0.2659818779939087 > 0.3
E        +  where 0.2659818779939087 = namespace(seconds=0.2659818779939087).seconds

Maybe build server be a little bit slower and test passes.

Yes, this is a recurring issue tracked in #923
I'm going to bump this threshold to 22s, but simply retrying your pipeline execution should be enough to have it pass for now.

codecov-commenter · 2023-10-10T11:57:22Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (a99de0a) 93.56% compared to head (058b9e5) 93.57%.
Report is 1 commits behind head on master.

❗ Current head 058b9e5 differs from pull request most recent head 2f0cd05. Consider uploading reports for the commit 2f0cd05 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #851   +/-   ##
=======================================
  Coverage   93.56%   93.57%           
=======================================
  Files          28       28           
  Lines        8209     8213    +4     
  Branches     1500     1501    +1     
=======================================
+ Hits         7681     7685    +4     
  Misses        326      326           
  Partials      202      202

Files	Coverage Δ
fpdf/line_break.py	`99.14% <100.00%> (+<0.01%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Lucas-C · 2023-10-10T11:57:39Z

This PR looks good to me 🙂

@gmischler would you like to get a look at it before we merge?

gmischler · 2023-10-11T11:40:12Z

@gmischler would you like to get a look at it before we merge?

Looks good to me. Let's send it.

rysson requested a review from Lucas-C as a code owner July 11, 2023 06:33

Lucas-C mentioned this pull request Jul 17, 2023

Possible to create a new release? #856

Closed

gmischler reviewed Jul 23, 2023

View reviewed changes

Lucas-C added the pending-answer label Aug 1, 2023

rysson force-pushed the issue-834-nbsp branch from ca9b3fa to d51dcb9 Compare September 2, 2023 11:38

Fix NBSP support. Closes: py-pdf#834.

811bb95

rysson force-pushed the issue-834-nbsp branch from f41a5e9 to b0e2ea4 Compare October 10, 2023 07:55

rysson force-pushed the issue-834-nbsp branch from b0e2ea4 to 56fddc9 Compare October 10, 2023 08:23

rysson added 3 commits October 10, 2023 10:25

Update docs about NBSP (py-pdf#834)

5b6b19b

Update unittest for py-pdf#834 (test_charmap)

07f483a

Update unittest for py-pdf#834 (test_html_whitespace_handling)

cbf448e

rysson force-pushed the issue-834-nbsp branch from 56fddc9 to cbf448e Compare October 10, 2023 08:28

Update unittest for py-pdf#834 (test_html)

2f0cd05

gmischler merged commit 1546f70 into py-pdf:master Oct 11, 2023
10 checks passed

Lucas-C removed the pending-answer label Nov 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix NBSP support. Closes: #834. #851

Fix NBSP support. Closes: #834. #851

rysson commented Jul 11, 2023

rysson commented Jul 11, 2023

Lucas-C commented Jul 11, 2023 •

edited

Loading

rysson commented Jul 11, 2023

gmischler commented Jul 12, 2023

Lucas-C commented Jul 12, 2023 •

edited

Loading

gmischler Jul 23, 2023

Lucas-C commented Aug 1, 2023

rysson commented Aug 2, 2023

Lucas-C commented Aug 3, 2023

rysson commented Sep 2, 2023 •

edited by Lucas-C

Loading

Lucas-C commented Sep 4, 2023

Lucas-C commented Oct 9, 2023

rysson commented Oct 10, 2023 •

edited

Loading

rysson commented Oct 10, 2023 •

edited

Loading

Lucas-C commented Oct 10, 2023

codecov-commenter commented Oct 10, 2023

Lucas-C commented Oct 10, 2023

gmischler commented Oct 11, 2023

Fix NBSP support. Closes: #834. #851

Fix NBSP support. Closes: #834. #851

Conversation

rysson commented Jul 11, 2023

rysson commented Jul 11, 2023

Lucas-C commented Jul 11, 2023 • edited Loading

rysson commented Jul 11, 2023

gmischler commented Jul 12, 2023

Lucas-C commented Jul 12, 2023 • edited Loading

gmischler Jul 23, 2023

Choose a reason for hiding this comment

Lucas-C commented Aug 1, 2023

rysson commented Aug 2, 2023

Lucas-C commented Aug 3, 2023

rysson commented Sep 2, 2023 • edited by Lucas-C Loading

Lucas-C commented Sep 4, 2023

Lucas-C commented Oct 9, 2023

rysson commented Oct 10, 2023 • edited Loading

rysson commented Oct 10, 2023 • edited Loading

Lucas-C commented Oct 10, 2023

codecov-commenter commented Oct 10, 2023

Codecov Report

Lucas-C commented Oct 10, 2023

gmischler commented Oct 11, 2023

Lucas-C commented Jul 11, 2023 •

edited

Loading

Lucas-C commented Jul 12, 2023 •

edited

Loading

rysson commented Sep 2, 2023 •

edited by Lucas-C

Loading

rysson commented Oct 10, 2023 •

edited

Loading

rysson commented Oct 10, 2023 •

edited

Loading