rspreadsheet icon indicating copy to clipboard operation
rspreadsheet copied to clipboard

Size does not ignore formatted cells without content

Open gorn opened this issue 4 years ago • 8 comments

Seet the pull request #52 by @jglauche.

If I understand it correctly some software (which one?) saves fows with the last item beeing something like this: <table:table-cell table:number-columns-repeated="1024"/>. In the current implementation this leads to row.size include this and return number bigger than 1024, which is not probably what user expects.

Does this happens with rows as well? Can @jglauche please give more details on this issue?

gorn avatar Oct 09 '19 23:10 gorn

It seems to me that f5a4e01 only resolves this partially, what about many rows which have two empty cellspans.

gorn avatar Oct 09 '19 23:10 gorn

In my tests, I've seen some rows to have unusual size limits, gonna dig into that. I also found yet another issue that content with .fods files is not being parsed correctly, not sure if that is related:

 = Rspreadsheet.new("spec/testfile4.fods"); @sheet = r.worksheet(1)
 => #<Rspreadsheet::Worksheet:0x0000555e1aae3210 @itemcache={}, @xmlnode=#<LibXML::XML::Node:0x0000555e1aae3288>> 
2.6.3 :008 > @sheet.xmlsubnodes.map{|x| x.content}
 => ["\n     \n      mew\n     \n     \n      1,00 €\n     \n     \n    ", "\n     \n      mew\n     \n     \n     \n      10\n     \n     \n    ", "\n     \n      mew\n     \n     \n    ", "\n     \n      mew\n     \n     \n    ", "\n     \n      mewmew\n     \n     \n    ", "\n     \n      mew\n     \n     \n    ", "\n     \n      meow\n     \n     \n    ", "\n     \n      meow\n     \n     \n    ", "\n     \n      meow\n     \n     \n    ", "\n     \n    ", "\n     \n     \n      1,00 €\n     \n     \n    ", "\n     \n    ", "\n     \n      mew\n     \n     \n    ", "\n     \n     \n     \n    ", "\n     \n     \n      1,00 €\n     \n     \n    ", "\n     \n    ", "\n     \n     \n    ", "\n     \n    ", "\n     \n    "]

I don't know how to distinguish between manually created but empty cellspans vs. empty styles being automatically applied to large row/column number. Do you have a suggestions or a test case I could work with?

jglauche avatar Oct 11 '19 14:10 jglauche

I added a bunch of tests to my upstream. .fods file and gnumeric .ods file tests still fail.

It seems like merged cells are counted as individual cells: [cell1] [merged cell 2+3] [cell4] would currently behave in returning row.size of 4. I could use some input on how it's supposed to work.

jglauche avatar Oct 13 '19 12:10 jglauche

I fixed the issues with flat ods and gnumeric files by checking against content.strip.empty?

jglauche avatar Oct 13 '19 12:10 jglauche

The way it is supposed to work is that if row=[cell1] [merged cell 2+3] [cell4] than row.size should be 4. If is more row.width than "number of cells". It is a good point however, it just did not come to my mind, that there can be different approach to this.

gorn avatar Oct 13 '19 16:10 gorn

If we go by different ways to approach this, we should make it consistent for merged rows and cells My question would be how we would name the different functions that act as between merged and unmerged content and which mode would be the default?

jglauche avatar Oct 13 '19 17:10 jglauche

@jglauche Surely the behaviour must be consistent. Is the behaviour different for rows? (Sorry did not have chance to pull your code and hand test it, from diffs it looks good however)

gorn avatar Oct 13 '19 18:10 gorn

@gorn no, my current implementation is consistently showing merged cells in any direction as individual items.

jglauche avatar Oct 14 '19 12:10 jglauche

Accepting the oull request

gorn avatar Sep 28 '22 21:09 gorn