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

Fixing: bullet/top_margin combo & width of indented paragraphs & some HTML bugs #1218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions fpdf/line_break.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def __init__(
self,
characters: Union[list, str],
graphics_state: dict,
k: float,
k: Number,
link: Optional[Union[int, str]] = None,
):
if isinstance(characters, str):
Expand Down Expand Up @@ -352,11 +352,11 @@ def render_pdf_text_core(self, frag_ws, current_ws):

class TextLine(NamedTuple):
fragments: tuple
text_width: float
text_width: Number
number_of_spaces: int
align: Align
height: float
max_width: float
height: Number
max_width: Number
trailing_nl: bool = False
trailing_form_feed: bool = False

Expand Down Expand Up @@ -389,7 +389,7 @@ class SpaceHint(NamedTuple):
original_character_index: int
current_line_fragment_index: int
current_line_character_index: int
line_width: float
line_width: Number
number_of_spaces: int


Expand All @@ -398,16 +398,16 @@ class HyphenHint(NamedTuple):
original_character_index: int
current_line_fragment_index: int
current_line_character_index: int
line_width: float
line_width: Number
number_of_spaces: int
curchar: str
curchar_width: float
curchar_width: Number
graphics_state: dict
k: float
k: Number


class CurrentLine:
def __init__(self, max_width: float, print_sh: bool = False):
def __init__(self, max_width: Number, print_sh: bool = False):
"""
Per-line text fragment management for use by MultiLineBreak.
Args:
Expand Down Expand Up @@ -444,12 +444,12 @@ def width(self):
def add_character(
self,
character: str,
character_width: float,
character_width: Number,
graphics_state: dict,
k: float,
k: Number,
original_fragment_index: int,
original_character_index: int,
height: float,
height: Number,
url: str = None,
):
assert character != NEWLINE
Expand Down Expand Up @@ -573,12 +573,13 @@ class MultiLineBreak:
def __init__(
self,
fragments: Sequence[Fragment],
max_width: Union[float, callable],
max_width: Union[Number, callable],
margins: Sequence[Number],
indent: Number = 0.0,
align: Align = Align.L,
print_sh: bool = False,
wrapmode: WrapMode = WrapMode.WORD,
line_height: float = 1.0,
line_height: Number = 1.0,
skip_leading_spaces: bool = False,
):
"""Accept text as Fragments, to be split into individual lines depending
Expand All @@ -593,6 +594,7 @@ def __init__(
lateral boundaries of the enclosing TextRegion() are not vertical.
margins (sequence of floats): The extra clearance that may apply at the beginning
and/or end of a line (usually either FPDF.c_margin or 0.0 for each side).
indent (float): How much left edge is moved to the right, shortening the line.
align (Align): The horizontal alignment of the current text block.
print_sh (bool): If True, a soft-hyphen will be rendered
normally, instead of triggering a line break. Default: False
Expand All @@ -608,6 +610,7 @@ def __init__(
self.get_width = max_width
else:
self.get_width = lambda height: max_width
self.indent = indent
self.margins = margins
self.align = align
self.print_sh = print_sh
Expand All @@ -629,7 +632,7 @@ def get_line(self):

current_font_height = 0

max_width = self.get_width(current_font_height)
max_width = self.get_width(current_font_height) - self.indent
# The full max width will be passed on via TextLine to FPDF._render_styled_text_line().
current_line = CurrentLine(max_width=max_width, print_sh=self.print_sh)
# For line wrapping we need to use the reduced width.
Expand Down Expand Up @@ -659,7 +662,7 @@ def get_line(self):

if current_fragment.font_size > current_font_height:
current_font_height = current_fragment.font_size # document units
max_width = self.get_width(current_font_height)
max_width = self.get_width(current_font_height) - self.indent
current_line.max_width = max_width
for margin in self.margins:
max_width -= margin
Expand Down
22 changes: 12 additions & 10 deletions fpdf/text_region.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,9 @@ def generate_bullet_frags_and_tl(self, bullet_string: str, bullet_r_margin: floa
bullet_fragments,
max_width=self._region.get_width,
margins=(
self.pdf.c_margin + (self.indent - fragments_width - bullet_r_margin),
self.pdf.c_margin,
self._region.h_margins[0]
+ (self.indent - fragments_width - bullet_r_margin),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't indent=self.indent be provided there, instead of adding self.indent to the left margin?

self._region.h_margins[1],
),
align=self.text_align or self._region.text_align or Align.L,
wrapmode=self.wrapmode,
Expand All @@ -182,7 +183,8 @@ def build_lines(self, print_sh) -> List[LineWrapper]:
multi_line_break = MultiLineBreak(
self._text_fragments,
max_width=self._region.get_width,
margins=(self.pdf.c_margin + self.indent, self.pdf.c_margin),
margins=self._region.h_margins,
indent=self.indent,
align=self.text_align or self._region.text_align or Align.L,
print_sh=print_sh,
wrapmode=self.wrapmode,
Expand Down Expand Up @@ -222,7 +224,7 @@ def __init__(
title=None,
alt_text=None,
):
self.region = region
self._region = region
self.name = name
if align:
align = Align.coerce(align)
Expand All @@ -246,7 +248,7 @@ def build_line(self):
# We do double duty as a "text line wrapper" here, since all the necessary
# information is already in the ImageParagraph object.
self.name, self.img, self.info = preload_image(
self.region.pdf.image_cache, self.name
self._region.pdf.image_cache, self.name
)
return self

Expand All @@ -261,11 +263,11 @@ def render(self, col_left, col_width, max_height):
if self.height:
h = self.height
else:
native_h = self.info["h"] / self.region.pdf.k
native_h = self.info["h"] / self._region.pdf.k
if self.width:
w = self.width
else:
native_w = self.info["w"] / self.region.pdf.k
native_w = self.info["w"] / self._region.pdf.k
if native_w > col_width or self.fill_width:
w = col_width
else:
Expand All @@ -281,7 +283,7 @@ def render(self, col_left, col_width, max_height):
elif self.align == Align.C:
x += (col_width - w) / 2
if is_svg:
return self.region.pdf._vector_image(
return self._region.pdf._vector_image(
name=self.name,
svg=self.img,
info=self.info,
Expand All @@ -294,7 +296,7 @@ def render(self, col_left, col_width, max_height):
alt_text=self.alt_text,
keep_aspect_ratio=self.keep_aspect_ratio,
)
return self.region.pdf._raster_image(
return self._region.pdf._raster_image(
name=self.name,
img=self.img,
info=self.info,
Expand Down Expand Up @@ -519,7 +521,6 @@ def _render_column_lines(self, text_lines, top, bottom):
if (
text_rendered
and tl_wrapper.first_line
and not cur_bullet
and cur_paragraph.top_margin
and self.pdf.y > self.pdf.t_margin
):
Expand Down Expand Up @@ -631,6 +632,7 @@ def __init__(
self.balance = balance
total_w = self.extents.right - self.extents.left
col_width = (total_w - (ncols - 1) * gutter) / ncols
self.h_margins = (pdf.c_margin, pdf.c_margin)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the current usage of this attribute, I wonder if having 2 distincts attributes .l_margin & .r_margin would not make the code & intent clearer there? 🙂

# We calculate the column extents once in advance, and store them for lookup.
c_left = self.extents.left
self._cols = [Extents(c_left, c_left + col_width)]
Expand Down
Binary file added test/text_region/tcols_bullets_indent.pdf
Binary file not shown.
69 changes: 69 additions & 0 deletions test/text_region/test_text_columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,72 @@ def test_tcols_break_top_margin(tmp_path): # regression test for #1214
) as par:
par.write(text=LOREM_IPSUM)
assert_pdf_equal(pdf, HERE / "tcols_break_top_margin.pdf", tmp_path)


def test_tcols_bullets_indent(tmp_path):
"""Ensure that the top/bottom margins work with indented/bulleted
paragraphs.
Ensure that indented paragraphs have a reduced total width.
Comment on lines +333 to +335
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test 👍

"""
pdf = FPDF()
pdf.add_page()
pdf.set_font("Helvetica", "", 14)
pdf.set_top_margin(50)
pdf.set_auto_page_break(True, 50)
pdf.rect(pdf.l_margin, pdf.t_margin, pdf.epw, pdf.eph)
with pdf.text_columns(text_align="L", ncols=2) as cols:
cols.write("Paragraphs with Top Margin\n\n")
for _ in range(4):
with cols.paragraph(
text_align="J",
top_margin=pdf.font_size * 4,
) as par:
par.write(text=LOREM_IPSUM[:100])
for _ in range(5):
with cols.paragraph(
text_align="J",
top_margin=pdf.font_size * 4,
indent=10,
bullet_string="\x95",
) as par:
par.write(text=LOREM_IPSUM[:100])
pdf.add_page()
pdf.rect(pdf.l_margin, pdf.t_margin, pdf.epw, pdf.eph)
with pdf.text_columns(text_align="L", ncols=2) as cols:
cols.write("Paragraphs with Bottom Margin\n\n")
for _ in range(4):
with cols.paragraph(
text_align="J",
bottom_margin=pdf.font_size * 4,
) as par:
par.write(text=LOREM_IPSUM[:100])
for _ in range(5):
with cols.paragraph(
text_align="J",
bottom_margin=pdf.font_size * 4,
indent=10,
bullet_string="\x95",
) as par:
par.write(text=LOREM_IPSUM[:100])
pdf.add_page()
pdf.rect(pdf.l_margin, pdf.t_margin, pdf.epw, pdf.eph)
with pdf.text_columns(text_align="L", ncols=2) as cols:
cols.write("Paragraphs with Top and Bottom Margin\n\n")
for _ in range(4):
with cols.paragraph(
text_align="J",
top_margin=pdf.font_size * 1.5,
bottom_margin=pdf.font_size * 2.5,
) as par:
par.write(text=LOREM_IPSUM[:100])
for _ in range(5):
with cols.paragraph(
text_align="J",
top_margin=pdf.font_size * 1.5,
bottom_margin=pdf.font_size * 2.5,
indent=10,
bullet_string="\x95",
) as par:
par.write(text=LOREM_IPSUM[:100])
assert_pdf_equal(pdf, HERE / "tcols_bullets_indent.pdf", tmp_path)

Loading