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


##########
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:
   > hmm, there is no consensus on that so tempted to say it will not happen so 
it is ok, worse case it would be easy to make it working too
   
   Agree that modularizing Maven is not on the agenda soon. However, it will 
not be easy at all to make the pull request works in that case. It will be 
blocked by the JVM, unless we put `--add-opens` options in the Maven launch 
script, which is strongly discouraged except for testing. Generally, using 
internal API is discouraged even in a non-JPMS world, and it is precisely for 
avoiding such practices that JPMS makes it hard.
   
   Furthermore, we still have the issue of resource strings not applicable (the 
problem of having no value to supply in "{0}" emplacements).



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