Hello,
I'm pretty new to the Trinidad community, so let me apologize in advance
if there's something I've misunderstood.
What I'd like to talk about are the differences between the client-side
message formatter:
trinidad-impl\src\main\javascript\META-INF\adf\jsLibs\Core.js ->
function _formatErrorString(errorFormat, tokens)
and the server-side message formatter:
trinidad-api\src\main\java\org\apache\myfaces\trinidad\util\FastMessageFormat.java
-> public String format(Object[] source)
The problem occurs when the application developer provides a custom
error message, such as messageDetailXYZ="Isn't {0} cool?". When this is
formatted by the client formatter, everything works fine. But let's say,
for whatever reason, client validation is now disabled, and therefore
this error message is formatted by the server formatter. When it hits
the apostrophe, the rest of the message is interpreted literally, so the
resulting formatted error message is "Isnt {0} cool?". This is bound to
confuse the app developer. If the app developer instead provides
messageDetailXYZ="Isn''t {0} cool?", then both the client and server
formatter return "Isn't token0 cool?" (that is, it's formatted
correctly), but I think requiring special escaping complicates things as
well. Ultimately, I think messageDetailXYZ="Isn't {0} cool?" should be
formatted correctly by both the client and server formatter.
I realize that since the server formatter is part of the public api, it
may be difficult to change now. However, if others think it might be
possible to resolve the client and server formatter differences, then
please see the code pasted below, where I have inserted several
questions and comments of the form:
///
// my comment here
///
Thank you,
Cale
---
Let's start with the server formatter. Personally, I prefer the server
formatter's method of replacing patterns with tokens. However, if a
pattern doesn't have an associated token, I'd prefer the server
formatter just copy the pattern to the output string (in other words,
interpret it as literal text), instead of throwing an exception. This
way, there's no need to use single quotes for special escaping behavior.
trinidad-api\src\main\java\org\apache\myfaces\trinidad\util\FastMessageFormat.java:
/**
* Formats the given array of strings based on the initial
* pattern. It is legal for this array to be shorter
* than that indicated by the pattern, or to have null
* entries - these will simply be ignored.
* <p>
* @param source an array of strings
*/
public String format(Object[] source)
{
int formatLength = _formatText.length;
int length = 0;
int sourceCount = source.length;
for (int i = 0; i < sourceCount; i++)
{
Object sourceString = source[i];
if (sourceString != null)
{
length += sourceString.toString().length();
}
}
StringBuffer buffer = new StringBuffer(length + formatLength);
*///
// The above buffer size assumes that each pattern is only replaced
once, i.e.
// if {0} occurs two or more times, only one instance is replaced. If we
are
// making this assumption, then it should be enforced when replacing
patterns below.
// If it's perfectly legal for {0} to be replaced in several spots, then
we should comment
// that the above buffer size is just a rough initial estimate.
// Furthermore, if this function is only accessed by a single thread, we
should
// instead use StringBuilder for performance purposes.
/// *
int lastStart = 0;
boolean inQuote = false;
for (int i = 0; i < formatLength; i++)
{
char ch = _formatText[i];
if (inQuote)
{
if (ch == '\'')
{
buffer.append(_formatText, lastStart, i - lastStart);
i++;
lastStart = i;
inQuote = false;
}
}
else
{
if (ch == '\'')
{
buffer.append(_formatText, lastStart, i - lastStart);
i++;
lastStart = i;
// Check for doubled-up quotes
if ((i < formatLength) && (_formatText[i] == '\''))
{
// Do nothing; we'll add the doubled-up quote later
;
}
else
{
inQuote = true;
}
}
*///
// I think using single quotes to escape text is unnecessary. We should
only try to
// replace patters of the type "{number}" for which 'number' is a valid
token index.
///*
else if (ch == '{')
{
buffer.append(_formatText, lastStart, i - lastStart);
int sourceIndex = 0;
int j = i + 1;
for (; j < formatLength; j++)
{
char patternChar = _formatText[j];
if (patternChar == '}')
{
break;
}
else
{
if ((patternChar < '0') || (patternChar > '9'))
throw new
IllegalArgumentException(_LOG.getLogger().getResourceBundle().getString("FASTMESSAGEFORMAT_ONLY_SUPPORT_NUMERIC_ARGUMENTS"));
sourceIndex = (sourceIndex * 10) + (patternChar - '0');
}
}
*///
// This seems overly complex. Can't we put a hard-coded limit on the
pattern number?
// For example, are we ever going to have more than 10 tokens? It would
be much simpler
// if we only had to look for a single digit number followed by a '}'.
///*
if (j == formatLength)
throw new
IllegalArgumentException(_LOG.getLogger().getResourceBundle().getString("END_OF_PATTERN_NOT_FOUND"));
if (j == i + 1)
throw new
IllegalArgumentException(_LOG.getLogger().getResourceBundle().getString("FASTMESSAGEFORMAT_FIND_EMPTY_ARGUMENT"));
*///
// I think for the above cases we should just interpret the text
literally instead
// of throwing exceptions.
///*
if (sourceIndex < sourceCount)
{
Object sourceString = source[sourceIndex];
if (sourceString != null)
buffer.append(sourceString.toString());
}
*///
// If the pattern "{*}" contains anything besides a
// non-negative number, we throw an exception, but if it contains a
number >= the
// number of tokens, the whole "{*}" is replaced by the empty string
(I'm surprised
// that for consistency purposes that an exception isn't thrown
instead). However,
// I think that in both of these cases the pattern should just be
interpreted as literal text,
// and that the pattern should only be replaced with the empty string
when it has an
// associated token and the token is null or empty.
///*
i = j;
lastStart = i + 1;
}
else
{
// Do nothing. The character will be added in later
;
}
}
}
buffer.append(_formatText, lastStart, formatLength - lastStart);
return new String(buffer);
}
---
Now let's talk about the client formatter. Using regular expressions to
match patterns is elegant, but personally I think it's overkill for what
we need here. The server formatter's method of replacing patterns with
tokens is faster, plus it completely avoids the issue of infinite
pattern replacement, should the token itself contain "{0}" or something
like that.
trinidad-impl\src\main\javascript\META-INF\adf\jsLibs\Core.js:
/**
* Performs token replacement on the the error format, replacing each
* token found in the token Object with the value for that token.
*/
function _formatErrorString(
errorFormat, // error format string with embedded tokens to be replaced
tokens // tokens Object containin token names and values to be
replaced
)
{
var currString = errorFormat;
*///
// We have a fundamental difference in behavior between this formatter
and the
// server formatter. This formatter only tries to replace patterns for
which we
// actually have a token; the server formatter throws exceptions.
///*
// loop through all of the tokens, replacing them if they are present
for (var currToken in tokens)
{
var currValue = tokens[currToken];
// if the token has no value
if (!currValue)
{
currValue = "";
}
// TRINIDAD-829:
// we replace '{' and '}' to ensure, that tokens containing values
// like {3} aren't parsed more than once...
// Only do this if it is typeof string (see TRINIDAD-873)
if (typeof currValue == "string")
{
currValue = currValue.replace("{","{'");
currValue = currValue.replace("}","'}");
}
*///
// 1) - footnote for above text
// This is a good way to avoid the infinite pattern replacement problem;
however,
// even better would be to use the server formatter's replacement method :)
///*
// the tokens are delimited by '{' before and '}' after the token
var currRegExp = "{" + currToken + "}";
// support tokens of the form %token% as well as {token}
currString = currString.replace(new RegExp('%' + currToken + '%', 'g'),
currRegExp);
*///
// Do we still need to support tokens of the type %number%? The server
formatter
// only supports tokens of the type {number}. If one supports both
types, so should
// the other.
///*
// Replace the token. Don't use String.replace, as the value may
// include dollar signs, which leads Netscape astray (bug 2242675)
var indexOf = currString.indexOf(currRegExp);
*///
// Doesn't footnote 1) make the following check unnecessary?
/// *
if (currValue.indexOf && currValue.indexOf(currRegExp) >= 0)
{
var b1 = '';
for (i=0; i<currValue.length; i++)
{
b1 = b1 + 'placeHolderString';
}
while (indexOf >= 0)
{
currString=(currString.substring(0,indexOf)
+ b1
+ currString.substring(indexOf+currRegExp.length));
indexOf = currString.indexOf(currRegExp);
}
indexOf = currString.indexOf(b1);
while (indexOf >= 0)
{
currString =(currString.substring(0,indexOf)
+ currValue
+ currString.substring(indexOf+b1.length));
indexOf = currString.indexOf(b1);
}
}
else
while (indexOf >= 0)
{
currString = (currString.substring(0, indexOf)
+ currValue
+ currString.substring(indexOf + currRegExp.length));
indexOf = currString.indexOf(currRegExp);
}
}
// TRINIDAD-829:
// we finally re-replace the '{' and '}'...
while(currString.indexOf("{'")!=-1)
{
currString= currString.replace("{'","{");
currString= currString.replace("'}","}");
}
*///
// Having the above behavior means already existing "{'" will be
replaced as well;
// if we must keep this behavior, it should be well documented.
///*
// And now take any doubled-up single quotes down to one,
// to handle escaping
var twoSingleQuotes = /''/g;
*///
// Having the above behavior means already existing "''" will be
replaced as well;
// if we must keep this behavior, it should be well documented.
///*
return currString.replace(twoSingleQuotes, "'");
}
