Merge pull request #1592 from obsidian-tasks-group/reduce-cyclic-depe…

…ndencies refactor: Reduce cyclic dependencies
obsidian-tasks-group · Jan 27, 2023 · b165429 · b165429
2 parents 68f5eac + 14d59e0
commit b165429
Show file tree

Hide file tree

Showing 11 changed files with 56 additions and 58 deletions.
diff --git a/src/Config/CustomStatusModal.ts b/src/Config/CustomStatusModal.ts
@@ -1,6 +1,6 @@
 import { Modal, Notice, Setting, TextComponent } from 'obsidian';
+import type { Plugin } from 'obsidian';
 import { StatusConfiguration, StatusType } from '../StatusConfiguration';
-import type TasksPlugin from '../main';
 import { StatusValidator } from '../StatusValidator';
 import { Status } from '../Status';
 
@@ -16,7 +16,7 @@ export class CustomStatusModal extends Modal {
     saved: boolean = false;
     error: boolean = false;
     private isCoreStatus: boolean;
-    constructor(public plugin: TasksPlugin, statusType: StatusConfiguration, isCoreStatus: boolean) {
+    constructor(public plugin: Plugin, statusType: StatusConfiguration, isCoreStatus: boolean) {
         super(plugin.app);
         this.statusSymbol = statusType.symbol;
         this.statusName = statusType.name;

diff --git a/src/Config/SettingsTab.ts b/src/Config/SettingsTab.ts
@@ -454,7 +454,10 @@ export class SettingsTab extends PluginSettingTab {
                 .setCta()
                 .onClick(async () => {
                     const tasks = this.plugin.getTasks();
-                    const unknownStatuses = StatusRegistry.getInstance().findUnknownStatuses(tasks!);
+                    const allStatuses = tasks!.map((task) => {
+                        return task.status;
+                    });
+                    const unknownStatuses = StatusRegistry.getInstance().findUnknownStatuses(allStatuses);
                     if (unknownStatuses.length === 0) {
                         return;
                     }

diff --git a/src/Query/Filter/DateField.ts b/src/Query/Filter/DateField.ts
@@ -3,6 +3,7 @@ import type { Task } from '../../Task';
 import { DateParser } from '../DateParser';
 import { Explanation } from '../Explain/Explanation';
 import type { Comparator } from '../Sorter';
+import { compareByDate } from '../../lib/DateTools';
 import { Field } from './Field';
 import { Filter, FilterOrErrorMessage } from './Filter';
 import { FilterInstructions } from './FilterInstructions';
@@ -127,31 +128,7 @@ export abstract class DateField extends Field {
 
     public comparator(): Comparator {
         return (a: Task, b: Task) => {
-            return DateField.compareByDate(this.date(a), this.date(b));
+            return compareByDate(this.date(a), this.date(b));
         };
     }
-
-    public static compareByDate(a: moment.Moment | null, b: moment.Moment | null): -1 | 0 | 1 {
-        if (a !== null && b === null) {
-            return -1;
-        } else if (a === null && b !== null) {
-            return 1;
-        } else if (a !== null && b !== null) {
-            if (a.isValid() && !b.isValid()) {
-                return -1;
-            } else if (!a.isValid() && b.isValid()) {
-                return 1;
-            }
-
-            if (a.isAfter(b)) {
-                return 1;
-            } else if (a.isBefore(b)) {
-                return -1;
-            } else {
-                return 0;
-            }
-        } else {
-            return 0;
-        }
-    }
 }
diff --git a/src/Query/Filter/HappensDateField.ts b/src/Query/Filter/HappensDateField.ts
@@ -3,6 +3,7 @@ import type { Task } from '../../Task';
 import { DateParser } from '../DateParser';
 import { Explanation } from '../Explain/Explanation';
 import type { Comparator } from '../Sorter';
+import { compareByDate } from '../../lib/DateTools';
 import { Field } from './Field';
 import { Filter, FilterOrErrorMessage } from './Filter';
 import { FilterInstructions } from './FilterInstructions';
@@ -92,7 +93,7 @@ export class HappensDateField extends Field {
      */
     public earliestDate(task: Task): Moment | null {
         const happensDates = new HappensDateField().dates(task);
-        const sortedHappensDates = happensDates.sort(DateField.compareByDate);
+        const sortedHappensDates = happensDates.sort(compareByDate);
         return sortedHappensDates[0];
     }
 
@@ -120,7 +121,7 @@ export class HappensDateField extends Field {
      */
     public comparator(): Comparator {
         return (a: Task, b: Task) => {
-            return DateField.compareByDate(this.earliestDate(a), this.earliestDate(b));
+            return compareByDate(this.earliestDate(a), this.earliestDate(b));
         };
     }
 }
diff --git a/src/Recurrence.ts b/src/Recurrence.ts
@@ -1,6 +1,6 @@
 import type { Moment } from 'moment';
 import { RRule } from 'rrule';
-import { DateField } from './Query/Filter/DateField';
+import { compareByDate } from './lib/DateTools';
 
 export class Recurrence {
     private readonly rrule: RRule;
@@ -195,13 +195,13 @@ export class Recurrence {
         }
 
         // Compare Date fields
-        if (DateField.compareByDate(this.startDate, other.startDate) !== 0) {
+        if (compareByDate(this.startDate, other.startDate) !== 0) {
             return false;
         }
-        if (DateField.compareByDate(this.scheduledDate, other.scheduledDate) !== 0) {
+        if (compareByDate(this.scheduledDate, other.scheduledDate) !== 0) {
             return false;
         }
-        if (DateField.compareByDate(this.dueDate, other.dueDate) !== 0) {
+        if (compareByDate(this.dueDate, other.dueDate) !== 0) {
             return false;
         }
 

diff --git a/src/StatusRegistry.ts b/src/StatusRegistry.ts
@@ -1,6 +1,5 @@
 import { Status } from './Status';
 import { StatusConfiguration, StatusType } from './StatusConfiguration';
-import type { Task } from './Task';
 
 /**
  * Tracks all the registered statuses a task can have.
@@ -187,19 +186,15 @@ export class StatusRegistry {
     }
 
     /**
-     * Find any statuses in the given tasks that are not known to this registry.
+     * Find any statuses in the given list that are not known to this registry.
      * This can be used to add all unknown status types to the settings,
      * to save users from having to do that manually.
      *
      * Statuses are returned in the order that they are first found in the
-     * supplied tasks.
-     * @param tasks
+     * supplied list.
+     * @param allStatuses
      */
-    public findUnknownStatuses(tasks: Task[]): Status[] {
-        const allStatuses = tasks.map((task) => {
-            return task.status;
-        });
-
+    public findUnknownStatuses(allStatuses: Status[]): Status[] {
         const unknownStatuses = allStatuses.filter((s) => {
             return !this.hasSymbol(s.symbol);
         });

diff --git a/src/Task.ts b/src/Task.ts
@@ -6,11 +6,11 @@ import { getSettings } from './Config/Settings';
 import { StatusRegistry } from './StatusRegistry';
 import type { Status } from './Status';
 import { Urgency } from './Urgency';
-import { DateField } from './Query/Filter/DateField';
 import { renderTaskLine } from './TaskLineRenderer';
 import type { TaskLineRenderDetails } from './TaskLineRenderer';
 import { DateFallback } from './DateFallback';
 import * as RegExpTools from './lib/RegExpTools';
+import { compareByDate } from './lib/DateTools';
 
 /**
  * When sorting, make sure low always comes after none. This way any tasks with low will be below any exiting
@@ -686,7 +686,7 @@ export class Task {
         for (const el of args) {
             const date1 = this[el] as Moment | null;
             const date2 = other[el] as Moment | null;
-            if (DateField.compareByDate(date1, date2) !== 0) {
+            if (compareByDate(date1, date2) !== 0) {
                 return false;
             }
         }

diff --git a/src/lib/DateTools.ts b/src/lib/DateTools.ts
@@ -0,0 +1,23 @@
+export function compareByDate(a: moment.Moment | null, b: moment.Moment | null): -1 | 0 | 1 {
+    if (a !== null && b === null) {
+        return -1;
+    } else if (a === null && b !== null) {
+        return 1;
+    } else if (a !== null && b !== null) {
+        if (a.isValid() && !b.isValid()) {
+            return -1;
+        } else if (!a.isValid() && b.isValid()) {
+            return 1;
+        }
+
+        if (a.isAfter(b)) {
+            return 1;
+        } else if (a.isBefore(b)) {
+            return -1;
+        } else {
+            return 0;
+        }
+    } else {
+        return 0;
+    }
+}
diff --git a/tests/CustomMatchers/CustomMatchersForSorting.ts b/tests/CustomMatchers/CustomMatchersForSorting.ts
@@ -1,8 +1,8 @@
 import type moment from 'moment';
 import { DateParser } from '../../src/Query/DateParser';
-import { DateField } from '../../src/Query/Filter/DateField';
 import type { Sorter } from '../../src/Query/Sorter';
 import type { Task } from '../../src/Task';
+import { compareByDate } from '../../src/lib/DateTools';
 
 declare global {
     namespace jest {
@@ -26,7 +26,7 @@ expect.extend({
         let b: moment.Moment | null = null;
         if (dateB !== null) b = DateParser.parseDate(dateB);
 
-        const actual = DateField.compareByDate(a, b);
+        const actual = compareByDate(a, b);
 
         const pass = actual === expected;
         const message = () => `${dateA} < ${dateB}: expected=${expected} actual=${actual}`;

diff --git a/tests/StatusRegistry.test.ts b/tests/StatusRegistry.test.ts
@@ -7,7 +7,6 @@ import { Status } from '../src/Status';
 import { StatusConfiguration, StatusType } from '../src/StatusConfiguration';
 import { Task } from '../src/Task';
 import * as TestHelpers from './TestHelpers';
-import { TaskBuilder } from './TestingTools/TaskBuilder';
 
 jest.mock('obsidian');
 window.moment = moment;
@@ -278,20 +277,20 @@ describe('StatusRegistry', () => {
             expect(registry.bySymbol('!').type).toEqual(StatusType.EMPTY);
             expect(registry.bySymbol('X').type).toEqual(StatusType.EMPTY);
             expect(registry.bySymbol('d').type).toEqual(StatusType.EMPTY);
-            const tasks = [
-                new TaskBuilder().statusValues('!', 'Unknown', 'X', false, StatusType.TODO).build(),
-                new TaskBuilder().statusValues('X', 'Unknown', '!', false, StatusType.DONE).build(),
-                new TaskBuilder().statusValues('d', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+            const allStatuses = [
+                new Status(new StatusConfiguration('!', 'Unknown', 'X', false, StatusType.TODO)),
+                new Status(new StatusConfiguration('X', 'Unknown', '!', false, StatusType.DONE)),
+                new Status(new StatusConfiguration('d', 'Unknown', '!', false, StatusType.IN_PROGRESS)),
                 // Include some tasks with duplicate statuses, to make sure duplicates are discarded
-                new TaskBuilder().statusValues('!', 'Unknown', 'X', false, StatusType.TODO).build(),
-                new TaskBuilder().statusValues('X', 'Unknown', '!', false, StatusType.DONE).build(),
-                new TaskBuilder().statusValues('d', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+                new Status(new StatusConfiguration('!', 'Unknown', 'X', false, StatusType.TODO)),
+                new Status(new StatusConfiguration('X', 'Unknown', '!', false, StatusType.DONE)),
+                new Status(new StatusConfiguration('d', 'Unknown', '!', false, StatusType.IN_PROGRESS)),
                 // Check that it does not add copies of any core statuses
-                new TaskBuilder().statusValues('-', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+                new Status(new StatusConfiguration('-', 'Unknown', '!', false, StatusType.IN_PROGRESS)),
             ];
 
             // Act
-            const unknownStatuses = registry.findUnknownStatuses(tasks);
+            const unknownStatuses = registry.findUnknownStatuses(allStatuses);
 
             // Assert
             expect(unknownStatuses.length).toEqual(3);

diff --git a/tests/Query/Filter/DateField.test.ts → tests/lib/DateTools.test.ts b/tests/Query/Filter/DateField.test.ts → tests/lib/DateTools.test.ts
@@ -9,7 +9,7 @@ import {
     expectDateComparesAfter,
     expectDateComparesBefore,
     expectDateComparesEqual,
-} from '../../CustomMatchers/CustomMatchersForSorting';
+} from '../CustomMatchers/CustomMatchersForSorting';
 
 // These are lower-level tests that the Task-based ones above, for ease of test coverage.
 describe('compareBy', () => {