Merge pull request #1250 from pjkaufman/master

Fix Trailing Spaces Breaking List Items and Checklist Items
platers · Dec 24, 2024 · 25a3e05 · 25a3e05
2 parents f744752 + fa17823
commit 25a3e05
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 26 deletions.
diff --git a/__tests__/trailing-spaces.test.ts b/__tests__/trailing-spaces.test.ts
@@ -98,5 +98,61 @@ ruleTest({
         [[File with  spaces]]
       `,
     },
+    { // accounts for https://github.com/platers/obsidian-linter/issues/868
+      testName: 'A list item with no content should not have the space indicating it is a list item removed',
+      before: dedent`
+        - List item 1
+        -${' '}
+        -${'   '}
+      `,
+      after: dedent`
+        - List item 1
+        -${' '}
+        -${' '}
+      `,
+    },
+    { // relates to for https://github.com/platers/obsidian-linter/issues/868
+      testName: 'Make sure that checklists are properly handled with trailing spaces',
+      before: dedent`
+        - [ ] List item 1
+        - [ ]${' '}
+        - [ ] ${'   '}
+      `,
+      after: dedent`
+        - [ ] List item 1
+        - [ ]${' '}
+        - [ ]${' '}
+      `,
+    },
+    { // relates to for https://github.com/platers/obsidian-linter/issues/868
+      testName: 'Make sure that indented lists are properly handled with trailing spaces',
+      before: dedent`
+        Text here
+        - List item 1
+          -${' '}
+          -${'   '}
+      `,
+      after: dedent`
+        Text here
+        - List item 1
+          -${' '}
+          -${' '}
+      `,
+    },
+    { // relates to for https://github.com/platers/obsidian-linter/issues/868
+      testName: 'Make sure that ordered lists are properly handled with trailing spaces',
+      before: dedent`
+        Text here
+        1. List item 1
+        2.${' '}
+        3.${'   '}
+      `,
+      after: dedent`
+        Text here
+        1. List item 1
+        2.${' '}
+        3.${' '}
+      `,
+    },
   ],
 });
diff --git a/src/rules/trailing-spaces.ts b/src/rules/trailing-spaces.ts
@@ -1,7 +1,8 @@
-import {IgnoreTypes} from '../utils/ignore-types';
+import {ignoreListOfTypes, IgnoreTypes} from '../utils/ignore-types';
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {BooleanOptionBuilder, ExampleBuilder, OptionBuilderBase} from './rule-builder';
 import dedent from 'ts-dedent';
+import {updateListItemText} from '../utils/mdast';
 
 class TrailingSpacesOptions implements Options {
   twoSpaceLineBreak: boolean = false;
@@ -22,14 +23,27 @@ export default class TrailingSpaces extends RuleBuilder<TrailingSpacesOptions> {
     return TrailingSpacesOptions;
   }
   apply(text: string, options: TrailingSpacesOptions): string {
-    if (!options.twoSpaceLineBreak) {
-      return text.replace(/[ \t]+$/gm, '');
-    } else {
-      text = text.replace(/(\S)[ \t]$/gm, '$1'); // one whitespace
-      text = text.replace(/(\S)[ \t]{3,}$/gm, '$1'); // three or more whitespaces
-      text = text.replace(/(\S)( ?\t\t? ?)$/gm, '$1'); // two whitespaces with at least one tab
-      return text;
-    }
+    text = ignoreListOfTypes([IgnoreTypes.list], text, (text: string): string => {
+      if (!options.twoSpaceLineBreak) {
+        return text.replace(/[ \t]+$/gm, '');
+      } else {
+        text = text.replace(/(\S)[ \t]$/gm, '$1'); // one whitespace
+        text = text.replace(/(\S)[ \t]{3,}$/gm, '$1'); // three or more whitespaces
+        text = text.replace(/(\S)( ?\t\t? ?)$/gm, '$1'); // two whitespaces with at least one tab
+        return text;
+      }
+    });
+
+    return updateListItemText(text, (text: string): string => {
+      if (!options.twoSpaceLineBreak) {
+        return text.replace(/[ \t]+$/gm, '');
+      } else {
+        text = text.replace(/(\S)[ \t]$/gm, '$1'); // one whitespace
+        text = text.replace(/(\S)[ \t]{3,}$/gm, '$1'); // three or more whitespaces
+        text = text.replace(/(\S)( ?\t\t? ?)$/gm, '$1'); // two whitespaces with at least one tab
+        return text;
+      }
+    }, true);
   }
   get exampleBuilders(): ExampleBuilder<TrailingSpacesOptions>[] {
     return [

diff --git a/src/utils/mdast.ts b/src/utils/mdast.ts
@@ -19,6 +19,11 @@ import {getTextInLanguage} from '../lang/helpers';
 
 const LRU = new QuickLRU({maxSize: 200});
 
+type PositionPlusEmptyIndicator = {
+  position: Position,
+  isEmpty: boolean,
+}
+
 export enum MDAstTypes {
   Link = 'link',
   Footnote = 'footnoteDefinition',
@@ -107,27 +112,40 @@ export function getPositions(type: MDAstTypes, text: string): Position[] {
 /**
  * Gets the positions of the list item text in the given text.
  * @param {string} text - The markdown text
- * @return {Position[]} The positions of the list item text in the given text
+ * @param {boolean} includeEmptyNodes - Whether or not empty list items should be
+ * returned to be handled by the calling function
+ * @return {PositionPlusEmptyIndicator[]} The positions of the list item text in the given text
+ * with a status as to whether or not they are empty
  */
-function getListItemTextPositions(text: string): Position[] {
+function getListItemTextPositions(text: string, includeEmptyNodes: boolean = false): Position[] {
   const ast = parseTextToAST(text);
-  const positions: Position[] = [];
+  const positions: PositionPlusEmptyIndicator[] = [];
   visit(ast, MDAstTypes.ListItem as string, (node) => {
     // @ts-ignore the fact that not all nodes have a children property since I am skipping any that do not
-    if (!node.children) {
+    if (!node.children || node.children.length === 0) {
+      if (includeEmptyNodes) {
+        positions.push({
+          position: node.position,
+          isEmpty: true,
+        });
+      }
+
       return;
     }
 
     // @ts-ignore the fact that not all nodes have a children property since I have already exited the function if that is the case
     for (const childNode of node.children) {
       if (childNode.type === (MDAstTypes.Paragraph as string)) {
-        positions.push(childNode.position);
+        positions.push({
+          position: childNode.position,
+          isEmpty: false,
+        });
       }
     }
   });
 
   // Sort positions by start position in reverse order
-  positions.sort((a, b) => b.start.offset - a.start.offset);
+  positions.sort((a, b) => b.position.start.offset - a.position.start.offset);
   return positions;
 }
 
@@ -587,29 +605,40 @@ export function updateBoldText(text: string, func:(text: string) => string): str
   return text;
 }
 
-export function updateListItemText(text: string, func:(text: string) => string): string {
-  const positions: Position[] = getListItemTextPositions(text);
+export function updateListItemText(text: string, func:(text: string) => string, includeEmptyNodes: boolean = false): string {
+  const positions: PositionPlusEmptyIndicator[] = getListItemTextPositions(text, includeEmptyNodes);
 
   for (const position of positions) {
-    let startIndex = position.start.offset;
-    // get the actual start of the list item leaving only 1 whitespace between the indicator and the text
-    while (startIndex > 0 && text.charAt(startIndex - 1).trim() === '') {
-      startIndex--;
-    }
-    // keep a single space for the indicator
-    if (startIndex === 0 || text.charAt(startIndex - 1).trim() != '') {
+    let startIndex = position.position.start.offset;
+    if (position.isEmpty) {
+      // get the actual start of the list item leaving only 1 whitespace between the indicator and the text
+      while (startIndex < position.position.end.offset && text.charAt(startIndex).trim() !== '') {
+        startIndex++;
+      }
+
       startIndex++;
+    } else {
+      // get the actual start of the list item leaving only 1 whitespace between the indicator and the text
+      while (startIndex > 0 && text.charAt(startIndex - 1).trim() === '') {
+        startIndex--;
+      }
+
+      // keep a single space for the indicator
+      if (startIndex === 0 || text.charAt(startIndex - 1).trim() != '') {
+        startIndex++;
+      }
     }
 
-    let listText = text.substring(startIndex, position.end.offset);
+    let listText = text.substring(startIndex, position.position.end.offset);
     // for some reason some checklists are not getting treated as such and this causes the task indicator to be included in the text
     if (checklistBoxStartsTextRegex.test(listText)) {
       startIndex += 4;
       listText = listText.substring(4);
     }
+
     listText = func(listText);
 
-    text = replaceTextBetweenStartAndEndWithNewValue(text, startIndex, position.end.offset, listText);
+    text = replaceTextBetweenStartAndEndWithNewValue(text, startIndex, position.position.end.offset, listText);
   }
 
   return text;