Merge pull request #134 from sillsdev/fix-image-link-double-bang

Fix two problems with image links caused by regex fix

Author: andrew-polk

src/plugins/embedTweaks.spec.ts

@@ -3,6 +3,33 @@ import { IPlugin } from "./pluginTypes";
 import { setLogLevel } from "../log";
 import { blocksToMarkdown } from "./pluginTestRun";
 import { gifEmbed, imgurGifEmbed } from "./embedTweaks";
+import { standardExternalLinkConversion } from "./externalLinks";
+import defaultConfig from "../config/default.docunotion.config";
+import { NotionBlock as NB } from "../types";
+
+function paragraph(text: string): NB {
+  return {
+    type: "paragraph",
+    paragraph: {
+      rich_text: [
+        {
+          type: "text",
+          text: { content: text, link: null },
+          annotations: {
+            bold: false,
+            italic: false,
+            strikethrough: false,
+            underline: false,
+            code: false,
+            color: "default",
+          },
+          plain_text: text,
+          href: null,
+        },
+      ],
+    },
+  } as unknown as NB;
+}
 
 test("imgur", async () => {
   setLogLevel("verbose");
@@ -18,6 +45,104 @@ test("imgur", async () => {
   expect(result.trim()).toBe(`![](https://imgur.com/gallery/U8TTNuI.gif)`);
 });
 
+// Regression test for the "!![](...)" bug: with both plugins active (the
+// default config order), the imgur mod turns a bare imgur link into an image,
+// then the gif mod sees that ".gif" image and must not prepend a second "!".
+test("imgur + gif together produce a single leading bang", async () => {
+  setLogLevel("verbose");
+  const config = { plugins: [imgurGifEmbed, gifEmbed] };
+  const result = await blocksToMarkdown(config, [
+    {
+      object: "block",
+      id: "e36710d8-98ad-40dc-b41b-b376ebdd6894",
+      type: "bookmark",
+      bookmark: { caption: [], url: "https://imgur.com/E83qLj6" },
+    } as unknown as NotionBlock,
+  ]);
+  expect(result.trim()).toBe(`![](https://imgur.com/E83qLj6.gif)`);
+  expect(result).not.toContain("!![]");
+});
+
+// An inline link the user gave real text to must stay a clickable link: we must
+// not turn it into an image (which also discards the text). Uses the default
+// config order so both mods get a crack at it.
+test("imgur link with author text is left as a clickable link", async () => {
+  setLogLevel("verbose");
+  const config = { plugins: [imgurGifEmbed, gifEmbed] };
+  const result = await blocksToMarkdown(config, [
+    paragraph(
+      "all at once ([see animation](https://imgur.com/gcrxl5k))."
+    ),
+    paragraph(
+      "(See an animation of these [new overlay features](https://imgur.com/E83qLj6))"
+    ),
+  ]);
+  expect(result).toContain("[see animation](https://imgur.com/gcrxl5k)");
+  expect(result).toContain(
+    "[new overlay features](https://imgur.com/E83qLj6)"
+  );
+  // nothing should have been turned into an image
+  expect(result).not.toContain("![]");
+  expect(result).not.toContain(".gif");
+});
+
+// Production-realistic: standardExternalLinkConversion runs in an earlier phase
+// and rewrites a Notion `[bookmark](url)` into `[url](url)` before the embed
+// regexes run. The embed must still recognize that as an auto-label and embed it.
+test("imgur bookmark still embeds after external-link conversion", async () => {
+  setLogLevel("verbose");
+  const config = {
+    plugins: [standardExternalLinkConversion, imgurGifEmbed, gifEmbed],
+  };
+  const result = await blocksToMarkdown(config, [
+    {
+      object: "block",
+      id: "e36710d8-98ad-40dc-b41b-b376ebdd6894",
+      type: "bookmark",
+      bookmark: { caption: [], url: "https://imgur.com/gallery/U8TTNuI" },
+    } as unknown as NotionBlock,
+  ]);
+  expect(result.trim()).toBe(`![](https://imgur.com/gallery/U8TTNuI.gif)`);
+  expect(result).not.toContain("!![]");
+});
+
+// ...but an authored inline link must survive that same pipeline as a link.
+test("authored imgur link survives external-link conversion as a link", async () => {
+  setLogLevel("verbose");
+  const config = {
+    plugins: [standardExternalLinkConversion, imgurGifEmbed, gifEmbed],
+  };
+  const result = await blocksToMarkdown(config, [
+    paragraph("all at once ([see animation](https://imgur.com/gcrxl5k))."),
+  ]);
+  expect(result).toContain("[see animation](https://imgur.com/gcrxl5k)");
+  expect(result).not.toContain("![]");
+  expect(result).not.toContain(".gif");
+});
+
+// Strongest guard: run the ACTUAL default production config (full plugin set
+// and order) rather than a hand-picked subset, so future config drift (plugin
+// reordering, the external-link converter changing) can't silently reopen the
+// bug. A Notion bookmark to imgur must embed; an authored inline link must stay
+// a clickable link.
+test("default production config: bookmark embeds, authored link stays a link", async () => {
+  setLogLevel("verbose");
+  const result = await blocksToMarkdown(defaultConfig, [
+    {
+      object: "block",
+      id: "e36710d8-98ad-40dc-b41b-b376ebdd6894",
+      type: "bookmark",
+      bookmark: { caption: [], url: "https://imgur.com/gallery/U8TTNuI" },
+    } as unknown as NotionBlock,
+    paragraph("all at once ([see animation](https://imgur.com/gcrxl5k))."),
+  ]);
+  // the bookmark became an embedded gif...
+  expect(result).toContain("![](https://imgur.com/gallery/U8TTNuI.gif)");
+  expect(result).not.toContain("!![]");
+  // ...while the authored link is untouched
+  expect(result).toContain("[see animation](https://imgur.com/gcrxl5k)");
+});
+
 test("gif", async () => {
   setLogLevel("verbose");
   const config = { plugins: [gifEmbed] };

src/plugins/embedTweaks.ts

@@ -5,8 +5,26 @@ export const gifEmbed: IPlugin = {
   regexMarkdownModifications: [
     {
       // I once saw a gif coming from Notion that wasn't a full
-      // url, which wouldn't work, hence the "http" requirement
-      regex: /\[.*?\]\((http.*?(\.(gif|GIF)))\)/,
+      // url, which wouldn't work, hence the "http" requirement.
+      //
+      // We only embed when the link's text is an AUTO-GENERATED label, not
+      // something the author typed. By the time this regex runs the label can
+      // take three auto shapes:
+      //   - empty `[]` (an embed)
+      //   - the literal `[bookmark]` placeholder notion-to-md emits for
+      //     bookmark blocks (seen when standardExternalLinkConversion isn't in
+      //     the config, e.g. the unit tests)
+      //   - the URL repeated as the label, `[http...gif](http...gif)`, which is
+      //     what standardExternalLinkConversion rewrites `[bookmark]` into in
+      //     the production config (it runs in an earlier phase, see transform.ts)
+      // A link the author gave real text, e.g. `[see animation](...gif)`, is
+      // none of these, so it's left as a clickable link rather than turned into
+      // an image (which would also throw the text away).
+      //
+      // The optional leading "!" lets us match (and replace) a link that
+      // another mod has already turned into an image, instead of prepending
+      // a second "!" and producing "!![](...)".
+      regex: /!?\[(?:bookmark|https?:\/\/[^\]]*\.(?:gif|GIF)[^\]]*)?\]\((http.*?(\.(gif|GIF)))\)/,
       replacementPattern: `![]($1)`,
     },
   ],
@@ -16,7 +34,14 @@ export const imgurGifEmbed: IPlugin = {
   name: "imgur",
   regexMarkdownModifications: [
     {
-      regex: /\[.*?\]\((.*?imgur\.com\/.*?)\)/, // imgur.com
+      // Only embed links whose text is an auto-generated label (empty `[]`,
+      // `[bookmark]`, or the URL repeated as the label `[http...imgur.com/...]`),
+      // so that a link the author gave real text to is kept as a clickable link
+      // rather than converted to an image. See the longer note in gifEmbed above
+      // for why all three label shapes can occur here.
+      // The optional leading "!" lets us match a link that's already an image
+      // without prepending a second "!".
+      regex: /!?\[(?:bookmark|https?:\/\/[^\]]*imgur\.com\/[^\]]*)?\]\((.*?imgur\.com\/.*?)\)/, // imgur.com
       // imgur links to gifs need a .gif at the end, but the url they give you doesn't have one.
       replacementPattern: `![]($1.gif)`,
     },