You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Paul Smith <ps...@aconex.com> on 2007/04/24 08:57:20 UTC

Proposed patch to LocationInfo in 1.2.15

A new constructor was added to LocationInfo for 1.2.15, but it  
doesn't quite work properly.  The other get*() methods use the  
fullInfo field to calculate values, but the new constructor  
effectively sets them.

I'm proposing the following diff:

Index: src/java/org/apache/log4j/spi/LocationInfo.java
===================================================================
--- src/java/org/apache/log4j/spi/LocationInfo.java     (revision  
531299)
+++ src/java/org/apache/log4j/spi/LocationInfo.java     (working copy)
@@ -179,6 +179,8 @@
      */
      public
      String getClassName() {
+        if(className != null ) return className;
+
        if(fullInfo == null) return NA;
        if(className == null) {
         // Starting the search from '(' is safer because there is
@@ -219,6 +221,8 @@
      */
      public
      String getFileName() {
+        if(fileName !=null) return fileName;
+
        if(fullInfo == null) return NA;
        if(fileName == null) {
@@ -240,6 +244,8 @@
      */
      public
      String getLineNumber() {
+        if(lineNumber !=null ) return lineNumber;
+
        if(fullInfo == null) return NA;
        if(lineNumber == null) {
@@ -258,6 +264,8 @@
      */
      public
      String getMethodName() {
+        if( methodName !=null ) return methodName;
+
        if(fullInfo == null) return NA;
        if(methodName == null) {
         int iend = fullInfo.lastIndexOf('(');


This will then give the juli-log4j bridge complete LocationInfo  
support (other than java.util.logging not supporting the line # and  
source file data AFAIK)

Paul

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Re: Proposed patch to LocationInfo in 1.2.15

Posted by Curt Arnold <ca...@apache.org>.
On Apr 24, 2007, at 10:35 AM, Curt Arnold wrote:

>
> On Apr 24, 2007, at 1:57 AM, Paul Smith wrote:
>
>> A new constructor was added to LocationInfo for 1.2.15, but it  
>> doesn't quite work properly.  The other get*() methods use the  
>> fullInfo field to calculate values, but the new constructor  
>> effectively sets them.
>>
>
> I fell down on that one.  I can't believe that I didn't write any  
> unit tests or mark the method as @since 1.2.15.
>
> As for the patch, I think it is safer to try to compensate in the  
> new constructor for the old behavior instead of the other way  
> around.  I have a rewrite of the constructor that will build the  
> fullinfo so that the getClass() et al methods would work as they  
> are.  I'll commit when I have got the tests in place.
>
> The parameter order could be a bit improved.  It mixes up the  
> logical location (class, method) fragments with the physical  
> location (file, line) fragments.  I'll review it and possibly  
> switch the order to class, method, file, line.  Unfortunately,  
> there would be no signature change so we have to make sure that we  
> fix all the existing uses of the constructor.
>

Committed change rev 531995  The parameter order was set several  
years ago in log4j 1.3.  Can't change it.



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Re: Proposed patch to LocationInfo in 1.2.15

Posted by Curt Arnold <ca...@apache.org>.
On Apr 24, 2007, at 1:57 AM, Paul Smith wrote:

> A new constructor was added to LocationInfo for 1.2.15, but it  
> doesn't quite work properly.  The other get*() methods use the  
> fullInfo field to calculate values, but the new constructor  
> effectively sets them.
>

I fell down on that one.  I can't believe that I didn't write any  
unit tests or mark the method as @since 1.2.15.

As for the patch, I think it is safer to try to compensate in the new  
constructor for the old behavior instead of the other way around.  I  
have a rewrite of the constructor that will build the fullinfo so  
that the getClass() et al methods would work as they are.  I'll  
commit when I have got the tests in place.

The parameter order could be a bit improved.  It mixes up the logical  
location (class, method) fragments with the physical location (file,  
line) fragments.  I'll review it and possibly switch the order to  
class, method, file, line.  Unfortunately, there would be no  
signature change so we have to make sure that we fix all the existing  
uses of the constructor.



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org