From 96569aab0cafe9f67f07e74a9c9e00151d8fddf3 Mon Sep 17 00:00:00 2001
From: Guillermo Vaya <guivaya@gmail.com>
Date: Mon, 25 Jul 2022 20:52:05 +0200
Subject: [PATCH] address code review

---
 packages/plugs/lib/settings_page.ts | 19 +++++++-------
 packages/plugs/lib/util.ts          | 20 +++++++++++++++
 packages/plugs/lib/yaml_page.ts     | 39 -----------------------------
 3 files changed, 29 insertions(+), 49 deletions(-)

diff --git a/packages/plugs/lib/settings_page.ts b/packages/plugs/lib/settings_page.ts
index a10ae22..c2bc2a9 100644
--- a/packages/plugs/lib/settings_page.ts
+++ b/packages/plugs/lib/settings_page.ts
@@ -1,5 +1,7 @@
-import { flashNotification } from "@silverbulletmd/plugos-silverbullet-syscall/editor";
-import { readYamlPage, writeYamlPage } from "./yaml_page";
+import { readYamlPage } from "./yaml_page";
+import { notifyUser } from "./util";
+import YAML from "yaml";
+import { writePage } from "@silverbulletmd/plugos-silverbullet-syscall/space";
 
 /**
  * Convenience function to read a specific set of settings from the `SETTINGS` page as well as default values
@@ -12,7 +14,6 @@ import { readYamlPage, writeYamlPage } from "./yaml_page";
  */
 
 const SETTINGS_PAGE = "SETTINGS";
-const SETTINGS_TEMPLATE = `This page contains settings for configuring SilverBullet and its Plugs:\n\`\`\`yaml\n\`\`\``; // might need \r\n for windows?
 
 export async function readSettings<T extends object>(settings: T): Promise<T> {
   try {
@@ -46,13 +47,11 @@ export async function writeSettings<T extends object>(settings: T) {
   try {
     readSettings = (await readYamlPage(SETTINGS_PAGE, ["yaml"])) || {};
   } catch (e: any) {
-    console.log("Couldn't read settings, generating a new settings page");
-    flashNotification("Creating a new SETTINGS page...", "info");
+    await notifyUser("Creating a new SETTINGS page...", "info");
   }
   const writeSettings = {...readSettings, ...settings};
-  if(await writeYamlPage(SETTINGS_PAGE, writeSettings, SETTINGS_TEMPLATE)) {
-    flashNotification("SETTINGS page written successfully", "info");
-  } else {
-    flashNotification("SETTINGS page failed to update", "error");
-  }
+  const doc = new YAML.Document();
+  doc.contents = writeSettings;
+  const contents = `This page contains settings for configuring SilverBullet and its Plugs.\nAny changes outside of the yaml block will be overwritten.\n\`\`\`yaml\n${doc.toString()}\n\`\`\``; // might need \r\n for windows?
+  await writePage(SETTINGS_PAGE, contents)
 }
\ No newline at end of file
diff --git a/packages/plugs/lib/util.ts b/packages/plugs/lib/util.ts
index 6bc6993..af0a2ce 100644
--- a/packages/plugs/lib/util.ts
+++ b/packages/plugs/lib/util.ts
@@ -1,3 +1,5 @@
+import { flashNotification } from "@silverbulletmd/plugos-silverbullet-syscall/editor";
+
 export async function replaceAsync(
   str: string,
   regex: RegExp,
@@ -12,3 +14,21 @@ export async function replaceAsync(
   const data = await Promise.all(promises);
   return str.replace(regex, () => data.shift()!);
 }
+
+export function isServer() {
+  return typeof window === 'undefined' || typeof window.document === "undefined"; // if something defines window the same way as the browser, this will fail.
+}
+
+// this helps keep if's condition as positive
+export function isBrowser() {
+  return !isServer();
+}
+
+export async function notifyUser(message: string, type?: "info"|"error") {
+  if (isBrowser()) {
+    return flashNotification(message, type);
+  }
+  const log = type === "error" ? console.error : console.log;
+  log(message); // we should end up sending the message to the user, users dont read logs.
+  return;
+}
\ No newline at end of file
diff --git a/packages/plugs/lib/yaml_page.ts b/packages/plugs/lib/yaml_page.ts
index 4d97da1..c0deaaf 100644
--- a/packages/plugs/lib/yaml_page.ts
+++ b/packages/plugs/lib/yaml_page.ts
@@ -40,42 +40,3 @@ export async function readYamlPage(
 
   return data;
 }
-
-export async function writeYamlPage(
-  pageName: string,
-  properties: any,
-  templateOnEmpty: string,
-): Promise<Boolean> {
-  let text;
-  try {
-    const page = await readPage(pageName);
-    text = page.text;
-  } catch {
-    // page doesn't exist, so let's create one.
-    text = templateOnEmpty;
-  }
-  const tree = await parseMarkdown(text);
-  // if we use any other language than yaml... how to create it?
-  const doc = new YAML.Document();
-  doc.contents = properties;
-  // generate a new node for the properties
-  const newCode = `\`\`\`yaml\n${doc.toString()}\n\`\`\``;
-  const subtree = await parseMarkdown(newCode);
-  // find the original set of properties and replace
-  let replaced = false;
-  replaceNodesMatching(tree, (node: ParseTree) => {
-    if (node.type !== 'FencedCode') {
-      return;
-    }
-    const codeinfoNode = findNodeOfType(node, "CodeInfo");
-    if (!codeinfoNode || codeinfoNode.children![0].text! !== "yaml") {
-      return;
-    }
-    replaced = true;
-    return subtree;
-  });
-  if (replaced) {
-    await writePage(pageName, renderToText(tree));
-  }
-  return replaced;
-}
\ No newline at end of file