Add a utility to wrap long lines of text

elibarzilay · elibarzilay · commit a00662fdf5e8 · 2021-06-04T13:25:47.000-04:00
diff --git a/src/comments.ts b/src/comments.ts
@@ -1,4 +1,4 @@
-import { sha256 } from "./util/util";
+import { sha256, txt } from "./util/util";
 
 // use `deletedWhenNotPresent` for comments that should be removed if not in the actions
 export const tagsToDeleteIfNotPosted: string[] = [];
@@ -11,85 +11,113 @@ export type Comment = { tag: string, status: string };
 
 export const HadError = (user: string | undefined, error: string) => ({
     tag: "had-error",
-    status: `${user ? `@${user} — ` : ""}There was an error that prevented me from properly processing this PR:
-
-    ${error}`
+    status: txt`
+        |${user ? `@${user} — ` : ""}There was an error that prevented me from properly processing
+         this PR:
+        |
+        |    ${error}`
 });
 
 export const CIFailed = (abbrOid: string, user: string, ciUrl: string) => ({
     tag: `gh-actions-complaint-${abbrOid}`,
-    status: `@${user} The CI build failed! Please [review the logs for more information](${ciUrl}).
-
-Once you've pushed the fixes, the build will automatically re-run. Thanks!
-
-**Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.**`
+    status: txt`
+        |@${user} The CI build failed! Please [review the logs for more information](${ciUrl}).
+        |
+        |Once you've pushed the fixes, the build will automatically re-run. Thanks!
+        |
+        |**Note: builds which are failing do not end up on the list of PRs for the DT
+         maintainers to review.**`
 });
 
 export const MergeConflicted = (abbrOid: string, user: string) => ({
     tag: `merge-complaint-${abbrOid}`,
-    status: `@${user} Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!`
+    status: txt`
+        |@${user} Unfortunately, this pull request currently has a merge conflict 😥.
+         Please update your PR branch to be up-to-date with respect to master. Have a nice day!`
 });
 
 export const ChangesRequest = (abbrOid: string, user: string) => ({
     tag: `reviewer-complaint-${abbrOid}`,
-    status: `@${user} One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!`
+    status: txt`
+        |@${user} One or more reviewers has requested changes. Please address their comments.
+         I'll be back once they sign off or you've pushed new commits. Thank you!`
 });
 
 export const SuggestTesting = deletedWhenNotPresent("suggest-testing", tag =>
     (user: string, testsLink: string) => ({
-        tag, status: `Hey @${user},
-
-:unamused: Your PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Please consider [adding tests](${testsLink}) to cover the change you're making. Including tests allows this PR to be merged by yourself and the owners of this module.
-
-***This can potentially save days of time for you!***`
+        tag, status: txt`
+            |Hey @${user},
+            |
+            |:unamused: Your PR doesn't modify any tests, so it's hard to know what's being fixed,
+             and your changes might regress in the future. Please consider [adding tests](${testsLink})
+             to cover the change you're making. Including tests allows this PR to be merged by
+             yourself and the owners of this module.
+            |
+            |***This can potentially save days of time for you!***`
     }));
 
 export const PingReviewers = (names: readonly string[], reviewLink: string) => ({
     tag: "pinging-reviewers",
-    status: `🔔 ${names.map(n => `@${n}`).join(" ")} — please [review this PR](${reviewLink}) in the next few days. Be sure to explicitly select **\`Approve\`** or **\`Request Changes\`** in the GitHub UI so I know what's going on.`
+    status: txt`
+        |🔔 ${names.map(n => `@${n}`).join(" ")} — please [review this PR](${reviewLink}) in the
+         next few days. Be sure to explicitly select **\`Approve\`** or **\`Request Changes\`**
+         in the GitHub UI so I know what's going on.`
 });
 
 export const PingReviewersOther = (user: string, reviewLink: string) => ({
     tag: "pinging-reviewers-others",
-    status: `🔔 @${user} — you're the only owner, but it would still be good if you find someone to [review this PR](${reviewLink}) in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...)`
+    status: txt`
+        |🔔 @${user} — you're the only owner, but it would still be good if you find someone to
+         [review this PR](${reviewLink}) in the next few days, otherwise a maintainer will look at it.
+         (And if you do find someone, maybe even recruit them to be a second owner to make future
+         changes easier...)`
 });
 
 export const PingReviewersTooMany = (names: readonly string[]) => ({
     tag: "pinging-reviewers-too-many",
-    status: `⚠️ There are too many reviewers for this PR change (${names.length}). Merging can only be handled by a DT maintainer.
-
-<details>
-<summary>People who would have been pinged</summary>
-${names.map(n => `${n}`).join(" ")}
-</details>`
+    status: txt`
+        |⚠️ There are too many reviewers for this PR change (${names.length}).
+         Merging can only be handled by a DT maintainer.
+        |
+        |<details>
+        |<summary>People who would have been pinged</summary>
+        |${names.map(n => `${n}`).join(" ")}
+        |</details>`
 });
 
 export const PingStaleReviewer = (reviewedAbbrOid: string, reviewers: string[]) => ({
     tag: `stale-ping-${sha256(reviewers.join("-")).substr(0, 6)}-${reviewedAbbrOid}`,
-    status: `@${reviewers.join(", @")} Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?`
+    status: txt`
+        |@${reviewers.join(", @")} Thank you for reviewing this PR! The author has pushed new
+         commits since your last review. Could you take another look and submit a fresh review?`
 });
 
 export const OfferSelfMerge = deletedWhenNotPresent("merge-offer", tag =>
     (user: string, otherOwners: string[], abbrOid: string) => ({
         // Note: pr-info.ts searches for the `(at ${abbrOid})`
-        tag, status: `@${user} Everything looks good here. Great job! I am ready to merge this PR (at ${abbrOid}) on your behalf.
-
-If you'd like that to happen, please post a comment saying:
-
-> Ready to merge
-
-and I'll merge this PR almost instantly. Thanks for helping out! :heart:
-${otherOwners.length === 0 ? "" : `
-(${otherOwners.map(o => "@" + o).join(", ")}: you can do this too.)`}`}));
+        tag, status: txt`
+            |@${user} Everything looks good here. Great job! I am ready to merge this PR
+             (at ${abbrOid}) on your behalf.
+            |
+            |If you'd like that to happen, please post a comment saying:
+            |
+            |> Ready to merge
+            |
+            |and I'll merge this PR almost instantly. Thanks for helping out! :heart:
+            |${otherOwners.length === 0 ? "" : `
+            |(${otherOwners.map(o => "@" + o).join(", ")}: you can do this too.)`}`}));
 
 export const WaitUntilMergeIsOK = (user: string, abbrOid: string, uri: string, mainCommentID: number | undefined) => ({
     // at most one reminder per update
     tag: `wait-for-merge-offer-${abbrOid}`,
-    status: `:passport_control: Hi @${user},
-
-I can't [accept a pull request](${uri}) until all of the checks in the "Status" section of [this comment](#issuecomment-${mainCommentID || "???"}) are green. I will let you know once that happens.
-
-Thanks, and happy typing!`
+    status: txt`
+        |:passport_control: Hi @${user},
+        |
+        |I can't [accept a pull request](${uri}) until all of the checks in the "Status" section of
+         [this comment](#issuecomment-${mainCommentID || "???"}) are green.
+         I will let you know once that happens.
+        |
+        |Thanks, and happy typing!`
 });
 
 // Explanation for the stalness count in the welcome message
