prawn icon indicating copy to clipboard operation
prawn copied to clipboard

Add :dry_run to text_box and formatted_text_box

Open iamjohnford opened this issue 9 years ago • 3 comments

I made a patch based on @practicingruby's suggestions on issue #763. The main changes are that text_box and formatted_text_box return the box object and allow access to attributes such as font_size and height. Passing :dry_run => true provides a way to get font sizes from :shrink_to_fit and box heights before rendering text to the pdf.

iamjohnford avatar Sep 27 '15 15:09 iamjohnford

Thanks for the PR! My concern is that this would change the return value for an API that was marked as stable, which would necessitate a major version bump.

I think I get what you are trying to do and agree this result could be more useful. What does everyone think of having this functionality in a new lower level API call that you could make and get the box return value, and the existing API call calls that lower level value, then returns the render result so we maintain backwards API compatibility?

I'm also concerned with you changed an API return and it didn't look like you had to change an existing test so we probably have at least one missing test that should be written as well.

packetmonkey avatar Sep 28 '15 19:09 packetmonkey

Yeah, I noticed your concern when I was working on the patch. I probably should have asked about that sooner.

One thing I noticed while testing this was that height_of doesn't take into account :inline_format. It should, right? So instead of:

def height_of(string, options = {})
  height_of_formatted([{ :text => string }], options)
end

It should be something like:

def height_of(string, options = {})
  options = options.dup

  if p = options[:inline_format]
    p = [] unless p.is_a?(Array)
    options.delete(:inline_format)
    array = self.text_formatter.format(string, *p)
  else
    array = [{ :text => string }]
  end

  height_of_formatted(array, options)
end

The heights are pretty different between the two as you can see with this example:

Prawn::Document.generate("test-height-of.pdf") do
  string = "Prawn love <sup>superscript</sup> " * 10

  options = {
    inline_format: true,
    size: 20,
    width: 180
  }

  height = height_of(string, options)
  puts "height_of: #{height}"
  bounding_box [0, bounds.height], width: options[:width], height: height do
    stroke_bounds
    text string, options
  end

  array = text_formatter.format(string, options)
  height = height_of_formatted(array, options)
  puts "height_of_formatted: #{height}"
  bounding_box [options[:width] + 10, bounds.height], width: options[:width], height: height do
    stroke_bounds
    text string, options
  end
end

Does that seem correct? I'm happy to create a pull request with a patch for that.

iamjohnford avatar Sep 30 '15 04:09 iamjohnford

@packetmonkey: What if we only return the box when :dry_run is set? This is backwards compatible because it'd keep the return value the same for any code following the 2.0 API, but would provide the box object if you use the newly introduced :dry_run param

In Prawn 3.0, we could consider unifying the interface.

practicingruby avatar Mar 29 '16 22:03 practicingruby