OK, great! Two random small things that I happened to spot (but eventually I will go through all this properly):
- Instead of things like YEARMONTH_FORMAT_KEY_CAMEL_CASE, "yearmonth_format", and "year_month_format", let's use the more natural YEAR_MONTH_FORMAT, "year_month_format", and "yearMonthFormat". Same for the other temporal types as well. I assume you just tried to be consistent with "datetime", but that's actually a mistake (that's the SQL-ish name of the type, that somehow get into releases unfortunately... maybe we should add date_time etc. as an alias). - String getTemporalFormat(Temporal temporal): Maybe fine for convenience, but the basic overload should be String getTemporalFormat(Class<? extends Temporal> temporalClass), as this function doesn't really care about the instance. When you have the CLA (and are yo sure that your employee can't claim this or such), we can merge into the FREEMARKER-35 branch. When it's finished, which also means the me or some others here went through it, polished it, etc., then it will be merged into the 2.3-gae branch. Thanks! On Mon, Feb 24, 2020 at 1:28 PM Teun <t...@betterbe.com> wrote: > Ah yes the temporalFormat. > I've removed temporalFormat and added instantFormat, localDateFormat, > localDateTimeFormat, localTimeFormat, offsetDateTimeFormat, > offsetTimeFormat, yearFormat, yearMonthFormat and zonedDateTimeFormat. > > See the commit: > > https://github.com/Jaaap/freemarker/commit/b55d03d10e4e9531a85674dfed8825e9dd230d8b > In branch: > https://github.com/Jaaap/freemarker/tree/FREEMARKER-35 > > > Regards, Teun > > > On 2020-02-15, at 00:34, Daniel Dekany <daniel.dek...@gmail.com> wrote: > > > > I have created a FREEMARKER-35 branch for this activity, branching of off > > 2.3-gae. You should use that branch in your local clone as well, but > > anyhow, when you open the PR, select FREEMARKER-35 as the target branch > too. > > > > For us to accept the PR the Contributor License Agreement will be needed > > (see https://freemarker.apache.org/contribute.html). But you can create > the > > PR whenever you prefer, and later commits you make in your clone in the > > same branch will automatically belong to it till it's accepted. > > > > As of the interesting part, the code. I will review it closer eventually, > > but at first quick sight I think it can be good. The "temporal_format" > > setting surely will need refinement, because you want to be able to > specify > > one format per specific temporal type, like one for local date, one for > > local time, one for year month, etc. But of course that can be done in > > later commits too. Also, the minimum Java version will have to be > > raised from 7 to 8 for 2.3.31 (where this feature can appear earliest), > but > > w will do that then. > > > > On Fri, Feb 14, 2020 at 5:13 PM Teun <t...@betterbe.com> wrote: > > > >> Yes it is a lot of work. > >> The commit below focusses on the formatting of various Temporals, so no > >> parsing etc. > >> > >> > https://github.com/Jaaap/freemarker/commit/8bf5a91de2c47b191b5d68c7506abfbb350626ab > >> < > >> > https://github.com/Jaaap/freemarker/commit/8bf5a91de2c47b191b5d68c7506abfbb350626ab > >>> > >> > >> Should i make a pull request based on this commit or are there > >> intermediate steps to be taken? > >> > >> regards, Teun > >> > >>> On 2020-02-12, at 23:50, Daniel Dekany <daniel.dek...@gmail.com> > wrote: > >>> > >>> No, you will only needed CLA when your pull request needs to be merged. > >>> > >>> Note that adding Java 8 support will have quite a lot consequences in > FM. > >>> Many parts touched. At least in principle, we should add a lot of new > >>> TemplateModel subinterfaces... given how many types are there in > >> java.time, > >>> that will be awkward. And then integrating all that with all the places > >>> that can now explicitly handle TemplateDateModel, > TemplateDateTimeModel, > >>> and TemplateTimeModel. So, yeah, it's not accidental that nobody has > >>> invested into this yet. :) > >>> -- > >>> Best regards, > >>> Daniel Dekany > >> > >> > > > > -- > > Best regards, > > Daniel Dekany > > -- Best regards, Daniel Dekany