@@ -107,24 +135,38 @@ export const StalenessComment = (author: string, ownersToPing: string[], expires
     const ownerPing = ownersToPing.map(o => "@"+o).join(", ");
     return {
         // --Unmerged--
-        "Unmerged:nearly": `Re-ping @${author} / ${ownerPing}:
-
-This PR has been ready to merge for over a week, and I haven't seen any requests to merge it. I will close it on ${expires} (in three weeks) if this doesn't happen.
-
-(If there's no reason to avoid merging it, please do so.  Otherwise, if it shouldn't be merged or if it needs more time, please close it or turn it into a draft.)`,
-        "Unmerged:done": `After a month, no one has requested merging the PR 😞. I'm going to assume that the change is not wanted after all, and will therefore close it.`,
+        "Unmerged:nearly": txt`
+            |Re-ping @${author} / ${ownerPing}:
+            |
+            |This PR has been ready to merge for over a week, and I haven't seen any requests to
+             merge it. I will close it on ${expires} (in three weeks) if this doesn't happen.
+            |
+            |(If there's no reason to avoid merging it, please do so.  Otherwise, if it shouldn't
+             be merged or if it needs more time, please close it or turn it into a draft.)`,
+        "Unmerged:done": txt`
+            |After a month, no one has requested merging the PR 😞. I'm going to assume that the
+             change is not wanted after all, and will therefore close it.`,
         // --Abandoned--
-        "Abandoned:nearly": `@${author} I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on ${expires} (in a week) if the issues aren't addressed.`,
-        "Abandoned:done": `@${author} To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!`,
+        "Abandoned:nearly": txt`
+            |@${author} I haven't seen any activity on this PR in more than three weeks, and it
+             still has problems that prevent it from being merged. The PR will be closed on
+             ${expires} (in a week) if the issues aren't addressed.`,
+        "Abandoned:done": txt`
+            |@${author} To keep things tidy, we have to close PRs that aren't mergeable and don't
+             have activity in the last month. No worries, though — please open a new PR if you'd
+             like to continue with this change. Thank you!`,
         // --Unreviewed--
-        "Unreviewed:nearly": `Re-ping ${ownerPing}:
-
-This PR has been out for over a week, yet I haven't seen any reviews.
-
-Could someone please give it some attention? Thanks!`,
-        "Unreviewed:done": `It has been more than two weeks and this PR still has no reviews.
-
-I'll bump it to the DT maintainer queue. Thank you for your patience, @${author}.
-
-(Ping ${ownerPing}.)`} as { [k: string]: string };
+        "Unreviewed:nearly": txt`
+            |Re-ping ${ownerPing}:
+            |
+            |This PR has been out for over a week, yet I haven't seen any reviews.
+            |
+            |Could someone please give it some attention? Thanks!`,
+        "Unreviewed:done": txt`
+            |It has been more than two weeks and this PR still has no reviews.
+            |
+            |I'll bump it to the DT maintainer queue. Thank you for your patience, @${author}.
+            |
+            |(Ping ${ownerPing}.)`
+    } as { [k: string]: string };
 };
