Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1453 (2023-01-01 18:24:55)

refactor: Partial adoption of StatusRegistry from obsidian-tasks-x
<!--- Provide a general summary of your changes in the Title above -->

# Description

This is the first in a series of steps to adopt use of the `StatusRegistry` from obsidian-tasks-x.

The changes were:

- fix: Add support for task lines with unknown characters in []
- refactor: Add most of `StatusRegistry` from obsidian-tasks-x
- test: Make `Task.toggle()` tests check number of tasks returned
- refactor: `Task.fromLine()` and `Task.toggle()` now use `StatusRegistry`.
- refactor: Copy in most of remaining `StatusRegistry` tests
- comment: Update a comment that was out-of-date
- refactor: Remove optional status parameter from `Task.toggle()`

## Non-trivial differences between obsidian-tasks-x and the obsidian-tasks behaviour in this PR

- Completion of `Todo`
    - In obsidian-tasks-x, `Todo` goes to `In Progress`.
    - In obsidian-tasks, `Todo` goes to `Done`, for consistency with the existing released behaviour. (Users will later have the option to adopt `In Progress` if they wish)
- Handling of unrecognised states
    - In obsidian-tasks-x, reading a line with an unknown status character throws an exception.
    - In In obsidian-tasks, instead a custom status called `Unknown` is created. (I may later change this to include the status character in the name, to avoid duplicate status names.)

The revision copied from is:
https://github.com/sytone/obsidian-tasks-x/commit/2c0b659457cc80806ff18585c955496c76861b87

Co-Authored-By: Sytone <1399443+sytone@users.noreply.github.com>

## Motivation and Context

This is preparation for adding support for more task status types - making the changes in small, testable steps.

## How has this been tested?

By running existing tests and adding new ones.

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

Internal changes:

