Skip to content

Commit

Permalink
Fix indent_paragrahs/no final_gap line spacing bug
Browse files Browse the repository at this point in the history
In case of no indent_paragraphs when formatted_text is called it
passes the text to render to Text.fill_formatted_text_box. Then the text
is rendered using the Box class and the Box class in combination with
the Wrap module takes care of spacing between lines. Spacing is present
regardless of final_gap value.

In case of indent_paragraphs set the situation is different.
formatted_text itself prints the first line separately and then the rest
of the text. As such, it has the responsibility to add spacing between
the first line and the rest on its own. When final_gap is set, as it is
by default, it works just fine. When final_gap is not set though, there
was no space between the first line and the second line of the text.

Since formatted_text calls draw_indented_formatted_line to render the
first line and draw_indented_formatted_line calls
fill_formatted_text_box with the single_line flag we can use the flag as
a secondary way to trigger adding a gap.

I also added a test case for a single line scenario because my initial
take on this issue introduced a different bug that caused a single-line
paragraph and another paragraph to have too much space between them.
  • Loading branch information
jstasiak authored and pointlessone committed Jan 4, 2024
1 parent 22fec8b commit 2a10105
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/prawn/text.rb
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,13 @@ def fill_formatted_text_box(text, options)
@all_text_printed = box.everything_printed?

self.y -= box.height
self.y -= box.line_gap + box.leading if @final_gap

# If there's no remaining_text we don't really want to treat this line
# in a special way, we printed everything we wanted so the special
# single_line logic should not be triggered here.
if @final_gap || (options[:single_line] && !@all_text_printed)
self.y -= box.line_gap + box.leading
end

remaining_text
end
Expand Down
38 changes: 38 additions & 0 deletions spec/prawn/text_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,44 @@
end
end

describe 'when :final_gap is set to false, there should still be ' \
'a gap between the first line and the rest' do
it 'spaces the first and the second line correctly' do
text = 'hello ' * 30
original_y = pdf.y
# First rendering with no indentation so we know what the correct
# render height should be
pdf.text(text, final_gap: false)
height_with_no_indentation = original_y - pdf.y
y_between_paragraphs = pdf.y
# The indentation size doesn't matter, it's about triggering
# a different path in the code
pdf.text(text, indent_paragraphs: 1, final_gap: false)
height_with_indentation = y_between_paragraphs - pdf.y
expect(height_with_indentation).to be_within(0.0001)
.of(height_with_no_indentation)
end
end

describe 'single line height with no final_gap should not depend on '\
'indentation' do
it 'does not affect the height of a single line' do
text = 'hello'
original_y = pdf.y
# First rendering with no indentation so we know what the correct
# render height should be
pdf.text(text, final_gap: false)
height_with_no_indentation = original_y - pdf.y
y_between_paragraphs = pdf.y
# The indentation size doesn't matter, it's about triggering
# a different path in the code
pdf.text(text, indent_paragraphs: 1, final_gap: false)
height_with_indentation = y_between_paragraphs - pdf.y
expect(height_with_indentation).to be_within(0.0001)
.of(height_with_no_indentation)
end
end

describe 'when wrap to new page, and first line of new page' \
' is not the start of a new paragraph, that line should' \
' not be indented' do
Expand Down

0 comments on commit 2a10105

Please sign in to comment.