diff --git a/src/compute-pr-actions.ts b/src/compute-pr-actions.ts
@@ -4,7 +4,7 @@ import * as emoji from "./emoji";
 import * as urls from "./urls";
 import { PrInfo, BotResult, FileInfo } from "./pr-info";
 import { ReviewInfo } from "./pr-info";
-import { noNullish, flatten, unique, sameUser, min, sha256, abbrOid } from "./util/util";
+import { noNullish, flatten, unique, sameUser, min, sha256, abbrOid, txt } from "./util/util";
 import * as dayjs from "dayjs";
 import * as advancedFormat from "dayjs/plugin/advancedFormat";
 dayjs.extend(advancedFormat);
@@ -370,7 +370,8 @@ function createWelcomeComment(info: ExtendedPrInfo, post: (c: Comments.Comment)
     const testsLink = info.hasNewPackages ? urls.testingNewPackages : urls.testingEditedPackages;
 
     const specialWelcome = !info.isFirstContribution ? `` :
-        ` I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.`;
+        txt`| I see this is your first time submitting to DefinitelyTyped 👋
+             — I'm the local bot who will help you through the process of getting things through.`;
     display(`@${info.author} Thank you for submitting this PR!${specialWelcome}`,
             ``,
             `***This is a live comment which I will keep updated.***`);
@@ -448,7 +449,9 @@ function createWelcomeComment(info: ExtendedPrInfo, post: (c: Comments.Comment)
     }
     if (addedSelfToManyOwners > 0) {
         display(``,
-                `@${info.author}: I see that you have added yourself as an owner${addedSelfToManyOwners > 1 ? " to several packages" : ""}, are you sure you want to [become an owner](${urls.definitionOwners})?`);
+                txt`@${info.author}: I see that you have added yourself as an
+                    owner${addedSelfToManyOwners > 1 ? " to several packages" : ""},
+                    are you sure you want to [become an owner](${urls.definitionOwners})?`);
     }
 
     // Lets the author know who needs to review this
