Merge pull request #3328 from ilandikov/refactor-list-item-getters

refactor: make ListItem class more similar to Task

Author: claremacrae

src/Task/ListItem.ts

@@ -1,6 +1,7 @@
+import type { TasksFile } from '../Scripting/TasksFile';
+import type { Task } from './Task';
 import type { TaskLocation } from './TaskLocation';
 import { TaskRegularExpressions } from './TaskRegularExpressions';
-import type { Task } from './Task';
 
 export class ListItem {
     // The original line read from file.
@@ -94,12 +95,24 @@ export class ListItem {
             return false;
         }
 
-        if (this.originalMarkdown !== other.originalMarkdown) {
-            return false;
+        // Note: sectionStart changes every time a line is added or deleted before
+        //       any of the tasks in a file. This does mean that redrawing of tasks blocks
+        //       happens more often than is ideal.
+        const args: Array<keyof ListItem> = [
+            'originalMarkdown',
+            'description',
+            'statusCharacter',
+            'path',
+            'lineNumber',
+            'sectionStart',
+            'sectionIndex',
+            'precedingHeader',
+        ];
+
+        for (const el of args) {
+            if (this[el]?.toString() !== other[el]?.toString()) return false;
         }
 
-        // Not testing status character as it is implied from the original markdown
-
         return ListItem.listsAreIdentical(this.children, other.children);
     }
 
@@ -123,4 +136,40 @@ export class ListItem {
 
         return list1.every((item, index) => item.identicalTo(list2[index]));
     }
+
+    public get path(): string {
+        return this.taskLocation.path;
+    }
+
+    public get file(): TasksFile {
+        return this.taskLocation.tasksFile;
+    }
+
+    /**
+     * Return the name of the file containing this object, with the .md extension removed.
+     */
+    public get filename(): string | null {
+        const fileNameMatch = this.path.match(/([^/]+)\.md$/);
+        if (fileNameMatch !== null) {
+            return fileNameMatch[1];
+        } else {
+            return null;
+        }
+    }
+
+    public get lineNumber(): number {
+        return this.taskLocation.lineNumber;
+    }
+
+    public get sectionStart(): number {
+        return this.taskLocation.sectionStart;
+    }
+
+    public get sectionIndex(): number {
+        return this.taskLocation.sectionIndex;
+    }
+
+    public get precedingHeader(): string | null {
+        return this.taskLocation.precedingHeader;
+    }
 }

src/Task/Task.ts

@@ -6,7 +6,6 @@ import type { Status } from '../Statuses/Status';
 import { compareByDate } from '../DateTime/DateTools';
 import { TasksDate } from '../DateTime/TasksDate';
 import { StatusType } from '../Statuses/StatusConfiguration';
-import type { TasksFile } from '../Scripting/TasksFile';
 import { PriorityTools } from '../lib/PriorityTools';
 import { logging } from '../lib/logging';
 import { logEndOfTaskEdit, logStartOfTaskEdit } from '../lib/LogTasksHelper';
@@ -46,8 +45,6 @@ export class Task extends ListItem {
     public readonly indentation: string;
     public readonly listMarker: string;
 
-    public readonly taskLocation: TaskLocation;
-
     public readonly tags: string[];
 
     public readonly priority: Priority;
@@ -126,7 +123,6 @@ export class Task extends ListItem {
         this.description = description;
         this.indentation = indentation;
         this.listMarker = listMarker;
-        this.taskLocation = taskLocation;
 
         this.tags = tags;
 
@@ -619,10 +615,6 @@ export class Task extends ListItem {
         return this._urgency;
     }
 
-    public get path(): string {
-        return this.taskLocation.path;
-    }
-
     /**
      * Return {@link cancelledDate} as a {@link TasksDate}, so the field names in scripting docs are consistent with the existing search instruction names, and null values are easy to deal with.
      */
@@ -729,38 +721,6 @@ export class Task extends ListItem {
         return this.precedingHeader !== null;
     }
 
-    public get file(): TasksFile {
-        return this.taskLocation.tasksFile;
-    }
-
-    /**
-     * Return the name of the file containing the task, with the .md extension removed.
-     */
-    public get filename(): string | null {
-        const fileNameMatch = this.path.match(/([^/]+)\.md$/);
-        if (fileNameMatch !== null) {
-            return fileNameMatch[1];
-        } else {
-            return null;
-        }
-    }
-
-    get lineNumber(): number {
-        return this.taskLocation.lineNumber;
-    }
-
-    get sectionStart(): number {
-        return this.taskLocation.sectionStart;
-    }
-
-    get sectionIndex(): number {
-        return this.taskLocation.sectionIndex;
-    }
-
-    public get precedingHeader(): string | null {
-        return this.taskLocation.precedingHeader;
-    }
-
     /**
      * Returns the text that should be displayed to the user when linking to the origin of the task
      *
@@ -817,13 +777,8 @@ export class Task extends ListItem {
         //       happens more often than is ideal.
         let args: Array<keyof Task> = [
             'description',
-            'path',
             'indentation',
             'listMarker',
-            'lineNumber',
-            'sectionStart',
-            'sectionIndex',
-            'precedingHeader',
             'priority',
             'blockLink',
             'scheduledDateIsInferred',

tests/Task/ListItem.test.ts

@@ -173,7 +173,7 @@ describe('identicalTo', () => {
         expect(listItem1.identicalTo(listItem2)).toEqual(true);
     });
 
-    it('should test different markdown', () => {
+    it('should recognise different descriptions', () => {
         const listItem1 = new ListItem('- description', null, taskLocation);
         const listItem2 = new ListItem('- description two', null, taskLocation);
         expect(listItem1.identicalTo(listItem2)).toEqual(false);
@@ -211,6 +211,13 @@ describe('identicalTo', () => {
 
         expect(listItem.identicalTo(task)).toEqual(false);
     });
+
+    it('should recognise different path', () => {
+        const item1 = new ListItem('- same', null, TaskLocation.fromUnknownPosition(new TasksFile('anything.md')));
+        const item2 = new ListItem('- same', null, TaskLocation.fromUnknownPosition(new TasksFile('something.md')));
+
+        expect(item2.identicalTo(item1)).toEqual(false);
+    });
 });
 
 describe('checking if list item lists are identical', () => {