Re: empty wicket:message
BTW, I just wanted to say how much I love Wicket! (Which you can tell by the minor issues that I'm mailing about on this list, which underlines the fact that I can't find any major ones...) :-) The resulting code is so short and so clean, Wicket it is so easy to work with, and everything just works. :-) It really makes web development fun again. I also loved the WIA book so far and since I'm localizing my site right now I'm very interested in that chapter you're finishing Eelco. :-) Regards, Sebastiaan Eelco Hillenius wrote: Btw, I would mind a decision about this in this weekend, as I'm finishing the chapter on localization right now :-) FWIW, I don't like the silent failure we have now, and believe that using the body as a default isn't very helpful; e.g. if you have the default body the same as what you intended to put in the properties file for the default locale (which is something that I often do, and I can imagine other people doing that as well), and you make a spelling error in the key (of either the tag or in the properties file), you'd never notice Wicket not finding the property. Either outputting the key as the body or throwing an exception is way better imho. However, I wouldn't vote against people wanting to stick with the current pattern as it is quite late in the game to change this. Eelco On 11/3/07, Eelco Hillenius [EMAIL PROTECTED] wrote: On 11/3/07, Igor Vaynberg [EMAIL PROTECTED] wrote: then you are violating the contract of wicket:message. if we do this then there is little point to allowing body inside wicket:message Erm, where is that contract defined? The body is for previewing. I don't think I ever thought about this as a default, nor do I think we should, since that is inconsistent with what we do elsewhere. Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] smime.p7s Description: S/MIME Cryptographic Signature
Re: empty wicket:message
On 11/5/07, Sebastiaan van Erk [EMAIL PROTECTED] wrote: BTW, I just wanted to say how much I love Wicket! As long as you don't give all your Love to Wicket, you have to keep some for Morena! (and her beautiful wicket site http://www.denherdervarga.com/ ) johan
Re: empty wicket:message
Johan Compagner wrote: On 11/5/07, Sebastiaan van Erk [EMAIL PROTECTED] wrote: BTW, I just wanted to say how much I love Wicket! As long as you don't give all your Love to Wicket, you have to keep some for Morena! (and her beautiful wicket site http://www.denherdervarga.com/ ) Don't worry about that, I'll be sure to keep some in reserve. :) Haven't had any complaints so far. ;-) You kinda caught me unprepared by the posting the site. ;-) But since it's out there now, comments and suggestions are welcome. johan Sebastiaan smime.p7s Description: S/MIME Cryptographic Signature
Re: empty wicket:message
thats better. I think that throw exceptions on missing resources shouldn't happen in deployment mode anyway i think there is no much use for it a good log statement in the server log should be enough. i dont find it very dangerous that it shows the body IF it has a body because that you see it on your page you could see [THIS SHOULD BE A KEY] which is very easy to spot. But also the default text which is also fine. (except ofcourse if you think it comes from he db and you change something there and you don't see it, but then you change something or update something and then it should be able to look it up) johan On 11/4/07, Eelco Hillenius [EMAIL PROTECTED] wrote: On 11/3/07, Al Maw [EMAIL PROTECTED] wrote: Johan Compagner wrote: Yes we could do that. Just remove that default value. But what i would like to have IF you have a body specified then we don't throw anything This way we keep old behavior and we have the new one FWIW, I entirely agree with this. If we just change it for tags that have a default body then we'll almost certainly break people's sites. How about keeping this for the deployment mode (and log an error or warning once) but throw that exception in development mode? Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
Johan Compagner wrote: thats better. I think that throw exceptions on missing resources shouldn't happen in deployment mode anyway i think there is no much use for it a good log statement in the server log should be enough. i dont find it very dangerous that it shows the body IF it has a body because that you see it on your page you could see [THIS SHOULD BE A KEY] which is very easy to spot. But also the default text which is also fine. (except ofcourse if you think it comes from he db and you change something there and you don't see it, but then you change something or update something and then it should be able to look it up) johan I definitely agree that in production mode it should not throw an exception for missing message resources. I don't like it if it shows the default body in development mode though. Either you have to type [THIS SHOULD BE A KEY] all the time, or you have to make an empty body, in both cases ruining your preview. If backwards compatiblity is necessary, why not just make it an option via the settings to turn off the exception throwing and revert to the body replacement? I would hate it if throwing the exceptions option is lost as soon as you fill in a body in the wicket:message tag... :-( Personally I think it would really help to have exceptions because they are hard to miss and scanning huge html pages for [MISSING KEY] is error prone too. Regards, Sebastiaan smime.p7s Description: S/MIME Cryptographic Signature
Re: empty wicket:message
im afraid we cannot do that. because the contract is that when a resource is not found we output the body, which is supposed to be the default text. to do what you want we need to change that contract. i am not opposed to that per se, but there needs to be a discussion followed by a vote. -igor On 11/3/07, Sebastiaan van Erk [EMAIL PROTECTED] wrote: Hi, I was wondering if it is possible to configure wicket to make wicket:message output the key in braces when the key is not found (at least in development mode), because that would make it a lot easier to spot missing labels... That is, what I'd like to do is: wicket:message key=bla / And have wicket output [bla] if the key bla cannot be found. I know I could do this: wicket:message key=bla[bla]/wicket:message but this is a lot more verbose and it requires me to correctly type the key name twice every time. Currently if the resource is not found it just outputs nothing at all (which is hard to spot). Regards, Sebastiaan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
Sebastiaan van Erk wrote: I was wondering if it is possible to configure wicket to make wicket:message output the key in braces when the key is not found (at least in development mode), because that would make it a lot easier to spot missing labels... That is, what I'd like to do is: wicket:message key=bla / And have wicket output [bla] if the key bla cannot be found. I know I could do this: wicket:message key=bla[bla]/wicket:message but this is a lot more verbose and it requires me to correctly type the key name twice every time. Currently if the resource is not found it just outputs nothing at all (which is hard to spot). Mmmm. I must say I agree with this. I'd actually prefer it to throw an exception. ;-) Maybe we should add this as a feature? getMarkupSettings().setThrowExceptionOnEmptyMessageTagKeyMissing(true) or something equally descriptive. ;-) Regards, Al - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
Thanks for the reply, I was afraid I was going to get this answer. :-) I can understand it though. Igor Vaynberg wrote: im afraid we cannot do that. because the contract is that when a resource is not found we output the body, which is supposed to be the default text. Am I pushing my luck if I interpret this as: if the resource is not found, replace it by the body *if there is one*. :-))) Practically speaking if I don't supply a body I'd like to be informed (at least in development mode) if the resource is not found... If I really intend an empty string to be shown I can always define it to be empty in the properties file. Just my two cents... :-) Regards, Sebastiaan to do what you want we need to change that contract. i am not opposed to that per se, but there needs to be a discussion followed by a vote. -igor On 11/3/07, Sebastiaan van Erk [EMAIL PROTECTED] wrote: Hi, I was wondering if it is possible to configure wicket to make wicket:message output the key in braces when the key is not found (at least in development mode), because that would make it a lot easier to spot missing labels... That is, what I'd like to do is: wicket:message key=bla / And have wicket output [bla] if the key bla cannot be found. I know I could do this: wicket:message key=bla[bla]/wicket:message but this is a lot more verbose and it requires me to correctly type the key name twice every time. Currently if the resource is not found it just outputs nothing at all (which is hard to spot). Regards, Sebastiaan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] smime.p7s Description: S/MIME Cryptographic Signature
Re: empty wicket:message
Mmmm. I must say I agree with this. I'd actually prefer it to throw an exception. ;-) I'm surprised we don't do this already! I would have expected that if IResourceSettings#getThrowExceptionOnMissingResource returns true, an exception would be thrown here as well. I think we should fix it. Maybe we should add this as a feature? getMarkupSettings().setThrowExceptionOnEmptyMessageTagKeyMissing(true) or something equally descriptive. ;-) If we have the exception that outputs the missing key, that should be enough imho. I'm not against adding this, but I don't we we need it once we implement throwing that exception. Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
then you are violating the contract of wicket:message. if we do this then there is little point to allowing body inside wicket:message -igor On 11/3/07, Eelco Hillenius [EMAIL PROTECTED] wrote: Mmmm. I must say I agree with this. I'd actually prefer it to throw an exception. ;-) I'm surprised we don't do this already! I would have expected that if IResourceSettings#getThrowExceptionOnMissingResource returns true, an exception would be thrown here as well. I think we should fix it. Maybe we should add this as a feature? getMarkupSettings().setThrowExceptionOnEmptyMessageTagKeyMissing(true) or something equally descriptive. ;-) If we have the exception that outputs the missing key, that should be enough imho. I'm not against adding this, but I don't we we need it once we implement throwing that exception. Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
Btw, I would mind a decision about this in this weekend, as I'm finishing the chapter on localization right now :-) FWIW, I don't like the silent failure we have now, and believe that using the body as a default isn't very helpful; e.g. if you have the default body the same as what you intended to put in the properties file for the default locale (which is something that I often do, and I can imagine other people doing that as well), and you make a spelling error in the key (of either the tag or in the properties file), you'd never notice Wicket not finding the property. Either outputting the key as the body or throwing an exception is way better imho. However, I wouldn't vote against people wanting to stick with the current pattern as it is quite late in the game to change this. Eelco On 11/3/07, Eelco Hillenius [EMAIL PROTECTED] wrote: On 11/3/07, Igor Vaynberg [EMAIL PROTECTED] wrote: then you are violating the contract of wicket:message. if we do this then there is little point to allowing body inside wicket:message Erm, where is that contract defined? The body is for previewing. I don't think I ever thought about this as a default, nor do I think we should, since that is inconsistent with what we do elsewhere. Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
Eelco Hillenius wrote: Btw, I would mind a decision about this in this weekend, as I'm finishing the chapter on localization right now :-) FWIW, I don't like the silent failure we have now, and believe that using the body as a default isn't very helpful; e.g. if you have the default body the same as what you intended to put in the properties file for the default locale (which is something that I often do, and I can imagine other people doing that as well), and you make a spelling error in the key (of either the tag or in the properties file), you'd never notice Wicket not finding the property. Either outputting the key as the body or throwing an exception is way better imho. However, I wouldn't vote against people wanting to stick with the current pattern as it is quite late in the game to change this. Eelco This is precisely the issue I was having, and the reason I was putting [key] in there, so I could actually see that it was getting the value from my properties file and not just the preview default, which happened to be the same (of course this kinda ruins the preview a bit). Thinking about it a bit more I agree with Eelco that I would like the exception thrown even if there is a default filled in (i.e., the message body is not empty) and the key is not defined, because it would kill previewability if you're forced to put empty wicket messages to get this behavior. I realize it's quite late in the game as Eelco puts it, but couldn't this just be a setting which you can turn on (and defaults to off), like Al Maw suggested? Or even defaults to on and in the migration notes it could say to get the old behavior you have to turn it off (being hopeful here ;-)) I really hate unresolved i18n messages, and with the silent failure it's just a pain to find them all. Regards, Sebastiaan On 11/3/07, Eelco Hillenius [EMAIL PROTECTED] wrote: On 11/3/07, Igor Vaynberg [EMAIL PROTECTED] wrote: then you are violating the contract of wicket:message. if we do this then there is little point to allowing body inside wicket:message Erm, where is that contract defined? The body is for previewing. I don't think I ever thought about this as a default, nor do I think we should, since that is inconsistent with what we do elsewhere. Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] smime.p7s Description: S/MIME Cryptographic Signature
Re: empty wicket:message
The easiest solution would be this:
Index:
/Users/eelcohillenius/Documents/workspace_wicket/trunk/jdk-1.4/wicket/src/main/java/org/apache/wicket/markup/resolver/WicketMessageResolver.java
===
---
/Users/eelcohillenius/Documents/workspace_wicket/trunk/jdk-1.4/wicket/src/main/java/org/apache/wicket/markup/resolver/WicketMessageResolver.java
(revision
591166)
+++
/Users/eelcohillenius/Documents/workspace_wicket/trunk/jdk-1.4/wicket/src/main/java/org/apache/wicket/markup/resolver/WicketMessageResolver.java
(working
copy)
@@ -43,13 +43,6 @@
{
private static final Logger log =
LoggerFactory.getLogger(WicketMessageResolver.class);
- /**
-* If the key can't be resolved and the default is null, an
exception will be thrown. Instead,
-* we default to a unique string and check against this later. Don't
just use an empty string
-* here, as people might want to override wicket:messages to empty
strings.
-*/
- private static final String DEFAULT_VALUE =
DEFAULT_WICKET_MESSAGE_RESOLVER_VALUE;
-
static
{
// register wicket:message
@@ -74,7 +67,7 @@
* @return true, if componentId was handle by the resolver. False,
otherwise
*/
public boolean resolve(final MarkupContainer container, final
MarkupStream markupStream,
- final ComponentTag tag)
+ final ComponentTag tag)
{
if (tag instanceof WicketTag)
{
@@ -85,7 +78,7 @@
if ((messageKey == null) ||
(messageKey.trim().length() == 0))
{
throw new MarkupException(
- Wrong format of
wicket:message key='xxx': attribute 'key'
is missing);
+ Wrong format of
wicket:message key='xxx': attribute 'key' is
missing);
}
final String id = _message_ +
container.getPage().getAutoIndex();
@@ -91,7 +84,7 @@
final String id = _message_ +
container.getPage().getAutoIndex();
MessageLabel label = new MessageLabel(id,
messageKey);
label.setRenderBodyOnly(container.getApplication().getMarkupSettings()
- .getStripWicketTags());
+ .getStripWicketTags());
container.autoAdd(label, markupStream);
// Yes, we handled the tag
@@ -125,8 +118,8 @@
protected void onComponentTagBody(MarkupStream markupStream,
ComponentTag openTag)
{
final String key = getModelObjectAsString();
- final String value = getLocalizer().getString(key,
getParent(),
DEFAULT_VALUE);
- if (value != null !DEFAULT_VALUE.equals(value))
+ final String value = getLocalizer().getString(key,
getParent());
+ if (value != null)
{
replaceComponentTagBody(markupStream, openTag,
value.trim());
}
But that would produce a lousy error message:
WicketMessage: Exception in rendering component: [Component id =
_message_4, page = org.apache.wicket.examples.helloworld.HelloWorld,
path = 6:_message_4.WicketMessageResolver$MessageLabel, isVisible =
true, isVersioned = true]
Root cause:
java.util.MissingResourceException: Unable to find resource: test for
component: [class=org.apache.wicket.examples.helloworld.HelloWorld]
at org.apache.wicket.Localizer.getString(Localizer.java:261)
at org.apache.wicket.Localizer.getString(Localizer.java:91)
at
org.apache.wicket.markup.resolver.WicketMessageResolver$MessageLabel.onComponentTagBody(WicketMessageResolver.java:128)
at org.apache.wicket.Component.renderComponent(Component.java:2416)
Eelco
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
Yes we could do that. Just remove that default value.
But what i would like to have IF you have a body specified then we don't
throw anything
This way we keep old behavior and we have the new one
Because if i specifiy a body then thats the default value for me. And if not
then i still can see
very easy that i miss a key.
But i guess that is not that easy to do because we have no idea inside the
tag that it really has a body right?
but i am fine with this change.
johan
On 11/4/07, Eelco Hillenius [EMAIL PROTECTED] wrote:
The easiest solution would be this:
Index: /Users/eelcohillenius/Documents/workspace_wicket/trunk/jdk-1.4
/wicket/src/main/java/org/apache/wicket/markup/resolver/WicketMessageResolver.java
===
--- /Users/eelcohillenius/Documents/workspace_wicket/trunk/jdk-1.4
/wicket/src/main/java/org/apache/wicket/markup/resolver/WicketMessageResolver.java
(revision
591166)
+++ /Users/eelcohillenius/Documents/workspace_wicket/trunk/jdk-1.4
/wicket/src/main/java/org/apache/wicket/markup/resolver/WicketMessageResolver.java
(working
copy)
@@ -43,13 +43,6 @@
{
private static final Logger log =
LoggerFactory.getLogger(WicketMessageResolver.class);
- /**
-* If the key can't be resolved and the default is null, an
exception will be thrown. Instead,
-* we default to a unique string and check against this later.
Don't
just use an empty string
-* here, as people might want to override wicket:messages to empty
strings.
-*/
- private static final String DEFAULT_VALUE =
DEFAULT_WICKET_MESSAGE_RESOLVER_VALUE;
-
static
{
// register wicket:message
@@ -74,7 +67,7 @@
* @return true, if componentId was handle by the resolver. False,
otherwise
*/
public boolean resolve(final MarkupContainer container, final
MarkupStream markupStream,
- final ComponentTag tag)
+ final ComponentTag tag)
{
if (tag instanceof WicketTag)
{
@@ -85,7 +78,7 @@
if ((messageKey == null) || (
messageKey.trim().length() == 0))
{
throw new MarkupException(
- Wrong format of
wicket:message key='xxx': attribute 'key'
is missing);
+ Wrong format of
wicket:message key='xxx': attribute 'key' is
missing);
}
final String id = _message_ +
container.getPage().getAutoIndex();
@@ -91,7 +84,7 @@
final String id = _message_ +
container.getPage().getAutoIndex();
MessageLabel label = new MessageLabel(id,
messageKey);
label.setRenderBodyOnly(
container.getApplication().getMarkupSettings()
- .getStripWicketTags());
+ .getStripWicketTags());
container.autoAdd(label, markupStream);
// Yes, we handled the tag
@@ -125,8 +118,8 @@
protected void onComponentTagBody(MarkupStream
markupStream,
ComponentTag openTag)
{
final String key = getModelObjectAsString();
- final String value = getLocalizer().getString(key,
getParent(),
DEFAULT_VALUE);
- if (value != null !DEFAULT_VALUE.equals(value))
+ final String value = getLocalizer().getString(key,
getParent());
+ if (value != null)
{
replaceComponentTagBody(markupStream,
openTag, value.trim());
}
But that would produce a lousy error message:
WicketMessage: Exception in rendering component: [Component id =
_message_4, page = org.apache.wicket.examples.helloworld.HelloWorld,
path = 6:_message_4.WicketMessageResolver$MessageLabel, isVisible =
true, isVersioned = true]
Root cause:
java.util.MissingResourceException: Unable to find resource: test for
component: [class=org.apache.wicket.examples.helloworld.HelloWorld]
at org.apache.wicket.Localizer.getString(Localizer.java:261)
at org.apache.wicket.Localizer.getString(Localizer.java:91)
at
org.apache.wicket.markup.resolver.WicketMessageResolver$MessageLabel.onComponentTagBody
(WicketMessageResolver.java:128)
at org.apache.wicket.Component.renderComponent(Component.java:2416)
Eelco
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
On 11/3/07, Johan Compagner [EMAIL PROTECTED] wrote: Yes we could do that. Just remove that default value. But what i would like to have IF you have a body specified then we don't throw anything This way we keep old behavior and we have the new one Because if i specifiy a body then thats the default value for me. And if not then i still can see very easy that i miss a key. But i guess that is not that easy to do because we have no idea inside the tag that it really has a body right? Well, my point was that keeping the body as a default value is actually dangerous: if you have the default body the same as what you intended to put in the properties file for the default locale (which is something that I often do, and I can imagine other people doing that as well), and you make a spelling error in the key (of either the tag or in the properties file), you'd never notice Wicket not finding the property You want to define a body for previewability, but you don't want to default to that because this silent failure makes it hard to detect errors in your message keys. Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
Johan Compagner wrote: Yes we could do that. Just remove that default value. But what i would like to have IF you have a body specified then we don't throw anything This way we keep old behavior and we have the new one FWIW, I entirely agree with this. If we just change it for tags that have a default body then we'll almost certainly break people's sites. Regards, Al - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: empty wicket:message
On 11/3/07, Al Maw [EMAIL PROTECTED] wrote: Johan Compagner wrote: Yes we could do that. Just remove that default value. But what i would like to have IF you have a body specified then we don't throw anything This way we keep old behavior and we have the new one FWIW, I entirely agree with this. If we just change it for tags that have a default body then we'll almost certainly break people's sites. How about keeping this for the deployment mode (and log an error or warning once) but throw that exception in development mode? Eelco - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
