Bug 58339 - [PATCH] Make OFFSET() accept missing parameters
Summary: [PATCH] Make OFFSET() accept missing parameters
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: 3.13-dev
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
Keywords: PatchAvailable
Depends on:
Reported: 2015-09-07 11:37 UTC by Patrick Böker
Modified: 2016-02-17 08:53 UTC (History)
0 users

offset_missing_arg.patch (1.30 KB, patch)
2015-09-07 11:37 UTC, Patrick Böker
Details | Diff
FormulaEvalTestData.xls (174.00 KB, application/octet-stream)
2015-09-07 11:38 UTC, Patrick Böker
FormulaEvalTestData_Copy.xlsx (63.67 KB, application/octet-stream)
2015-09-07 11:38 UTC, Patrick Böker

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Böker 2015-09-07 11:37:11 UTC
Created attachment 33072 [details]

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.
Comment 1 Patrick Böker 2015-09-07 11:38:14 UTC
Created attachment 33073 [details]
Comment 2 Patrick Böker 2015-09-07 11:38:57 UTC
Created attachment 33074 [details]
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.