Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1456 (2023-01-01 23:03:35)

refactor: remove originalStatusCharacter from Task and TaskBuilder
<!--- Provide a general summary of your changes in the Title above -->

# Description

Remove `originalStatusCharacter` from `Task` and `TaskBuilder`.

It has been totally replaced by `Task.status.indicator`.

## Motivation and Context

Simplifying code: the new `Status` class from obsidian-tasks-x is now fully adopted in Tasks, for the original status of `TODO` and `DONE`.

## How has this been tested?

By running existing tests.

## 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)

## 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/Commands/CreateOrEditTaskParser.ts b/src/Commands/CreateOrEditTaskParser.ts
index 6c7108bd..e887b489 100644
--- a/src/Commands/CreateOrEditTaskParser.ts
+++ b/src/Commands/CreateOrEditTaskParser.ts
@@ -43,7 +43,6 @@ export const taskFromLine = ({ line, path }: { line: string; path: string }): Ta
             path,
             indentation: '',
             listMarker: '-',
-            originalStatusCharacter: ' ',
             priority: Priority.None,
             startDate: null,
             scheduledDate: null,
@@ -80,7 +79,6 @@ export const taskFromLine = ({ line, path }: { line: string; path: string }): Ta
         path,
         indentation,
         listMarker,
-        originalStatusCharacter: statusString,
         blockLink,
         priority: Priority.None,
         startDate: null,