- [x] **Refactor** (prefix: `refactor` - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## Checklist

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [ ] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#updating-documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#maintaining-the-tests).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md) and
[Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md).
Put an x in the boxes to confirm you agree.
-->

- [x] My contribution follow this project's [contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md)
- [x] I agree to follow this project's [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md)



Patch:
diff --git a/src/Status.ts b/src/Status.ts
index aabd2479..e433d91f 100644
--- a/src/Status.ts
+++ b/src/Status.ts
@@ -73,13 +73,22 @@ export class Status {
     public static DONE: Status = new Status(new StatusConfiguration('x', 'Done', ' ', true));
 
     /**
-     * The default Todo status. Goes to In Progress when toggled.
+     * A default status of empty, used when things go wrong.
+     *
+     * @static
+     * @memberof Status
+     */
+    public static EMPTY: Status = new Status(new StatusConfiguration('', 'EMPTY', '', true));
+
+    /**
+     * The default Todo status. Goes to Done when toggled.
+     * User may later be able to override this to go to In Progress instead.
      *
      * @static
      * @type {Status}
      * @memberof Status
      */
-    public static TODO: Status = new Status(new StatusConfiguration(' ', 'Todo', '/', true));
+    public static TODO: Status = new Status(new StatusConfiguration(' ', 'Todo', 'x', true));
 
     /**
      * The configuration stored in the data.json file.
diff --git a/src/StatusRegistry.ts b/src/StatusRegistry.ts
new file mode 100644
index 00000000..86d7fb77
--- /dev/null
+++ b/src/StatusRegistry.ts
@@ -0,0 +1,135 @@
+import { Status, StatusConfiguration } from './Status';
+
+/**
+ * Tracks all the registered statuses a task can have.
+ *
+ * @export
+ * @class StatusRegistry
+ */
+export class StatusRegistry {
+    private static instance: StatusRegistry;
+
+    private _registeredStatuses: Status[] = [];
+
+    /**
+     * Creates an instance of Status and registers it for use. It will also check to see
+     * if the default todo and done are registered and if not handle it internally.
+     *
+     * @memberof StatusRegistry
+     */
+    private constructor() {
+        this.addDefaultStatusTypes();
+    }
+
+    /**
+     * The static method that controls the access to the StatusRegistry instance.
+     *
+     * @static
+     * @return {*}  {StatusRegistry}
+     * @memberof StatusRegistry
+     */
+    public static getInstance(): StatusRegistry {
+        if (!StatusRegistry.instance) {
+            StatusRegistry.instance = new StatusRegistry();
+        }
+
+        return StatusRegistry.instance;
+    }
+
+    /**
+     * Adds a new Status to the registry if not already registered.
+     *
+     * @param {StatusConfiguration | Status} status
+     * @memberof StatusRegistry
+     */
+    public add(status: StatusConfiguration | Status): void {
+        if (!this.hasIndicator(status.indicator)) {
+            this._registeredStatuses.push(new Status(status));
+        }
+    }
+
+    /**
+     * Returns the registered status by the indicator between the
+     * square braces in the markdown task.
+     *
+     * @param {string} indicator
+     * @return {*}  {Status}
+     * @memberof StatusRegistry
+     */
+    public byIndicator(indicator: string): Status {
+        if (this.hasIndicator(indicator)) {
+            return this.getIndicator(indicator);
+        }
+
+        return Status.EMPTY;
+    }
+
+    /**
+     * Resets the array os Status types to be empty.
+     *
+     * @memberof StatusRegistry
+     */
+    public clearStatuses(): void {
+        this._registeredStatuses = [];
+        this.addDefaultStatusTypes();
+    }
+
+    /**
+     * To allow custom progression of task status each status knows
+     * which status can come after it as a state transition.
+     *
+     * @return {*}  {Status}
+     * @memberof StatusRegistry
+     */
+    public getNextStatus(status: Status): Status {
+        if (status.nextStatusIndicator !== '') {
+            const nextStatus = this.byIndicator(status.nextStatusIndicator);
+            if (nextStatus !== null) {
+                return nextStatus;
+            }
+        }
+        return Status.EMPTY;
+    }
+
+    /**
+     * Filters the Status types by the indicator and returns the first one found.
+     *
+     * @private
+     * @param {string} indicatorToFind
+     * @return {*}  {Status}
+     * @memberof StatusRegistry
+     */
+    private getIndicator(indicatorToFind: string): Status {
+        return this._registeredStatuses.filter(({ indicator }) => indicator === indicatorToFind)[0];
+    }
+
+    /**
+     * Filters all the Status types by the indicator and returns true if found.
+     *
+     * @private
+     * @param {string} indicatorToFind
+     * @return {*}  {boolean}
+     * @memberof StatusRegistry
+     */
+    private hasIndicator(indicatorToFind: string): boolean {
+        return (
+            this._registeredStatuses.find((element) => {
+                return element.indicator === indicatorToFind;
+            }) !== undefined
+        );
+    }
+
+    /**
+     * Checks the registry and adds the default status types.
+     *
+     * @private
+     * @memberof StatusRegistry
+     */
+    private addDefaultStatusTypes(): void {
+        const defaultStatuses = [Status.TODO, Status.DONE, Status.EMPTY];
+
+        defaultStatuses.forEach((status) => {
+            this.add(status);
+        });
+    }
+}
diff --git a/src/Task.ts b/src/Task.ts
index d9204e8d..56457b36 100644
--- a/src/Task.ts
+++ b/src/Task.ts
@@ -3,7 +3,8 @@ import { LayoutOptions, TaskLayout } from './TaskLayout';
 import type { TaskLayoutComponent } from './TaskLayout';
 import { Recurrence } from './Recurrence';
 import { getSettings } from './Config/Settings';
-import { Status } from './Status';
+import { StatusRegistry } from './StatusRegistry';
+import { Status, StatusConfiguration } from './Status';
 import { Urgency } from './Urgency';
 import { DateField } from './Query/Filter/DateField';
 import { renderTaskLine } from './TaskLineRenderer';
@@ -268,16 +269,11 @@ export class Task {
         const indentation = regexMatch[1];
         const listMarker = regexMatch[2];
 
-        // Get the status of the task, only todo and done supported.
-        // But custom ones are retained and displayed as-is.
+        // Get the status of the task.
         const statusString = regexMatch[3];
-        let status: Status;
-        switch (statusString) {
-            case ' ':
-                status = Status.TODO;
-                break;
-            default:
-                status = Status.DONE;
+        let status = StatusRegistry.getInstance().byIndicator(statusString);
+        if (status === Status.EMPTY) {
+            status = new Status(new StatusConfiguration(statusString, 'Unknown', 'x', false));
         }
 
         // Match for block link and remove if found. Always expected to be
@@ -529,7 +525,7 @@ export class Task {
      * task is not recurring, it will return `[toggled]`.
      */
     public toggle(): Task[] {
-        const newStatus: Status = this.status === Status.TODO ? Status.DONE : Status.TODO;
+        const newStatus = StatusRegistry.getInstance().getNextStatus(this.status);
 
         let newDoneDate = null;
 
@@ -539,7 +535,7 @@ export class Task {
             dueDate: Moment | null;
         } | null = null;
 
-        if (newStatus !== Status.TODO) {
+        if (newStatus.isCompleted()) {
             // Set done date only if setting value is true
             const { setDoneDate } = getSettings();
             if (setDoneDate) {
@@ -556,7 +552,7 @@ export class Task {
             ...this,
             status: newStatus,
             doneDate: newDoneDate,
-            originalStatusCharacter: newStatus === Status.DONE ? 'x' : ' ',
+            originalStatusCharacter: newStatus.isCompleted() ? 'x' : ' ',
         });
 
         const newTasks: Task[] = [];


Test patch:
diff --git a/tests/StatusRegistry.test.ts b/tests/StatusRegistry.test.ts
new file mode 100644
index 00000000..2314aab7
--- /dev/null
+++ b/tests/StatusRegistry.test.ts
@@ -0,0 +1,99 @@
+/**
+ * @jest-environment jsdom
+ */
+import moment from 'moment';
+import { StatusRegistry } from '../src/StatusRegistry';
+import { Status, StatusConfiguration } from '../src/Status';
+import { Task } from '../src/Task';
+
+jest.mock('obsidian');
+window.moment = moment;
+
+describe('StatusRegistry', () => {
+    it('should create a new instance and populate default status indicators', () => {
+        // Arrange
+
+        // Act
+        const statusRegistry = StatusRegistry.getInstance();
+        const doneStatus = statusRegistry.byIndicator('x');
+
+        // Assert
+        expect(statusRegistry).not.toBeNull();
+
+        expect(doneStatus.indicator).toEqual(Status.DONE.indicator);
+
+        expect(statusRegistry.byIndicator('x').indicator).toEqual(Status.DONE.indicator);
+        expect(statusRegistry.byIndicator('').indicator).toEqual(Status.EMPTY.indicator);
+        expect(statusRegistry.byIndicator(' ').indicator).toEqual(Status.TODO.indicator);
+
+        // Detect unrecognised indicator:
+        expect(statusRegistry.byIndicator('?').indicator).toEqual(Status.EMPTY.indicator);
+    });
+
+    it('should allow lookup of next status for a status', () => {
+        // Arrange
+        const statusRegistry = StatusRegistry.getInstance();
+        statusRegistry.clearStatuses();
+        const statusA = new Status(new StatusConfiguration('a', 'A', 'b', false));
+        const statusB = new Status(new StatusConfiguration('b', 'B', 'c', false));
+        const statusC = new Status(new StatusConfiguration('c', 'C', 'd', false));
+        const statusD = new Status(new StatusConfiguration('d', 'D', 'a', false));
+
+        // Act
+        statusRegistry.add(statusA);
+        statusRegistry.add(statusB);
+        statusRegistry.add(statusC);
+        statusRegistry.add(statusD);
+
+        // Assert
+        expect(statusRegistry.getNextStatus(statusA).indicator).toEqual('b');
+        expect(statusRegistry.getNextStatus(statusB).indicator).toEqual('c');
+        expect(statusRegistry.getNextStatus(statusC).indicator).toEqual('d');
+        expect(statusRegistry.getNextStatus(statusD).indicator).toEqual('a');
+    });
+
+    it('should allow task to toggle through custom transitions', () => {
+        // Arrange
+        const statusRegistry = StatusRegistry.getInstance();
+        statusRegistry.clearStatuses();
+        const statusA = new Status(new StatusConfiguration('a', 'A', 'b', false));
+        const statusB = new Status(new StatusConfiguration('b', 'B', 'c', false));
+        const statusC = new Status(new StatusConfiguration('c', 'C', 'd', false));
+        const statusD = new Status(new StatusConfiguration('d', 'D', 'a', false));
+        statusRegistry.add(statusA);
+        statusRegistry.add(statusB);
+        statusRegistry.add(statusC);
+        statusRegistry.add(statusD);
+        const line = '- [a] this is a task starting at A';
+        const path = 'file.md';
+        const sectionStart = 1337;
+        const sectionIndex = 1209;
+        const precedingHeader = 'Eloquent Section';
+        const task = Task.fromLine({
+            line,
+            path,
+            sectionStart,
+            sectionIndex,
+            precedingHeader,
+            fallbackDate: null,
+        });
+
+        // Act
+
+        // Assert
+        expect(task).not.toBeNull();
+        expect(task!.status.indicator).toEqual(statusA.indicator);
+
+        const toggledA = task?.toggle()[0];
+        expect(toggledA?.status.indicator).toEqual(statusB.indicator);
+
+        const toggledB = toggledA?.toggle()[0];
+        expect(toggledB?.status.indicator).toEqual(statusC.indicator);
+
+        const toggledC = toggledB?.toggle()[0];
+        expect(toggledC?.status.indicator).toEqual(statusD.indicator);
+
+        const toggledD = toggledC?.toggle()[0];
+        expect(toggledD?.status.indicator).toEqual(statusA.indicator);
+    });
+});
diff --git a/tests/Task.test.ts b/tests/Task.test.ts
index fee3b61b..94f90bf6 100644
--- a/tests/Task.test.ts
+++ b/tests/Task.test.ts
@@ -27,7 +27,7 @@ describe('parsing', () => {
         expect(task).not.toBeNull();
         expect(task!.listMarker).toEqual('-');
         expect(task!.description).toEqual('this is a done task');
-        expect(task!.status).toStrictEqual(Status.DONE);
+        expect(task!.status.indicator).toStrictEqual(Status.DONE.indicator);
         expect(task!.dueDate).not.toBeNull();
         expect(task!.dueDate!.isSame(moment('2021-09-12', 'YYYY-MM-DD'))).toStrictEqual(true);
         expect(task!.doneDate).not.toBeNull();
@@ -63,7 +63,7 @@ describe('parsing', () => {
         expect(task).not.toBeNull();
         expect(task!.listMarker).toEqual('1.');
         expect(task!.description).toEqual('this is a done task');
-        expect(task!.status).toStrictEqual(Status.DONE);
+        expect(task!.status.indicator).toStrictEqual(Status.DONE.indicator);
         expect(task!.originalMarkdown).toStrictEqual(line);
     });
 
@@ -80,7 +80,7 @@ describe('parsing', () => {
         expect(task).not.toBeNull();
         expect(task!.listMarker).toEqual('909999.');
         expect(task!.description).toEqual('this is a todo task');
-        expect(task!.status).toStrictEqual(Status.TODO);
+        expect(task!.status.indicator).toStrictEqual(Status.TODO.indicator);
         expect(task!.originalMarkdown).toStrictEqual(line);
     });
 
@@ -109,7 +109,7 @@ describe('parsing', () => {
         const task = fromLine({ line: '- [D] this is a deferred task' });
 
         // Assert
-        expect(task!.originalStatusCharacter).toStrictEqual('D');
+        expect(task!.status.indicator).toStrictEqual('D');
     });
 
     it('allows signifier emojis as part of the description', () => {
@@ -124,7 +124,7 @@ describe('parsing', () => {
         // Assert
         expect(task).not.toBeNull();
         expect(task!.description).toEqual('this is a ✅ done task');
-        expect(task!.status).toStrictEqual(Status.DONE);
+        expect(task!.status.indicator).toStrictEqual(Status.DONE.indicator);
         expect(task!.dueDate).not.toBeNull();
         expect(task!.dueDate!.isSame(moment('2021-09-12', 'YYYY-MM-DD'))).toStrictEqual(true);
         expect(task!.doneDate).not.toBeNull();
@@ -143,7 +143,7 @@ describe('parsing', () => {
         // Assert
         expect(task).not.toBeNull();
         expect(task!.description).toEqual('this is a ✅ done task');
-        expect(task!.status).toStrictEqual(Status.DONE);
+        expect(task!.status.indicator).toStrictEqual(Status.DONE.indicator);
         expect(task!.dueDate).not.toBeNull();
         expect(task!.dueDate!.isSame(moment('2021-09-12', 'YYYY-MM-DD'))).toStrictEqual(true);
         expect(task!.doneDate).not.toBeNull();
@@ -467,13 +467,15 @@ describe('toggle done', () => {
         const task: Task = fromLine({
             line,
         }) as Task;
-        const toggled: Task = task.toggle()[0];
+        const tasks = task.toggle();
+        expect(tasks.length).toEqual(1);
+        const toggled: Task = tasks[0];
 
         // Assert
         expect(toggled).not.toBeNull();
-        expect(toggled!.status).toStrictEqual(Status.DONE);
+        expect(toggled!.status.indicator).toStrictEqual(Status.DONE.indicator);
         expect(toggled!.doneDate).not.toBeNull();
-        expect(toggled!.originalStatusCharacter).toStrictEqual('x');
+        expect(toggled!.status.indicator).toStrictEqual('x');
         expect(toggled!.blockLink).toEqual(' ^my-precious');
     });
 
@@ -485,12 +487,14 @@ describe('toggle done', () => {
         const task: Task = fromLine({
             line,
         }) as Task;
-        const toggled: Task = task.toggle()[0];
+        const tasks = task.toggle();
+        expect(tasks.length).toEqual(1);
+        const toggled: Task = tasks[0];
 
         // Assert
         expect(toggled).not.toBeNull();
-        expect(toggled!.status).toStrictEqual(Status.TODO);
-        expect(toggled!.originalStatusCharacter).toStrictEqual(' ');
+        expect(toggled!.status.indicator).toStrictEqual(Status.TODO.indicator);
+        expect(toggled!.status.indicator).toStrictEqual(' ');
         expect(toggled!.doneDate).toBeNull();
     });
 
@@ -788,7 +792,9 @@ describe('toggle done', () => {
                 line,
             });
 
-            const nextTask: Task = task!.toggle()[0];
+            const tasks = task!.toggle();
+            expect(tasks.length).toEqual(2);
+            const nextTask: Task = tasks[0];
 
             expect({
                 nextDue: nextTask.dueDate?.format('YYYY-MM-DD'),
@@ -823,7 +829,9 @@ describe('toggle done', () => {
         expect(task!.dueDate).not.toBeNull();
         expect(task!.dueDate!.isSame(moment('2021-09-12', 'YYYY-MM-DD'))).toStrictEqual(true);
 
-        const nextTask: Task = task!.toggle()[0];
+        const tasks = task!.toggle();
+        expect(tasks.length).toEqual(2);
+        const nextTask: Task = tasks[0];
         expect({
             nextDue: nextTask.dueDate?.format('YYYY-MM-DD'),
             nextScheduled: nextTask.scheduledDate?.format('YYYY-MM-DD'),