You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Sergiu Dumitriu <se...@gmail.com> on 2016/11/21 06:22:00 UTC

Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Although RFC 3339 says that "Applications [...] may choose, for the sake
of readability, to specify a full-date and full-time separated by (say)
a space character", the standard is to use T as a separator.

And if I'm already nagging, ISO and RFC are two different things, as are
ISO-8601 and RFC 3339, so don't use ISO in the name and RFC in the
comments as if they're interchangeable, pick one and stick with it.

While the RFC allows space as the separator, ISO-8601 actually requires
T as the date-time separator.

On 11/20/2016 05:19 AM, cbrisson@apache.org wrote:
> Author: cbrisson
> Date: Sun Nov 20 10:19:41 2016
> New Revision: 1770547
> 
> URL: http://svn.apache.org/viewvc?rev=1770547&view=rev
> Log:
> [tools] introduce 'iso' (date/time with time zone, locale independent) and 'intl' (date/time without time zone, locale and time zone independent) formats
> 
> Modified:
>     velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/ConversionUtils.java
>     velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/generic/DateTool.java
> 
> Modified: velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/ConversionUtils.java
> URL: http://svn.apache.org/viewvc/velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/ConversionUtils.java?rev=1770547&r1=1770546&r2=1770547&view=diff
> ==============================================================================
> --- velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/ConversionUtils.java (original)
> +++ velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/ConversionUtils.java Sun Nov 20 10:19:41 2016
> @@ -30,7 +30,9 @@ import java.text.SimpleDateFormat;
>  import java.util.Collection;
>  import java.util.Date;
>  import java.util.Calendar;
> +import java.util.HashMap;
>  import java.util.Locale;
> +import java.util.Map;
>  import java.util.TimeZone;
>  import java.util.concurrent.ConcurrentMap;
>  import java.util.concurrent.ConcurrentHashMap;
> @@ -52,6 +54,18 @@ public class ConversionUtils
>      //NOTE: '3' belongs to a non-public "scientific" style
>      private static final int STYLE_INTEGER      = 4;
>  
> +    /* Java DateFormat standard constants extensions */
> +    private static final int STYLE_INTL = 5; /* international format without time zone*/
> +    private static final int STYLE_ISO = 6;  /* international format with timezone (RFC 3339) */
> +
> +
> +    /* iso/intl date/time formats (locale-independant) */
> +    private static DateFormat isoDateFormat = new SimpleDateFormat("yyyy-MM-dd");  /* date for iso/intl */
> +    private static DateFormat intlTimeFormat = new SimpleDateFormat("HH:mm:ss");   /* time without timezone */
> +    private static DateFormat isoTimeFormat = new SimpleDateFormat("HH:mm:ssXXX"); /* time with ISO 8601 timezone */
> +    private static DateFormat intlTimestampFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");   /* timestamp without timezone */
> +    private static DateFormat isoTimestampFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ssXXX"); /* timestamp with ISO 88601 timezone */
> +
>      // cache custom formats
>      private static ConcurrentMap<String,NumberFormat> customFormatsCache = new ConcurrentHashMap<String,NumberFormat>();
>  
> @@ -401,19 +415,53 @@ public class ConversionUtils
>              else if (timeStyle < 0)
>              {
>                  // only a date style was specified
> -                df = DateFormat.getDateInstance(dateStyle, locale);
> +                switch (dateStyle)
> +                {
> +                    case STYLE_INTL:
> +                    case STYLE_ISO:
> +                        df = isoDateFormat;
> +                        break;
> +                    default:
> +                        df = DateFormat.getDateInstance(dateStyle, locale);
> +                        df.setTimeZone(timezone);
> +                        break;
> +                }
>              }
>              else if (dateStyle < 0)
>              {
>                  // only a time style was specified
> -                df = DateFormat.getTimeInstance(timeStyle, locale);
> +                switch (timeStyle)
> +                {
> +                    case STYLE_INTL:
> +                        df = intlTimeFormat;
> +                        break;
> +                    case STYLE_ISO:
> +                        df = (DateFormat) isoTimeFormat.clone();
> +                        df.setTimeZone(timezone);
> +                        break;
> +                    default:
> +                        df = DateFormat.getTimeInstance(timeStyle, locale);
> +                        df.setTimeZone(timezone);
> +                        break;
> +                }
>              }
>              else
>              {
> -                df = DateFormat.getDateTimeInstance(dateStyle, timeStyle,
> -                                                    locale);
> +                switch (dateStyle)
> +                {
> +                    case STYLE_INTL:
> +                        df = intlTimestampFormat;
> +                        break;
> +                    case STYLE_ISO:
> +                        df = (DateFormat) isoTimestampFormat.clone();
> +                        df.setTimeZone(timezone);
> +                        break;
> +                    default:
> +                        df = DateFormat.getDateTimeInstance(dateStyle, timeStyle, locale);
> +                        df.setTimeZone(timezone);
> +                        break;
> +                }
>              }
> -            df.setTimeZone(timezone);
>              return df;
>          }
>          catch (Exception suppressed)
> @@ -423,6 +471,19 @@ public class ConversionUtils
>          }
>      }
>  
> +    static Map<String, Integer> stylesMap;
> +    static
> +    {
> +        stylesMap = new HashMap<String, Integer>();
> +        stylesMap.put("FULL", DateFormat.FULL);
> +        stylesMap.put("LONG", DateFormat.LONG);
> +        stylesMap.put("MEDIUM", DateFormat.MEDIUM);
> +        stylesMap.put("SHORT", DateFormat.SHORT);
> +        stylesMap.put("DEFAULT", DateFormat.DEFAULT);
> +        stylesMap.put("INTL", STYLE_INTL);
> +        stylesMap.put("ISO", STYLE_ISO);
> +    }
> +
>      /**
>       * Checks a string to see if it matches one of the standard DateFormat
>       * style patterns: full, long, medium, short, or default.  If it does,
> @@ -435,32 +496,8 @@ public class ConversionUtils
>       */
>      public static int getDateStyleAsInt(String style)
>      {
> -        // avoid needlessly running through all the string comparisons
> -        if (style == null || style.length() < 4 || style.length() > 7) {
> -            return -1;
> -        }
> -        if (style.equalsIgnoreCase("full"))
> -        {
> -            return DateFormat.FULL;
> -        }
> -        if (style.equalsIgnoreCase("long"))
> -        {
> -            return DateFormat.LONG;
> -        }
> -        if (style.equalsIgnoreCase("medium"))
> -        {
> -            return DateFormat.MEDIUM;
> -        }
> -        if (style.equalsIgnoreCase("short"))
> -        {
> -            return DateFormat.SHORT;
> -        }
> -        if (style.equalsIgnoreCase("default"))
> -        {
> -            return DateFormat.DEFAULT;
> -        }
> -        // ok, it's not any of the standard patterns
> -        return -1;
> +        Integer intStyle = stylesMap.get(style.toUpperCase());
> +        return intStyle == null ? -1 : intStyle.intValue();
>      }
>  
>  
> 
> Modified: velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/generic/DateTool.java
> URL: http://svn.apache.org/viewvc/velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/generic/DateTool.java?rev=1770547&r1=1770546&r2=1770547&view=diff
> ==============================================================================
> --- velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/generic/DateTool.java (original)
> +++ velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools/generic/DateTool.java Sun Nov 20 10:19:41 2016
> @@ -28,16 +28,22 @@ import java.util.Calendar;
>  import java.util.Locale;
>  import java.util.TimeZone;
>  import org.apache.velocity.tools.ConversionUtils;
> -import org.apache.velocity.tools.ToolContext;
>  import org.apache.velocity.tools.config.DefaultKey;
>  
>  /**
> - * Tool for working with {@link Date} and {@link Calendar}
> + * <p>Tool for working with {@link Date} and {@link Calendar}
>   * in Velocity templates.  It is useful for accessing and
>   * formatting the "current" date as well as for formatting
>   * arbitrary {@link Date} and {@link Calendar} objects. Also
>   * the tool can be used to retrieve {@link DateFormat} instances
> - * or make conversions to and from various date types.
> + * or make conversions to and from various date types.</p>
> + * <p>Possible formats include:
> + * <ul>
> + *     <li>'short', 'medium', 'long', 'full' (from {@link java.text.DateFormat}, optionally suffixed by '_time' or '_date' to get a specific time or date format</li>
> + *     <li>'intl' for RFC 3339 without time zone (locale and time-zone independent), optionally suffixed by '_time' or '_date' to get a specific time or date format</li>
> + *     <li>'iso' for RFC 3339 with time zone (locale independent), optionally suffixed by '_time' or '_date' to get a specific time or date format</li>
> + *     <li>a custom format, as specified in {@link SimpleDateFormat}</li>
> + * </ul></p>
>   * <p><pre>
>   * Example of formatting the "current" date:
>   *  $date                         -> Oct 19, 2003 9:54:50 PM
> @@ -46,6 +52,7 @@ import org.apache.velocity.tools.config.
>   *  $date.full_date               -> Sunday, October 19, 2003
>   *  $date.get('default','short')  -> Oct 19, 2003 9:54 PM
>   *  $date.get('yyyy-M-d H:m:s')   -> 2003-10-19 21:54:50
> + *  $date.iso                     -> 2003-10-19 21:54:50-07:00
>   *
>   * Example of formatting an arbitrary date:
>   *  $myDate                        -> Tue Oct 07 03:14:50 PDT 2003
> @@ -77,7 +84,7 @@ public class DateTool extends FormatConf
>       */
>      public static final String TIMEZONE_KEY = "timezone";
>  
> -    private TimeZone timezone = TimeZone.getDefault(); 
> +    private TimeZone timezone = TimeZone.getDefault();
>  
>      /**
>       * Does the actual configuration. This is protected, so
> @@ -567,7 +574,7 @@ public class DateTool extends FormatConf
>       * @param locale the {@link Locale} to be used
>       * @param timezone the {@link TimeZone} to be used
>       * @return an instance of {@link DateFormat}
> -     * @see #getDateFormat(int timeStyle, int dateStyle, Locale locale, TimeZone timezone)
> +     * @see {@link DateFormat#getDateFormat(int timeStyle, int dateStyle, Locale locale, TimeZone timezone)}
>       * @since VelocityTools 1.1
>       */
>      public DateFormat getDateFormat(String dateStyle, String timeStyle,
> 
> 


-- 
Sergiu Dumitriu
http://purl.org/net/sergiu/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Claude Brisson <cl...@renegat.net>.
>>> 6. I consider 435 to 441 being inconsisting to their counterpart 420
>>> to 423.
>>>
>>
>> Oh, you're speaking about line numbers in ConversionUtils.java... why is
>> it inconsistent? When we're speaking about date only, there is no
>> timezone or separator involved, so it's the same format.
>
> Yes, but it seems to be superseded by another commit already.
>

Which still has the problem, but I'll fix it, thanks.

>>> I would really try to make a difference between ISO and ISO for humans
>>> because people mostly do not care about timezones, they want to have
>>> it in their local one.
>>>
>>> iso, iso-human, iso-tz (does not apply to date only), iso-human-tz
>>> (does not apply to date only) (ideally with an Olson TZ ID?)
>>>
>>
>> I like it.
>>
>> By Olson TZ ID, I guess you refer to "zzz" (aka CET), and not "zzzz"
>> (aka Central European Time), don't you?
>
> No, never ever use this braindead RFC three-letter timezones. They are 
> not standardized, not unambigious. Invented in the dark ages of the 
> Internet.
>
> See here: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
> They are fully supported by java.util.TimeZone.
>
> Unfortunately, SimpleDateFormat does not support them as format option.
> I highly recommend to drop those bits in r1771188.
>

Ok, which means I have to replace zzz by TimeZone.getId(), correct?


   Claude


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Michael Osipov <mi...@apache.org>.
Salut Claude,

Am 2016-11-29 um 17:22 schrieb Claude Brisson:
> The DateTool should now be fixed:
>
> - the stupid bug about date formats ignoring time zone has been fixed
>
> - the new standard formats are iso / iso_tz for ISO 8601, and intl /
> intl_tz for the human-readable versions which use time zone IDs for
> formatting.
>
> See commit http://svn.apache.org/viewvc?view=revision&revision=1771916
>
> All of this would have been simpler to code using java8 java.time or
> joda-time libs, but each version has enough reenginering trouble of its
> own. Plus, SimpleDateFormat not being thread-safe, a new format is
> instantiated for each parsing or rendering. All of this leaves a bit of
> an aftertaste of sub-optimality,
>
> I totally agree with the fact that datetimes should be formatted into
> users timezone as often as possible. But we still have to rely on the
> webapp author to convey users timezones...

just checked the commit. The new code looks really great now, clean and 
consistent. However, the comments have a few typos: ISO-8601, ISO 88601. 
Replace it with "ISO 8601" and you are done.

Thanks for making Velocity even better...

Merci,

Michael

> On 26/11/2016 02:08, Michael Osipov wrote:
>> Am 2016-11-25 um 01:08 schrieb Claude Brisson:
>>>
>>>>> By Olson TZ ID, I guess you refer to "zzz" (aka CET), and not "zzzz"
>>>>> (aka Central European Time), don't you?
>>>>
>>>> No, never ever use this braindead RFC three-letter timezones. They are
>>>> not standardized, not unambigious. Invented in the dark ages of the
>>>> Internet.
>>>>
>>>> See here: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
>>>> They are fully supported by java.util.TimeZone.
>>>>
>>>> Unfortunately, SimpleDateFormat does not support them as format option.
>>>> I highly recommend to drop those bits in r1771188.
>>>>
>>>
>>> Ok, which means I have to replace zzz by TimeZone.getId(), correct?
>>
>> Yes.
>>
>>
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Claude Brisson <cl...@renegat.net>.
The DateTool should now be fixed:

- the stupid bug about date formats ignoring time zone has been fixed

- the new standard formats are iso / iso_tz for ISO 8601, and intl / 
intl_tz for the human-readable versions which use time zone IDs for 
formatting.

See commit http://svn.apache.org/viewvc?view=revision&revision=1771916

All of this would have been simpler to code using java8 java.time or 
joda-time libs, but each version has enough reenginering trouble of its 
own. Plus, SimpleDateFormat not being thread-safe, a new format is 
instantiated for each parsing or rendering. All of this leaves a bit of 
an aftertaste of sub-optimality,

I totally agree with the fact that datetimes should be formatted into 
users timezone as often as possible. But we still have to rely on the 
webapp author to convey users timezones...

   Claude

On 26/11/2016 02:08, Michael Osipov wrote:
> Am 2016-11-25 um 01:08 schrieb Claude Brisson:
>>
>>>> By Olson TZ ID, I guess you refer to "zzz" (aka CET), and not "zzzz"
>>>> (aka Central European Time), don't you?
>>>
>>> No, never ever use this braindead RFC three-letter timezones. They are
>>> not standardized, not unambigious. Invented in the dark ages of the
>>> Internet.
>>>
>>> See here: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
>>> They are fully supported by java.util.TimeZone.
>>>
>>> Unfortunately, SimpleDateFormat does not support them as format option.
>>> I highly recommend to drop those bits in r1771188.
>>>
>>
>> Ok, which means I have to replace zzz by TimeZone.getId(), correct?
>
> Yes.
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Michael Osipov <mi...@apache.org>.
Am 2016-11-24 um 08:29 schrieb Claude Brisson:
> Glad to receive an enlightened opinion on the subject.
>
>> 4. "yyyy-MM-dd'T'HH:mm:ssX and "yyyy-MM-dd HH:mm:ssX" are incomplete
>> if you have offsets by 30 or 45 minutes. You are simply truncating
>> them. Unless you know and you probably don't, always stick with XXX in
>> extended format.
>
> It's XXX in the code. ssX appeared only in the mail discussion on the
> dev list.

I know, I am just telling intentionally not have this copied to the code.

>> 6. I consider 435 to 441 being inconsisting to their counterpart 420
>> to 423.
>>
>
> Oh, you're speaking about line numbers in ConversionUtils.java... why is
> it inconsistent? When we're speaking about date only, there is no
> timezone or separator involved, so it's the same format.

Yes, but it seems to be superseded by another commit already.

>> I would really try to make a difference between ISO and ISO for humans
>> because people mostly do not care about timezones, they want to have
>> it in their local one.
>>
>> iso, iso-human, iso-tz (does not apply to date only), iso-human-tz
>> (does not apply to date only) (ideally with an Olson TZ ID?)
>>
>
> I like it.
>
> By Olson TZ ID, I guess you refer to "zzz" (aka CET), and not "zzzz"
> (aka Central European Time), don't you?

No, never ever use this braindead RFC three-letter timezones. They are 
not standardized, not unambigious. Invented in the dark ages of the 
Internet.

See here: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
They are fully supported by java.util.TimeZone.

Unfortunately, SimpleDateFormat does not support them as format option.
I highly recommend to drop those bits in r1771188.

Michael


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Claude Brisson <cl...@renegat.net>.

On 24/11/2016 20:41, Michael Osipov wrote:
> Am 2016-11-24 um 17:40 schrieb Sergiu Dumitriu:
>> On 11/24/2016 02:29 AM, Claude Brisson wrote:
>>>> 6. I consider 435 to 441 being inconsisting to their counterpart 420
>>>> to 423.
>>>>
>>>
>>> Oh, you're speaking about line numbers in ConversionUtils.java... 
>>> why is
>>> it inconsistent? When we're speaking about date only, there is no
>>> timezone or separator involved, so it's the same format.
>>>
>>
>> Not necessarily, depends on what you mean by "date only". If I'm talking
>> with someone on the other side of the dateline, and ask him what _date_
>> is now, he'll answer something different than what I know is the date.
>>
>> With the current code, the same date could be printed inconsistently as:
>>
>> 2001-02-03
>> 2001-02-02T23:05:06-05:00
>>
>> So, if by date we mean "date in Greenwich", then yes, the two formats
>> are the same, but if we mean "local date", then it depends on where
>> "local" is.
>
> Exactly, users always expects that everything is normalized to their 
> timezone. A very bad example for this is Git command line. By default 
> it shows the commit datetime with the timezone offset of the 
> committer/author, doing "git log" on Maven master gives me six 
> different offsets, WTF! Impossible to read w/o changing to 'iso-local'.
>
>> If we want dates to be timezone agnostic, then we must explicitly set
>> the timezone offset to 0.
>
> Exactly!
>

You're definitely right, we need to take it into account.


   Claude

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Michael Osipov <mi...@apache.org>.
Am 2016-11-24 um 17:40 schrieb Sergiu Dumitriu:
> On 11/24/2016 02:29 AM, Claude Brisson wrote:
>>> 6. I consider 435 to 441 being inconsisting to their counterpart 420
>>> to 423.
>>>
>>
>> Oh, you're speaking about line numbers in ConversionUtils.java... why is
>> it inconsistent? When we're speaking about date only, there is no
>> timezone or separator involved, so it's the same format.
>>
>
> Not necessarily, depends on what you mean by "date only". If I'm talking
> with someone on the other side of the dateline, and ask him what _date_
> is now, he'll answer something different than what I know is the date.
>
> With the current code, the same date could be printed inconsistently as:
>
> 2001-02-03
> 2001-02-02T23:05:06-05:00
>
> So, if by date we mean "date in Greenwich", then yes, the two formats
> are the same, but if we mean "local date", then it depends on where
> "local" is.

Exactly, users always expects that everything is normalized to their 
timezone. A very bad example for this is Git command line. By default it 
shows the commit datetime with the timezone offset of the 
committer/author, doing "git log" on Maven master gives me six different 
offsets, WTF! Impossible to read w/o changing to 'iso-local'.

> If we want dates to be timezone agnostic, then we must explicitly set
> the timezone offset to 0.

Exactly!

Michael


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Sergiu Dumitriu <se...@gmail.com>.
On 11/24/2016 02:29 AM, Claude Brisson wrote:
>> 6. I consider 435 to 441 being inconsisting to their counterpart 420
>> to 423.
>>
> 
> Oh, you're speaking about line numbers in ConversionUtils.java... why is
> it inconsistent? When we're speaking about date only, there is no
> timezone or separator involved, so it's the same format.
> 

Not necessarily, depends on what you mean by "date only". If I'm talking
with someone on the other side of the dateline, and ask him what _date_
is now, he'll answer something different than what I know is the date.

With the current code, the same date could be printed inconsistently as:

2001-02-03
2001-02-02T23:05:06-05:00

So, if by date we mean "date in Greenwich", then yes, the two formats
are the same, but if we mean "local date", then it depends on where
"local" is.

If we want dates to be timezone agnostic, then we must explicitly set
the timezone offset to 0.
-- 
Sergiu Dumitriu
http://purl.org/net/sergiu

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Claude Brisson <cl...@renegat.net>.
Glad to receive an enlightened opinion on the subject.


On 23/11/2016 20:59, Michael Osipov wrote:
> Hi Claude,
>
> not being subscribed to the Velocity dev list, though being a happy 
> Velocity user and Maven Doxia developer relying on Velocity, here are 
> my thoughts on this:
>
> Having access to ISO 8601 (neither ISO-8601, nor ISO8601) in the 
> original German version through my employer (unfortunately, I cannot 
> share due to legal reasons), I can shed some light here.
>
> I have raised the same issues with Commons Lang, all of them have been 
> addressed some time ago.
>
> 1. The 'T' is always mandatory, *unless* it has been mutually agreed 
> to replace it for a space (U+0020). It is a safe bet to name iso-human 
> but not having it as default.
> 2. Consider that you have used the extended format (s. Common Lang). 
> Don't forget that there is also a basic format. Document that at least.
> 3. There is no need to reference to RFC when you have always an ISO 
> standard applied by half of the planet.

Ok.

> 4. "yyyy-MM-dd'T'HH:mm:ssX and "yyyy-MM-dd HH:mm:ssX" are incomplete 
> if you have offsets by 30 or 45 minutes. You are simply truncating 
> them. Unless you know and you probably don't, always stick with XXX in 
> extended format.

It's XXX in the code. ssX appeared only in the mail discussion on the 
dev list.

> 5. ISO operates on timezone *offsets* (except Zulu) not on timezones 
> like Olson TZ DB.

I agree there is a linguistic confusion, here. Even the SimpleDateFormat 
javadocs are imprecise on this. But well... when there's a sign it's an 
offset, otherwise it's a timezone...

> 6. I consider 435 to 441 being inconsisting to their counterpart 420 
> to 423.
>

Oh, you're speaking about line numbers in ConversionUtils.java... why is 
it inconsistent? When we're speaking about date only, there is no 
timezone or separator involved, so it's the same format.

> I would really try to make a difference between ISO and ISO for humans 
> because people mostly do not care about timezones, they want to have 
> it in their local one.
>
> iso, iso-human, iso-tz (does not apply to date only), iso-human-tz 
> (does not apply to date only) (ideally with an Olson TZ ID?)
>

I like it.

By Olson TZ ID, I guess you refer to "zzz" (aka CET), and not "zzzz" 
(aka Central European Time), don't you?



   Claude


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: svn commit: r1770547 - in /velocity/tools/trunk/velocity-tools-generic/src/main/java/org/apache/velocity/tools: ConversionUtils.java generic/DateTool.java

Posted by Claude Brisson <cl...@renegat.net>.
Thanks again for your review.

On 21/11/2016 07:22, Sergiu Dumitriu wrote:
> Although RFC 3339 says that "Applications [...] may choose, for the sake
> of readability, to specify a full-date and full-time separated by (say)
> a space character", the standard is to use T as a separator.
>
> And if I'm already nagging, ISO and RFC are two different things, as are
> ISO-8601 and RFC 3339, so don't use ISO in the name and RFC in the
> comments as if they're interchangeable, pick one and stick with it.
>
> While the RFC allows space as the separator, ISO-8601 actually requires
> T as the date-time separator.

I agree I made a compromise, here: we're definitely targeting RFC 3339 
(with the space separator, as we're mostly doing display), but 
nicknaming a format 'rfc' wasn't very illustrative for the end user. 
After all, RFC 3339 is a "profile of the ISO-8601 standard" for Internet.

But anyway, I'm not yet happy with the current 'intl' and 'iso' 
formatting styles so far, so we can review my choices.

Let's summarize. The goal here is to extend the standard Java formatting 
styles ('full', 'long', 'medium', 'short' and 'default' which equals 
'medium') with standardized YMD formats. The current implementation 
proposes 'iso' (for "yyyy-MM-dd HH:mm:ssX", but as Sergiu noticed, it's 
not the ISO format but rather an RFC 3339 one), and 'intl' (but 
precisely, not having the time zone part for a format name 'intl' looks 
rather ankward).

Here is another proposal:

  - 'iso' for the strict RFC-3339 / ISO-8601 format (aka 
"yyyy-MM-dd'T'HH:mm:ssX")

  - 'intl' for the relaxed RFC-3339 using the space separator (aka 
"yyyy-MM-dd HH:mm:ssX")

  - 'standard' for the same format, with a space separator but without 
timezone (aka "yyyy-MM-dd HH:mm:ss"). I thought of 'local', but it seems 
rather inappropriate for a locale independent format. If you have 
another proposal...


   Claude



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org