From 3fdbda8037f0e9d29259c069daf9e5ab23e46b0d Mon Sep 17 00:00:00 2001 From: Jakub Stasiak Date: Mon, 24 Sep 2018 14:52:48 +0200 Subject: [PATCH] Fix indent_paragrahs/no final_gap line spacing bug 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. --- lib/prawn/text.rb | 8 +++++++- spec/prawn/text_spec.rb | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/prawn/text.rb b/lib/prawn/text.rb index 49097f42f..b535eeb2e 100644 --- a/lib/prawn/text.rb +++ b/lib/prawn/text.rb @@ -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 diff --git a/spec/prawn/text_spec.rb b/spec/prawn/text_spec.rb index 391d62c17..dfba20b62 100644 --- a/spec/prawn/text_spec.rb +++ b/spec/prawn/text_spec.rb @@ -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