feat: add support to set 1904 date system based on global configuration - #435
Conversation
|
Thank you for the information. I will check to see if it is supported. |
|
I will refactor the unit tests for datewindowing ASAP. |
This reverts commit 70de920.
alaahong
left a comment
alaahong
left a comment
There was a problem hiding this comment.
if we want to implement a feature, the unit test should not just run without any assertion or verify.
as you've achieve the major part, parameter for 1904 of xlsx, so just leave comment but no blocking here.
| } | ||
|
|
||
| @Test | ||
| void test03WriteAndRead() { |
There was a problem hiding this comment.
seems no assertion or verify here. it should be throwing exception, or write warn log to keep no breaking change at least.
but the test highlight that FastExcel won't support but test pass with a true value.
There was a problem hiding this comment.
updated, others assertions in class DateWindowingListener
alaahong
left a comment
alaahong
left a comment
There was a problem hiding this comment.
Generally looks good. just leave a question for detail, please kindly help to check if you've a time.
| .excelType(ExcelTypeEnum.XLS) | ||
| .use1904windowing(Boolean.TRUE) | ||
| .sheet() | ||
| .doWrite(data()); |
There was a problem hiding this comment.
may I ask for this setting behavior? will the excel show the 1904 model since wrote with true?
There was a problem hiding this comment.
Yes,only xlsx is supported,writing to xls is not supported.If the provided files have been set up using MS Excel, there will be no problem reading xls files.
I have provided two xls files that have been set up for testing the reading of xls files.
There was a problem hiding this comment.
I tried using org.apache.poi.hssf.model.InternalWorkbook#createWorkbook(), but it couldn't set this option in the actual file. I think Apache POI no longer supports this.
There was a problem hiding this comment.
InternalWorkbook#createWorkbook() always set false ~so I thought above case still involving some confused
There was a problem hiding this comment.
😞Me too. I haven't found a way to set this for xls files yet.
There was a problem hiding this comment.
In my opinion, the test case should be same behavior if always set false by POI. Might be we can reflect to get the value in unit test, just fetch the value but not set to avoid any hack on private field.
3 participants
Purpose of the pull request
Closes: #433 , #434
What's changed?
These changes only for the
xlsxfile:setDate1904method to set 1904 date systemChecklist