Repository: platers/obsidian-linter
Pull request #658 (2023-03-18 20:54:40)

Refactor Logic Around Blank Line Being Put Around Content
This makes the empty line around content logic more robust and reusable.

Changes Made:
- Converted the logic over to get the start of the line ignoring non-whitespace and greater than characters (>)
- Made sure the logic would decide which empty line value to use when blockquotes were involved to make things more uniform
- Updated several examples to show the change in logic


Patch:
diff --git a/src/rules/empty-line-around-code-fences.ts b/src/rules/empty-line-around-code-fences.ts
index f49ae50..40ce538 100644
--- a/src/rules/empty-line-around-code-fences.ts
+++ b/src/rules/empty-line-around-code-fences.ts
@@ -79,7 +79,7 @@ export default class EmptyLineAroundCodeFences extends RuleBuilder<EmptyLineArou
         `,
         after: dedent`
           # Make sure that code blocks in blockquotes are accounted for correctly
-          >
+
           > \`\`\`js
           > var text = 'this is some text';
           > \`\`\`
@@ -87,13 +87,11 @@ export default class EmptyLineAroundCodeFences extends RuleBuilder<EmptyLineArou
           > \`\`\`js
           > var other text = 'this is more text';
           > \`\`\`
-          >
           ${''}
           **Note that the blanks blockquote lines added do not have whitespace after them**
           ${''}
           # Doubly nested code block
           ${''}
-          > >
           > > \`\`\`js
           > > var other text = 'this is more text';
           > > \`\`\`
diff --git a/src/rules/empty-line-around-math-block.ts b/src/rules/empty-line-around-math-block.ts
index 83656dd..c5aa804 100644
--- a/src/rules/empty-line-around-math-block.ts
+++ b/src/rules/empty-line-around-math-block.ts
@@ -112,12 +112,11 @@ export default class EmptyLineAroundMathBlock extends RuleBuilder<EmptyLineAroun
           > $$
           > \\boldsymbol{a}=\\begin{bmatrix}a_x \\\\ a_y\\end{bmatrix}
           > $$
-          >
           ${''}
           More content here
           ${''}
           > Math block doubly nested in blockquote
-          > >
+          >
           > > $$
           > > \\boldsymbol{a}=\\begin{bmatrix}a_x \\\\ a_y\\end{bmatrix}
           > > $$
diff --git a/src/rules/empty-line-around-tables.ts b/src/rules/empty-line-around-tables.ts
index cef4e3d..216fb68 100644
--- a/src/rules/empty-line-around-tables.ts
+++ b/src/rules/empty-line-around-tables.ts
@@ -135,12 +135,11 @@ export default class EmptyLineAroundTables extends RuleBuilder<EmptyLineAroundTa
           > | foo      | bar      | blob     |
           > | baz      | qux      | trust    |
           > | quux     | quuz     | glob     |
-          >
           ${''}
           More content here
           ${''}
           > Table doubly nested in blockquote
-          > >
+          >
           > > | Column 1 | Column 2 | Column 3 |
           > > |----------|----------|----------|
           > > | foo      | bar      | blob     |
diff --git a/src/utils/mdast.ts b/src/utils/mdast.ts
index 13ec521..ed65cb3 100644
--- a/src/utils/mdast.ts
+++ b/src/utils/mdast.ts
@@ -436,7 +436,13 @@ export function ensureEmptyLinesAroundMathBlock(text: string, numberOfDollarSign
 export function ensureEmptyLinesAroundBlockquotes(text: string): string {
   const positions: Position[] = getPositions(MDAstTypes.Blockquote, text);
   for (const position of positions) {
-    text = makeSureContentHasEmptyLinesAddedBeforeAndAfter(text, position.start.offset, position.end.offset, true);
+    // make sure to shift end to the next new line character just in case blocquotes are nested which can cause changes to move content out of the original position expected
+    let endIndex = position.end.offset;
+    while (endIndex < text.length - 1 && text.charAt(endIndex) !== '\n') {
+      endIndex++;
+    }
+
+    text = makeSureContentHasEmptyLinesAddedBeforeAndAfter(text, position.start.offset, endIndex);
   }
 
   return text;
diff --git a/src/utils/regex.ts b/src/utils/regex.ts
index fc98dd4..310a3fd 100644
--- a/src/utils/regex.ts
+++ b/src/utils/regex.ts
@@ -69,7 +69,7 @@ export function ensureEmptyLinesAroundTables(text: string): string {
   }
 
   for (const table of tableMatches) {
-    let start = text.indexOf(table);
+    let start = text.indexOf(table.trimStart());
     const end = start + table.length;
     if (table.trim().startsWith('>')) {
       while (text.charAt(start).trim() === '' || text.charAt(start) === '>') {
diff --git a/src/utils/strings.ts b/src/utils/strings.ts
index 2882263..12b628a 100644
--- a/src/utils/strings.ts
+++ b/src/utils/strings.ts
@@ -1,5 +1,3 @@
-import {escapeRegExp} from './regex';
-
 /**
  * Inserts a string at the given position in a string.
  * @param {string} str - The string to insert into
@@ -32,91 +30,193 @@ export function stripCr(text: string): string {
   return text.replace(/\r/g, '');
 }
 
-/**
- * Checks whether the expected text and actual text are the same allowing for requiring an exact match versus a close match on whitespace
- * @param {string} expectedText - The expected text
- * @param {string} actualText - The actual text
- * @param {boolean} requireSameTrailingWhitespace - Whether or not to do an exact comparison or allow whitespace to differ
- * @return {boolean} Whether or not the text matched the expected value
- */
-function textMatches(expectedText: string, actualText: string, requireSameTrailingWhitespace: boolean = false): boolean {
-  if (requireSameTrailingWhitespace) {
-    return expectedText == actualText;
+function getStartOfLineWhitespaceOrBlockquoteLevel(text: string, startPosition: number): [string, number] {
+  if (startPosition === 0) {
+    return ['', 0];
+  }
+
+  let startOfLine = '';
+  let index = startPosition;
+  while (index >= 0) {
+    const char = text.charAt(index);
+    if (char === '\n') {
+      break;
+    } else if (char.trim() === '' || char === '>') {
+      startOfLine = char + startOfLine;
+    } else {
+      startOfLine = '';
+    }
+
+    index--;
   }
 
-  return actualText.match(new RegExp('^' + escapeRegExp(expectedText) + '( |\\t)*$', 'm')) != null;
+  return [startOfLine, index];
 }
 
-/**
- * Makes sure that the specified content has an empty line around it so long as it does not start or end a file.
- * @param {string} text - The entire file's contents
- * @param {number} start - The starting index of the content to escape
- * @param {number} end - The ending index of the content to escape
- * @param {boolean} isForBlockquotes - Whether or not to apply logic to allow a blank line or one less nesting of blockquotes to be the empty line
- * @return {string} The new file contents after the empty lines have been added
- */
-export function makeSureContentHasEmptyLinesAddedBeforeAndAfter(text: string, start: number, end: number, isForBlockquotes: boolean = false): string {
-  const content = text.substring(start, end);
-  let startOfLine = '';
-  let contentPriorToContent = text.substring(0, start);
-  if (contentPriorToContent.length > 0) {
-    const contentLinesPriorToContent = contentPriorToContent.split('\n');
-    startOfLine = contentLinesPriorToContent[contentLinesPriorToContent.length - 1] ?? '';
-    startOfLine = startOfLine.trimEnd();
+function getEmptyLine(startOfLine: string, priorLine: string = ''): string {
+  const [priorLineStart] = getStartOfLineWhitespaceOrBlockquoteLevel(priorLine, priorLine.length);
 
+  return '\n' + priorLineStart.trim();
+}
 
-    let numberOfIndexesToRemove = 0;
-    while (contentLinesPriorToContent.length - (2 + numberOfIndexesToRemove) >= 0) {
-      const lineContent = contentLinesPriorToContent[contentLinesPriorToContent.length - (2 + numberOfIndexesToRemove)];
-      if (!textMatches(startOfLine, lineContent) && (!isForBlockquotes || !textMatches('', lineContent, true))) {
-        break;
-      }
+function makeSureContentHasASingleEmptyLineBeforeItUnlessItStartsAFile(text: string, startOfContent: number): string {
+  if (startOfContent === 0) {
+    return text;
+  }
 
-      numberOfIndexesToRemove++;
+  let index = startOfContent;
+  let startOfNewContent = startOfContent;
+  while (index >= 0) {
+    const currentChar = text.charAt(index);
+    if (currentChar.trim() !== '') {
+      break; // if non-whitespace is encountered, then the line has content
+    } else if (currentChar === '\n') {
+      startOfNewContent = index;
     }
+    index--;
+  }
 
-    contentLinesPriorToContent.splice(contentLinesPriorToContent.length - (1 + numberOfIndexesToRemove), numberOfIndexesToRemove);
+  if (index < 0 || startOfNewContent === 0) {
+    return text.substring(startOfContent + 1);
+  }
 
-    if (contentLinesPriorToContent.length > 1) {
-      if ((isForBlockquotes && contentLinesPriorToContent[contentLinesPriorToContent.length - 2].match(/^> ?.*$/m)) ||
-      (!isForBlockquotes && !textMatches(startOfLine, contentLinesPriorToContent[contentLinesPriorToContent.length - 2]))) {
-        contentLinesPriorToContent.splice(contentLinesPriorToContent.length - 1, 0, startOfLine);
-      } else if (!textMatches('', contentLinesPriorToContent[contentLinesPriorToContent.length - 2], true)) {
-        contentLinesPriorToContent.splice(contentLinesPriorToContent.length - 1, 0, '');
-      }
-    }
+  return text.substring(0, startOfNewContent) + '\n' + text.substring(startOfContent);
+}
 
-    contentPriorToContent = contentLinesPriorToContent.join('\n');
+function makeSureContentHasASingleEmptyLineBeforeItUnlessItStartsAFileForBlockquote(text: string, startOfLine: string, startOfContent: number): string {
+  if (startOfContent === 0) {
+    return text;
   }
 
-  let contentAfterContent = text.substring(end);
-  if (contentAfterContent.length > 0) {
-    const contentLinesAfterBlock = contentAfterContent.split('\n');
-    let numberOfIndexesToRemove = 0;
-    while (numberOfIndexesToRemove + 1 < contentLinesAfterBlock.length) {
-      const lineContent = contentLinesAfterBlock[1+numberOfIndexesToRemove];
-      if (!textMatches(startOfLine, lineContent) && (!isForBlockquotes || !textMatches('', lineContent, true))) {
+  const nestingLevel = startOfLine.split('>').length - 1;
+  let index = startOfContent;
+  let startOfNewContent = startOfContent;
+  let lineNestingLevel = 0;
+  while (index >= 0) {
+    const currentChar = text.charAt(index);
+    if (currentChar.trim() !== '' && currentChar !== '>') {
+      break; // if non-whitespace, non-gt-bracket is encountered, then the line has content
+    } else if (currentChar === '>') {
+      lineNestingLevel++;
+    } else if (currentChar === '\n') {
+      if (lineNestingLevel === 0 || lineNestingLevel === nestingLevel || (lineNestingLevel + 1) === nestingLevel) {
+        startOfNewContent = index;
+        lineNestingLevel = 0;
+      } else {
         break;
       }
+    }
+    index--;
+  }
 
-      numberOfIndexesToRemove++;
+  if (index < 0 || startOfNewContent === 0) {
+    return text.substring(startOfContent + 1);
+  }
+
+  const indexOfLastNewLine = text.lastIndexOf('\n', startOfNewContent - 1);
+  let priorLine = '';
+  if (indexOfLastNewLine === -1) {
+    priorLine = text.substring(0, startOfNewContent);
+  } else {
+    priorLine = text.substring(indexOfLastNewLine, startOfNewContent);
+  }
+
+  return text.substring(0, startOfNewContent) + getEmptyLine(startOfLine, priorLine) + text.substring(startOfContent);
+}
+
+function makeSureContentHasASingleEmptyLineAfterItUnlessItEndsAFile(text: string, endOfContent: number): string {
+  if (endOfContent === (text.length - 1)) {
+    return text;
+  }
+
+  let index = endOfContent;
+  let endOfNewContent = endOfContent;
+  let isFirstNewLine = true;
+  while (index < text.length) {
+    const currentChar = text.charAt(index);
+    if (currentChar.trim() !== '') {
+      break; // if non-whitespace is encountered, then the line has content
+    } else if (currentChar === '\n') {
+      if (isFirstNewLine) {
+        isFirstNewLine = false;
+      } else {
+        endOfNewContent = index;
+      }
     }
+    index++;
+  }
 
-    contentLinesAfterBlock.splice(1, numberOfIndexesToRemove);
+  if (index === text.length || endOfNewContent === text.length - 1) {
+    return text.substring(0, endOfContent);
+  }
 
-    if (contentLinesAfterBlock.length > 1) {
-      if ((isForBlockquotes && contentLinesAfterBlock[1].match(/^> ?.*$/m)) ||
-      (!isForBlockquotes && !textMatches(startOfLine, contentLinesAfterBlock[1]))) {
-        contentLinesAfterBlock.splice(1, 0, startOfLine);
-      } else if (isForBlockquotes && !textMatches('', contentLinesAfterBlock[1])) {
-        contentLinesAfterBlock.splice(1, 0, '');
+  return text.substring(0, endOfContent) + '\n' + text.substring(endOfNewContent);
+}
+
+function makeSureContentHasASingleEmptyLineAfterItUnlessItEndsAFileForBlockquote(text: string, startOfLine: string, endOfContent: number): string {
+  if (endOfContent === (text.length - 1)) {
+    return text;
+  }
+
+  const nestingLevel = startOfLine.split('>').length - 1;
+  let index = endOfContent;
+  let endOfNewContent = endOfContent;
+  let isFirstNewLine = true;
+  let lineNestingLevel = 0;
+  while (index < text.length) {
+    const currentChar = text.charAt(index);
+    if (currentChar.trim() !== '' && currentChar !== '>') {
+      break; // if non-whitespace is encountered, then the line has content
+    } else if (currentChar === '>') {
+      lineNestingLevel++;
+    } else if (currentChar === '\n') {
+      if (lineNestingLevel === 0 || lineNestingLevel === nestingLevel || (lineNestingLevel + 1) === nestingLevel) {
+        lineNestingLevel = 0;
+        if (isFirstNewLine) {
+          isFirstNewLine = false;
+        } else {
+          endOfNewContent = index;
+        }
+      } else {
+        break;
       }
     }
+    index++;
+  }
 
-    contentAfterContent = contentLinesAfterBlock.join('\n');
+  if (index === text.length || endOfNewContent === text.length - 1) {
+    return text.substring(0, endOfContent);
   }
 
-  return contentPriorToContent + content + contentAfterContent;
+  const indexOfSecondNewLineAfterContent = text.indexOf('\n', endOfNewContent + 1);
+  let nextLine = '';
+  if (indexOfSecondNewLineAfterContent === -1) {
+    nextLine = text.substring(endOfNewContent);
+  } else {
+    nextLine = text.substring(endOfNewContent + 1, indexOfSecondNewLineAfterContent);
+  }
+
+  return text.substring(0, endOfContent) + getEmptyLine(startOfLine, nextLine) + text.substring(endOfNewContent);
+}
+
+/**
+ * Makes sure that the specified content has an empty line around it so long as it does not start or end a file.
+ * @param {string} text - The entire file's contents
+ * @param {number} start - The starting index of the content to escape
+ * @param {number} end - The ending index of the content to escape
+ * @return {string} The new file contents after the empty lines have been added
+ */
+export function makeSureContentHasEmptyLinesAddedBeforeAndAfter(text: string, start: number, end: number): string {
+  const [startOfLine, startOfLineIndex] = getStartOfLineWhitespaceOrBlockquoteLevel(text, start);
+  if (startOfLine.trim() !== '') {
+    const newText = makeSureContentHasASingleEmptyLineAfterItUnlessItEndsAFileForBlockquote(text, startOfLine, end);
+
+    return makeSureContentHasASingleEmptyLineBeforeItUnlessItStartsAFileForBlockquote(newText, startOfLine, startOfLineIndex);
+  }
+
+  const newText = makeSureContentHasASingleEmptyLineAfterItUnlessItEndsAFile(text, end);
+
+  return makeSureContentHasASingleEmptyLineBeforeItUnlessItStartsAFile(newText, startOfLineIndex);
 }
 
 // from https://stackoverflow.com/a/52171480/8353749


Test patch:
diff --git a/__tests__/make-sure-content-has-empty-lines-added-before-and-after.test.ts b/__tests__/make-sure-content-has-empty-lines-added-before-and-after.test.ts
new file mode 100644
index 0000000..e7429ff
--- /dev/null
+++ b/__tests__/make-sure-content-has-empty-lines-added-before-and-after.test.ts
@@ -0,0 +1,222 @@
+import {makeSureContentHasEmptyLinesAddedBeforeAndAfter} from '../src/utils/strings';
+import dedent from 'ts-dedent';
+
+type EmptyStringBeforeAndAfterTestCase = {
+  testName: string,
+  startOfContent: number,
+  endOfContent: number
+  before: string,
+  after: string,
+}
+
+const testCases: EmptyStringBeforeAndAfterTestCase[] = [
+  {
+    testName: 'Content with no empty line before or after it should have one added before and after',
+    startOfContent: 9,
+    endOfContent: 26,
+    before: dedent`
+      # Header
+      Text content here
+      ## Other Header
+    `,
+    after: dedent`
+      # Header
+      ${''}
+      Text content here
+      ${''}
+      ## Other Header
+    `,
+  },
+  {
+    testName: 'Content with multiple empty lines before it should have one added before and after',
+    startOfContent: 13,
+    endOfContent: 29,
+    before: dedent`
+      # Header
+      ${''}
+      ${''}
+      ${''}
+      Text content here
+      ${''}
+      ## Other Header
+    `,
+    after: dedent`
+      # Header
+      ${''}
+      Text content here
+      ${''}
+      ## Other Header
+    `,
+  },
+  {
+    testName: 'Content at the start of the file should not get an empty line added before it',
+    startOfContent: 0,
+    endOfContent: 17,
+    before: dedent`
+      Text content here
+      ${''}
+      ## Other Header
+    `,
+    after: dedent`
+      Text content here
+      ${''}
+      ## Other Header
+    `,
+  },
+  {
+    testName: 'Content at the end of the file should not get an empty line added after it',
+    startOfContent: 9,
+    endOfContent: 26,
+    before: dedent`
+      # Header
+      Text content here
+    `,
+    after: dedent`
+      # Header
+      ${''}
+      Text content here
+    `,
+  },
+  {
+    testName: 'Content in blockquote has en empty line added before and after it',
+    startOfContent: 11,
+    endOfContent: 28,
+    before: dedent`
+      # Header
+      > Text content here
+      More unrelated content here
+    `,
+    after: dedent`
+      # Header
+      ${''}
+      > Text content here
+      ${''}
+      More unrelated content here
+    `,
+  },
+  {
+    testName: 'Content in blockquote that is surrounded by more content in the same blockquote does not end the blockquote',
+    startOfContent: 64,
+    endOfContent: 81,
+    before: dedent`
+      # Header
+      > Content prior to content to put empty lines around
+      > Text content here
+      > Content after content to put empty lines around
+    `,
+    after: dedent`
+      # Header
+      > Content prior to content to put empty lines around
+      >
+      > Text content here
+      >
+      > Content after content to put empty lines around
+    `,
+  },
+  {
+    testName: 'A table in a blockquote or callout should have a blank line added before and after the table',
+    startOfContent: 24, // start of the table header: "|"
+    endOfContent: 206, // end of the table "\n"
+    before: dedent`
+      > Table in blockquote
+      > | Column 1 | Column 2 | Column 3 |
+      > |----------|----------|----------|
+      > | foo      | bar      | blob     |
+      > | baz      | qux      | trust    |
+      > | quux     | quuz     | glob     |
+      ${''}
+      More content here
+    `,
+    after: dedent`
+      > Table in blockquote
+      >
+      > | Column 1 | Column 2 | Column 3 |
+      > |----------|----------|----------|
+      > | foo      | bar      | blob     |
+      > | baz      | qux      | trust    |
+      > | quux     | quuz     | glob     |
+      ${''}
+      More content here
+    `,
+  },
+  {
+    testName: 'A table in a nested blockquote or callout should have a blank line added before and after the table',
+    startOfContent: 59, // start of the table header: "|"
+    endOfContent: 249, // end of the table "\n"
+    before: dedent`
+      More content here
+      ${''}
+      > Table doubly nested in blockquote
+      > > | Column 1 | Column 2 | Column 3 |
+      > > |----------|----------|----------|
+      > > | foo      | bar      | blob     |
+      > > | baz      | qux      | trust    |
+      > > | quux     | quuz     | glob     |
+      More content here
+    `,
+    after: dedent`
+      More content here
+      ${''}
+      > Table doubly nested in blockquote
+      >
+      > > | Column 1 | Column 2 | Column 3 |
+      > > |----------|----------|----------|
+      > > | foo      | bar      | blob     |
+      > > | baz      | qux      | trust    |
+      > > | quux     | quuz     | glob     |
+      ${''}
+      More content here
+    `,
+  },
+  {
+    startOfContent: 124, // start of the 2nd code block: "`"
+    endOfContent: 175, // end of the 2nd code block: "\n"
+    testName: 'Fenced code blocks that are in a blockquote have the proper empty line added',
+    before: dedent`
+      # Make sure that code blocks in blockquotes are accounted for correctly
+      > \`\`\`js
+      > var text = 'this is some text';
+      > \`\`\`
+      >
+      > \`\`\`js
+      > var other text = 'this is more text';
+      > \`\`\`
+      ${''}
+      **Note that the blanks blockquote lines added do not have whitespace after them**
+      ${''}
+      # Doubly nested code block
+      ${''}
+      > > \`\`\`js
+      > > var other text = 'this is more text';
+      > > \`\`\`
+    `,
+    after: dedent`
+      # Make sure that code blocks in blockquotes are accounted for correctly
+      > \`\`\`js
+      > var text = 'this is some text';
+      > \`\`\`
+      >
+      > \`\`\`js
+      > var other text = 'this is more text';
+      > \`\`\`
+      ${''}
+      **Note that the blanks blockquote lines added do not have whitespace after them**
+      ${''}
+      # Doubly nested code block
+      ${''}
+      > > \`\`\`js
+      > > var other text = 'this is more text';
+      > > \`\`\`
+    `,
+  },
+];
+
+
+describe('Make Sure Content Has Empty Lines Added Before and After', () => {
+  for (const testCase of testCases) {
+    it(testCase.testName, () => {
+      // console.log(testCase.testName, testCase.before.indexOf('Text content here')); // helpful for adding new tests
+      expect(makeSureContentHasEmptyLinesAddedBeforeAndAfter(testCase.before, testCase.startOfContent, testCase.endOfContent)).toStrictEqual(testCase.after);
+    });
+  }
+});