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.

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

Make sure 'lift' is reset if Fragment changes lift in _render_styled_text_line #862

Merged
merged 2 commits into from
Jul 25, 2023

Conversation

Tolker-KU
Copy link

@Tolker-KU Tolker-KU commented Jul 24, 2023

TextLines with Framents with alternating lift is not rendered correctly as lift operator is not reset. This is fixed here.

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 "N/A"

  • A mention of the change is present in CHANGELOG.md

@Tolker-KU Tolker-KU marked this pull request as ready for review July 24, 2023 13:53
@Tolker-KU Tolker-KU requested a review from Lucas-C as a code owner July 24, 2023 13:53
@Lucas-C
Copy link
Member

Lucas-C commented Jul 24, 2023

Hi @Tolker-KU

Thank you for this PR! 👍

Could you please add a unit test with some minimal code
to explain what you are fixing there?

If you are fixing a bug that happened with FPDF.write(),
you can add a test function in test/text/test_write.py,
and ensure that everything works by running pytest test/text/test_write.py -k your_new_function_name

See also: https://pyfpdf.github.io/fpdf2/Development.html#testing

@Tolker-KU Tolker-KU force-pushed the bugfix/lift_not_stick branch from ee77ca3 to 0e46d95 Compare July 24, 2023 14:36
@Tolker-KU
Copy link
Author

Hi @Lucas-C,

I found some other test related to this functionality and have added a new test in that file

@Lucas-C
Copy link
Member

Lucas-C commented Jul 24, 2023

I found some other test related to this functionality and have added a new test in that file

Perfect!

One last thing before merging: could you add a mention of this fix in the ### Fixed section of CHANGELOG.md, for the next release v2.7.5, please?

@Tolker-KU
Copy link
Author

Tolker-KU commented Jul 24, 2023

Tests will probably fail due to the issue discussed here #863

@Lucas-C
Copy link
Member

Lucas-C commented Jul 24, 2023

Tests will probably fail due to the issue discussed here #863

Alright 👍
This PR should then be rebased once #863 is merged.

@Tolker-KU Tolker-KU force-pushed the bugfix/lift_not_stick branch from bc8ea15 to 06f1411 Compare July 25, 2023 11:38
@Tolker-KU
Copy link
Author

Tests will probably fail due to the issue discussed here #863

Alright 👍 This PR should then be rebased once #863 is merged.

Done! Tests should pass now :)

@Lucas-C
Copy link
Member

Lucas-C commented Jul 25, 2023

Done! Tests should pass now :)

Yes! Merging now

Thank you for your work on this @Tolker-KU

@Lucas-C Lucas-C merged commit df762e3 into py-pdf:master Jul 25, 2023
@Tolker-KU Tolker-KU deleted the bugfix/lift_not_stick branch July 25, 2023 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants