gnodet-bot commented on code in PR #13180:
URL: https://github.com/apache/maven/pull/13180#discussion_r4061045230


##########
impl/maven-cli/src/main/java/org/apache/maven/cling/event/RichExecutionEventLogger.java:
##########
@@ -0,0 +1,377 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.cling.event;
+
+import java.time.Duration;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.maven.api.MonotonicClock;
+import org.apache.maven.api.services.MessageBuilder;
+import org.apache.maven.api.services.MessageBuilderFactory;
+import org.apache.maven.cling.utils.CLIReportingUtils;
+import org.apache.maven.execution.AbstractExecutionListener;
+import org.apache.maven.execution.BuildFailure;
+import org.apache.maven.execution.BuildSuccess;
+import org.apache.maven.execution.BuildSummary;
+import org.apache.maven.execution.ExecutionEvent;
+import org.apache.maven.execution.MavenExecutionResult;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.project.MavenProject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.maven.cling.utils.CLIReportingUtils.formatDuration;
+
+/**
+ * Execution event logger for the rich terminal mode ({@code --console=rich}).
+ * <p>
+ * In rich mode, the {@link RichBuildEventListener} manages a JLine status bar 
at
+ * the bottom of the terminal showing live reactor progress. This logger is 
deliberately
+ * minimal — it suppresses the verbose per-mojo and per-project banners that
+ * {@link ExecutionEventLogger} produces, since the status bar replaces them.
+ * <p>
+ * What this logger DOES print (above the status bar):
+ * <ul>
+ *   <li>One line per completed module (like {@link 
PlainExecutionEventLogger})</li>
+ *   <li>Build result summary ({@code BUILD SUCCESS/FAILURE})</li>
+ *   <li>Compact module statistics and timing</li>
+ *   <li>Pointer to the structured build report</li>
+ * </ul>
+ * <p>
+ * What the status bar shows (managed by {@link RichBuildEventListener}):
+ * <ul>
+ *   <li>Currently building modules with active mojo name</li>
+ *   <li>Reactor progress ({@code [n/total]}) and elapsed time</li>
+ *   <li>Active downloads with progress</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ * @see RichBuildEventListener
+ * @see PlainExecutionEventLogger
+ */
+public class RichExecutionEventLogger extends AbstractExecutionListener {
+
+    private static final int MAX_LOG_PREFIX_SIZE = 8; // "[ERROR] "
+    private static final int PROJECT_STATUS_SUFFIX_SIZE = 20; // "SUCCESS [  
0.000 s]"
+    private static final int MIN_TERMINAL_WIDTH = 60;
+    private static final int DEFAULT_TERMINAL_WIDTH = 80;
+    private static final int MAX_TERMINAL_WIDTH = 130;
+    private static final int MAX_PADDED_BUILD_TIME_DURATION_LENGTH = 9;
+
+    private final MessageBuilderFactory messageBuilderFactory;
+    private final Logger logger;
+    private final RichBuildEventListener buildEventListener;
+    private int terminalWidth;
+    private int lineLength;
+    private int maxProjectNameLength;
+    private int totalProjects;
+    private volatile int currentVisitedProjectCount;

Review Comment:
   nit: `volatile int currentVisitedProjectCount` — same misleading `volatile` 
as was fixed in `PlainExecutionEventLogger`.
   
   This field is only ever incremented inside `synchronized (this)` (line 258 
of the current file), and the lock already establishes the happens-before 
needed for safe publication. `volatile` implies the field can be safely read 
lock-free, which is false — the compound `++currentVisitedProjectCount` is not 
atomic and is only safe because of the lock. The previous review explicitly 
called out both `PlainExecutionEventLogger` and `RichExecutionEventLogger`; 
`Plain` was fixed but `Rich` was not.
   
   ```suggestion
       private int currentVisitedProjectCount;
   ```



-- 
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