You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Eelco Hillenius <ee...@gmail.com> on 2006/11/18 09:19:02 UTC

i18n message loading

Hi all,

I'm writing a chapter on internationalization/ localization, and found
out the message loading works differently from what I expected (and
I'm quite sure how it used to work pre 1.2?).

The first thing I stumbled on was the fact that messages are located
searching from parent to child instead of the inverse. What is the
point of doing that? I would expect something like this to work:

MyPage.properties:
my.property=some value

MyPanel.properties:
my.properties=override

If MyPanel is added to MyPage, currently my.property would resolve to
'some value', while I would expect it to resolve to 'override'.

This patch would fix that:
Index: /Users/eelcohillenius/Documents/workspace/wicket-2.0/src/main/java/wicket/Localizer.java
===================================================================
--- /Users/eelcohillenius/Documents/workspace/wicket-2.0/src/main/java/wicket/Localizer.java	(revision
476204)
+++ /Users/eelcohillenius/Documents/workspace/wicket-2.0/src/main/java/wicket/Localizer.java	(working
copy)
@@ -325,7 +325,8 @@

 		// Build the search stack
 		final List<Class> searchStack = new ArrayList<Class>();
-		searchStack.add(component.getClass());
+		Class<? extends Component> componentClass = component.getClass();
+		searchStack.add(componentClass);

 		if (!(component instanceof Page))
 		{
@@ -333,7 +334,8 @@
 			MarkupContainer container = component.getParent();
 			while (container != null)
 			{
-				searchStack.add(container.getClass());
+				componentClass = container.getClass();
+				searchStack.add(componentClass);
 				if (container instanceof Page)
 				{
 					break;
@@ -409,7 +411,11 @@
 			if ((searchStack != null) && (searchStack.size() > 0))
 			{
 				// Walk the component hierarchy down from page to the component
-				for (int i = searchStack.size() - 1; (i >= 0) && (string == null); i--)
+				// for (int i = searchStack.size() - 1; (i >= 0) && (string ==
+				// null); i--)
+				// {
+				int len = searchStack.size();
+				for (int i = 0; (i < len) && (string == null); i++)
 				{
 					Class clazz = (Class)searchStack.get(i);


But I'm wondering whether there was a reason to do it starting from
the parent, as treating it like a stack seems to be deliberate.

Another thing I found out is that resolving using inheritance gives up
too quickly in some cases. For example:

[Base.properties]
[Base_nl.properties]
  s1=invoer
Foo.properties
  s1=input

If Foo extends Base, and you try to get s1 for the Dutch (nl) locale
relative to Foo, it will never actually reach Base_nl.properties but
instead get the value in Foo.properties (the English default in this
case). I think this is wrong too, though the fix might be nasty.

WDYT? Anyone noticed some other unexpected behavior here?

Eelco

Re: i18n message loading

Posted by Eelco Hillenius <ee...@gmail.com>.
On 11/18/06, Igor Vaynberg <ig...@gmail.com> wrote:
> i think that was done so you can override properties of deeper components.
>
> suppose i create a custom component, and this component uses key my.key in
> its markup and provides a default value in its .properties file.
>
> so how do you override that? right now its simple, just redefine my.key in
> the parent's properties file - since you built the parent you are in control
> there.

Why would the parent overwrite the child? That doesn't make sense to
me, and it is not how, e.g., our markup loading works either.

Eelco

Re: i18n message loading

Posted by Igor Vaynberg <ig...@gmail.com>.
i think that was done so you can override properties of deeper components.

suppose i create a custom component, and this component uses key my.key in
its markup and provides a default value in its .properties file.

so how do you override that? right now its simple, just redefine my.key in
the parent's properties file - since you built the parent you are in control
there.

-igor


On 11/18/06, Eelco Hillenius <ee...@gmail.com> wrote:
>
> On 11/18/06, Johan Compagner <jc...@gmail.com> wrote:
> > we should first go through it for the given language and then do it for
> the
> > default language
> > But quickly looking at that.
> > That won't be an easy fix, because the iteration through components is
> in
> > Localizer and the iteration over locale/style is in
> > XxxxStringResourceLoader
>
> Yeah, I noticed that too. The functionality is a bit shattered.
>
> > My personal opinion about the current resource loading that it does to
> much,
> > we support to many cases. It should be simplified.
>
> This one time I don't agree that simpler is better. We/ our users need
> real power here.
>
> What do you think about the the other point I mentioned?
>
> Eelco
>

Re: i18n message loading

Posted by Eelco Hillenius <ee...@gmail.com>.
On 11/18/06, Johan Compagner <jc...@gmail.com> wrote:
> we should first go through it for the given language and then do it for the
> default language
> But quickly looking at that.
> That won't be an easy fix, because the iteration through components is in
> Localizer and the iteration over locale/style is in
> XxxxStringResourceLoader

Yeah, I noticed that too. The functionality is a bit shattered.

> My personal opinion about the current resource loading that it does to much,
> we support to many cases. It should be simplified.

This one time I don't agree that simpler is better. We/ our users need
real power here.

What do you think about the the other point I mentioned?

Eelco

Re: i18n message loading

Posted by Eelco Hillenius <ee...@gmail.com>.
Cool that the page is there. And funny the author misunderstood the
matching algorithm just as I misunderstood it :). I found out that
AbstractStringResourceLoader has a decent description of how the
component stack lookup works - though the actual work for that happens
in Localizer.

I'll look at the WIKI again (some things are not mentioned, like
style/ variant) though others... don't hold back.

Eelco

On 11/20/06, Gwyn Evans <gw...@gmail.com> wrote:
> On 20/11/06, Juergen Donnerstag <ju...@gmail.com> wrote:
> > please open an RFE. And may be we should use our Wiki to describe the
> > matching algorithm there.
>
> The i18n pages are at http://cwiki.apache.org/confluence/display/WICKET/i18n
>
> (Although it looks like the 2nd pretty much duplicates the last part
> of the 1st - mainly because I'd grabbed the info from the authors post
> to the mailing list, but it looks like he didn't notice & created a
> new wiki page the next day!)
>
> --
> Download Wicket 1.2.3 now! - http://wicketframework.org
>

Re: i18n message loading

Posted by Juergen Donnerstag <ju...@gmail.com>.
than lets use it as a starting point.

Juergen

On 11/20/06, Gwyn Evans <gw...@gmail.com> wrote:
> On 20/11/06, Juergen Donnerstag <ju...@gmail.com> wrote:
> > please open an RFE. And may be we should use our Wiki to describe the
> > matching algorithm there.
>
> The i18n pages are at http://cwiki.apache.org/confluence/display/WICKET/i18n
>
> (Although it looks like the 2nd pretty much duplicates the last part
> of the 1st - mainly because I'd grabbed the info from the authors post
> to the mailing list, but it looks like he didn't notice & created a
> new wiki page the next day!)
>
> --
> Download Wicket 1.2.3 now! - http://wicketframework.org
>

Re: i18n message loading

Posted by Gwyn Evans <gw...@gmail.com>.
On 20/11/06, Juergen Donnerstag <ju...@gmail.com> wrote:
> please open an RFE. And may be we should use our Wiki to describe the
> matching algorithm there.

The i18n pages are at http://cwiki.apache.org/confluence/display/WICKET/i18n

(Although it looks like the 2nd pretty much duplicates the last part
of the 1st - mainly because I'd grabbed the info from the authors post
to the mailing list, but it looks like he didn't notice & created a
new wiki page the next day!)

-- 
Download Wicket 1.2.3 now! - http://wicketframework.org

Re: i18n message loading

Posted by Juergen Donnerstag <ju...@gmail.com>.
please open an RFE. And may be we should use our Wiki to describe the
matching algorithm there.

Juergen

On 11/20/06, Eelco Hillenius <ee...@gmail.com> wrote:
> Here is the 100,000 dollar question (not payable by yours truly): when
> you implemented the change you want, what would be the exact default
> matching algorithm in plain English?
>
> Can I (or do you want to) open up a JIRA issue for this? Even if it is
> not currently implemented correctly, I'd still like to include the
> ideal matching algorithm in Wicket In Action.
>
> Thanks,
>
> Eelco
>
>
> On 11/18/06, Juergen Donnerstag <ju...@gmail.com> wrote:
> > It has been on my todo list for a long time already. It's just lack of time
> >
> > Juergen
> >
>

Re: i18n message loading

Posted by Eelco Hillenius <ee...@gmail.com>.
Here is the 100,000 dollar question (not payable by yours truly): when
you implemented the change you want, what would be the exact default
matching algorithm in plain English?

Can I (or do you want to) open up a JIRA issue for this? Even if it is
not currently implemented correctly, I'd still like to include the
ideal matching algorithm in Wicket In Action.

Thanks,

Eelco


On 11/18/06, Juergen Donnerstag <ju...@gmail.com> wrote:
> It has been on my todo list for a long time already. It's just lack of time
>
> Juergen
>

Re: i18n message loading

Posted by Juergen Donnerstag <ju...@gmail.com>.
It has been on my todo list for a long time already. It's just lack of time

Juergen

On 11/18/06, Eelco Hillenius <ee...@gmail.com> wrote:
> On 11/18/06, Juergen Donnerstag <ju...@gmail.com> wrote:
> > > > To Johans point, I think the current implementation is too inflexible,
> > > > it is far to difficult to change/extend/limit the search order easily
> > > > if required. IMO this general design problem needs to be fixed.
> > >
> > > Yep, I agree. The logic should be brought back to one place. So... we
> > > could open an issue for that, right?
> > >
> >
> > May be not bringing it back to one place but may be a single Interface
> > which allows to easily chain implementations. Than you can have one
> > implementation that searches the locale and styles first, another
> > implementation takeing care of markup inheritance, the next one
> > traversing the component hierachy, one for the super class hierarchy,
> > the next one what ever. By simply more or less plugging them together
> > the search order would be obvious and could easily be
> > changed/extended/simplified.
>
> Sounds good. If you have a good idea on how to implement it... don't
> hold back :)
>
> Eelco
>

Re: i18n message loading

Posted by Eelco Hillenius <ee...@gmail.com>.
On 11/18/06, Juergen Donnerstag <ju...@gmail.com> wrote:
> > > To Johans point, I think the current implementation is too inflexible,
> > > it is far to difficult to change/extend/limit the search order easily
> > > if required. IMO this general design problem needs to be fixed.
> >
> > Yep, I agree. The logic should be brought back to one place. So... we
> > could open an issue for that, right?
> >
>
> May be not bringing it back to one place but may be a single Interface
> which allows to easily chain implementations. Than you can have one
> implementation that searches the locale and styles first, another
> implementation takeing care of markup inheritance, the next one
> traversing the component hierachy, one for the super class hierarchy,
> the next one what ever. By simply more or less plugging them together
> the search order would be obvious and could easily be
> changed/extended/simplified.

Sounds good. If you have a good idea on how to implement it... don't
hold back :)

Eelco

Re: i18n message loading

Posted by Juergen Donnerstag <ju...@gmail.com>.
> > To Johans point, I think the current implementation is too inflexible,
> > it is far to difficult to change/extend/limit the search order easily
> > if required. IMO this general design problem needs to be fixed.
>
> Yep, I agree. The logic should be brought back to one place. So... we
> could open an issue for that, right?
>

May be not bringing it back to one place but may be a single Interface
which allows to easily chain implementations. Than you can have one
implementation that searches the locale and styles first, another
implementation takeing care of markup inheritance, the next one
traversing the component hierachy, one for the super class hierarchy,
the next one what ever. By simply more or less plugging them together
the search order would be obvious and could easily be
changed/extended/simplified.

> Eelco
>

Re: i18n message loading

Posted by Eelco Hillenius <ee...@gmail.com>.
> The reason for the Stack order, provided I recall correctly, assume
> you have self-contained component with all default properties
> required. Next you place the component in your apps, but you want to
> change the default properties, than the only place outside the jar is
> the any of the parent properties files. Make sense?

Ok, it makes sense (though I just replied to Igor it didn't :)) And
thinking a little bit about it, I guess it is better than what I
proposed from the view point of reusable components. So strike that.

> To Johans point, I think the current implementation is too inflexible,
> it is far to difficult to change/extend/limit the search order easily
> if required. IMO this general design problem needs to be fixed.

Yep, I agree. The logic should be brought back to one place. So... we
could open an issue for that, right?

Eelco

Re: i18n message loading

Posted by Juergen Donnerstag <ju...@gmail.com>.
I remember we had endless discussions on the sequence of finding
properties and I'm not able to recall all of them. I'm not aware of
any changes to the code for quite a while. And as it has been fairly
silent with respect to users asking questions or requesting
functionality, means to me that most users are happy with what we
have. I don't say it can't be improved and it most likely has
ambiguities and bugs as well.

The reason for the Stack order, provided I recall correctly, assume
you have self-contained component with all default properties
required. Next you place the component in your apps, but you want to
change the default properties, than the only place outside the jar is
the any of the parent properties files. Make sense?

To Johans point, I think the current implementation is too inflexible,
it is far to difficult to change/extend/limit the search order easily
if required. IMO this general design problem needs to be fixed.

Juergen

On 11/18/06, Johan Compagner <jc...@gmail.com> wrote:
> we should first go through it for the given language and then do it for the
> default language
> But quickly looking at that.
> That won't be an easy fix, because the iteration through components is in
> Localizer and the iteration over locale/style is in
> XxxxStringResourceLoader
>
> My personal opinion about the current resource loading that it does to much,
> we support to many cases. It should be simplified.
>
> johan
>
>
> On 11/18/06, Eelco Hillenius <ee...@gmail.com> wrote:
> >
> > Hi all,
> >
> > I'm writing a chapter on internationalization/ localization, and found
> > out the message loading works differently from what I expected (and
> > I'm quite sure how it used to work pre 1.2?).
> >
> > The first thing I stumbled on was the fact that messages are located
> > searching from parent to child instead of the inverse. What is the
> > point of doing that? I would expect something like this to work:
> >
> > MyPage.properties:
> > my.property=some value
> >
> > MyPanel.properties:
> > my.properties=override
> >
> > If MyPanel is added to MyPage, currently my.property would resolve to
> > 'some value', while I would expect it to resolve to 'override'.
> >
> > This patch would fix that:
> > Index: /Users/eelcohillenius/Documents/workspace/wicket-2.0
> > /src/main/java/wicket/Localizer.java
> > ===================================================================
> > --- /Users/eelcohillenius/Documents/workspace/wicket-2.0
> > /src/main/java/wicket/Localizer.java    (revision
> > 476204)
> > +++ /Users/eelcohillenius/Documents/workspace/wicket-2.0
> > /src/main/java/wicket/Localizer.java    (working
> > copy)
> > @@ -325,7 +325,8 @@
> >
> >                 // Build the search stack
> >                 final List<Class> searchStack = new ArrayList<Class>();
> > -               searchStack.add(component.getClass());
> > +               Class<? extends Component> componentClass =
> > component.getClass();
> > +               searchStack.add(componentClass);
> >
> >                 if (!(component instanceof Page))
> >                 {
> > @@ -333,7 +334,8 @@
> >                         MarkupContainer container = component.getParent();
> >                         while (container != null)
> >                         {
> > -                               searchStack.add(container.getClass());
> > +                               componentClass = container.getClass();
> > +                               searchStack.add(componentClass);
> >                                 if (container instanceof Page)
> >                                 {
> >                                         break;
> > @@ -409,7 +411,11 @@
> >                         if ((searchStack != null) && (searchStack.size() >
> > 0))
> >                         {
> >                                 // Walk the component hierarchy down from
> > page to the component
> > -                               for (int i = searchStack.size() - 1; (i >=
> > 0) && (string == null); i--)
> > +                               // for (int i = searchStack.size() - 1; (i
> > >= 0) && (string ==
> > +                               // null); i--)
> > +                               // {
> > +                               int len = searchStack.size();
> > +                               for (int i = 0; (i < len) && (string ==
> > null); i++)
> >                                 {
> >                                         Class clazz =
> > (Class)searchStack.get(i);
> >
> >
> > But I'm wondering whether there was a reason to do it starting from
> > the parent, as treating it like a stack seems to be deliberate.
> >
> > Another thing I found out is that resolving using inheritance gives up
> > too quickly in some cases. For example:
> >
> > [Base.properties]
> > [Base_nl.properties]
> >   s1=invoer
> > Foo.properties
> >   s1=input
> >
> > If Foo extends Base, and you try to get s1 for the Dutch (nl) locale
> > relative to Foo, it will never actually reach Base_nl.properties but
> > instead get the value in Foo.properties (the English default in this
> > case). I think this is wrong too, though the fix might be nasty.
> >
> > WDYT? Anyone noticed some other unexpected behavior here?
> >
> > Eelco
> >
>
>

Re: i18n message loading

Posted by Johan Compagner <jc...@gmail.com>.
we should first go through it for the given language and then do it for the
default language
But quickly looking at that.
That won't be an easy fix, because the iteration through components is in
Localizer and the iteration over locale/style is in
XxxxStringResourceLoader

My personal opinion about the current resource loading that it does to much,
we support to many cases. It should be simplified.

johan


On 11/18/06, Eelco Hillenius <ee...@gmail.com> wrote:
>
> Hi all,
>
> I'm writing a chapter on internationalization/ localization, and found
> out the message loading works differently from what I expected (and
> I'm quite sure how it used to work pre 1.2?).
>
> The first thing I stumbled on was the fact that messages are located
> searching from parent to child instead of the inverse. What is the
> point of doing that? I would expect something like this to work:
>
> MyPage.properties:
> my.property=some value
>
> MyPanel.properties:
> my.properties=override
>
> If MyPanel is added to MyPage, currently my.property would resolve to
> 'some value', while I would expect it to resolve to 'override'.
>
> This patch would fix that:
> Index: /Users/eelcohillenius/Documents/workspace/wicket-2.0
> /src/main/java/wicket/Localizer.java
> ===================================================================
> --- /Users/eelcohillenius/Documents/workspace/wicket-2.0
> /src/main/java/wicket/Localizer.java    (revision
> 476204)
> +++ /Users/eelcohillenius/Documents/workspace/wicket-2.0
> /src/main/java/wicket/Localizer.java    (working
> copy)
> @@ -325,7 +325,8 @@
>
>                 // Build the search stack
>                 final List<Class> searchStack = new ArrayList<Class>();
> -               searchStack.add(component.getClass());
> +               Class<? extends Component> componentClass =
> component.getClass();
> +               searchStack.add(componentClass);
>
>                 if (!(component instanceof Page))
>                 {
> @@ -333,7 +334,8 @@
>                         MarkupContainer container = component.getParent();
>                         while (container != null)
>                         {
> -                               searchStack.add(container.getClass());
> +                               componentClass = container.getClass();
> +                               searchStack.add(componentClass);
>                                 if (container instanceof Page)
>                                 {
>                                         break;
> @@ -409,7 +411,11 @@
>                         if ((searchStack != null) && (searchStack.size() >
> 0))
>                         {
>                                 // Walk the component hierarchy down from
> page to the component
> -                               for (int i = searchStack.size() - 1; (i >=
> 0) && (string == null); i--)
> +                               // for (int i = searchStack.size() - 1; (i
> >= 0) && (string ==
> +                               // null); i--)
> +                               // {
> +                               int len = searchStack.size();
> +                               for (int i = 0; (i < len) && (string ==
> null); i++)
>                                 {
>                                         Class clazz =
> (Class)searchStack.get(i);
>
>
> But I'm wondering whether there was a reason to do it starting from
> the parent, as treating it like a stack seems to be deliberate.
>
> Another thing I found out is that resolving using inheritance gives up
> too quickly in some cases. For example:
>
> [Base.properties]
> [Base_nl.properties]
>   s1=invoer
> Foo.properties
>   s1=input
>
> If Foo extends Base, and you try to get s1 for the Dutch (nl) locale
> relative to Foo, it will never actually reach Base_nl.properties but
> instead get the value in Foo.properties (the English default in this
> case). I think this is wrong too, though the fix might be nasty.
>
> WDYT? Anyone noticed some other unexpected behavior here?
>
> Eelco
>