search

tfussell / xlnt

:bar_chart: Cross-platform user-friendly xlsx library for C++11+
Other
1.49k stars 418 forks source link

Streaming: skip empty rows in has_cell()/read_cell() #493

Closed adamhooper closed 3 years ago

adamhooper commented 4 years ago

Previously, an empty row would mess with the parser: if we're in an empty row, our helper methods don't detect us as being in the "row" or in the "sheetData". So has_cell() would return false when it shouldn't. Similarly, read_cell() wouldn't skip rows; so read_cell() would return an invalid cell when placed in an empty row, causing a segfault when the caller tried to use the cell.

Callers must take care to call has_next() before read_next(). In the future, perhaps we can make read_next() return a std::optional and nix has_next() altogether?

[Closes #492]

(The test file is a modified file from the Bureau of Labor Statistics -- public domain.)

adamhooper commented 4 years ago

Whoop. I'm seeing errors in my other test suite. I'll hack at this some more.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.02%) to 83.594% when pulling 3127e9c0165e175216bdc5e0063653fe8e8833f2 on adamhooper:issue-492-stream-empty-row into 3c7122a78ce9c163d26f833f8f69f43aac59a16b on tfussell:master.

adamhooper commented 4 years ago

Oh, never mind my trepidation. The errors I mentioned 40 minutes ago were because I didn't apply the patch correctly. All is well here :).

tfussell commented 4 years ago

Nice one! I'll review and merge soon.