desruisseaux commented on code in PR #1027:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/1027#discussion_r2753986846


##########
src/main/java/org/apache/maven/plugin/compiler/DiagnosticLogger.java:
##########
@@ -191,50 +211,93 @@ Optional<String> firstError(Throwable cause) {
      * Reports summary after the compilation finished.
      */
     void logSummary() {
-        MessageBuilder message = messageBuilderFactory.builder();
-        final String patternForCount;
         if (!codeCount.isEmpty()) {
-            @SuppressWarnings("unchecked")
-            Map.Entry<String, Integer>[] entries = 
codeCount.entrySet().toArray(Map.Entry[]::new);
-            Arrays.sort(entries, (a, b) -> Integer.compare(b.getValue(), 
a.getValue()));
-            patternForCount = patternForCount(Math.max(entries[0].getValue(), 
Math.max(numWarnings, numErrors)));
-            message.strong("Summary of compiler messages:").newline();
-            for (Map.Entry<String, Integer> entry : entries) {
+            final var bundle = tryGetCompilerBundle();
+            for (final var entry : codeCount.entrySet().stream()
+                    // sort occurrence then key to have an absolute ordering
+                    .sorted(Map.Entry.<String, Integer>comparingByValue()
+                            .reversed()
+                            .thenComparing(Map.Entry.comparingByKey()))
+                    .toList()) {
                 int count = entry.getValue();
-                message.format(patternForCount, count, 
entry.getKey()).newline();
+                String key = entry.getKey();
+                if (bundle != null) {
+                    try {
+                        // not great but the code is worse when you read it
+                        key = key + " (" + bundle.getString(key) + ")";
+                    } catch (final RuntimeException re) {
+                        // ignore, use the plain key
+                    }
+                }
+                Consumer<String> log;
+                if (entry.getKey().startsWith("compiler.")) {
+                    final var sub = 
entry.getKey().substring("compiler.".length());
+                    if (sub.startsWith("err.")) {
+                        log = logger::error;
+                    } else if (sub.startsWith("warn.")) {
+                        log = logger::warn;
+                    } else {
+                        log = logger::info;
+                    }
+                } else {
+                    log = logger::info;
+                }
+                log.accept(messageBuilderFactory
+                        .builder()
+                        .strong(key + ": ")
+                        .append(String.valueOf(count))
+                        .build());
             }
-        } else {
-            patternForCount = patternForCount(Math.max(numWarnings, 
numErrors));
         }
+
         if ((numWarnings | numErrors) != 0) {
+            MessageBuilder message = messageBuilderFactory.builder();
             message.strong("Total:");
+            if (numWarnings != 0) {
+                message.append(' 
').append(String.valueOf(numWarnings)).append(" warning");
+                if (numWarnings > 1) {
+                    message.append('s');
+                }
+            }
+            if (numErrors != 0) {
+                message.append(' ').append(String.valueOf(numErrors)).append(" 
error");
+                if (numErrors > 1) {
+                    message.append('s');
+                }
+            }
+            logger.info(message.build());
         }
-        if (numWarnings != 0) {
-            writeCount(message, patternForCount, numWarnings, "warning");
-        }
-        if (numErrors != 0) {
-            writeCount(message, patternForCount, numErrors, "error");
-        }
-        logger.info(message.toString());
-    }
-
-    /**
-     * {@return the pattern for formatting the specified number followed by a 
label}
-     * The given number should be the widest number to format.
-     * A margin of 4 spaces is added at the beginning of the line.
-     */
-    private static String patternForCount(int n) {
-        return "    %" + Integer.toString(n).length() + "d %s";
     }
 
-    /**
-     * Appends the count of warnings or errors, making them plural if needed.
-     */
-    private static void writeCount(MessageBuilder message, String 
patternForCount, int count, String name) {
-        message.newline();
-        message.format(patternForCount, count, name);
-        if (count > 1) {
-            message.append('s');
+    // we mainly know the one for javac as of today, this impl is best effort
+    private ResourceBundle tryGetCompilerBundle() {

Review Comment:
   > It does in our context, give it a try
   
   Yes but this is not a JPMS context. While Maven is not yet modularized, this 
is one of my long term goal.
   
   > Not sure what you mean cause it is literally the pattern message for the 
code you used
   
   Yes but we parameters that we don't have when writing a summary. The _{0} 
uses or overrides a deprecated API"_ is a message reported when one particular 
class or method use a deprecated API, and {0} is the name of that class or 
method. But when we write the summary, we are saying that (for example) this 
warnings occurred 100 times. We have no particular class or method that we can 
insert in the {0} part.



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