I should add that since you've identified them it would be good to make sure
there is a unit test for each of them.
Ralph
On Feb 1, 2013, at 12:04 PM, Ralph Goers wrote:
> Yes - this all sounds good to me.
>
> Ralph
>
> On Feb 1, 2013, at 11:27 AM, Joanne Polsky wrote:
>
>> Thanks Ralph and Gary, that makes more sense. I indicated it would default
>> to "\n" since that is what is hard coded in the source now. I can update to
>> Constants.LINE_SEP while adding this new feature.
>>
>> The ThrowablePatternConverter currently doesn't parse the filters, so having
>> delim as it's own option may be simpler in the end. I would think the
>> following patterns should be supported:
>>
>> %throwable{full}{delim( | )}
>> %throwable{delim( | )}
>> %throwable
>> %xThrowable{full}{filters(package1,package2)}{delim( | )}
>> %xThrowable{full}{delim( | )}
>> %xThrowable{delim( | )}
>> %xThrowable
>>
>> Also the delim could be more than one character, for instance we use
>> space+pipe+space as our separator.
>>
>> Would those patterns make sense?
>>
>> Joanne
>>
>> From: [email protected]
>> Subject: Re: Feature Request from 1.x: Include option in throwable pattern
>> converter to control stack trace separator
>> Date: Fri, 1 Feb 2013 11:19:35 -0800
>> To: [email protected]
>>
>> Yes. That is available as Constants.LINE_SEP.
>>
>> Ralph
>>
>> On Feb 1, 2013, at 11:03 AM, Gary Gregory wrote:
>>
>> Shouldn't the default be the line.separator system property?
>>
>> Gary
>>
>>
>> On Fri, Feb 1, 2013 at 1:54 PM, Ralph Goers <[email protected]>
>> wrote:
>> First, thanks for participating! The process you are following is perfectly
>> fine. It is always great to start a discussion of a new feature on the dev
>> list to work out any details. You can create a Jira with the feature
>> request and then attach a patch once you have completed the work. Assigning
>> the Jira to yourself lets us know that you plan to work on it so that is a
>> good idea.
>>
>> We could either use the syntax you are proposing or
>>
>> %xEx{full}{filters(package1,package2),delim(|)}
>>
>> which would be a little shorter. In both cases I would expect specifying
>> the filters should be optional if the delimiter is included.
>>
>> FWIW, this change will be a bit more extensive than just changing line 115.
>> The extended throwable pattern converters do most of their formatting in
>> ThrowableProxy, so it would need to be made aware of the delimiter.
>>
>> Ralph
>>
>> On Feb 1, 2013, at 9:54 AM, Joanne Polsky wrote:
>>
>> Hello,
>>
>> Some time ago, I had submitted a feature request for Log4j 1.x which I never
>> got around to actually implementing:
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=51122
>>
>> I see that this feature request would apply to the new Log4j 2.x source as
>> well:
>> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverter.java?view=markup
>>
>> Now that I actually have some time, I'd like to make this contribution if no
>> one has any concerns.
>>
>> Essentially the change is to make the appended new line configurable to be
>> any string delimiter default being "\n":
>> [ThrowablePatternConverter.java:115] sb.append(array[i]).append("\n");
>>
>> I had originally thought that a pattern like the following might work
>> "%throwable{full}{ | }". However, it looks like in 2.0, there are some
>> classes which extend the ThrowablePatternConverter that assume the second
>> option may be a list of filters:
>> https://svn.apache.org/repos/asf/logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/pattern/ExtendedThrowablePatternConverter.java
>> https://svn.apache.org/repos/asf/logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/pattern/RootThrowablePatternConverter.java
>>
>> I was thinking I could still implement this change in
>> ThrowablePatternConverter where it would iterate through the options looking
>> for the specific prefix of delim? So for instance the user would specify
>> something like the following (not sure if I like that delim but I can't
>> think of another identifier):
>> %throwable{full}{delim( | )}
>> %XThrowable{full}{filters(package1,package2)}{delim( | )}
>> %rThrowable{full}{filters(package1,package2)}{delim( | )}
>>
>> This would be my first contribution to any Apache software if this feature
>> was accepted so I'm not exactly sure how to proceed if I'm interested in
>> making the contribution myself. I guess the first step would be to to copy
>> the feature request from the 1.0 bugzilla to the 2.0 JIRA
>> (https://issues.apache.org/jira/browse/LOG4J2). Is there some approval
>> process for the feature before I start implementing the change? Or would I
>> simply assign the JIRA to myself, make the code change I think makes sense,
>> then submit for code review?
>>
>> Thanks,
>> Joanne
>>
>>
>>
>>
>> --
>> E-Mail: [email protected] | [email protected]
>> JUnit in Action, 2nd Ed: http://bit.ly/ECvg0
>> Spring Batch in Action: http://bit.ly/bqpbCK
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>>
>
