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

fpdf.ln() not working in version 2.5.7 if last fpdf.cell() was called with empty txt #601

Closed
rgarus opened this issue Nov 12, 2022 · 5 comments · Fixed by #604
Closed

fpdf.ln() not working in version 2.5.7 if last fpdf.cell() was called with empty txt #601

rgarus opened this issue Nov 12, 2022 · 5 comments · Fixed by #604

Comments

@rgarus
Copy link

rgarus commented Nov 12, 2022

In the following example line feed should use as height the height value of the last printed cell, which is the height of the current font size. This works in fpdf2 version 2.5.6, but no longer in version 2.5.7. In version 2.5.7 the used height value is 0.

Minimal code

from fpdf import FPDF

pdf = FPDF()
pdf.add_page()
pdf.set_font("Times", size=12)

for i in range(1, 10):
    pdf.cell(w=100, h=None, txt=f"Printing line number {i}")
    pdf.cell(w=100, h=None, txt="")
    pdf.ln()

pdf.output("test.pdf") 

Environment

  • OS Linux
  • Python version 3.10
  • fpdf2 version 2.5.7

test_2.5.6.pdf
test_2.5.7.pdf

@rgarus rgarus added the bug label Nov 12, 2022
@gmischler
Copy link
Collaborator

Thanks for reporting this!

Looks like it's my fault and happened in #519.
A tactical fix should be easy and quick, I will look at it more closely tonight.

On a strategic level though, I'm not sure about the wisdom of maintaining self.lasth for this purpose.
@Lucas-C, wouldn't it make more sense to simply use the current font height?
It doesn't look like self.lasth is documented anywhere outside of one short code comment, so it's unlikely that many people will miss it.

@Lucas-C
Copy link
Member

Lucas-C commented Nov 14, 2022

On a strategic level though, I'm not sure about the wisdom of maintaining self.lasth for this purpose.

lasth is now private: https://github.com/PyFPDF/fpdf2/blob/master/fpdf/fpdf.py#L266
It is set by _render_styled_text_line() called from text(), cell(), multi_cell() & write().
My understanding is that its value does not always come from self.font_height. It can be specified for example from the h parameter of cell().

Hence, while I would be OK to get rid of self._lasth, I do not see how to do so without impacting the library behaviour.

If you can submit a PR to fix the issue @gmischler, it would be very welcome 😊

@gmischler
Copy link
Collaborator

Hence, while I would be OK to get rid of self._lasth, I do not see how to do so without impacting the library behaviour.

Since it is not currently possible to change the font size within one of the relevant methods, I don't think that self._lasth can actually differ from the current font size (except through this bug).
Either way, the fix should be quite straightforward.

@Lucas-C
Copy link
Member

Lucas-C commented Nov 15, 2022

Since it is not currently possible to change the font size within one of the relevant methods, I don't think that self._lasth can actually differ from the current font size (except through this bug).

@gmischler: consider the following code

from fpdf import FPDF

pdf = FPDF()
pdf.add_page()
pdf.set_font("helvetica", size=18)
pdf.cell(txt="Hello world")
print(pdf._lasth)  # prints: 6.35
pdf.cell(txt="Hello world", h=50)
print(pdf._lasth)  # prints: 50

As you can see self._lasth can change even if .font_size does not.

@gmischler
Copy link
Collaborator

As you can see self._lasth can change even if .font_size does not.

Ah I see.
To be honest, this feels like a rather weird behaviour to me, but it also appears to be rather old, so we'll have to live with it for now...

gmischler added a commit to gmischler/fpdf2 that referenced this issue Nov 16, 2022
gmischler added a commit to gmischler/fpdf2 that referenced this issue Nov 16, 2022
gmischler added a commit to gmischler/fpdf2 that referenced this issue Nov 17, 2022
gmischler added a commit to gmischler/fpdf2 that referenced this issue Nov 18, 2022
Lucas-C pushed a commit that referenced this issue Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants