Add support for LocalDate/LocalDateTime

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Add support for LocalDate/LocalDateTime

Axel Howind-3
Hi,

since version 4.0.1, POI requires at least Java 8. I suggest adding support for the new Java Date/Time API introduced with that version of the JDK, i.e. `Cell.getLocalDateValue()`  `Cell.getLocalDateTimeValue()` and two matching overloads of `Cell.setCellValue()`. What do you think? If I get positive feedback, I'll probably find the time to submit a patch after mid october.

Axel


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Add support for LocalDate/LocalDateTime

Nick Burch-2
On Fri, 27 Sep 2019, Axel Howind wrote:
> since version 4.0.1, POI requires at least Java 8. I suggest adding
> support for the new Java Date/Time API introduced with that version of
> the JDK, i.e. `Cell.getLocalDateValue()` `Cell.getLocalDateTimeValue()`
> and two matching overloads of `Cell.setCellValue()`. What do you think?
> If I get positive feedback, I'll probably find the time to submit a
> patch after mid october.

Seems like a useful update to me!

DateUtil could also potentially use similar methods too

Thanks
Nick

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Add support for LocalDate/LocalDateTime

Axel Howind-3
Ok, I'll create an RFE and add a pull request.

On 2019/09/27 15:57:08, Nick Burch <[hidden email]> wrote:

> On Fri, 27 Sep 2019, Axel Howind wrote:
> > since version 4.0.1, POI requires at least Java 8. I suggest adding
> > support for the new Java Date/Time API introduced with that version of
> > the JDK, i.e. `Cell.getLocalDateValue()` `Cell.getLocalDateTimeValue()`
> > and two matching overloads of `Cell.setCellValue()`. What do you think?
> > If I get positive feedback, I'll probably find the time to submit a
> > patch after mid october.
>
> Seems like a useful update to me!
>
> DateUtil could also potentially use similar methods too
>
> Thanks
> Nick
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Please review: Add support for new java.time API (LocalDate/LocalDateTime)

Axel Howind-3
Done. CI for the pull request still fails due to an unrelated issue (Bug 63794 - Synchronization of Github mirror is broken). When commenting out the unrelated test case, build succeeds. I hope that one can be resolved soon so that we can get this into trunk. As an alternative, I could create a patch against SVN trunk.

- DateUtil: Added new methods for LocalDate and LocalDateTime
- Cell: added getCellLocalDateTimeValue(), Cell.setCellValue(LocalDateTime) and Cell.setCellValue(LocalDate). I did not add Cell.getCellLocalDateValue() as that would truncate the time information (java.util.Date actually stores date and time, the derived(!) class java.sql.Date then strips support for time - what a mess).

I implemented Cell.setCellValue(LocalDate) as a default method in the interface. IMHO that increases maintainability. Now that Java 8 is required, there are multiple places in the code base where that could be used to make POI a little more lightweight.

Please review and comment.
Axel

On 2019/09/28 08:59:56, Axel Howind <[hidden email]> wrote:

> Ok, I'll create an RFE and add a pull request.
>
> On 2019/09/27 15:57:08, Nick Burch <[hidden email]> wrote:
> > On Fri, 27 Sep 2019, Axel Howind wrote:
> > > since version 4.0.1, POI requires at least Java 8. I suggest adding
> > > support for the new Java Date/Time API introduced with that version of
> > > the JDK, i.e. `Cell.getLocalDateValue()` `Cell.getLocalDateTimeValue()`
> > > and two matching overloads of `Cell.setCellValue()`. What do you think?
> > > If I get positive feedback, I'll probably find the time to submit a
> > > patch after mid october.
> >
> > Seems like a useful update to me!
> >
> > DateUtil could also potentially use similar methods too
> >
> > Thanks
> > Nick
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Please review: Add support for new java.time API (LocalDate/LocalDateTime)

Dominik Stadler
Hi,

thanks for the work on LocalDateTime, we will try to review and apply the
changes soon.

As you offered to look at some other topics in Apache POI that need help,
here some pointers:

* As usually, there are many open bug-reports, so if you find some time to
triage some and add unit tests to make it easy to reproduce the problem or
even patches for fixes (and unit-tests) where possible it would help a lot,
also simply asking for some more information or closing outdated/invalid
bug reports always helps
* Otherwise there is a list of more general topics at
http://poi.apache.org/devel/guidelines.html#WhereHelpNeeded which we came
up with
* In general the support for shifting/moving/copying rows is causing some
problems in XSSF/HSSF, but be aware that this is a rather low-level topic
where some initial code-research will be necessary to understand the causes
of the trouble, see
https://bz.apache.org/bugzilla/buglist.cgi?bug_id=63463%2C63352%2C63228%2C63189%2C62581%2C62328%2C62030%2C61851%2C61474%2C60902%2C60642%2C60072%2C59733%2C59731%2C59677%2C59306%2C59239%2C58348%2C58221%2C57885%2C57423%2C56454%2C56123%2C54533%2C54509%2C54470%2C54399%2C53832%2C53769%2C53678%2C53320%2C46742%2C46266&list_id=183205&order=bug_id%20DESC&query_format=advanced
for a list of related issues

Dominik.



Thanks... Dominik.


On Tue, Oct 1, 2019 at 3:10 PM Axel Howind <[hidden email]> wrote:

> Done. CI for the pull request still fails due to an unrelated issue (Bug
> 63794 - Synchronization of Github mirror is broken). When commenting out
> the unrelated test case, build succeeds. I hope that one can be resolved
> soon so that we can get this into trunk. As an alternative, I could create
> a patch against SVN trunk.
>
> - DateUtil: Added new methods for LocalDate and LocalDateTime
> - Cell: added getCellLocalDateTimeValue(),
> Cell.setCellValue(LocalDateTime) and Cell.setCellValue(LocalDate). I did
> not add Cell.getCellLocalDateValue() as that would truncate the time
> information (java.util.Date actually stores date and time, the derived(!)
> class java.sql.Date then strips support for time - what a mess).
>
> I implemented Cell.setCellValue(LocalDate) as a default method in the
> interface. IMHO that increases maintainability. Now that Java 8 is
> required, there are multiple places in the code base where that could be
> used to make POI a little more lightweight.
>
> Please review and comment.
> Axel
>
> On 2019/09/28 08:59:56, Axel Howind <[hidden email]> wrote:
> > Ok, I'll create an RFE and add a pull request.
> >
> > On 2019/09/27 15:57:08, Nick Burch <[hidden email]> wrote:
> > > On Fri, 27 Sep 2019, Axel Howind wrote:
> > > > since version 4.0.1, POI requires at least Java 8. I suggest adding
> > > > support for the new Java Date/Time API introduced with that version
> of
> > > > the JDK, i.e. `Cell.getLocalDateValue()`
> `Cell.getLocalDateTimeValue()`
> > > > and two matching overloads of `Cell.setCellValue()`. What do you
> think?
> > > > If I get positive feedback, I'll probably find the time to submit a
> > > > patch after mid october.
> > >
> > > Seems like a useful update to me!
> > >
> > > DateUtil could also potentially use similar methods too
> > >
> > > Thanks
> > > Nick
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>