Bug 58339

Summary: [PATCH] Make OFFSET() accept missing parameters
Product: POI Reporter: Patrick Böker <patrick.boeker>
Component: SS CommonAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: enhancement Keywords: PatchAvailable
Priority: P2    
Version: 3.13-dev   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: offset_missing_arg.patch
FormulaEvalTestData.xls
FormulaEvalTestData_Copy.xlsx

Description Patrick Böker 2015-09-07 11:37:11 UTC
Created attachment 33072 [details]
offset_missing_arg.patch

This patch makes the OFFSET function also work with one or both of the last two parameters missing. Excel 2013 also behaves that way.

I'll attach the changed test files separately.
test-data/spreadsheet/FormulaEvalTestData_Copy.xlsx
test-data/spreadsheet/FormulaEvalTestData.xls
Comment 1 Patrick Böker 2015-09-07 11:38:14 UTC
Created attachment 33073 [details]
FormulaEvalTestData.xls
Comment 2 Patrick Böker 2015-09-07 11:38:57 UTC
Created attachment 33074 [details]
FormulaEvalTestData_Copy.xlsx
Comment 3 Patrick Böker 2016-02-15 09:29:23 UTC
Is there anything I can do to get this merged or is it just missing tuits?
Comment 4 Javen O'Neal 2016-02-15 18:50:31 UTC
Is there an ignored failing unit test to show that the offset function doesn't work when one or both of the last two parameters are missing? If not, that's probably all this bug needs before it can be merged.
Comment 5 Javen O'Neal 2016-02-15 19:17:24 UTC
Nevermind, TestFormulasFromSpreadsheet.java handles the extra tests. No need for new unit tests.

The current attached xls/xlsx files fail at TestFormulasFromSpreadsheet.java because their locale is DE, so they use OktIndex rather than Oct2Dec, and other locale-specific functions. However, Offset passes, so the changes are good.

I don't have access to EN locale Excel to copy your edits over at the moment. I'll get to this in a week if no one else beats me to it. Presumably the only changes were the addition of Offset formulas:

H1040 =SUM(OFFSET(K7:L8,0,0,1,1))
I1040 =SUM(OFFSET(K7:L8,0,0,1,))
J1040 =SUM(OFFSET(K7:L8,0,0,,1))
K1040 =SUM(OFFSET(K7:L8,0,0,1))
L1040 =SUM(OFFSET(K7:L8,0,0))
M1040 =SUM(OFFSET(K7:L8,0,0,,))
N1040 =SUM(OFFSET(K7:L8,0,0,))
Comment 6 Javen O'Neal 2016-02-15 20:30:49 UTC
Merged in r1730606 and r1730607.

Thanks for the patch, Patrick!
Comment 7 Patrick Böker 2016-02-17 08:53:01 UTC
As you said, those seven added tests were the only changes in the Excel files.