Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExcelNPOIStorage - Spreadsheets with blank cells #291

Closed
wants to merge 1 commit into from

Conversation

jiffgithub
Copy link

@jiffgithub jiffgithub commented Apr 11, 2018

ExtractRecords - Fix Exception index was outside the bounds of the array

The MissingCellPolicy in NPOI library is set to MissingCellPolicy.RETURN_NULL_AND_BLANK by default on both the HSSFWorkbook and XSSFWorkbook classes.

Changing the value to MissingCellPolicy.CREATE_NULL_AS_BLANK on the relevant Workbook instance resolves the issue.

FileHelpers.ExcelNPOIStorage
ExcelNPOIStorage.cs (OpenWorkbook method)
Fix on Line 101

ExtractRecords - Fix Exception index was outside the bounds of the array
@steffanv
Copy link
Contributor

I found this, because I have to deal with spreadsheets that contain empty cells.

The proposed fix works for me. Unfortunately, I had to build the project on my own so please merge the fix for the next release.

@jiffgithub jiffgithub changed the title MissingCellPolicy.CREATE_NULL_AS_BLANK ExcelNPOIStorage - Spreadsheets that contain empty cells Oct 24, 2018
@jiffgithub jiffgithub changed the title ExcelNPOIStorage - Spreadsheets that contain empty cells ExcelNPOIStorage - Spreadsheets with empty cells Oct 24, 2018
@jiffgithub jiffgithub changed the title ExcelNPOIStorage - Spreadsheets with empty cells ExcelNPOIStorage - Spreadsheets with blank cells Oct 24, 2018
@thatsjohnson
Copy link

thatsjohnson commented May 14, 2019

I also found this issue and verified the fix resolves the problem. Can this be escalated? FileHelpers is an incredibly helpful library and would benefit from this change. The 3.4 version still has this issue and would be very helpful to have fix in a official release. Thank you.

@mcavigelli
Copy link
Collaborator

Hi there

Thank you for your code. I see that this looks like a small change.
However we need to have a unittest to verify the changed behaviour.
Would that be possible for you?

Thank you, Matthias

@thatsjohnson
Copy link

thatsjohnson commented Jul 11, 2019 via email

@steffanv
Copy link
Contributor

Of course, I will work on that this week. Chris J

On Tue, Jul 9, 2019 at 8:28 AM Matthias Cavigelli @.***> wrote: Hi there Thank you for your code. I see that this looks like a small change. However we need to have a unittest to verify the changed behaviour. Would that be possible for you? Thank you, Matthias — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#291?email_source=notifications&email_token=AESRUAGOBTEUWOA3LJQTPRDP6R763A5CNFSM4EZ7RNK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZQDNVY#issuecomment-509621975>, or mute the thread https://github.com/notifications/unsubscribe-auth/AESRUAH336WB4UPFLUHYSOTP6R763ANCNFSM4EZ7RNKQ .

Dear all,

I have already done this yesterday. Please refer to Pull Request #354 (Spreadsheets with blank cells and Unit Test. #354). This one is also based on the latest code base. Thus it is easy to merge.

Steffan

@mcavigelli mcavigelli self-assigned this Aug 20, 2019
@mcavigelli
Copy link
Collaborator

Thank you for your contribution!
Your requested change is contained in #352

Matthias

@mcavigelli mcavigelli closed this Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants