Re: [tools] Adding localization method to DateTool

2009-11-15 Thread Christopher Schultz
Nathan,

Resurrecting this because I'm ready to commit.

On 10/29/2009 3:04 PM, Nathan Bubna wrote:
 On Thu, Oct 29, 2009 at 8:06 AM, Christopher Schultz
 ch...@christopherschultz.net wrote:
 Nathan,

 On 10/29/2009 11:02 AM, Christopher Schultz wrote:
 I like re-coding into SimpleDataFormat better.

 D'oh: that's not possible because DateFormat doesn't expose the pattern
 it's using, so I'll have to return something unrelated. Hmm. I'm not a
 big fan of NULL but it's also not exactly consistent with the API if an
 error message is returned.

 Any ideas other than NULL?
 
 Not really, because a) it's unlikely to ever occur in practice and b)
 that's the traditional VelocityTools response to bad inputs.  If you
 call $date.toLocalizedPattern($invalidInput) then that's little better
 than $date.noSuchMethod($validinput).  Either way, part of the
 reference is invalid, and we want to render it as an invalid
 reference.  Returning null accomplishes that.

While I agree that VelocityTools' traditional response to bad input is
null, the problem here isn't the user's bad input: instead, it's our
reliance on an unpublished but highly likely coincidence that DateFormat
always hands-out SimpleDateFormat objects.

For patterns other than simple_date and stuff like that, I could
simply construct a new SimpleDateFormat object and use that, but then
the behavior of toLocalizedPattern would differ from the other methods
-- that is, short_date would work, except when it doesn't :(

Thanks,
-chris



signature.asc
Description: OpenPGP digital signature


Re: [tools] Adding localization method to DateTool

2009-11-15 Thread Nathan Bubna
On Sun, Nov 15, 2009 at 7:26 AM, Christopher Schultz
ch...@christopherschultz.net wrote:
 Nathan,

 Resurrecting this because I'm ready to commit.

 On 10/29/2009 3:04 PM, Nathan Bubna wrote:
 On Thu, Oct 29, 2009 at 8:06 AM, Christopher Schultz
 ch...@christopherschultz.net wrote:
 Nathan,

 On 10/29/2009 11:02 AM, Christopher Schultz wrote:
 I like re-coding into SimpleDataFormat better.

 D'oh: that's not possible because DateFormat doesn't expose the pattern
 it's using, so I'll have to return something unrelated. Hmm. I'm not a
 big fan of NULL but it's also not exactly consistent with the API if an
 error message is returned.

 Any ideas other than NULL?

 Not really, because a) it's unlikely to ever occur in practice and b)
 that's the traditional VelocityTools response to bad inputs.  If you
 call $date.toLocalizedPattern($invalidInput) then that's little better
 than $date.noSuchMethod($validinput).  Either way, part of the
 reference is invalid, and we want to render it as an invalid
 reference.  Returning null accomplishes that.

 While I agree that VelocityTools' traditional response to bad input is
 null, the problem here isn't the user's bad input: instead, it's our
 reliance on an unpublished but highly likely coincidence that DateFormat
 always hands-out SimpleDateFormat objects.

ah, but i didn't mean to imply that it is *just* the standard response
to bad input.  it's the convention for all errors when attempting to
execute a tool method.  it tells the user there's a problem in a
consistent manner, responds to silent notation, is easily caught by an
event handler, and properly blows up when VelocityEngine is running in
the new strict mode.  returning an error string would be inconsistent
and fail to draw the attention of other facilities for error
checking/handling that Velocity provides.

 For patterns other than simple_date and stuff like that, I could
 simply construct a new SimpleDateFormat object and use that, but then
 the behavior of toLocalizedPattern would differ from the other methods
 -- that is, short_date would work, except when it doesn't :(

yeah, but i think that's acceptable.  do the best we can, return null
when nothing can be done.

 Thanks,
 -chris



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



Re: [tools] Adding localization method to DateTool

