Merge pull request #3126 from ilandikov/fix-list-item-rerendering

fix: rerender search results when child list items are edited

src/Obsidian/Cache.ts

@@ -377,7 +377,7 @@ export class Cache {
 
         // If there are no changes in any of the tasks, there's
         // nothing to do, so just return.
-        if (Task.tasksListsIdentical(oldTasks, newTasks)) {
+        if (ListItem.listsAreIdentical(oldTasks, newTasks)) {
             // This code kept for now, to allow for debugging during development.
             // It is too verbose to release to users.
             // if (this.getState() == State.Warm) {

src/Task/ListItem.ts

@@ -48,4 +48,46 @@ export class ListItem {
     get isRoot(): boolean {
         return this.parent === null;
     }
+
+    /**
+     * Compare all the fields in another ListItem, to detect any differences from this one.
+     *
+     * If any field is different in any way, it will return false.
+     *
+     * @note Use {@link Task.identicalTo} to compare {@link Task} objects.
+     *
+     * @param other - if this is in fact a {@link Task}, the result of false.
+     */
+    identicalTo(other: ListItem) {
+        if (this.constructor.name !== other.constructor.name) {
+            return false;
+        }
+
+        if (this.originalMarkdown !== other.originalMarkdown) {
+            return false;
+        }
+
+        return ListItem.listsAreIdentical(this.children, other.children);
+    }
+
+    /**
+     * Compare two lists of ListItem objects, and report whether their
+     * contents, including any children, are identical and in the same order.
+     *
+     * This can be useful for optimising code if it is guaranteed that
+     * there are no possible differences in the tasks in a file
+     * after an edit, for example.
+     *
+     * If any field is different in any task or list item, it will return false.
+     *
+     * @param list1
+     * @param list2
+     */
+    static listsAreIdentical(list1: ListItem[], list2: ListItem[]): boolean {
+        if (list1.length !== list2.length) {
+            return false;
+        }
+
+        return list1.every((item, index) => item.identicalTo(list2[index]));
+    }
 }

src/Task/Task.ts

@@ -784,26 +784,6 @@ export class Task extends ListItem {
         return linkText;
     }
 
-    /**
-     * Compare two lists of Task objects, and report whether their
-     * tasks are identical in the same order.
-     *
-     * This can be useful for optimising code if it is guaranteed that
-     * there are no possible differences in the tasks in a file
-     * after an edit, for example.
-     *
-     * If any field is different in any task, it will return false.
-     *
-     * @param oldTasks
-     * @param newTasks
-     */
-    static tasksListsIdentical(oldTasks: Task[], newTasks: Task[]): boolean {
-        if (oldTasks.length !== newTasks.length) {
-            return false;
-        }
-        return oldTasks.every((oldTask, index) => oldTask.identicalTo(newTasks[index]));
-    }
-
     /**
      * Compare all the fields in another Task, to detect any differences from this one.
      *
@@ -816,6 +796,11 @@ export class Task extends ListItem {
      * @param other
      */
     public identicalTo(other: Task) {
+        // First compare child Task and ListItem objects, and any other data in ListItem:
+        if (!super.identicalTo(other)) {
+            return false;
+        }
+
         // NEW_TASK_FIELD_EDIT_REQUIRED
 
         // Based on ideas from koala. AquaCat and javalent in Discord:

tests/Task/ListItem.test.ts

@@ -6,6 +6,9 @@ import { TasksFile } from '../../src/Scripting/TasksFile';
 import { Task } from '../../src/Task/Task';
 import { TaskLocation } from '../../src/Task/TaskLocation';
 import { ListItem } from '../../src/Task/ListItem';
+import { TaskBuilder } from '../TestingTools/TaskBuilder';
+import { fromLine } from '../TestingTools/TestHelpers';
+import { createChildListItem } from './ListItemHelpers';
 
 window.moment = moment;
 
@@ -96,3 +99,107 @@ describe('list item tests', () => {
         }
     });
 });
+
+describe('identicalTo', () => {
+    it('should test same markdown', () => {
+        const listItem1 = new ListItem('- same description', null);
+        const listItem2 = new ListItem('- same description', null);
+        expect(listItem1.identicalTo(listItem2)).toEqual(true);
+    });
+
+    it('should test different markdown', () => {
+        const listItem1 = new ListItem('- description', null);
+        const listItem2 = new ListItem('- description two', null);
+        expect(listItem1.identicalTo(listItem2)).toEqual(false);
+    });
+
+    it('should recognise list items with different number of children', () => {
+        const item1 = new ListItem('- item', null);
+        createChildListItem('- child of item1', item1);
+
+        const item2 = new ListItem('- item', null);
+
+        expect(item2.identicalTo(item1)).toEqual(false);
+    });
+
+    it('should recognise list items with different children', () => {
+        const item1 = new ListItem('- item', null);
+        createChildListItem('- child of item1', item1);
+
+        const item2 = new ListItem('- item', null);
+        createChildListItem('- child of item2', item2);
+
+        expect(item2.identicalTo(item1)).toEqual(false);
+    });
+
+    it('should recognise ListItem and Task as different', () => {
+        const listItem = new ListItem('- [ ] description', null);
+        const task = fromLine({ line: '- [ ] description' });
+
+        expect(listItem.identicalTo(task)).toEqual(false);
+    });
+});
+
+describe('checking if list item lists are identical', () => {
+    it('should treat empty lists as identical', () => {
+        const list1: ListItem[] = [];
+        const list2: ListItem[] = [];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('should treat different sized lists as different', () => {
+        const list1: ListItem[] = [];
+        const list2: ListItem[] = [new ListItem('- x', null)];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+
+    it('should detect matching list items as same', () => {
+        const list1: ListItem[] = [new ListItem('- 1', null)];
+        const list2: ListItem[] = [new ListItem('- 1', null)];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('- should detect non-matching list items as different', () => {
+        const list1: ListItem[] = [new ListItem('- 1', null)];
+        const list2: ListItem[] = [new ListItem('- 2', null)];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+});
+
+describe('checking if task lists are identical', () => {
+    it('should treat empty lists as identical', () => {
+        const list1: Task[] = [];
+        const list2: Task[] = [];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('should treat different sized lists as different', () => {
+        const list1: Task[] = [];
+        const list2: Task[] = [new TaskBuilder().build()];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+
+    it('should detect matching tasks as same', () => {
+        const list1: Task[] = [new TaskBuilder().description('1').build()];
+        const list2: Task[] = [new TaskBuilder().description('1').build()];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(true);
+    });
+
+    it('should detect non-matching tasks as different', () => {
+        const list1: Task[] = [new TaskBuilder().description('1').build()];
+        const list2: Task[] = [new TaskBuilder().description('2').build()];
+        expect(ListItem.listsAreIdentical(list1, list2)).toBe(false);
+    });
+});
+
+describe('checking if mixed lists are identical', () => {
+    it('should recognise mixed lists as unequal', () => {
+        const list1 = [new ListItem('- [ ] description', null)];
+        const list2 = [fromLine({ line: '- [ ] description' })];
+
+        expect(ListItem.listsAreIdentical(list1, list1)).toEqual(true);
+        expect(ListItem.listsAreIdentical(list1, list2)).toEqual(false);
+        expect(ListItem.listsAreIdentical(list2, list1)).toEqual(false);
+        expect(ListItem.listsAreIdentical(list2, list2)).toEqual(true);
+    });
+});

tests/Task/ListItemHelpers.ts

@@ -0,0 +1,7 @@
+import { ListItem } from '../../src/Task/ListItem';
+
+export function createChildListItem(originalMarkdown: string, parent: ListItem) {
+    // This exists purely to silence WebStorm about typescript:S1848
+    // See https://sonarcloud.io/organizations/obsidian-tasks-group/rules?open=typescript%3AS1848&rule_key=typescript%3AS1848
+    new ListItem(originalMarkdown, parent);
+}

tests/Task/Task.test.ts

@@ -22,6 +22,7 @@ import type { TasksDate } from '../../src/DateTime/TasksDate';
 import { example_kanban } from '../Obsidian/__test_data__/example_kanban';
 import { jason_properties } from '../Obsidian/__test_data__/jason_properties';
 import { OnCompletion } from '../../src/Task/OnCompletion';
+import { createChildListItem } from './ListItemHelpers';
 
 window.moment = moment;
 
@@ -1730,30 +1731,21 @@ describe('identicalTo', () => {
         expect(task2.status.identicalTo(task1.status)).toEqual(true);
         expect(task2.identicalTo(task1)).toEqual(true);
     });
-});
 
-describe('checking if task lists are identical', () => {
-    it('should treat empty lists as identical', () => {
-        const list1: Task[] = [];
-        const list2: Task[] = [];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(true);
-    });
+    it('should recognise different numbers of child items', () => {
+        const task1 = new TaskBuilder().build();
+        const task2 = new TaskBuilder().build();
+        createChildListItem('- child of task2', task2);
 
-    it('should treat different sized lists as different', () => {
-        const list1: Task[] = [];
-        const list2: Task[] = [new TaskBuilder().build()];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(false);
+        expect(task2.identicalTo(task1)).toEqual(false);
     });
 
-    it('should detect matching tasks as same', () => {
-        const list1: Task[] = [new TaskBuilder().description('1').build()];
-        const list2: Task[] = [new TaskBuilder().description('1').build()];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(true);
-    });
+    it('should recognise different description in child list items', () => {
+        const task1 = new TaskBuilder().build();
+        const task2 = new TaskBuilder().build();
+        createChildListItem('- child of task1', task1);
+        createChildListItem('- child of task2', task2);
 
-    it('should detect non-matching tasks as different', () => {
-        const list1: Task[] = [new TaskBuilder().description('1').build()];
-        const list2: Task[] = [new TaskBuilder().description('2').build()];
-        expect(Task.tasksListsIdentical(list1, list2)).toBe(false);
+        expect(task2.identicalTo(task1)).toEqual(false);
     });
 });