Hi Robert,
As you can see I've had a go at fixing the odd diagnostic message. I'd
be grateful if you could double-check this sometime.
====
The initial code did:
// match package plus first 4 chars
if (candidate.regionMatches(true, 0, name, 0, 38)) {...}
I presume the "38" was meant to be
length("org.apache.commons.logging.impl.") + 4.
However that works out to be 32+4=36. So the old code was actually
testing the first 6 chars.
I decided to make the code a little more self-explanatory (defined
constants etc) and went with the 4 chars as stated by the comment,
rather than the 4 chars as implemented.
====
I also switched from candidate.regionMatches(true, 0, name, ..) to
name.regionMatches(true, 0, candidate, ...). Actually, on hindsight that
wasn't necessary; I had another test in there for a while which used
name and therefore made them all consistent. However
x.regionMatches(.., y, ..) and
y.regionMatches(.., x, ..)
are both the same, yes? (as long as x and y are both non-null which they
always are).
====
Also, rather than trimming the user-specified classname only before
doing diagnostic output, the name is now trimmed within the
findUserSpecifiedLogClassName.
Cheers,
Simon
On Wed, 2006-04-19 at 08:57 +0000, [EMAIL PROTECTED] wrote:
> Author: skitching
> Date: Wed Apr 19 01:57:54 2006
> New Revision: 395181
>
> URL: http://svn.apache.org/viewcvs?rev=395181&view=rev
> Log:
> Fix problem with "suggested" alternative for invalid log adapter class.
> Always trim whitespace from user-specified log adapter class name.
>
> Modified:
>
> jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
>
> Modified:
> jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
> URL:
> http://svn.apache.org/viewcvs/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java?rev=395181&r1=395180&r2=395181&view=diff
> ==============================================================================
> ---
> jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
> (original)
> +++
> jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
> Wed Apr 19 01:57:54 2006
> @@ -77,6 +77,8 @@
> /** SimpleLog class name */
> private static final String LOGGING_IMPL_SIMPLE_LOGGER =
> "org.apache.commons.logging.impl.SimpleLog";
>
> + private static final String PKG_IMPL="org.apache.commons.logging.impl.";
> + private static final int PKG_LEN = PKG_IMPL.length();
>
> // -----------------------------------------------------------
> Constructors
>
> @@ -783,15 +785,13 @@
> messageBuffer.append(specifiedLogClassName);
> messageBuffer.append("' cannot be found or is not useable.");
>
> - //
> // Mistyping or misspelling names is a common fault.
> // Construct a good error message, if we can
> if (specifiedLogClassName != null) {
> - final String trimmedName = specifiedLogClassName.trim();
> - informUponSimilarName(messageBuffer, trimmedName,
> LOGGING_IMPL_LOG4J_LOGGER);
> - informUponSimilarName(messageBuffer, trimmedName,
> LOGGING_IMPL_JDK14_LOGGER);
> - informUponSimilarName(messageBuffer, trimmedName,
> LOGGING_IMPL_LUMBERJACK_LOGGER);
> - informUponSimilarName(messageBuffer, trimmedName,
> LOGGING_IMPL_SIMPLE_LOGGER);
> + informUponSimilarName(messageBuffer,
> specifiedLogClassName, LOGGING_IMPL_LOG4J_LOGGER);
> + informUponSimilarName(messageBuffer,
> specifiedLogClassName, LOGGING_IMPL_JDK14_LOGGER);
> + informUponSimilarName(messageBuffer,
> specifiedLogClassName, LOGGING_IMPL_LUMBERJACK_LOGGER);
> + informUponSimilarName(messageBuffer,
> specifiedLogClassName, LOGGING_IMPL_SIMPLE_LOGGER);
> }
> throw new
> LogConfigurationException(messageBuffer.toString());
> }
> @@ -845,19 +845,25 @@
> }
>
>
> -
> /**
> * Appends message if the given name is similar to the candidate.
> * @param messageBuffer <code>StringBuffer</code> the message should be
> appended to,
> * not null
> * @param name the (trimmed) name to be test against the candidate, not
> null
> - * @param candidate the candidate name
> + * @param candidate the candidate name (not null)
> */
> private void informUponSimilarName(final StringBuffer messageBuffer,
> final String name,
> final String candidate) {
> - // this formular (first four letters of the name excluding package)
> - // gives a reason guess
> - if (candidate.regionMatches(true, 0, name, 0, 38)) {
> + if (name.equals(candidate)) {
> + // Don't suggest a name that is exactly the same as the one the
> + // user tried...
> + return;
> + }
> +
> + // If the user provides a name that is in the right package, and gets
> + // the first 4 characters of the adapter class right (ignoring case),
> + // then suggest the candidate adapter class name.
> + if (name.regionMatches(true, 0, candidate, 0, PKG_LEN + 4)) {
> messageBuffer.append(" Did you mean '");
> messageBuffer.append(candidate);
> messageBuffer.append("'?");
> @@ -917,8 +923,14 @@
> }
> }
>
> + // Remove any whitespace; it's never valid in a classname so its
> + // presence just means a user mistake. As we know what they meant,
> + // we may as well strip the spaces.
> + if (specifiedClass != null) {
> + specifiedClass = specifiedClass.trim();
> + }
> +
> return specifiedClass;
> -
> }
>
>
>
>
>
> ---------------------------------------------------------------------
> 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]
