From 8c85c10212ce744164c8d778b6c708be1b321cdc Mon Sep 17 00:00:00 2001
From: Sidharth Vinod <sidharthv96@gmail.com>
Date: Thu, 1 Sep 2022 18:40:28 +0530
Subject: [PATCH] Fix TODO Qs

---
 src/diagram-api/detectType.ts      |  2 +-
 src/diagram-api/diagramAPI.spec.ts |  3 +--
 src/diagrams/common/common.ts      | 10 +++-------
 src/mermaidAPI.ts                  |  4 ++--
 src/styles.ts                      |  2 +-
 5 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/diagram-api/detectType.ts b/src/diagram-api/detectType.ts
index 62a37616b..34c78404a 100644
--- a/src/diagram-api/detectType.ts
+++ b/src/diagram-api/detectType.ts
@@ -73,7 +73,7 @@ export const detectType = function (text: string, config?: MermaidConfig): strin
     }
   }
 
-  return 'flowchart';
+  throw new Error(`No diagram type detected for text: ${text}`);
 };
 
 export const addDetector = (key: string, detector: DiagramDetector) => {
diff --git a/src/diagram-api/diagramAPI.spec.ts b/src/diagram-api/diagramAPI.spec.ts
index ce12a6433..9216552e2 100644
--- a/src/diagram-api/diagramAPI.spec.ts
+++ b/src/diagram-api/diagramAPI.spec.ts
@@ -12,8 +12,7 @@ describe('DiagramAPI', () => {
 
   it('should handle diagram registrations', () => {
     expect(() => getDiagram('loki')).toThrow();
-    // TODO Q: Shouldn't this be throwing an error?
-    expect(detectType('loki diagram')).toBe('flowchart');
+    expect(() => detectType('loki diagram')).toThrow();
     registerDiagram(
       'loki',
       {
diff --git a/src/diagrams/common/common.ts b/src/diagrams/common/common.ts
index 057481768..10edf9856 100644
--- a/src/diagrams/common/common.ts
+++ b/src/diagrams/common/common.ts
@@ -24,7 +24,6 @@ export const removeScript = (txt: string): string => {
 };
 
 const sanitizeMore = (text: string, config: MermaidConfig) => {
-  // TODO Q: Should this check really be here? Feels like we should be sanitizing it regardless.
   if (config.flowchart?.htmlLabels !== false) {
     const level = config.securityLevel;
     if (level === 'antiscript' || level === 'strict') {
@@ -54,7 +53,7 @@ export const sanitizeTextOrArray = (
   config: MermaidConfig
 ): string | string[] => {
   if (typeof a === 'string') return sanitizeText(a, config);
-  // TODO Q: Do we need flat?
+  // TODO: Refactor to avoid flat.
   return a.flat().map((x: string) => sanitizeText(x, config));
 };
 
@@ -108,7 +107,6 @@ const breakToPlaceholder = (s: string): string => {
  */
 const getUrl = (useAbsolute: boolean): string => {
   let url = '';
-  // TODO Q: If useAbsolute if false, empty string is returned. Bug?
   if (useAbsolute) {
     url =
       window.location.protocol +
@@ -116,7 +114,6 @@ const getUrl = (useAbsolute: boolean): string => {
       window.location.host +
       window.location.pathname +
       window.location.search;
-    // TODO Q: Why is this necessary?
     url = url.replaceAll(/\(/g, '\\(');
     url = url.replaceAll(/\)/g, '\\)');
   }
@@ -130,9 +127,8 @@ const getUrl = (useAbsolute: boolean): string => {
  * @param {string | boolean} val String or boolean to convert
  * @returns {boolean} The result from the input
  */
-// TODO Q: Should we make this check more specific? 'False', '0', 'null' all will evaluate to true.
-export const evaluate = (val: string | boolean): boolean =>
-  val === 'false' || val === false ? false : true;
+export const evaluate = (val?: string | boolean): boolean =>
+  val === false || ['false', 'null', '0'].includes(String(val).trim().toLowerCase()) ? false : true;
 
 /**
  * Makes generics in typescript syntax
diff --git a/src/mermaidAPI.ts b/src/mermaidAPI.ts
index 8ca4a9bab..303a78680 100644
--- a/src/mermaidAPI.ts
+++ b/src/mermaidAPI.ts
@@ -34,6 +34,7 @@ import theme from './themes';
 import utils, { directiveSanitizer } from './utils';
 import DOMPurify from 'dompurify';
 import { MermaidConfig } from './config.type';
+import { evaluate } from './diagrams/common/common';
 
 let hasLoadedDiagrams = false;
 
@@ -308,8 +309,7 @@ const render = function (
   let svgCode = root.select('#d' + id).node().innerHTML;
 
   log.debug('cnf.arrowMarkerAbsolute', cnf.arrowMarkerAbsolute);
-  // TODO Q: Needs verification.
-  if (!cnf.arrowMarkerAbsolute && cnf.securityLevel !== 'sandbox') {
+  if (!evaluate(cnf.arrowMarkerAbsolute) && cnf.securityLevel !== 'sandbox') {
     svgCode = svgCode.replace(/marker-end="url\(.*?#/g, 'marker-end="url(#', 'g');
   }
 
diff --git a/src/styles.ts b/src/styles.ts
index 54266ff9a..1b4105b99 100644
--- a/src/styles.ts
+++ b/src/styles.ts
@@ -13,7 +13,7 @@ import c4 from './diagrams/c4/styles';
 import { FlowChartStyleOptions } from './diagrams/flowchart/styles';
 import { log } from './logger';
 
-// TODO Q: Shouldn't registerDiagram be injecting data here?
+// TODO @knut: Inject from registerDiagram.
 const themes = {
   flowchart,
   'flowchart-v2': flowchart,