Hi Chris,

patch looks fine, but I think it would be better if the message was internationalized. Could you please improve it ?

Thanks,

Hi Erwan,

Internationalize which message? The tooltips are internationalized. Are you talking about the log message?:

Debug.logInfo("No help text found in [" + entityResourceName + "] with key [" + messageId + "]", module);

Many thanks,

Chris

Sorry Chris,
I was a bit too quick on reading the patch.... Please forget what I said before !
Will commit today !

Thanks Chris, done in trunk at 941261 and in 10.04 at 941262

I just modified your patch to add a test if logInfo is active.

Hi Erwan,

Sorry to be a pain, but the change with the Debug.infoOn() check is broken, it should be:

// return an empty field help description if there is no value found for the key
if (fieldDescription.equals(messageId)) {
    if (Debug.infoOn()) {
        Debug.logInfo("No help text found in [" + entityResourceName + "] with key [" + messageId + "]", module);
    }
    return "";
}

I.e. we need to return an empty string if there is no value found for the key, regardless of whether debuging is on or off.

Cheers,

Chris

see previous comment

Done at 941600 and 941603

Thanks Chris,

Two quick questions ...

1) Since Debug.logInfo ultimately determines if logging for "info" is on, isn't it better to just call Debug.logInfo directly (paying the string construction penalty of course) ?

2) While it is pretty unlikely that the help key here would be defined in the resource bundle with the same value; this code would turn those into empty string. Would it be better to overload getMessage to allow an optional default message ... so we could do ...

String fieldDescription = UtilProperties.getMessage(entityResourceName, messageId, locale, null);
if (fieldDescription == null) {
...

Hi Bob, I agree with you on both points...