Repository: obsidian-tasks-group/obsidian-tasks (ts)

Issue:
CreateOrEditTaskParser.ts looks like it can now lose custom task status information
### Description

I believe that this line probably needs updating to manage the where the user ran the edit command on a task with a custom status:

https://github.com/obsidian-tasks-group/obsidian-tasks/blob/5c5d28850045d38dca8a7fe90b9ab7c175077bdf/src/Commands/CreateOrEditTaskParser.ts#L66

There are unit tests for this code now, so it should be easy to confirm if this is true, and if so, fix it.

**Update:** I have confirmed that this is a bug.

### Steps to reproduce

- Open a vault that has a global filter enabled, for example `#task`
- Paste in this line:

`- [!] Hello world`

- Put the cursor in that line
- Run `Tasks: Create or edit task` command
    - Note: The Status drop-down should show an `!`, but it is empty
- Click Apply

Expected result:

`- [!] #task Hello world`

Actual result:

`- [x] #task Hello world`

### How to Test

_No response_

Pull request description:
fix: with global filter, the modal was losing custom status symbols
<!--- Provide a general summary of your changes in the Title above -->

# Description

- Fix a bug that any custom status was lost if the Edit task modal was used to add a global filter to a task that had a status symbol other than ` ` or `x`.
- Create `StatusRegistry.byIndicatorOrCreate()` and updated existing code to use it, to remove repetition in the bug fix

## Motivation and Context

Fixes #1488

## How has this been tested?

- By adding a failing test
- By testing manually in local Obsidian

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)
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/Commands/CreateOrEditTaskParser.ts b/src/Commands/CreateOrEditTaskParser.ts
index e887b489..c18920fe 100644
--- a/src/Commands/CreateOrEditTaskParser.ts
+++ b/src/Commands/CreateOrEditTaskParser.ts
@@ -1,6 +1,7 @@
 import { Status } from '../Status';
 import { Priority, Task, TaskRegularExpressions } from '../Task';
 import { DateFallback } from '../DateFallback';
+import { StatusRegistry } from '../StatusRegistry';
 
 /**
  * Read any markdown line and treat it as a task, for the purposes of
@@ -63,7 +64,8 @@ export const taskFromLine = ({ line, path }: { line: string; path: string }): Ta
     const indentation: string = nonTaskMatch[1];
     const listMarker = nonTaskMatch[2] ?? '-';
     const statusString: string = nonTaskMatch[4] ?? ' ';
-    const status = statusString === ' ' ? Status.TODO : Status.DONE;
+    const status = StatusRegistry.getInstance().byIndicatorOrCreate(statusString);
+
     let description: string = nonTaskMatch[5];
 
     const blockLinkMatch = line.match(TaskRegularExpressions.blockLinkRegex);
diff --git a/src/StatusRegistry.ts b/src/StatusRegistry.ts
index 032ce119..373c08e5 100644
--- a/src/StatusRegistry.ts
+++ b/src/StatusRegistry.ts
@@ -80,6 +80,9 @@ export class StatusRegistry {
     /**
      * Returns the registered status by the indicator between the
      * square braces in the markdown task.
+     * Returns an EMPTY status if indicator is unknown.
+     *
+     * @see byIndicatorOrCreate
      *
      * @param {string} indicator
      * @return {*}  {Status}
@@ -93,6 +96,27 @@ export class StatusRegistry {
         return Status.EMPTY;
     }
 
+    /**
+     * Returns the registered status by the indicator between the
+     * square braces in the markdown task.
+     *
+     * Creates a usable new Status with this given indicator if indicator is unknown.
+     * Note: An unknown indicator is not added to the registry.
+     *
+     * @see hasIndicator
+     *
+     * @param {string} indicator
+     * @return {*}  {Status}
+     * @memberof StatusRegistry
+     */
+    public byIndicatorOrCreate(indicator: string): Status {
+        if (this.hasIndicator(indicator)) {
+            return this.getIndicator(indicator);
+        }
+
+        return Status.createUnknownStatus(indicator);
+    }
+
     /**
      * Returns the registered status by the name assigned by the user.
      *
diff --git a/src/Task.ts b/src/Task.ts
index aae2353c..e58aad8c 100644
--- a/src/Task.ts
+++ b/src/Task.ts
@@ -264,10 +264,7 @@ export class Task {
 
         // Get the status of the task.
         const statusString = regexMatch[3];
-        let status = StatusRegistry.getInstance().byIndicator(statusString);
-        if (status === Status.EMPTY) {
-            status = Status.createUnknownStatus(statusString);
-        }
+        const status = StatusRegistry.getInstance().byIndicatorOrCreate(statusString);
 
         // Match for block link and remove if found. Always expected to be
         // at the end of the line.


Test patch:
diff --git a/tests/Commands/CreateOrEditTaskParser.test.ts b/tests/Commands/CreateOrEditTaskParser.test.ts
index 1f7fa638..0ccc2364 100644
--- a/tests/Commands/CreateOrEditTaskParser.test.ts
+++ b/tests/Commands/CreateOrEditTaskParser.test.ts
@@ -47,6 +47,11 @@ describe('CreateOrEditTaskParser - testing edited task if line is saved unchange
             '- [ ] Some existing test with ^block-link',
             '',
         ],
+        [
+            '- [!] Not a task as no global filter - unknown status symbol', // Ensure unknown status symbol is retained in non-tasks
+            '- [!] Not a task as no global filter - unknown status symbol', // The global filter doesn't get added until the Modal rewrites the line
+            '#task',
+        ],
     ])(
         'line loaded into "Create or edit task" command: "%s"',
         (line: string, expectedResult: string, globalFilter: string) => {
diff --git a/tests/StatusRegistry.test.ts b/tests/StatusRegistry.test.ts
index e33b2b35..c4963182 100644
--- a/tests/StatusRegistry.test.ts
+++ b/tests/StatusRegistry.test.ts
@@ -45,6 +45,30 @@ describe('StatusRegistry', () => {
         expect(statusRegistry.byIndicator('?').indicator).toEqual(StatusConfiguration.makeEmpty().indicator);
     });
 
+    it('should return empty status for lookup by unknown indicator with byIndicator()', () => {
+        // Arrange
+        const statusRegistry = new StatusRegistry();
+
+        // Act
+        const result = statusRegistry.byIndicator('?');
+
+        // Assert
+        expect(result).toEqual(Status.EMPTY);
+    });
+
+    it('should return Unknown status for lookup by unknown indicator with byIndicatorOrCreate()', () => {
+        // Arrange
+        const statusRegistry = new StatusRegistry();
+
+        // Act
+        const result = statusRegistry.byIndicatorOrCreate('?');
+
+        // Assert
+        expect(result.indicator).toEqual('?');
+        expect(result.name).toEqual('Unknown');
+        expect(result.nextStatusIndicator).toEqual('x');
+    });
+
     it('should allow lookup of next status for a status', () => {
         // Arrange
         const statusRegistry = new StatusRegistry();