prawnpdf / prawn

Fast, Nimble PDF Writer for Ruby
https://prawnpdf.org
Other
4.67k stars 688 forks source link

colspan table with fixed width #502

Closed ignasio closed 11 years ago

ignasio commented 11 years ago

When I trying to create table like this http://rghost.ru/46587227.view with prawn, it cause CannotFit error.Code below first = {:content=>"Foooo fo foooooo",:width=>50,:align=>:center} second = {:content=>"Foooo",:colspan=>2,:width=>70,:align=>:center} third = {:content=>"fooooooooooo, fooooooooooooo, fooo, foooooo fooooo",:width=>55,:align=>:center} fourth = {:content=>"Bar",:width=>15,:align=>:center}

table_content = [[ first, [[second],[third,fourth]] ]] pdf.move_down(20) pdf.table(table_content)

and when second width is 100 and third and forth 50 work fine, but this not for me

ahacking commented 11 years ago

This sounds like a duplicate of #407, and a similar issue to what I'm having, although I am not specifying any widths in each row, but instead using the column_widths option. Either way it isnt working for me.

hbrandl commented 11 years ago

Since this bug was referenced in issue #407, I tried to reproduce / fix it.

To reproduce this bug, I used the following test case

describe "bugreport" do
  it "illustrates issue #502" do
    pdf = Prawn::Document.new
    first = {:content=>"Foooo fo foooooo",:width=>50,:align=>:center}
    second = {:content=>"Foooo",:colspan=>2,:width=>70,:align=>:center}
    third = {:content=>"fooooooooooo, fooooooooooooo, fooo, foooooo fooooo",:width=>55,:align=>:center}
    fourth = {:content=>"Bar",:width=>15,:align=>:center}

    table_content = [[
    first,
    [[second],[third,fourth]]
    ]]
    pdf.move_down(20)
    pdf.table(table_content)

  end
end

Going back to commit 386998effa7ce299c749f47063d7ff426b8f980a, I was able to reproduce this bug.

1) Prawn::Table You can explicitly set the column widths and use a colspan > 1 illustrates issue #502
 Failure/Error: pdf.table(table_content)
 Prawn::Errors::CannotFit:
   Table's width was set larger than its contents' maximum width (max width 70, requested 90.0)
 # ./lib/prawn/table.rb:372:in `column_widths'
 # ./lib/prawn/table.rb:551:in `set_column_widths'
 # ./lib/prawn/table.rb:137:in `initialize'
 # ./lib/prawn/table/cell.rb:185:in `new'
 # ./lib/prawn/table/cell.rb:185:in `make'
 # ./lib/prawn/table.rb:439:in `block (2 levels) in make_cells'
 # ./lib/prawn/table.rb:433:in `each'
 # ./lib/prawn/table.rb:433:in `block in make_cells'
 # ./lib/prawn/table.rb:431:in `each'
 # ./lib/prawn/table.rb:431:in `make_cells'
 # ./lib/prawn/table.rb:128:in `initialize'
 # ./lib/prawn/table.rb:27:in `new'
 # ./lib/prawn/table.rb:27:in `table'
 # ./spec/table_spec.rb:62:in `block (3 levels) in <top (required)>'

To reproduce clone repository at https://github.com/hbrandl/prawn/tree/issue_502 and run rspec.

This test case passes without any problems using my fixes for issue #407 and #533.

After pull request #534 is merged, one may add this test case, but I'm not sure if it is necessary.

johnnyshields commented 11 years ago

@hbrandl I've tried your fixes for #502, #533, and #407, however I think I've come across one additional edge case. Please try this code with 3 levels of table nesting:

    data = [[{content: make_table([[make_table([['foo'*100]])],[make_table([['foo'*100]])]], column_widths: [bounds.width/2] *2), colspan: 12}]]
    table(data, column_widths: [bounds.width / 12] * 12)
hbrandl commented 11 years ago

@johnnyshields If you can build a failing test case, I'll look into the issue. Please try to minify the test case as much as possible so I can look at the isolated problem. A sketch of the expected pdf output would also be greatly appreciated. As well as any further info that you can provide.

