py-pdf / fpdf2

Simple PDF generation for Python
https://py-pdf.github.io/fpdf2/
GNU Lesser General Public License v3.0
1.13k stars 253 forks source link

Allow for using wrapmode='CHAR' in templates #1176

Closed carlhiggs closed 6 months ago

carlhiggs commented 6 months ago

Fixes #1159

Checklist:

In the flextemplate test for the wrapmode functionality, I tested a range of possibilities described in text of each template element. The first three elements test that the new code doesn't do anything strange to existing funcitonality; the last one specifically tests that wrapmode='CHAR' wraps accordingly:

elements = [
    {
        "name": "multi",
        "type": "T",
        "x1": 80,
        "y1": 10,
        "x2": 170,
        "y2": 15,
        "text": "If multiline is False, the text should still not wrap even if wrapmode is specified.",
        "background": 0xEEFFFF,
        "multiline": False,
        "wrapmode": "WORD",
    },
    {
        "name": "multi",
        "type": "T",
        "x1": 80,
        "y1": 40,
        "x2": 170,
        "y2": 45,
        "text": "If multiline is True, and wrapmode is omitted, the text should wrap by word and not "
                "cause an error due to omission of the wrapmode argument.",
        "background": 0xEEFFFF,
        "multiline": True,
    },
    {
        "name": "multi",
        "type": "T",
        "x1": 80,
        "y1": 70,
        "x2": 170,
        "y2": 75,
        "text": "If multiline is True and the wrapmode argument is provided, it should not cause a "
            "problem even if using the default wrapmode of 'WORD'.",
        "background": 0xEEFFFF,
        "multiline": True,
        "wrapmode": "WORD",
    },
    {
        "name": "multi",
        "type": "T",
        "x1": 80,
        "y1": 100,
        "x2": 170,
        "y2": 105,
        "text": "If multiline is True and the wrapmode argument is provided, it should result in "
            "wrapping based on characters instead of words, regardless of language (i.e. even though "
            "this is designed to support scripts like Chinese and Japanese, wrapping long sentences "
            "like this in English still demonstrates functionality.)",
        "background": 0xEEFFFF,
        "multiline": True,
        "wrapmode": "CHAR",
    },
]
pdf = FPDF()
pdf.add_page()
pdf.set_font("courier", "", 10)
templ = FlexTemplate(pdf, elements)
templ.render()
assert_pdf_equal(pdf, HERE / "flextemplate_wrapmode.pdf", tmp_path)

The output of the above looks correct to me: image

I ran black and checked pylint in vscode; I believe the changes I made should be all fine for these requirements.

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

gmischler commented 6 months ago

@allcontributors please add @carlhiggs for code

allcontributors[bot] commented 6 months ago

@gmischler

I've put up a pull request to add @carlhiggs! :tada:

carlhiggs commented 6 months ago

I've just addressed latest round of review requests: more accurately referring to when 'wrapmode is "CHAR"' in one specific test's self-descriptive text (and updated test pdfs used to validate this to have the changed text too), and externalising duplicate text to a shared python file containing elements now used in both template and flextemplate tests of the optional wrapmode functionality. Black and pylint were run using CLI, and through pre-commit hook. The pylint warnings that remained did not relate to my tests or modifications, but to various "pylint.extensions" that apparently were impossible to load ("bad_plugin-value"?). I believe that is not an issue with this pull request, but something upstream. If this passes checks, do you think this could be could to merge @gmischler ?

gmischler commented 6 months ago

Thanks for the contribution, @carlhiggs ! Merging now.