[GitHub] [logging-log4j2] vy commented on a diff in pull request #1137: LOG4J2-3627: PatternLayout: %xEx{ [ "short" | depth ] } not working

[GitBox <gi...@apache.org>]

vy commented on code in PR #1137:
URL: https://github.com/apache/logging-log4j2/pull/1137#discussion_r1017115243


##########
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ExtendedThrowablePatternConverter.java:
##########
@@ -71,8 +72,22 @@ public void format(final LogEvent event, final StringBuilder toAppendTo) {
             if (len > 0 && !Character.isWhitespace(toAppendTo.charAt(len - 1))) {
                 toAppendTo.append(' ');
             }
-            proxy.formatExtendedStackTraceTo(toAppendTo, options.getIgnorePackages(),
+            final String trace = proxy.getExtendedStackTraceAsString(options.getIgnorePackages(),

Review Comment:
   To be honest, I would appreciate a structural fix practicing some Boy Scout Rule: _"Always leave the campground cleaner than you found it."_ AFAIC, the entire `*Converter` family suffers from _"let me fix this particular issue for this particular converter"_ attempts. Please don't get me wrong, I am not pointing any fingers; I am as guilty as everybody else who is in a position to improve this situation. I also don't want to discourage you, I cannot emphasize enough how much I appreciate your effort. Long story short, I personally prefer a structural fix: moving this logic to a single place, testing it decently, and reusing it wherever needed. If you can provide one, that would be awesome. Otherwise, I will try to give it a shot myself, though I need to admit that my Log4j agenda is already pretty packed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org