@@ -458,18 +461,26 @@ function createWelcomeComment(info: ExtendedPrInfo, post: (c: Comments.Comment)
     if (info.blessingKind === "merge") {
         display("This PR can be merged.");
     } else if (info.hasNewPackages) {
-        display(`This PR adds a new definition, so it needs to be reviewed by ${requiredApprover} before it can be merged.`);
+        display(txt`This PR adds a new definition, so it needs to be reviewed by
+                    ${requiredApprover} before it can be merged.`);
     } else if (info.popularityLevel === "Critical" && info.blessingKind !== "review") {
-        display(`Because this is a widely-used package, ${requiredApprover} will need to review it before it can be merged.`);
+        display(txt`Because this is a widely-used package, ${requiredApprover}
+                    will need to review it before it can be merged.`);
     } else if (!info.requireMaintainer) {
-        const and = info.hasDefinitions && info.hasTests ? "and updated the tests (👏)" : "and there were no type definition changes";
-        display(`Because you edited one package ${and}, I can help you merge this PR once someone else signs off on it.`);
+        const and = info.hasDefinitions && info.hasTests
+            ? "and updated the tests (👏)"
+            : "and there were no type definition changes";
+        display(txt`Because you edited one package ${and}, I can help you merge this PR
+                    once someone else signs off on it.`);
     } else if (info.noOtherOwners && info.blessingKind !== "review") {
-        display(`There aren't any other owners of this package, so ${requiredApprover} will review it.`);
+        display(txt`There aren't any other owners of this package,
+                    so ${requiredApprover} will review it.`);
     } else if (info.hasMultiplePackages && info.blessingKind !== "review") {
-        display(`Because this PR edits multiple packages, it can be merged once it's reviewed by ${requiredApprover}.`);
+        display(txt`Because this PR edits multiple packages, it can be merged
+                    once it's reviewed by ${requiredApprover}.`);
     } else if (info.checkConfig && info.blessingKind !== "review") {
-        display(`Because this PR edits the configuration file, it can be merged once it's reviewed by ${requiredApprover}.`);
+        display(txt`Because this PR edits the configuration file, it can be merged
+                    once it's reviewed by ${requiredApprover}.`);
     } else if (info.blessingKind !== "review") {
         display(`This PR can be merged once it's reviewed by ${requiredApprover}.`);
     } else {
@@ -518,9 +529,12 @@ function createWelcomeComment(info: ExtendedPrInfo, post: (c: Comments.Comment)
 
     display(``);
     if (!info.canBeSelfMerged) {
-        display(`Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.`);
+        display(txt`Once every item on this list is checked,
+                    I'll ask you for permission to merge and publish the changes.`);
     } else {
-        display(`All of the items on the list are green. **To merge, you need to post a comment including the string "Ready to merge"** to bring in your changes.`);
+        display(txt`All of the items on the list are green.
+                    **To merge, you need to post a comment including the string "Ready to merge"**
+                    to bring in your changes.`);
     }
 
     if (info.staleness && info.staleness.state !== "fresh") {
diff --git a/src/util/util.ts b/src/util/util.ts
@@ -47,7 +47,8 @@ export function authorNotBot(node: { login: string } | { author?: { login: strin
 }
 
 export function scrubDiagnosticDetails(s: string) {
-    return s.replace(/<details><summary>Diagnostic Information.*?<\/summary>(?:\\n)+```json\\n{.*?\\n}\\n```(?:\\n)+<\/details>/sg, "... diagnostics scrubbed ...");
+    return s.replace(/<details><summary>Diagnostic Information.*?<\/summary>(?:\\n)+```json\\n{.*?\\n}\\n```(?:\\n)+<\/details>/sg,
+                     "... diagnostics scrubbed ...");
 }
 
 export function sha256(s: string) {
@@ -57,3 +58,18 @@ export function sha256(s: string) {
 export function abbrOid(s: string) {
     return s.slice(0, 7);
 }
+
+// Remove when the fix propagates to a published version
+interface HACK {
+    raw(template: { raw: readonly string[] | ArrayLike<string>}, ...substitutions: any[]): string;
+}
+
+// Convenient utility for long texts: trimmed, then remove all spaces up to a
+// "|" on each line, and lines that have no "|" at the beginning are joined with
+// the previous line (with a single space).  (Should really be mapped only on `strs`
+// only, otherwise there're compositionality problems (one place in comments.ts).)
+export function txt(strs: TemplateStringsArray, ...xs: any) {
+    return (String as HACK).raw({ raw: strs }, ...xs)
+        .trim().replace(/(^|\n) *([^\s])/g, (_m, pfx, sfx) =>
+            sfx === "|" ? pfx : pfx ? " " + sfx : sfx);
+}
