Fix rendering of code in links, HTML entities

Use a placeholder for inline code rather than splitting on them so
markup that spans them will be process properly.

Replace Slack special characters everywhere in the message, except
where they are already HTML entities.

Fixes #37

lib/Markdown.ts

@@ -123,6 +123,36 @@ function convertMarkdown(text: string): string {
     return splitProcessor(relinked, convertFormat, urlSplitter);
 }
 
+/** Provide a unique identifier for later replacement. */
+function codeTag(i: number): string {
+    return `%.%CODE_PROCESSOR_CODE${i}%.%`;
+}
+
+/**
+ * Transform everything but the interior of inline code segments,
+ * i.e., \`code\`, but still be able to process elements that wrap
+ * around inlide code formatting.
+ *
+ * @param text input string
+ * @param transform function that takes the whole string with inline
+ *                  code segments "hidden" and performs transformation
+ * @return transformed string with unchanged inline code segments
+ */
+function codeProcessor(text: string): string {
+    const hunks = text.split(/(`.*?`)/);
+    const codes = new Array<string>(hunks.length);
+    for (let i = 1; i < hunks.length; i += 2) {
+        codes[i] = hunks[i];
+        hunks[i] = codeTag(i);
+    }
+    const transformed = convertMarkdown(hunks.join(""));
+    let restored = transformed;
+    for (let i = 1; i < hunks.length; i += 2) {
+        restored = restored.replace(codeTag(i), codes[i]);
+    }
+    return restored;
+}
+
 /**
  * Convert GitHub-flavored Markdown to Slack message markup.  This is
  * not a complete implementation of a Markdown parser, but it does its
@@ -132,5 +162,6 @@ function convertMarkdown(text: string): string {
  * @return string with Slack markup
  */
 export function githubToSlack(text: string): string {
-    return splitProcessor(text, convertMarkdown);
+    const codeBlock = /(```[\S\s]*?```(?!`))/g;
+    return splitProcessor(text, codeProcessor, codeBlock);
 }

lib/SlackMessages.ts

@@ -44,11 +44,13 @@ export const MessageMimeTypes: { [key: string]: MessageMimeType } = {
  */
 
 /**
- * Escapes special Slack characters.
+ * Encode special Slack characters and HTML entities.
  */
 export function escape(text: string): string {
     if (text) {
-        return splitProcessor(text, i => i.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;"));
+        const entify = (i: string) => i.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
+        const htmlEntities = /(&(?:\w+|#\d+);)/;
+        return splitProcessor(text, entify, htmlEntities);
     } else {
         return "";
     }

lib/splitProcessor.ts

@@ -14,8 +14,6 @@
  * limitations under the License.
  */
 
-export const codeBlockSplitter = /(```[\S\s]*?```(?!`)|`.*?`)/mg;
-
 /**
  * Perform transformations on input string, skipping sections that match
  * the regular expression.
@@ -26,12 +24,7 @@ export const codeBlockSplitter = /(```[\S\s]*?```(?!`)|`.*?`)/mg;
  *                  and performs transformation
  * @return transformed string, stitched back together
  */
-export function splitProcessor(
-    text: string,
-    transform: (i: string) => string,
-    splitter: RegExp = codeBlockSplitter,
-): string {
-
+export function splitProcessor(text: string, transform: (i: string) => string, splitter: RegExp): string {
     const hunks = text.split(splitter);
     for (let i = 0; i < hunks.length; i += 2) {
         hunks[i] = transform(hunks[i]);

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@atomist/slack-messages",
-  "version": "1.0.2",
+  "version": "1.1.0",
   "description": "Atomist utilities for creating formatted Slack messages",
   "author": {
     "name": "Atomist",

test/Markdown.test.ts

@@ -182,7 +182,6 @@ describe("Markdown", () => {
             assert(convertImageLinks(md) === expected);
         });
 
-        /* tslint:disable:max-line-length */
         it("should convert img elements to raw links", () => {
             const md = `![I Am a Scientist](http://gbv.com/iaas.jpeg)
 ![Goldheart Mountaintop Queen Directory](http://gbv.com/gmqd.jpeg)![You're Not an Airplane](http://gbv.com/ynaa.jpeg)
@@ -254,7 +253,6 @@ There are many more.
 `;
             assert(convertImageLinks(md) === expected);
         });
-        /* tslint:enable:max-line-length */
 
         it("should convert named image links", () => {
             const md = `The EP "Clown Prince of the Menthol Trailer" has
@@ -739,7 +737,6 @@ Here is how the above list was created using Markdown:
             assert(githubToSlack(md) === expected);
         });
 
-        /* tslint:disable:max-line-length */
         it("should not mangle urls", () => {
             const md = `This is *some* markdown.  It has __some__ URLs.
 [UTBUTS](https://gbv.com/__under-the-bushes__under-the-stars/)
@@ -758,7 +755,32 @@ Named <http://www.gbv.org/song/Smothered*in*hugs.html|links> too.
 `;
             assert(githubToSlack(md) === expected);
         });
-        /* tslint:enable:max-line-length */
+
+        it("should render markup within URL titles", () => {
+            const m = "Markdown with [`code`](http://a.b/), etc.";
+            const e = "Markdown with <http://a.b/|`code`>, etc.";
+            const s = githubToSlack(m);
+            assert(s === e);
+        });
+
+        it("should not render URLs within code", () => {
+            const m = "Markdown with `[code](http://a.b/)`, etc.";
+            const s = githubToSlack(m);
+            assert(s === m);
+        });
+
+        it("should allow backticks adjacent to code blocks", () => {
+            const m = "````code` not code `code````";
+            const s = githubToSlack(m);
+            assert(s === m);
+        });
+
+        it("should match backticks", () => {
+            const m = "````code` not code `code```` __not__ code `*code*`";
+            const s = githubToSlack(m);
+            const e = "````code` not code `code```` *not* code `*code*`";
+            assert(s === e);
+        });
 
     });
 });

test/SlackMessages.test.ts

@@ -258,32 +258,59 @@ describe("SlackMessages", () => {
             assert.strictEqual(escape(""), "");
         });
 
-        it("will not escape characters in inline code", () => {
-            ["&", "<", ">"].forEach(c => {
-                const i = `Inline \`code ${c} whatnot\` should be safe`;
-                assert(escape(i) === i);
-            });
+        it("will escape characters in inline code", () => {
+            const cs: Record<string, string> = { amp: "&", lt: "<", gt: ">" };
+            for (const c in cs) {
+                if (cs.hasOwnProperty(c)) {
+                    const i = `Inline \`code ${cs[c]} whatnot\` should be safe`;
+                    const e = `Inline \`code &${c}; whatnot\` should be safe`;
+                    assert(escape(i) === e);
+                }
+            }
         });
 
-        it("will not escape characters in code blocks", () => {
-            ["&", "<", ">"].forEach(c => {
-                const i = `Code blocks such as this:
+        it("will escape characters in code blocks", () => {
+            const cs: Record<string, string> = { amp: "&", lt: "<", gt: ">" };
+            for (const c in cs) {
+                if (cs.hasOwnProperty(c)) {
+                    const i = `Code blocks such as this:
 
 \`\`\`
 function first(s: string): string {
-    if (s === "${c}") {
-        return "${c}";
+    if (s === "${cs[c]}") {
+        return "${cs[c]}";
     } else {
-        return \`not ${c}\`;
+        return \`not ${cs[c]}\`;
     }
 }
 \`\`\`
 
 should be safe
 `;
-                assert(escape(i) === i);
-            });
+                    const e = `Code blocks such as this:
+
+\`\`\`
+function first(s: string): string {
+    if (s === "&${c};") {
+        return "&${c};";
+    } else {
+        return \`not &${c};\`;
+    }
+}
+\`\`\`
+
+should be safe
+`;
+                    assert(escape(i) === e);
+                }
+            }
+        });
+
+        it("will not escape HTML entities", () => {
+            const t = "&lt;this &amp; that&gt;";
+            assert.strictEqual(escape(t), t);
         });
+
     });
 
     describe("Urls", () => {