Improve bot message formatting

Reviewed-by: ehelin

bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestWorkItem.java

@@ -24,6 +24,7 @@
 
 import org.openjdk.skara.bot.WorkItem;
 import org.openjdk.skara.forge.PullRequest;
+import org.openjdk.skara.issuetracker.Comment;
 import org.openjdk.skara.json.*;
 import org.openjdk.skara.storage.StorageBuilder;
 import org.openjdk.skara.vcs.Hash;
@@ -32,7 +33,7 @@
 import java.nio.file.Path;
 import java.util.*;
 import java.util.function.Consumer;
-import java.util.regex.Pattern;
+import java.util.regex.*;
 import java.util.stream.*;
 
 public class PullRequestWorkItem implements WorkItem {
@@ -50,21 +51,21 @@ public class PullRequestWorkItem implements WorkItem {
         this.integratorId = integratorId;
     }
 
-    private Hash resultingCommitHashFor(PullRequest pr) {
-       if (pr.labels().contains("integrated")) {
-           for (var comment : pr.comments()) {
-               if (comment.author().id().equals(integratorId)) {
-                   for (var line : comment.body().split("\n")) {
-                       if (line.startsWith("Pushed as commit")) {
-                           var parts = line.split(" ");
-                           var hash = parts[parts.length - 1].replace(".", "");
-                           return new Hash(hash);
-                       }
-                   }
-               }
-           }
-       }
-       return null;
+    private final static Pattern pushedPattern = Pattern.compile("Pushed as commit ([a-f0-9]{40})\\.");
+
+    private Hash resultingCommitHash() {
+        if (pr.labels().contains("integrated")) {
+            return pr.comments().stream()
+                     .filter(comment -> comment.author().id().equals(integratorId))
+                     .map(Comment::body)
+                     .map(pushedPattern::matcher)
+                     .filter(Matcher::find)
+                     .map(m -> m.group(1))
+                     .map(Hash::new)
+                     .findAny()
+                     .orElse(null);
+        }
+        return null;
     }
 
     private Set<PullRequestState> deserializePrState(String current) {
@@ -179,7 +180,7 @@ public Collection<WorkItem> run(Path scratchPath) {
                 .materialize(historyPath);
 
         var issues = parseIssues();
-        var commit = resultingCommitHashFor(pr);
+        var commit = resultingCommitHash();
         var state = new PullRequestState(pr, issues, commit);
         var stored = storage.current();
         if (stored.contains(state)) {
@@ -193,7 +194,7 @@ public Collection<WorkItem> run(Path scratchPath) {
                 .findAny();
         // The stored entry could be old and be missing commit information - if so, upgrade it
         if (storedState.isPresent() && storedState.get().commitId().equals(Optional.of(Hash.zero()))) {
-            var hash = resultingCommitHashFor(pr);
+            var hash = resultingCommitHash();
             storedState = Optional.of(new PullRequestState(pr, storedState.get().issueIds(), hash));
             storage.put(storedState.get());
         }

bots/notify/src/test/java/org/openjdk/skara/bots/notify/comment/CommitCommentNotifierTests.java

@@ -23,12 +23,11 @@
 package org.openjdk.skara.bots.notify.comment;
 
 import org.junit.jupiter.api.*;
-import org.openjdk.skara.bots.notify.*;
+import org.openjdk.skara.bots.notify.NotifyBot;
 import org.openjdk.skara.json.JSON;
 import org.openjdk.skara.test.*;
 
 import java.io.IOException;
-import java.net.URI;
 import java.util.*;
 import java.util.regex.Pattern;
 
@@ -76,7 +75,7 @@ void testCommitComment(TestInfo testInfo) throws IOException {
             var pr = credentials.createPullRequest(repo, "master", "master", "Fix an issue");
             pr.setBody("I made a fix");
             pr.addLabel("integrated");
-            pr.addComment("Pushed as commit " + editHash.hex() + ".");
+            pr.addComment("More text!\n\n@user Pushed as commit " + editHash.hex() + ". Even more text.\n\nAnd some additional text.");
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Check commit comment

bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java

@@ -155,7 +155,9 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
                 if (!finalRebaseMessage.isBlank()) {
                     reply.println(rebaseMessage.toString());
                 }
-                reply.println("Pushed as commit " + localHash.hex() + ". :bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.");
+                reply.println("Pushed as commit " + localHash.hex() + ".");
+                reply.println();
+                reply.println(":bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.");
                 localRepo.push(localHash, pr.repository().url(), pr.targetRef());
                 pr.setState(PullRequest.State.CLOSED);
                 pr.addLabel("integrated");

bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelerWorkItem.java

@@ -73,25 +73,28 @@ private void updateLabelMessage(List<Comment> comments, List<String> newLabels)
         if (newLabels.isEmpty()) {
             message.append("To determine the appropriate audience for reviewing this pull request, one or more ");
             message.append("labels corresponding to different subsystems will normally be applied automatically. ");
-            message.append("However, no automatic labelling rule matches the changes in this pull request. ");
+            message.append("However, no automatic labelling rule matches the changes in this pull request.\n\n");
             message.append("In order to have an RFR email automatically sent to the correct mailing list, you will ");
             message.append("need to add one or more labels manually using the `/label add \"label\"` command. ");
             message.append("The following labels are valid: ");
             var labels = bot.labelConfiguration().allowed().stream()
                             .sorted()
                             .map(label -> "`" + label + "`")
-                            .collect(Collectors.joining(", "));
+                            .collect(Collectors.joining(" "));
             message.append(labels);
             message.append(".");
         } else {
-            message.append("The following labels will be automatically applied to this pull request: ");
+            message.append("The following label");
+            if (newLabels.size() > 1) {
+                message.append("s");
+            }
+            message.append(" will be automatically applied to this pull request: ");
             var labels = newLabels.stream()
                                   .sorted()
                                   .map(label -> "`" + label + "`")
-                                  .collect(Collectors.joining(", "));
-            message.append("`");
+                                  .collect(Collectors.joining(" "));
             message.append(labels);
-            message.append("`. When this pull request is ready to be reviewed, an RFR email will be sent to the ");
+            message.append(".\n\nWhen this pull request is ready to be reviewed, an RFR email will be sent to the ");
             message.append("corresponding mailing list");
             if (newLabels.size() > 1) {
                 message.append("s");

bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java

@@ -129,7 +129,9 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
                 if (!finalRebaseMessage.isBlank()) {
                     reply.println(rebaseMessage.toString());
                 }
-                reply.println("Pushed as commit " + localHash.hex() + ". :bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.");
+                reply.println("Pushed as commit " + localHash.hex() + ".");
+                reply.println();
+                reply.println(":bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.");
                 localRepo.push(localHash, pr.repository().url(), pr.targetRef());
                 pr.setState(PullRequest.State.CLOSED);
                 pr.addLabel("integrated");

bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelTests.java

@@ -163,7 +163,7 @@ void adjustAutoApplied(TestInfo testInfo) throws IOException {
             // The bot should have applied one label automatically
             TestBotRunner.runPeriodicItems(prBot);
             assertEquals(Set.of("2", "rfr"), new HashSet<>(pr.labels()));
-            assertLastCommentContains(pr, "The following labels will be automatically applied");
+            assertLastCommentContains(pr, "The following label will be automatically applied");
             assertLastCommentContains(pr, "`2`");
 
             // It will refuse to remove it

bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelerTests.java

@@ -71,7 +71,7 @@ void simple(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(labelBot);
             assertEquals(Set.of("rfr"), new HashSet<>(pr.labels()));
             assertLastCommentContains(pr, "However, no automatic labelling rule matches the changes in this pull request.");
-            assertLastCommentContains(pr, "The following labels are valid: `test1`, `test2`");
+            assertLastCommentContains(pr, "The following labels are valid: `test1` `test2`");
 
             var fileA = localRepoFolder.resolve("a.txt");
             Files.writeString(fileA, "Hello");