Repository: obsidian-tasks-group/obsidian-tasks
Pull request #804 (2022-06-17 16:28:52)

refactor: Add StatusField class, for handling done/not done filters
## Description
<!--- Describe your changes in detail -->

1. Add `StatusField` class
2. Rework Query to use `StatusField` for `done` and `not done` filters
3. Add `StatusField.test.ts` with simple tests for this new class 
4. Two new methods added to Field, to encapsulate the checking whether lines match regexes, because the `StatusField` class does not support any regex-based instructions, so I needed a way for `StatusField` to indicate it didn't have any regex instructions by returning null. 

## Motivation and Context

More progress on #664 

## How has this been tested?

By running the existing tests, and adding some new ones.

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->

## Screenshots (if appropriate):

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Refactor (non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

## 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! -->
- [ ] 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 accordingly.
- [x] My change has adequate Unit Test coverage.

By creating a Pull Request you agree to our [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md). For further guidance on contributing please see [contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md)



Patch:
diff --git a/src/Query.ts b/src/Query.ts
index bbe505cf..ccd7ea25 100644
--- a/src/Query.ts
+++ b/src/Query.ts
@@ -3,7 +3,7 @@ import type { TaskGroups } from './Query/TaskGroups';
 
 import { LayoutOptions } from './LayoutOptions';
 import { Sort } from './Sort';
-import { Status, Task } from './Task';
+import type { Task } from './Task';
 import type { IQuery } from './IQuery';
 
 import type { Field } from './Query/Filter/Field';
@@ -16,6 +16,7 @@ import { PriorityField } from './Query/Filter/PriorityField';
 import { ScheduledDateField } from './Query/Filter/ScheduledDateField';
 import { StartDateField } from './Query/Filter/StartDateField';
 import { HappensDateField } from './Query/Filter/HappensDateField';
+import { StatusField } from './Query/Filter/StatusField';
 import { TagsField } from './Query/Filter/TagsField';
 
 export type SortingProperty =
@@ -58,9 +59,6 @@ export class Query implements IQuery {
     private _sorting: Sorting[] = [];
     private _grouping: Grouping[] = [];
 
-    private readonly doneString = 'done';
-    private readonly notDoneString = 'not done';
-
     // If a tag is specified the user can also add a number to specify
     // which one to sort by if there is more than one.
     private readonly sortByRegexp =
@@ -90,16 +88,6 @@ export class Query implements IQuery {
                 switch (true) {
                     case line === '':
                         break;
-                    case line === this.doneString:
-                        this._filters.push(
-                            (task) => task.status === Status.Done,
-                        );
-                        break;
-                    case line === this.notDoneString:
-                        this._filters.push(
-                            (task) => task.status !== Status.Done,
-                        );
-                        break;
                     case line === this.recurringString:
                         this._filters.push((task) => task.recurrence !== null);
                         break;
@@ -112,6 +100,8 @@ export class Query implements IQuery {
                     case this.shortModeRegexp.test(line):
                         this._layoutOptions.shortMode = true;
                         break;
+                    case this.parseFilter(line, new StatusField()):
+                        break;
                     case this.parseFilter(line, new PriorityField()):
                         break;
                     case this.parseFilter(line, new HappensDateField()):
diff --git a/src/Query/Filter/DateField.ts b/src/Query/Filter/DateField.ts
index 0991f713..e4f37ee7 100644
--- a/src/Query/Filter/DateField.ts
+++ b/src/Query/Filter/DateField.ts
@@ -36,7 +36,7 @@ export abstract class DateField extends Field {
             return result;
         }
 
-        const match = line.match(this.filterRegexp());
+        const match = Field.getMatch(this.filterRegexp(), line);
         if (match !== null) {
             const filterDate = DateParser.parseDate(match[2]);
             if (!filterDate.isValid()) {
diff --git a/src/Query/Filter/Field.ts b/src/Query/Filter/Field.ts
index 17d76974..03f9415e 100644
--- a/src/Query/Filter/Field.ts
+++ b/src/Query/Filter/Field.ts
@@ -22,7 +22,7 @@ export abstract class Field {
      * @param line - A line from a ```tasks``` block.
      */
     public canCreateFilterForLine(line: string): boolean {
-        return this.filterRegexp().test(line);
+        return Field.lineMatchesFilter(this.filterRegexp(), line);
     }
 
     /**
@@ -34,12 +34,49 @@ export abstract class Field {
         line: string,
     ): FilterOrErrorMessage;
 
+    /**
+     * Does the given line match the given filter?
+     * @param filter - A RegExp regular expression, that specifies one query instruction.
+     *                 Or null, if the field does not support regexp-based filtering.
+     * @param line - A line from a tasks code block query.
+     * @protected
+     */
+    protected static lineMatchesFilter(
+        filter: RegExp | null,
+        line: string,
+    ): boolean {
+        if (filter) {
+            return filter.test(line);
+        } else {
+            return false;
+        }
+    }
+
+    /**
+     * Return the match for the given filter, or null if it does not match
+     * @param filterRegexp - A RegExp regular expression, that specifies one query instruction.
+     *                       Or null, if the field does not support regexp-based filtering.
+     * @param line - A line from a tasks code block query.
+     * @protected
+     */
+    protected static getMatch(
+        filterRegexp: RegExp | null,
+        line: string,
+    ): RegExpMatchArray | null {
+        if (filterRegexp) {
+            return line.match(filterRegexp);
+        } else {
+            return null;
+        }
+    }
+
     /**
      * Return a regular expression that will match a correctly-formed
      * instruction line for filtering Tasks by inspecting the value of this field.
+     * Or null, if this field does not have a regex-based instruction.
      * @protected
      */
-    protected abstract filterRegexp(): RegExp;
+    protected abstract filterRegexp(): RegExp | null;
 
     /**
      * Return the name of this field, to be used in error messages.
diff --git a/src/Query/Filter/HappensDateField.ts b/src/Query/Filter/HappensDateField.ts
index 6a3924ff..264432d9 100644
--- a/src/Query/Filter/HappensDateField.ts
+++ b/src/Query/Filter/HappensDateField.ts
@@ -40,7 +40,7 @@ export class HappensDateField extends Field {
             return result;
         }
 
-        const happensMatch = line.match(this.filterRegexp());
+        const happensMatch = Field.getMatch(this.filterRegexp(), line);
         if (happensMatch !== null) {
             const filterDate = DateParser.parseDate(happensMatch[2]);
             if (!filterDate.isValid()) {
diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts
index 3037b066..b584cd98 100644
--- a/src/Query/Filter/PriorityField.ts
+++ b/src/Query/Filter/PriorityField.ts
@@ -8,7 +8,7 @@ export class PriorityField extends Field {
 
     createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
         const result = new FilterOrErrorMessage();
-        const priorityMatch = line.match(this.filterRegexp());
+        const priorityMatch = Field.getMatch(this.filterRegexp(), line);
         if (priorityMatch !== null) {
             const filterPriorityString = priorityMatch[3];
             let filterPriority: Priority | null = null;
diff --git a/src/Query/Filter/StatusField.ts b/src/Query/Filter/StatusField.ts
new file mode 100644
index 00000000..9ce30d54
--- /dev/null
+++ b/src/Query/Filter/StatusField.ts
@@ -0,0 +1,41 @@
+import { Status } from '../../Task';
+import { Field } from './Field';
+import { FilterOrErrorMessage } from './Filter';
+
+export class StatusField extends Field {
+    private readonly instructionForFieldPresence = 'done';
+    private readonly instructionForFieldAbsence = 'not done';
+
+    public canCreateFilterForLine(line: string): boolean {
+        return (
+            line === this.instructionForFieldAbsence ||
+            line === this.instructionForFieldPresence
+        );
+    }
+
+    public createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
+        const result = new FilterOrErrorMessage();
+
+        if (line === this.instructionForFieldPresence) {
+            result.filter = (task) => task.status === Status.Done;
+            return result;
+        }
+
+        if (line === this.instructionForFieldAbsence) {
+            result.filter = (task) => task.status !== Status.Done;
+            return result;
+        }
+
+        result.error = `do not understand filter: ${line}`;
+
+        return result;
+    }
+
+    protected fieldName(): string {
+        return 'status';
+    }
+
+    protected filterRegexp(): RegExp | null {
+        return null;
+    }
+}
diff --git a/src/Query/Filter/TagsField.ts b/src/Query/Filter/TagsField.ts
index 23dd26b2..87243365 100644
--- a/src/Query/Filter/TagsField.ts
+++ b/src/Query/Filter/TagsField.ts
@@ -14,7 +14,7 @@ export class TagsField extends Field {
 
     public createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
         const result = new FilterOrErrorMessage();
-        const tagMatch = line.match(this.filterRegexp());
+        const tagMatch = Field.getMatch(this.filterRegexp(), line);
         if (tagMatch !== null) {
             const filterMethod = tagMatch[2];
 
diff --git a/src/Query/Filter/TextField.ts b/src/Query/Filter/TextField.ts
index b12925b5..497f2725 100644
--- a/src/Query/Filter/TextField.ts
+++ b/src/Query/Filter/TextField.ts
@@ -10,7 +10,7 @@ import { FilterOrErrorMessage } from './Filter';
 export abstract class TextField extends Field {
     public createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
         const result = new FilterOrErrorMessage();
-        const match = line.match(this.filterRegexp());
+        const match = Field.getMatch(this.filterRegexp(), line);
         if (match !== null) {
             const filterMethod = match[1];
             if (filterMethod === 'includes') {
@@ -43,7 +43,7 @@ export abstract class TextField extends Field {
             .includes(needle.toLocaleLowerCase());
     }
 
-    protected abstract filterRegexp(): RegExp;
+    protected abstract filterRegexp(): RegExp | null;
 
     protected abstract fieldName(): string;
 


Test patch:
diff --git a/tests/Query.test.ts b/tests/Query.test.ts
index 9a511412..d5e37581 100644
--- a/tests/Query.test.ts
+++ b/tests/Query.test.ts
@@ -56,7 +56,6 @@ describe('Query parsing', () => {
             'no scheduled date',
             'no start date',
             'not done',
-            'not done',
             'path does not include some/path',
             'path includes some/path',
             'priority is above none',
diff --git a/tests/Query/Filter/StatusField.test.ts b/tests/Query/Filter/StatusField.test.ts
new file mode 100644
index 00000000..a4af3b4a
--- /dev/null
+++ b/tests/Query/Filter/StatusField.test.ts
@@ -0,0 +1,34 @@
+import { StatusField } from '../../../src/Query/Filter/StatusField';
+import type { FilterOrErrorMessage } from '../../../src/Query/Filter/Filter';
+import { TaskBuilder } from '../../TestingTools/TaskBuilder';
+import { testFilter } from '../../TestingTools/FilterTestHelpers';
+import { Status } from '../../../src/Task';
+
+function testStatusFilter(
+    filter: FilterOrErrorMessage,
+    status: Status,
+    expected: boolean,
+) {
+    const builder = new TaskBuilder();
+    testFilter(filter, builder.status(status), expected);
+}
+
+describe('status', () => {
+    it('done', () => {
+        // Arrange
+        const filter = new StatusField().createFilterOrErrorMessage('done');
+
+        // Assert
+        testStatusFilter(filter, Status.Todo, false);
+        testStatusFilter(filter, Status.Done, true);
+    });
+
+    it('not done', () => {
+        // Arrange
+        const filter = new StatusField().createFilterOrErrorMessage('not done');
+
+        // Assert
+        testStatusFilter(filter, Status.Todo, true);
+        testStatusFilter(filter, Status.Done, false);
+    });
+});