gnodet commented on code in PR #12695:
URL: https://github.com/apache/maven/pull/12695#discussion_r4059676683


##########
impl/maven-logging/src/main/java/org/apache/maven/slf4j/MavenSimpleLogger.java:
##########
@@ -120,6 +118,60 @@ protected void write(int level, String loggerName, String 
cleanMessage, StringBu
         }
     }
 
+    /**
+     * Append a colorized throwable rendering to the given builder.
+     * Reuses the existing formatting logic for consistency with console 
output.
+     */
+    private void appendFormattedThrowable(StringBuilder sb, Throwable t, 
String prefix) {
+        MessageBuilder builder = 
builder().a(prefix).failure(t.getClass().getName());
+        if (t.getMessage() != null) {
+            builder.a(": ").failure(t.getMessage());
+        }
+        sb.append(builder.toString()).append(System.lineSeparator());
+        appendStackTrace(sb, t, prefix);
+    }
+
+    private void appendStackTrace(StringBuilder sb, Throwable t, String 
prefix) {
+        MessageBuilder builder = builder();
+        for (StackTraceElement e : t.getStackTrace()) {
+            builder.a(prefix);
+            builder.a("    ");
+            builder.strong("at");
+            builder.a(" ");
+            builder.a(e.getClassName());
+            builder.a(".");
+            builder.a(e.getMethodName());
+            builder.a("(");
+            builder.strong(getLocation(e));
+            builder.a(")");
+            sb.append(builder.toString()).append(System.lineSeparator());
+            builder.setLength(0);
+        }
+        for (Throwable se : t.getSuppressed()) {
+            builder.a(prefix)
+                    .a("    ")
+                    .strong("Suppressed")
+                    .a(": ")
+                    .a(se.getClass().getName());
+            if (se.getMessage() != null) {
+                builder.a(": ").failure(se.getMessage());
+            }
+            sb.append(builder.toString()).append(System.lineSeparator());
+            builder.setLength(0);
+            appendStackTrace(sb, se, prefix + "    ");
+        }
+        Throwable cause = t.getCause();
+        if (cause != null && t != cause) {
+            builder.a(prefix).strong("Caused by").a(": 
").a(cause.getClass().getName());
+            if (cause.getMessage() != null) {
+                builder.a(": ").failure(cause.getMessage());
+            }
+            sb.append(builder.toString()).append(System.lineSeparator());
+            builder.setLength(0);
+            appendStackTrace(sb, cause, prefix);

Review Comment:
   Fixed in `32901219fc` — added `MAX_THROWABLE_DEPTH = 20` constant; 
`printStackTrace`/`writeThrowable` now pass a `depth` counter and emit a 
truncation message when the limit is reached.



##########
api/maven-api-core/src/main/java/org/apache/maven/api/plugin/Log.java:
##########
@@ -38,49 +38,38 @@
 public interface Log {
     /**
      * {@return true if the <b>trace</b> error level is enabled}
-     * <p>
-     * The default implementation returns {@code false} for backward
-     * compatibility with existing {@code Log} implementations.
+     * @since 4.1.0
      */
-    default boolean isTraceEnabled() {
-        return false;
-    }
+    boolean isTraceEnabled();

Review Comment:
   Fixed in `f656ea9567` — all six trace methods restored to `default` with 
backward-compatible no-op implementations.



##########
api/maven-api-core/src/main/java/org/apache/maven/api/build/report/LogEvent.java:
##########
@@ -157,7 +158,8 @@ default long threadId() {
      * providing a global ordering across all event sources (Log API,
      * JUL, and direct SLF4J).
      *
-     * @return the sequence number, or {@code -1} if unavailable
+     * @return the sequence number, always non-negative
+     * @since 4.1.0
      */
     default long sequenceNumber() {
         return -1;

Review Comment:
   Fixed in `f656ea9567` — Javadoc now says "or {@code -1} if unavailable".



##########
impl/maven-core/src/main/java/org/apache/maven/logging/LoggingExecutionListener.java:
##########
@@ -129,45 +129,40 @@ public void mojoStarted(ExecutionEvent event) {
     @Override
     public void mojoSucceeded(ExecutionEvent event) {
         setMdc(event);
-        delegate.mojoSucceeded(event);
         ProjectBuildLogAppender.setMojoId(null);
+        delegate.mojoSucceeded(event);
     }
 
     @Override
     public void mojoFailed(ExecutionEvent event) {
         setMdc(event);
-        delegate.mojoFailed(event);
         ProjectBuildLogAppender.setMojoId(null);
+        delegate.mojoFailed(event);
     }
 
     @Override
     public void mojoSkipped(ExecutionEvent event) {

Review Comment:
   Fixed in `f656ea9567` — `mojoSkipped()` now calls `setMojoId(null)`.



##########
api/maven-api-core/src/main/java/org/apache/maven/api/plugin/Log.java:
##########
@@ -38,49 +38,38 @@
 public interface Log {
     /**
      * {@return true if the <b>trace</b> error level is enabled}
-     * <p>
-     * The default implementation returns {@code false} for backward
-     * compatibility with existing {@code Log} implementations.
+     * @since 4.1.0
      */
-    default boolean isTraceEnabled() {
-        return false;
-    }
+    boolean isTraceEnabled();

Review Comment:
   Fixed in `f656ea9567` — `isTraceEnabled()` and all five `trace()` overloads 
restored to `default`.



##########
impl/maven-core/src/main/java/org/apache/maven/logging/LoggingExecutionListener.java:
##########
@@ -129,45 +129,40 @@ public void mojoStarted(ExecutionEvent event) {
     @Override
     public void mojoSucceeded(ExecutionEvent event) {
         setMdc(event);
-        delegate.mojoSucceeded(event);
         ProjectBuildLogAppender.setMojoId(null);
+        delegate.mojoSucceeded(event);
     }
 
     @Override
     public void mojoFailed(ExecutionEvent event) {
         setMdc(event);
-        delegate.mojoFailed(event);
         ProjectBuildLogAppender.setMojoId(null);
+        delegate.mojoFailed(event);
     }
 
     @Override
     public void mojoSkipped(ExecutionEvent event) {
         setMdc(event);
         delegate.mojoSkipped(event);

Review Comment:
   Both fixed in `f656ea9567`: (1) `mojoSkipped()` now calls 
`ProjectBuildLogAppender.setMojoId(null)`; (2) `forkStarted()` now calls 
`setForkingMojoId(ProjectBuildLogAppender.getMojoId())` and 
`forkSucceeded`/`forkFailed` clear it — the restore path is live.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to