And while I'm not affiliated with prawnpdf, I would guess, that a new issue would be warranted, since this seems like a much more complex bug.

johnnyshields commented 11 years ago

@hbrandl try this:

    it "should allow long strings in subtables when parent is width-constrained" do
      pdf = Prawn::Document.new
      child_1 = pdf.make_table([['foo'*100]])
      child_2 = pdf.make_table([['foo']])
      pdf.table([[child_1], [child_2]], column_widths: [pdf.bounds.width/2]*2)
    end
hbrandl commented 11 years ago

@johnnyshields Thank you. I'm getting the following output

1) Prawn::Table You can explicitly set the column widths and use a colspan > 1 should allow long strings in subtables when parent is width-constrained
 Failure/Error: pdf.table([[child_1], [child_2]], column_widths: [pdf.bounds.width/2]*2)
 Prawn::Errors::CannotFit:
   Table's width was set too small to contain its contents (min width 540.0, requested 270.0)
 # ./lib/prawn/table.rb:366:in `column_widths'
 # ./lib/prawn/table.rb:551:in `set_column_widths'
 # ./lib/prawn/table.rb:137:in `initialize'
 # ./lib/prawn/table.rb:27:in `new'
 # ./lib/prawn/table.rb:27:in `table'
 # ./spec/table_spec.rb:94:in `block (3 levels) in <top (required)>'

It is an error and probably a bug, but at least it's error description fits.

I tried to hunt the bug down, but was not able to find a cause. All I could find is some very strange behaviour of ruby (1.9.3 and 1.8.7) or rpsec (2.14.1) or prawn in the aggregate_cell_value function. Some puts are called and some aren't. To illustrate, take the code below and replace the current aggregate_cell_values function (only a few puts should be added)

  def aggregate_cell_values(row_or_column, meth, aggregate)
    values = {}
    puts "entering aggregate_cell_values"
    #calculate values for all cells that do not span accross multiple cells
    #this ensures that we don't have a problem if the first line includes
    #a cell that spans across multiple cells
    each do |cell|
      #don't take spanned cells
      if cell.colspan == 1 and cell.class != Prawn::Table::Cell::SpanDummy
        index = cell.send(row_or_column)
        values[index] = [values[index], cell.send(meth)].compact.send(aggregate)
      end
      index = cell.send(row_or_column)
      puts "A"
      puts "B values[index]=#{values[index]} cell.send(meth)=#{cell.send(meth)}"
      puts "values=#{values}"
    end

    each do |cell|
      index = cell.send(row_or_column)          
      if cell.colspan > 1
        #calculate current (old) return value before we do anything
        old_sum = 0
        cell.colspan.times { |i|
          old_sum += values[index+i] unless values[index+i].nil?
        }

        #calculate future return value 
        new_sum = cell.send(meth) * cell.colspan

        if new_sum >= old_sum
          #not entirely sure why we need this line, but with it the tests pass
          values[index] = [values[index], cell.send(meth)].compact.send(aggregate) 
          #overwrite the old values with the new ones, but only if all entries existed
          entries_exist = true
          cell.colspan.times { |i| entries_exist = false if values[index+i].nil? }
          cell.colspan.times { |i|
            values[index+i] = cell.send(meth) if entries_exist
          }
        end
      else
        if cell.class == Prawn::Table::Cell::SpanDummy
          values[index] = [values[index], cell.send(meth)].compact.send(aggregate) 
        end
      end
    end
    values.values.inject(0, &:+)
  end

The expected output would be something along the lines of entering aggregate_cell_values A B values[index]=... values=... A B values[index]=... values=...

But on my system it is A B values[index]=... values=... B values[index]=... values=...

I can't come up with any reason why the puts "A" wouldn't execute.

Since I don't feel like chasing a bug in ruby or rspec down and/or since I can't see my mistake. I'm giving up on this bug.

I'd suggest you file a new bug report, since this bug may be related, but is definitely a bug on a deeper level.