Merge pull request #777 from danger/hk-issue-776

Hk issue 776
danger · Nov 27, 2018 · 6dee41b · 6dee41b
2 parents 30b746a + de47c11
commit 6dee41b
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,8 @@
 - Update comment instead deleting, if it has replies (BitBucket Server) [@langovoi][]
 - Fix BitBucket Server GitDSL [@langovoi][]
 - Add support of paged APIs of BitBucket Server [@langovoi][]
+- Revert removal of implicit `<p>` tag from [danger/danger-js#754](https://github.com/danger/danger-js/pull/754) and add
+  disctinction depending on containing markdown or not - [@hanneskaeufler][]
 
 # 6.1.6
 

diff --git a/source/runner/templates/_tests/__snapshots__/_githubIssueTemplates.test.ts.snap b/source/runner/templates/_tests/__snapshots__/_githubIssueTemplates.test.ts.snap
@@ -43,7 +43,10 @@ exports[`generating inline messages Shows correct messages for inline/regular vi
   </thead>
   <tbody><tr>
       <td>:warning:</td>
-      <td>**File.swift#L10** - Test message</td>
+      <td>
+
+  **File.swift#L10** - Test message
+  </td>
     </tr>
   
 <tr>
@@ -61,7 +64,40 @@ exports[`generating inline messages Shows correct messages for inline/regular vi
 "
 `;
 
-exports[`generating messages leaves space between <td>s to allow GitHub to render message content as markdown 1`] = `
+exports[`generating messages avoids adding space inside the <td> for proper vertical alignment if the message does not contain any markdown 1`] = `
+"
+<!--
+  0 failure: 
+  0 warning: 
+  1 messages
+  
+  DangerID: danger-id-example-id;
+-->
+
+
+
+<table>
+  <thead>
+    <tr>
+      <th width=\\"50\\"></th>
+      <th width=\\"100%\\" data-danger-table=\\"true\\">Messages</th>
+    </tr>
+  </thead>
+  <tbody><tr>
+      <td>:book:</td>
+      <td>no markdown here</td>
+    </tr>
+  </tbody>
+</table>
+
+
+<p align=\\"right\\">
+  Generated by :no_entry_sign: <a href=\\"http://github.com/danger/danger-js/\\">dangerJS</a>
+</p>
+"
+`;
+
+exports[`generating messages leaves space between <td>s to allow GitHub to render message content as markdown if the message contains any 1`] = `
 "
 <!--
   1 failure:  **Failure:** Some...
@@ -80,7 +116,10 @@ exports[`generating messages leaves space between <td>s to allow GitHub to rende
   </thead>
   <tbody><tr>
       <td>:no_entry_sign:</td>
-      <td>**Failure:** Something failed!</td>
+      <td>
+
+  **Failure:** Something failed!
+  </td>
     </tr>
   </tbody>
 </table>
@@ -95,7 +134,10 @@ exports[`generating messages leaves space between <td>s to allow GitHub to rende
   </thead>
   <tbody><tr>
       <td>:warning:</td>
-      <td>_Maybe you meant to run \`yarn install\`?_</td>
+      <td>
+
+  _Maybe you meant to run \`yarn install\`?_
+  </td>
     </tr>
   </tbody>
 </table>
@@ -110,11 +152,14 @@ exports[`generating messages leaves space between <td>s to allow GitHub to rende
   </thead>
   <tbody><tr>
       <td>:book:</td>
-      <td>\`\`\`ts
+      <td>
+
+  \`\`\`ts
 function add(a: number, b: number): number {
   return a + b
 }
-\`\`\`</td>
+\`\`\`
+  </td>
     </tr>
   </tbody>
 </table>

diff --git a/source/runner/templates/_tests/_githubIssueTemplates.test.ts b/source/runner/templates/_tests/_githubIssueTemplates.test.ts
@@ -60,7 +60,7 @@ describe("generating messages", () => {
     expect(issues).toContain(expected)
   })
 
-  it("leaves space between <td>s to allow GitHub to render message content as markdown", () => {
+  it("leaves space between <td>s to allow GitHub to render message content as markdown if the message contains any", () => {
     const issues = githubResultsTemplate("example-id", {
       fails: [{ message: "**Failure:** Something failed!" }],
       warnings: [{ message: "_Maybe you meant to run `yarn install`?_" }],
@@ -70,6 +70,17 @@ describe("generating messages", () => {
 
     expect(issues).toMatchSnapshot()
   })
+
+  it("avoids adding space inside the <td> for proper vertical alignment if the message does not contain any markdown", () => {
+    const issues = githubResultsTemplate("example-id", {
+      fails: [],
+      warnings: [],
+      messages: [{ message: "no markdown here" }],
+      markdowns: [],
+    })
+
+    expect(issues).toMatchSnapshot()
+  })
 })
 
 describe("generating inline messages", () => {

diff --git a/source/runner/templates/githubIssueTemplate.ts b/source/runner/templates/githubIssueTemplate.ts
@@ -12,7 +12,7 @@ import { Violation, isInline } from "../../dsl/Violation"
  * @returns {string} HTML
  */
 function table(name: string, emoji: string, violations: Violation[]): string {
-  if (violations.length === 0 || violations.every(violation => !violation.message)) {
+  if (noViolationsOrAllOfThemEmpty(violations)) {
     return ""
   }
   return `
@@ -23,18 +23,33 @@ function table(name: string, emoji: string, violations: Violation[]): string {
       <th width="100%" data-danger-table="true">${name}</th>
     </tr>
   </thead>
-  <tbody>${violations
-    .map((v: Violation) => {
-      const message = isInline(v) ? `**${v.file!}#L${v.line!}** - ${v.message}` : v.message
-      return `<tr>
+  <tbody>${violations.map(violation => htmlForValidation(emoji, violation)).join("\n")}</tbody>
+</table>
+`
+}
+
+function htmlForValidation(emoji: string, violation: Violation) {
+  let message = isInline(violation)
+    ? `**${violation.file!}#L${violation.line!}** - ${violation.message}`
+    : violation.message
+
+  if (containsMarkdown(message)) {
+    message = `\n\n  ${message}\n  `
+  }
+
+  return `<tr>
       <td>:${emoji}:</td>
       <td>${message}</td>
     </tr>
   `
-    })
-    .join("\n")}</tbody>
-</table>
-`
+}
+
+function containsMarkdown(message: string): boolean {
+  return message.match(/[`*_~\[]+/g) ? true : false
+}
+
+function noViolationsOrAllOfThemEmpty(violations: Violation[]) {
+  return violations.length === 0 || violations.every(violation => !violation.message)
 }
 
 function getSummary(label: string, violations: Violation[]): string {