You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sk...@apache.org on 2006/04/19 10:57:55 UTC
svn commit: r395181 -
/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: svn commit: r395181 -
/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Wed, 2006-04-19 at 21:09 +1200, Simon Kitching wrote:
> 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.
the comment's wrong: Jdk13Lumberjacklog and Jdk14Logger have the same
first 4 characters. 5 should be enough. (will need to check this when i
can find the time)
> ====
> 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).
+1
> ====
> Also, rather than trimming the user-specified classname only before
> doing diagnostic output, the name is now trimmed within the
> findUserSpecifiedLogClassName.
+1
looks good
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: svn commit: r395181 -
/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Posted by Simon Kitching <sk...@apache.org>.
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, skitching@apache.org 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: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org