diff --git a/src/Task.ts b/src/Task.ts
index 9e2bc41c..f8e2c7ec 100644
--- a/src/Task.ts
+++ b/src/Task.ts
@@ -124,11 +124,6 @@ export class Task {
     public readonly sectionStart: number;
     /** The index of the nth task in its section. */
     public readonly sectionIndex: number;
-    /**
-     * The original character from within `[]` in the document.
-     * Required to be added to the LI the same way obsidian does as a `data-task` attribute.
-     */
-    public readonly originalStatusCharacter: string;
     public readonly precedingHeader: string | null;
 
     public readonly tags: string[];
@@ -162,7 +157,6 @@ export class Task {
         listMarker,
         sectionStart,
         sectionIndex,
-        originalStatusCharacter,
         precedingHeader,
         priority,
         startDate,
@@ -182,7 +176,6 @@ export class Task {
         listMarker: string;
         sectionStart: number;
         sectionIndex: number;
-        originalStatusCharacter: string;
         precedingHeader: string | null;
         priority: Priority;
         startDate: moment.Moment | null;
@@ -202,7 +195,6 @@ export class Task {
         this.listMarker = listMarker;
         this.sectionStart = sectionStart;
         this.sectionIndex = sectionIndex;
-        this.originalStatusCharacter = originalStatusCharacter;
         this.precedingHeader = precedingHeader;
 
         this.tags = tags;
@@ -417,7 +409,6 @@ export class Task {
             listMarker,
             sectionStart,
             sectionIndex,
-            originalStatusCharacter: statusString,
             precedingHeader,
             priority,
             startDate,
@@ -513,7 +504,7 @@ export class Task {
      * @memberof Task
      */
     public toFileLineString(): string {
-        return `${this.indentation}${this.listMarker} [${this.originalStatusCharacter}] ${this.toString()}`;
+        return `${this.indentation}${this.listMarker} [${this.status.indicator}] ${this.toString()}`;
     }
 
     /**
@@ -552,7 +543,6 @@ export class Task {
             ...this,
             status: newStatus,
             doneDate: newDoneDate,
-            originalStatusCharacter: newStatus.indicator,
         });
 
         const newTasks: Task[] = [];
@@ -669,7 +659,6 @@ export class Task {
             'listMarker',
             'sectionStart',
             'sectionIndex',
-            'originalStatusCharacter',
             'precedingHeader',
             'priority',
             'blockLink',
diff --git a/src/TaskLineRenderer.ts b/src/TaskLineRenderer.ts
index 4d0369a5..425f2f02 100644
--- a/src/TaskLineRenderer.ts
+++ b/src/TaskLineRenderer.ts
@@ -83,7 +83,7 @@ export async function renderTaskLine(
     li.prepend(checkbox);
 
     // Set these to be compatible with stock obsidian lists:
-    li.setAttribute('data-task', task.originalStatusCharacter.trim()); // Trim to ensure empty attribute for space. Same way as obsidian.
+    li.setAttribute('data-task', task.status.indicator.trim()); // Trim to ensure empty attribute for space. Same way as obsidian.
     li.setAttribute('data-line', renderDetails.listIndex.toString());
     checkbox.setAttribute('data-line', renderDetails.listIndex.toString());
 


Test patch:
diff --git a/tests/Query.test.ts b/tests/Query.test.ts
index e6aa2432..ae149e96 100644
--- a/tests/Query.test.ts
+++ b/tests/Query.test.ts
@@ -265,7 +265,6 @@ describe('Query', () => {
                     listMarker: '-',
                     sectionStart: 0,
                     sectionIndex: 0,
-                    originalStatusCharacter: ' ',
                     precedingHeader: null,
                     priority: Priority.None,
                     startDate: null,
@@ -286,7 +285,6 @@ describe('Query', () => {
                     listMarker: '-',
                     sectionStart: 0,
                     sectionIndex: 0,
-                    originalStatusCharacter: ' ',
                     precedingHeader: null,
                     priority: Priority.None,
                     startDate: null,
diff --git a/tests/Task.test.ts b/tests/Task.test.ts
index 5524f406..cd88c9c7 100644
--- a/tests/Task.test.ts
+++ b/tests/Task.test.ts
@@ -928,12 +928,6 @@ describe('identicalTo', () => {
         expect(lhs).not.toBeIdenticalTo(new TaskBuilder().sectionIndex(2));
     });
 
-    it('should check originalStatusCharacter', () => {
-        const lhs = new TaskBuilder().originalStatusCharacter(' ');
-        expect(lhs).toBeIdenticalTo(new TaskBuilder().originalStatusCharacter(' '));
-        expect(lhs).not.toBeIdenticalTo(new TaskBuilder().originalStatusCharacter('x'));
-    });
-
     it('should check precedingHeader', () => {
         const lhs = new TaskBuilder().precedingHeader('Heading 1');
         expect(lhs).toBeIdenticalTo(new TaskBuilder().precedingHeader('Heading 1'));
diff --git a/tests/TestingTools/TaskBuilder.test.ts b/tests/TestingTools/TaskBuilder.test.ts
index 9ce45df0..3d0576b8 100644
--- a/tests/TestingTools/TaskBuilder.test.ts
+++ b/tests/TestingTools/TaskBuilder.test.ts
@@ -1,5 +1,3 @@
-import type { Task } from '../../src/Task';
-import { Status } from '../../src/Status';
 import { TaskBuilder } from './TaskBuilder';
 
 export {};
@@ -10,36 +8,4 @@ describe('TaskBuilder', () => {
         const task = builder.build();
         expect(task.toFileLineString()).toStrictEqual('- [ ] hello #tag1 #tag2');
     });
-
-    describe('status and originalStatusCharacter', () => {
-        function checkStatuses(task: Task, status: Status, originalStatusCharacter: string, description: string) {
-            expect(task.status).toStrictEqual(status);
-            expect(task.originalStatusCharacter).toStrictEqual(originalStatusCharacter);
-            expect(task.toFileLineString()).toStrictEqual(description);
-        }
-
-        it('should set originalStatusCharacter when setting status TODO', () => {
-            const builder = new TaskBuilder().originalStatusCharacter('?').status(Status.TODO);
-            const task = builder.build();
-            checkStatuses(task, Status.TODO, ' ', '- [ ] my description');
-        });
-
-        it('should set originalStatusCharacter when setting status DONE', () => {
-            const builder = new TaskBuilder().originalStatusCharacter('?').status(Status.DONE);
-            const task = builder.build();
-            checkStatuses(task, Status.DONE, 'x', '- [x] my description');
-        });
-
-        it('should not change status when changing originalStatusCharacter to TODO', () => {
-            const builder = new TaskBuilder().status(Status.DONE).originalStatusCharacter(' ');
-            const task = builder.build();
-            checkStatuses(task, Status.DONE, ' ', '- [ ] my description');
-        });
-
-        it('should not change status when changing originalStatusCharacter to Half Done', () => {
-            const builder = new TaskBuilder().status(Status.DONE).originalStatusCharacter('/');
-            const task = builder.build();
-            checkStatuses(task, Status.DONE, '/', '- [/] my description');
-        });
-    });
 });
diff --git a/tests/TestingTools/TaskBuilder.ts b/tests/TestingTools/TaskBuilder.ts
index ea5cef5f..2de2a6a1 100644
--- a/tests/TestingTools/TaskBuilder.ts
+++ b/tests/TestingTools/TaskBuilder.ts
@@ -27,7 +27,6 @@ export class TaskBuilder {
     private _sectionStart: number = 0;
     private _sectionIndex: number = 0;
 
-    private _originalStatusCharacter: string = ' ';
     private _precedingHeader: string | null = null;
     private _tags: string[] = [];
     private _priority: Priority = Priority.None;
@@ -67,7 +66,6 @@ export class TaskBuilder {
             listMarker: this._listMarker,
             sectionStart: this._sectionStart,
             sectionIndex: this._sectionIndex,
-            originalStatusCharacter: this._originalStatusCharacter,
             precedingHeader: this._precedingHeader,
             priority: this._priority,
             startDate: this._startDate,
@@ -85,12 +83,10 @@ export class TaskBuilder {
     /**
      * Set the status.
      *
-     * This also sets {@link originalStatusCharacter}
      * @param status
      */
     public status(status: Status): TaskBuilder {
         this._status = status;
-        this._originalStatusCharacter = status === Status.TODO ? ' ' : 'x';
         return this;
     }
 
@@ -134,18 +130,6 @@ export class TaskBuilder {
         return this;
     }
 
-    /**
-     * Set the originalStatusCharacter.
-     *
-     * This does NOT set {@link status}, so call that method before calling this one,
-     * if you want to use a character other than ' ' or 'x'.
-     * @param originalStatusCharacter
-     */
-    public originalStatusCharacter(originalStatusCharacter: string): TaskBuilder {
-        this._originalStatusCharacter = originalStatusCharacter;
-        return this;
-    }
-
     public precedingHeader(precedingHeader: string | null): TaskBuilder {
         this._precedingHeader = precedingHeader;
         return this;