Plug Manager rework (#1042) (#1122)

plugs/editor/editor.plug.yaml

@@ -63,6 +63,10 @@ config:
     defaultLinkStyle:
       type: string
       nullable: true
+    plugs:
+      type: array
+      items:
+        type: string
 functions:
   setEditorMode:
     path: "./editor.ts:setEditorMode"

plugs/plug-manager/plugmanager.test.ts

@@ -0,0 +1,110 @@
+import { assertEquals } from "@std/assert";
+import { insertIntoPlugPage } from "./plugmanager.ts";
+
+/** Convenience function simulating repeatedly calling `editor.replaceRange` */
+function replaceRanges(
+  pageText: string,
+  ranges: Array<{ from: number; to: number; text: string }>,
+): string {
+  let result = pageText;
+  for (const { from, to, text } of ranges) {
+    result = result.substring(0, from) + text + result.substring(to);
+  }
+  return result;
+}
+
+const exampleURI = "test:my.plug.js";
+const exampleBlock = `\`\`\`space-config
+plugs:
+- ${exampleURI}
+\`\`\``;
+
+Deno.test("Updating PLUGS page", () => {
+  // Empty page
+  let before = "";
+  let after = replaceRanges(before, insertIntoPlugPage(exampleURI, before));
+  assertEquals(after, exampleBlock);
+
+  // Page with some content and newline
+  before = "Lorem ipsum dolor sit amet.\n";
+  let expected = `${before}${exampleBlock}`;
+  after = replaceRanges(before, insertIntoPlugPage(exampleURI, before));
+  assertEquals(after, expected);
+
+  // Page without a newline at the end
+  before = "Lorem ipsum dolor sit amet.";
+  expected = `${before}\n${exampleBlock}`;
+  after = replaceRanges(before, insertIntoPlugPage(exampleURI, before));
+  assertEquals(after, expected);
+
+  // Old PLUGS page
+  before = `Old prelude
+
+\`\`\`yaml
+- test:old.plug.js
+- test:another.plug.js
+\`\`\`
+Some stuff below`;
+  expected = `Old prelude
+
+\`\`\`space-config
+plugs:
+- test:old.plug.js
+- test:another.plug.js
+- test:my.plug.js
+\`\`\`
+Some stuff below`;
+  after = replaceRanges(before, insertIntoPlugPage("test:my.plug.js", before));
+  assertEquals(after, expected);
+
+  // Page with an existing space config block
+  before = `Page content
+\`\`\`space-config
+foo:
+  bar: baz
+\`\`\`
+Some stuff below`;
+  expected = `Page content
+\`\`\`space-config
+foo:
+  bar: baz
+plugs:
+- test:my.plug.js
+\`\`\`
+Some stuff below`;
+  after = replaceRanges(before, insertIntoPlugPage("test:my.plug.js", before));
+  assertEquals(after, expected);
+
+  // Append at the end of an existing plugs list
+  before = `Page content
+\`\`\`space-config
+plugs:
+- test:old.plug.js
+# some comment
+\`\`\`
+`;
+  expected = `Page content
+\`\`\`space-config
+plugs:
+- test:old.plug.js
+# some comment
+- test:my.plug.js
+\`\`\`
+`;
+  after = replaceRanges(before, insertIntoPlugPage("test:my.plug.js", before));
+  assertEquals(after, expected);
+
+  // Why would you do this to yourself?
+  before = `I love square brackets
+\`\`\`space-config
+plugs: [ "test:old.plug.js" ]
+\`\`\`
+`;
+  expected = `I love square brackets
+\`\`\`space-config
+plugs: [ "test:old.plug.js" , "test:my.plug.js" ]
+\`\`\`
+`;
+  after = replaceRanges(before, insertIntoPlugPage("test:my.plug.js", before));
+  assertEquals(after, expected);
+});

plugs/plug-manager/plugmanager.ts

@@ -6,38 +6,75 @@ import {
 } from "@silverbulletmd/silverbullet/syscalls";
 import { readYamlPage } from "@silverbulletmd/silverbullet/lib/yaml_page";
 import { builtinPlugNames } from "../builtin_plugs.ts";
+import { findNodeMatching } from "@silverbulletmd/silverbullet/lib/tree";
+import { parseMarkdown } from "$common/markdown_parser/parser.ts";
+import { addParentPointers } from "@silverbulletmd/silverbullet/lib/tree";
+import { findNodeOfType } from "@silverbulletmd/silverbullet/lib/tree";
+import { assert } from "@std/assert/assert";
+import { builtinLanguages } from "$common/languages.ts";
 
+const plugsPage = "PLUGS";
 const plugsPrelude =
-  "This file lists all plugs that SilverBullet will load. Run the {[Plugs: Update]} command to update and reload this list of plugs.\n\n";
+  "#meta\n\nThis file lists all plugs added with the {[Plugs: Add]} command. Run the {[Plugs: Update]} command to update all Plugs defined anywhere using Space Config.\n\n";
 
 export async function updatePlugsCommand() {
   await editor.save();
   await editor.flashNotification("Updating plugs...");
   try {
-    let plugList: string[] = [];
+    const plugList: string[] = [];
-    try {
+    const configPlugs: any[] = await system.getSpaceConfig("plugs", []);
-      const plugListRead: any[] = await readYamlPage("PLUGS");
+    if (!Array.isArray(configPlugs)) {
-      if (!Array.isArray(plugListRead)) {
+      throw new Error("Expected 'plugs' in Space Config to be an array");
-        await editor.flashNotification(
+    }
-          "PLUGS YAML does not contain a plug list, not loading anything",
+    const stringPlugs = configPlugs.filter((plug) => typeof plug === "string");
+    if (stringPlugs.length !== configPlugs.length) {
+      throw new Error(
+        `${
+          configPlugs.length - stringPlugs.length
+        } plugs in Space Config aren't set as strings`,
+      );
+    }
+    plugList.push(...stringPlugs);
+    if (await space.fileExists("PLUGS.md")) {
+      // This is not primary mode of managing plugs anymore, only here for backwards compatibility.
+      try {
+        const pagePlugs: any[] = await readYamlPage("PLUGS");
+        if (Array.isArray(pagePlugs)) {
+          // It's possible that the user is using it for something else, but if it has yaml with an array, assume it's plugs
+          const pageStringPlugs = pagePlugs.filter((plug) =>
+            typeof plug === "string"
+          );
+          if (pageStringPlugs.length !== pagePlugs.length) {
+            throw new Error(
+              `${
+                pagePlugs.length - pageStringPlugs.length
+              } plugs from PLUG page were not in a yaml list format`,
+            );
+          }
+          plugList.push(...pageStringPlugs);
+          if (pageStringPlugs.length > 0) {
+            editor.flashNotification(
+              `${pageStringPlugs.length} plugs in PLUGS page can be moved to Space Config for better editor support`,
+            );
+          }
+        }
+      } catch (e: any) {
+        editor.flashNotification(
+          `Error processing PLUGS page: ${e.message}`,
           "error",
         );
         return;
       }
-      plugList = plugListRead.filter((plug) => typeof plug === "string");
-      if (plugList.length !== plugListRead.length) {
-        throw new Error(
-          `Some of the plugs were not in a yaml list format, they were ignored`,
-        );
-      }
-    } catch (e: any) {
-      if (e.message.includes("Not found")) {
-        console.warn("No PLUGS page found, not loading anything");
-        return;
-      }
-      throw new Error(`Error processing PLUGS: ${e.message}`);
     }
-    console.log("Plug YAML", plugList);
+
+    // De-duplicate URIs, this is safe because by definition they point to the same plug
+    plugList.forEach((uri, index) => {
+      if (plugList.indexOf(uri) !== index) {
+        plugList.splice(index, 1);
+      }
+    });
+
+    console.log("Found Plug URIs:", plugList);
     const allCustomPlugNames: string[] = [];
     for (const plugUri of plugList) {
       const [protocol, ...rest] = plugUri.split(":");
@@ -65,6 +102,18 @@ export async function updatePlugsCommand() {
         plugName = plugNameMatch[1];
       }
 
+      // Validate the extracted name
+      if (builtinPlugNames.includes(plugName)) {
+        throw new Error(
+          `Plug name '${plugName}' is conflicting with a built-in plug`,
+        );
+      }
+      if (allCustomPlugNames.includes(plugName)) {
+        throw new Error(
+          `Plug name '${plugName}' defined by more than one URI`,
+        );
+      }
+
       const manifests = await events.dispatchEvent(
         `get-plug:${protocol}`,
         rest.join(":"),
@@ -99,38 +148,142 @@ export async function updatePlugsCommand() {
   }
 }
 
-export async function addPlugCommand() {
+export async function addPlugCommand(_cmdDef: any, uriSuggestion: string = "") {
-  let name = await editor.prompt("Plug URI:");
+  let uri = await editor.prompt("Plug URI:", uriSuggestion);
-  if (!name) {
+  if (!uri) {
     return;
   }
   // Support people copy & pasting the YAML version
-  if (name.startsWith("-")) {
+  if (uri.startsWith("-")) {
-    name = name.replace(/^\-\s*/, "");
+    uri = uri.replace(/^\-\s*/, "");
   }
-  let plugList: string[] = [];
+
-  try {
+  let plugPageContent = plugsPrelude;
-    plugList = await readYamlPage("PLUGS");
+  if (await space.fileExists(plugsPage + ".md")) {
-  } catch (e: any) {
+    plugPageContent = await space.readPage(plugsPage);
-    console.error("ERROR", e);
+  } else {
+    space.writePage(plugsPage, plugPageContent);
   }
-  if (plugList.includes(name)) {
+  await editor.navigate({ page: plugsPage });
-    await editor.flashNotification("Plug already installed", "error");
+  // Here we are on the PLUGS page, if it didn't exist before it's filled with prelude
-    return;
+  const changeList = insertIntoPlugPage(uri, plugPageContent);
+  for (const { from, to, text } of changeList) {
+    editor.replaceRange(from, to, text);
   }
-  plugList.push(name);
-  // await writeYamlPage("PLUGS", plugList, plugsPrelude);
-  await space.writePage(
-    "PLUGS",
-    plugsPrelude + "```yaml\n" + plugList.map((p) => `- ${p}`).join("\n") +
-      "\n```",
-  );
-  await editor.navigate({ page: "PLUGS" });
-  await updatePlugsCommand();
   await editor.flashNotification("Plug added!");
   system.reloadPlugs();
 }
 
+/** Add the plug to the end of the plugs list in Space Config inside the PLUGS page content
+ * Returns an array for `editor.replaceRange` syscalls.
+ *
+ * Rewrites the `yaml` block to `space-config` if present
+ * Appends the `space-config` block if needed.
+ * Appends the `plugs` key on root level if needed.
+ *
+ * It's exported only to allow testing.
+ * There are a bunch of asserts to please the type checker that will fail with a malformed page.
+ */
+export function insertIntoPlugPage(
+  uri: string,
+  pageContent: string,
+): Array<{ from: number; to: number; text: string }> {
+  const edits: Array<{ from: number; to: number; text: string }> = [];
+
+  const tree = parseMarkdown(pageContent);
+  addParentPointers(tree);
+
+  const yamlInfo = findNodeMatching(tree, (n) => {
+    return n.type === "CodeInfo" &&
+      n.children !== undefined &&
+      n.children.length === 1 &&
+      n.children[0].text === "yaml";
+  });
+  const configInfo = findNodeMatching(tree, (n) => {
+    return n.type === "CodeInfo" &&
+      n.children !== undefined &&
+      n.children.length === 1 &&
+      n.children[0].text === "space-config";
+  });
+
+  if (yamlInfo) {
+    // replace YAML with Space Config, add plugs: line at the start, and the new URI at the end
+    assert(yamlInfo.from && yamlInfo.to);
+    edits.push({ from: yamlInfo.from, to: yamlInfo.to, text: "space-config" });
+
+    assert(yamlInfo.parent);
+    const yamlText = findNodeOfType(yamlInfo.parent, "CodeText");
+    assert(yamlText && yamlText.from && yamlText.to);
+    edits.push({ from: yamlText.from, to: yamlText.from, text: "plugs:\n" });
+    edits.push({ from: yamlText.to, to: yamlText.to, text: `\n- ${uri}` });
+  } else if (configInfo) {
+    // Append the required parts into the Space Config block, using lezer's (upstream) parser
+    assert(configInfo.parent);
+    const configText = findNodeOfType(configInfo.parent, "CodeText");
+    assert(configText && configText.from && configText.to);
+    assert(configText.children?.length === 1 && configText.children[0].text);
+
+    const config = configText.children[0].text;
+    const configTree = builtinLanguages["yaml"].parser.parse(config);
+    configTree.iterate({
+      enter: (n) => {
+        if (
+          n.name === "Document" &&
+          config.substring(n.from, n.to).startsWith("plugs:")
+        ) {
+          assert(configText.from);
+          if (
+            n.node.lastChild &&
+            config.substring(n.node.lastChild.from, n.node.lastChild.to) === "]"
+          ) {
+            // This is a list with square brackets
+            edits.push({
+              from: configText.from + n.node.lastChild.from,
+              to: configText.from + n.node.lastChild.from,
+              text: `, "${uri}" `,
+            });
+          } else {
+            edits.push({
+              from: configText.from + n.to,
+              to: configText.from + n.to,
+              text: `\n- ${uri}`,
+            });
+          }
+          return false; // Found the right node, no need to traverse any more
+        } else {
+          return true;
+        }
+      },
+    });
+    if (edits.length === 0) {
+      // No plugs in this block
+      edits.push({
+        from: configText.to,
+        to: configText.to,
+        text: `\nplugs:\n- ${uri}`,
+      });
+    }
+  } else {
+    // Just add the whole block if there's nothing
+    const configBlock = `\`\`\`space-config
+plugs:
+- ${uri}
+\`\`\``;
+    edits.push({
+      from: pageContent.length,
+      to: pageContent.length,
+      // Start on an empty line
+      text: (pageContent.endsWith("\n") || pageContent === "")
+        ? configBlock
+        : ("\n" + configBlock),
+    });
+  }
+
+  // Sort edits from end to start, so they don't affect each other's positions
+  edits.sort((a, b) => b.from - a.from);
+  return edits;
+}
+
 export async function getPlugHTTPS(url: string): Promise<string> {
   const fullUrl = `https:${url}`;
   console.log("Now fetching plug code from", fullUrl);