2009-10-29 Thread Antonio Petrelli
2009/10/28 Nathan Bubna nbu...@gmail.com:
 One question, would it make sense to also add a:

 public String toLocalizedPattern(String format) {
     return toLocalizedPattern(format, getLocale()
 }

 for completeness?  Or is that redundant?

IMHO this method would be useful too, as long as getLocale returns
the currently-selected locale, IOW the client's locale.

Antonio

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



Re: [tools] Adding localization method to DateTool

2009-10-29 Thread Tomas Wredendal
2009/10/29 Antonio Petrelli antonio.petre...@gmail.com:
 2009/10/29 Claude Brisson cla...@renegat.net:
 On jeu., 2009-10-29 at 09:22 +0100, Antonio Petrelli wrote:
 2009/10/28 Nathan Bubna nbu...@gmail.com:
  One question, would it make sense to also add a:
 
  public String toLocalizedPattern(String format) {
      return toLocalizedPattern(format, getLocale()
  }
 
  for completeness?  Or is that redundant?

 IMHO this method would be useful too, as long as getLocale returns
 the currently-selected locale, IOW the client's locale.

 That's the server locale by default elsewhere in such localized methods
 (or, if present, the one defined in tools.xml). For consistency, I'd
 rather keep it the same here.

 If it's question of compatibility I agree, but you should consider
 that, in fact, the server locale is completely useless.

No, it.s not useless, maybe rare. If my company users/partners are
spread around the world, I want to force our locale regardless what
locale thay have on their borrowed/localy purchased computer...
...Just a thought...

 I am Italian and I wish to see pages in Italian (preferably) and dates
 in Italian format.
 Now, if the server is in Germany, I don't want absolutely to see
 Oktober as the current month :-D it makes no sense.
 However I admit that, getting the client's locale from DateTool might
 be problematic, because it should be independent from a Servlet-based
 application. The locale can be easily got from the request (or other
 mechanisms).
 So, at the end, probably the method without the locale should not be
 added, to avoid confusion.

 Antonio

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



--
Tomas

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



Re: [tools] Adding localization method to DateTool

2009-10-29 Thread Antonio Petrelli
2009/10/29 Claude Brisson cla...@renegat.net:
 On jeu., 2009-10-29 at 11:02 +0100, Antonio Petrelli wrote:
 2009/10/29 Claude Brisson cla...@renegat.net:
  On jeu., 2009-10-29 at 09:22 +0100, Antonio Petrelli wrote:
  2009/10/28 Nathan Bubna nbu...@gmail.com:
   One question, would it make sense to also add a:
  
   public String toLocalizedPattern(String format) {
       return toLocalizedPattern(format, getLocale()
   }
  
   for completeness?  Or is that redundant?
 
  IMHO this method would be useful too, as long as getLocale returns
  the currently-selected locale, IOW the client's locale.
 
  That's the server locale by default elsewhere in such localized methods
  (or, if present, the one defined in tools.xml). For consistency, I'd
  rather keep it the same here.

 If it's question of compatibility I agree, but you should consider
 that, in fact, the server locale is completely useless.
 I am Italian and I wish to see pages in Italian (preferably) and dates
 in Italian format.
 Now, if the server is in Germany, I don't want absolutely to see
 Oktober as the current month :-D it makes no sense.

 A tools.xml defined locale (either globally or by tool) would make some
 sense. This feature is already available in tools 2.0.

Uh right, never mind :-)

Antonio

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



Re: [tools] Adding localization method to DateTool

2009-10-29 Thread Christopher Schultz
Nathan,

On 10/28/2009 6:32 PM, Nathan Bubna wrote:
 You've not been forgotten.  :)  You're correct that this is not
 currently supported.  This only adds a simple method to a single
 tool's VTL API and thus would not impede any forthcoming long-wished
 for 2.0 release (at least in my book).  Go for it.
 
 One question, would it make sense to also add a:
 
 public String toLocalizedPattern(String format) {
  return toLocalizedPattern(format, getLocale());
 }
 
 for completeness?  Or is that redundant?

After reading everyone's comments, I'm not really sure a second method
is necessary. As Antonio mentions, the server Locale is next to useless,
although tools.xml can be used to define the default locale for
DateTool. For times when you know you want to use the server's Locale
(or the DateTool's Locale), you can easily get that Locale object and
pass it into the two-argument method.

The bottom line is that neither technique (using a custom Locale nor the
server/DateTool Locale) is possible at the moment, and adding a single
method allows them both if desired.

I'm happy to add a second method if there's a lot of support for it,
especially because it's nearly as trivial as the first.

Thanks,
-chris



signature.asc
Description: OpenPGP digital signature


Re: [tools] Adding localization method to DateTool

2009-10-29 Thread Christopher Schultz
All,

On 10/28/2009 5:10 PM, Christopher Schultz wrote:
   public String toLocalizedPattern(String dateFormat,
Locale targetLocale)
   {
 new SimpleDateFormat(formatPattern,
targetLocale).toLocalizedPattern();
   }

After looking at DateTool, I'm thinking that I should probably write
this method as:

   public String toLocalizedPattern(String dateFormat,
Locale targetLocale)
   {
DateFormat df = getDateFormat(dateFormat,
  targetLocale,
  getTimeZone());

return df.toLocalizedPattern();
   }

Using the getDateFormat will allow date formats such as short and
short_date and stuff like that, and will be more consistent with the
rest of the DateTool API.

Any comments?

Also, I noticed that some methods explicitly use this.getTimeZone() and
others reference the private member directly. Technically, DateUtil does
have a subclass (ComparisonDateTool) so it should always use getTimeZone
in case the subclass wishes to override it. On the other hand, this
class is not /really/ meant for subclassing and so direct member access
seems reasonable. Any comments on this while I'm mucking-around in
DateTool? (Of course, any changes to other parts of the code will be
made in a different commit).

Thanks,
-chris



signature.asc
Description: OpenPGP digital signature


Re: [tools] Adding localization method to DateTool

2009-10-29 Thread Christopher Schultz
Nathan,

On 10/29/2009 10:36 AM, Nathan Bubna wrote:
return df.toLocalizedPattern();
 
 I believe you'll have to cast to (SimpleDateFormat) to get the
 toLocalizedPattern() method.

Yup, I noticed that as I started to write the code. For now, I just have
it allowing a ClassCastException. I wasn't sure what to do if the
DateFormat returned wasn't a SimpleDateFormat. I suppose I could always
re-code whatever DateFormat comes back into a SimpleDateFormat and use
that. That ought to always work, even if it ends up being a bit slow.

 In practice, such a cast should always
 work, but i don't trust that.  So, perhaps cast, but try/catch any
 ClassCastException and return null (or attempt your original plan in
 such cases.

I like re-coding into SimpleDataFormat better.

I'm working on unit tests right now and everything looks good, including
the use of special formats such as 'short_time' and stuff like that.

I'll also fix the direct-access of the 'timezone' member when I'm
finished with this.

-chris



signature.asc
Description: OpenPGP digital signature


[tools] Adding localization method to DateTool

2009-10-28 Thread Christopher Schultz
All,

I'm a big fan of DateTool and use it all the time in my Velocity
templates. I recently had a revelation that, when providing hints to my
users like please format your date like this: MM/dd/, the date
format itself is being read from the user's preferences and always in
the SimpleDateFormat form (that is, using English-appropriate letters,
etc.).

SimpleDateFormat provides a simple way to localize a date format pattern
into a non-English locale: toLocalizedPattern

Basically, if you want to show MM/dd/ in, say, German, you want to
say MM/tt/. Or, in French, it should be MM/jj/. (Forget that
MM/dd/ is not exactly appropriate for my French and German friends,
it's the representation of the pattern I'm worried about).

Anyhow, it's not particularly convenient in a Velocity template to do
the equivalent of this:

new SimpleDateFormat(formatPattern, targetLocale).toLocalizedPattern()

So, I'm proposing adding a method to DateTool:

  public String toLocalizedPattern(String dateFormat,
   Locale targetLocale)

This method will pretty much just contain the above code.

Any thoughts? I can add the method myself using my committer ninja
skills (I'll bet you guys forgot about me, huh?) but I wanted to make
sure that:

1. There wasn't a better way to do this, possibly already in existence
2. Nobody minded me adding such a method
3. The addition wouldn't disrupt a 2.0 tools release

Thanks,
-chris



signature.asc
Description: OpenPGP digital signature