[ 
https://issues.apache.org/jira/browse/LOG4J2-160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13571694#comment-13571694
 ] 

Joanne Polsky edited comment on LOG4J2-160 at 2/5/13 8:35 PM:
--------------------------------------------------------------

I’ve attached the first patch for review; LOG4J2-160-1.patch.  I'd like to 
split this change into 2 changes lists if possible to reduce complexity.  The 
first being the change to include the new separator option and apply it to the 
pattern converters.  The second would address the performance improvements for 
ExtendedThrowablePatternConverter and RootThrowablePatternConverter in 
ThrowableProxy; there are some additional complexities that I would like to 
discuss outside of the context of this initial change.

I’ve created a new class called ThrowableFormatOptions which contains the 
parsing logic for the %throwable, %xThrowable and %rThrowable  patterns.  Each 
will support a new option to have a custom stack trace separator, for instance:
%ex{full}{separator(|)}
%ex{separator(|)}

This works fine for ThrowablePatternConverter, however, for 
ExtendedThrowablePatternConverter/RootThrowablePatternConverter there seems to 
be inconsistency between how options are delimited.  From the user guide 
(http://logging.apache.org/log4j/2.x/log4j-users-guide.pdf), these patterns can 
use a comma delimiter:
%rEx["none"|"short"|"full"|depth],[filters(packages)}

However, this doesn't seem consistent with the way other patterns delimit the 
options by enclosing them in braces { }, for instance the following seems more 
correct:
%rEx[{("none"|"short"|"full"|depth)}][{filters(packages)}]

I copied the existing code to support the case where 2 options are used 
{full,filters_or_separator), but I didn't add support for the case where all 3 
options are used {full,filters,separator}.  It complicates parsing considerably 
since filters uses commas to delimit the packages as well.  It requires these 
classes to use custom parsing techniques that are outside the standard 
framework which doesn't feel natural.  Is is possible to change the recommended 
usage in the user guide to use the standard delimiters instead?  Since this is 
still considered beta, is it acceptable to drop the comma syntax in favor of 
the braces exclusively?

                
      was (Author: jpolsky):
    I’ve attached the first patch for review; LOG4J2-160-1.patch.  I'd like to 
split this change into 2 changes lists if possible to reduce complexity.  The 
first being the change to include the new separator option and apply it to the 
pattern converters.  The second would address the performance improvements for 
ExtendedThrowablePatternConverter and RootThrowablePatternConverter in 
ThrowableProxy; there are some additional complexities that I would like to 
discuss outside of the context of this initial change.

I’ve created a new class called ThrowableFormatOptions which contains the 
parsing logic for the %throwable, %xThrowable and %rThrowable  patterns.  Each 
will support a new option to have a custom stack trace separator, for instance:
%ex{full}{separator(|)}
%ex{separator(|)}

This works fine for ThrowablePatternConverter, however, for 
ExtendedThrowablePatternConverter/RootThrowablePatternConverter there seems to 
be inconsistency between how options are delimited.  From the user guide 
(http://logging.apache.org/log4j/2.x/log4j-users-guide.pdf), these patterns can 
use a comma delimiter:
%rEx["none"|"short"|"full"|depth],[filters(packages)}

However, this doesn't seem consistent with the way other patterns delimit the 
options by enclosing them in braces { }, for instance the following seems more 
correct:
%rEx[{("none"|"short"|"full"|depth)}][{filters(packages)}]

I copied the existing code to support the case where 2 options are used 
{full,filters_or_separator), but I didn't add support for the case where all 3 
options are used {full,filters,separator}.  It complicates parsing considerably 
since filters uses commas to delimit the packages and requires these classes to 
use custom parsing techniques that are outside the standard framework.  Is is 
possible to change the recommended usage in the user guide to use the standard 
delimiters instead?  Since this is still considered beta, is it acceptable to 
drop the comma syntax in favor of the braces exclusively?

                  
> Include option in throwable pattern converter to control stack trace separator
> ------------------------------------------------------------------------------
>
>                 Key: LOG4J2-160
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-160
>             Project: Log4j 2
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Joanne Polsky
>            Priority: Trivial
>         Attachments: LOG4J2-160-1.patch
>
>
> Copying feature request from Bugzilla 1.x:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=51122

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to