Problems reading empty rows with OpenOffice

[mezza]

When trying to read an OpenOffice ODS file that contains empty rows
between rows with data, the gem seems to freeze - no errors are
logged.

As an example, suppose rows 1 and 3 contain one or more cells with
data (doesn't matter which columns) but row 2 is empty.

Here's the output from my IRB session:

---

require 'roo'
default formats are encoded in ISO-8859-1
=> true
oo = oo = Openoffice.new('/test.ods')
=> #<Openoffice:0x114e21c @doc=<UNDEFINED> ... </>, @cells_read={},
@first_row={}, @cell_type={}, @last_column={}, @filename="/test.ods",
@tmpdir="oo_1625", @file_warning=:error, @first_column={}, @cell={},
@file_nr=1, @default_sheet=nil, @last_row={}, @header_line=1,
@formula=
{}>
1.upto(3){|line| puts oo.row(line,oo.sheets.first)}
Northern bluefin tuna (E Stock)

---

And there the session gets stuck in a loop. I added a logger and think
the following patch is required:

Index: vendor/plugins/roo/lib/roo/generic_spreadsheet.rb
===================================================================
--- vendor/plugins/roo/lib/roo/generic_spreadsheet.rb (revision 681)
+++ vendor/plugins/roo/lib/roo/generic_spreadsheet.rb (working copy)
@@ -268,7 +268,7 @@
end
}
result = tmp_arr[1..-1]
- while result && result[-1] == nil
+ while result && !result.empty? && result[-1] == nil
result = result[0..-2]
end
result

[Hugh McGowan]

I've logged an issue on GitHub and will take a look. Thanks for reporting this - I'll see if I can reproduce it in my environment.

[Hugh McGowan]

I took a look this morning and have reproduced it on my side. It's
definitely in the right place. There are a few corner cases I need to
look at and while I'm in there I'll also make sure the other ways to
retrieve data (eg by column) also work with that dataset. I'll try to
get a release out in the next week or so with a fix.

Thanks!
Hugh

[mezza]

Great stuff, and thank _you_.

[mikhailov]

rails method .blank? method is more prefer than .empty?

[mezza]

Sure, if you only want to use the gem with rails applications. Better
that it doesn't have to depend on the entire rails (and associated
gems) I think for when it's used elsewhere.

Given the existing code already checks for the existence of the result
array, the only thing that's left is to ensure it's not empty, hence
my suggestion in the patch.