|Summary:||Can't read in rows without 'r' attribute|
|Product:||POI||Reporter:||John Claxton <jclaxton>|
|Component:||XSSF||Assignee:||POI Developers List <dev>|
Patch file for a potential fix & tests.
Description John Claxton 2020-02-20 17:33:36 UTC
Comment 1 John Claxton 2020-02-20 17:44:48 UTC
Created attachment 37030 [details] Proposed patch.tar.gz
Comment 2 Axel Howind 2020-02-20 20:19:10 UTC
IMHO this is a good idea. But consider a file where the row numbers are missing in between. Say the last row before the rows with missing numbers had the number n. Then the next row will be assigned the same number. That doesn’t look right Another thing to consider is that we might have to re-adjust row numbers of the following rows. Consider rows 1,2,3 all with correct row numbers. Then 5 rows without row numbers, followed by a row with number 4. How should row numbers be handled then? What does Excel do in that case? Do you have (or can you create) excel sheets for all three case? 1. row numbers missing from the start of the file 2. row numbers missing in between 3. row numbers missing in between, but would use the same row numbers as the following rows We should really run this on these test sheets and also create a unit test that covers all three cases. Thanks for contributing! Axel
Comment 3 John Claxton 2020-02-20 20:30:45 UTC
Good point, I didn't really consider sheets with partial row numbers, my examples were all or nothing. I'll send a new patch file that covers all three cases. Thanks, John Claxton
Comment 4 Dominik Stadler 2021-01-03 18:49:22 UTC
Any plans to provide the mentioned updated patch?
Comment 5 John Claxton 2021-02-09 15:45:26 UTC
Created attachment 37733 [details] Patch file for a potential fix & tests. Here's a new patch, with some caveats. 1. I was having lots of trouble setting up my project and couldn't get tests to run. It seems like a problem with my Ant setup, not the code itself, so hopefully the tests all pass. My apologies if they don't. (Through a kluge, I did run the tests that I added in this patch, and they pass.) 2. For the third case Axel mentioned, I'm not sure what the correct behavior would be. Excel interprets that as invalid xml, and does its best to recover, but it's not clear what logic it uses. So I wrote a test for that, but commented it out for now. There are some further details in a test comment in the patch file.