Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1225 (2022-10-12 12:49:21)

refactor: Add instruction field to Filter & FilterOrErrorMessage
<!--- Provide a general summary of your changes in the Title above -->

# Description

I've called this a refactoring, as it does not yet change the externally visible behaviour.

`Filter` & `FilterOrErrorMessage` now have a field called `instruction` which is the line supplied by the user.

## Motivation and Context

This will be useful later for providing an explain facility, showing how the user's filters get converted to actual searches.

## How has this been tested?

Running existing tests.

## 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/Query/Filter/BooleanField.ts b/src/Query/Filter/BooleanField.ts
index 63409f6c..222705e5 100644
--- a/src/Query/Filter/BooleanField.ts
+++ b/src/Query/Filter/BooleanField.ts
@@ -50,7 +50,7 @@ export class BooleanField extends Field {
      * 4. Returning a final function filter, which for each Task can run the complete query.
      */
     private parseLine(line: string): FilterOrErrorMessage {
-        const result = new FilterOrErrorMessage();
+        const result = new FilterOrErrorMessage(line);
         if (line.length === 0) {
             result.error = 'empty line';
             return result;
diff --git a/src/Query/Filter/DateField.ts b/src/Query/Filter/DateField.ts
index 92f1f346..3caac8b8 100644
--- a/src/Query/Filter/DateField.ts
+++ b/src/Query/Filter/DateField.ts
@@ -24,7 +24,7 @@ export abstract class DateField extends Field {
     }
 
     public createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
-        const result = new FilterOrErrorMessage();
+        const result = new FilterOrErrorMessage(line);
 
         if (line === this.instructionForFieldPresence) {
             result.filterFunction = (task: Task) => this.date(task) !== null;
diff --git a/src/Query/Filter/Filter.ts b/src/Query/Filter/Filter.ts
index d37359e5..62be314c 100644
--- a/src/Query/Filter/Filter.ts
+++ b/src/Query/Filter/Filter.ts
@@ -11,25 +11,33 @@ export type FilterFunction = (task: Task) => boolean;
  *
  * Currently it provides access to:
  *
+ * - The original {@link instruction}
  * - The {@link filterFunction} - a {@link FilterFunction} which tests whether a task matches the filter
  *
- * Later, the plan is to add storage of the user's instruction, and a human-readable explanation of the filter.
+ * Later, the plan is to add a human-readable explanation of the filter.
  */
 export class Filter {
+    readonly instruction: string;
     public filterFunction: FilterFunction;
 
-    public constructor(filterFunction: FilterFunction) {
+    public constructor(instruction: string, filterFunction: FilterFunction) {
+        this.instruction = instruction;
         this.filterFunction = filterFunction;
     }
 }
 
 /**
  * A class which stores one of:
- * - A Filter
- * - An error message
+ * - The original instruction string - a line from a tasks code block
+ * - An optional {@link Filter}
+ * - An optional error message
  *
- * This is really currently a convenience for returning date from
- * Field.createFilterOrErrorMessage() and derived classes.
+ * This is really currently a convenience for returning data from
+ * {@link Field.createFilterOrErrorMessage()} and derived classes.
+ *
+ * By the time the code has finished with parsing the line, typically the
+ * contained {@link Filter} will be saved, for later use in searching for Tasks
+ * that match the user's filter instruction.
  *
  * Later, it may gain helper functions for constructing parser error messages,
  * as currently these are created by some rather repetitious code, and also
@@ -37,9 +45,14 @@ export class Filter {
  * problem line, and perhaps listing allowed options).
  */
 export class FilterOrErrorMessage {
+    readonly instruction: string;
     private _filter: Filter | undefined;
     error: string | undefined;
 
+    constructor(instruction: string) {
+        this.instruction = instruction;
+    }
+
     public get filter(): Filter | undefined {
         return this._filter;
     }
@@ -54,7 +67,7 @@ export class FilterOrErrorMessage {
 
     set filterFunction(value: FilterFunction | undefined) {
         if (value) {
-            this._filter = new Filter(value);
+            this._filter = new Filter(this.instruction, value);
         } else {
             this._filter = undefined;
         }
@@ -62,20 +75,22 @@ export class FilterOrErrorMessage {
 
     /**
      * Construct a FilterOrErrorMessage with the filter.
+     * @param instruction
      * @param filter
      */
-    public static fromFilter(filter: FilterFunction): FilterOrErrorMessage {
-        const result = new FilterOrErrorMessage();
+    public static fromFilter(instruction: string, filter: FilterFunction): FilterOrErrorMessage {
+        const result = new FilterOrErrorMessage(instruction);
         result.filterFunction = filter;
         return result;
     }
 
     /**
      * Construct a FilterOrErrorMessage with the given error message.
+     * @param instruction
      * @param errorMessage
      */
-    public static fromError(errorMessage: string): FilterOrErrorMessage {
-        const result = new FilterOrErrorMessage();
+    public static fromError(instruction: string, errorMessage: string): FilterOrErrorMessage {
+        const result = new FilterOrErrorMessage(instruction);
         result.error = errorMessage;
         return result;
     }
diff --git a/src/Query/Filter/FilterInstruction.ts b/src/Query/Filter/FilterInstruction.ts
index 61acf9ca..b4fd0c9b 100644
--- a/src/Query/Filter/FilterInstruction.ts
+++ b/src/Query/Filter/FilterInstruction.ts
@@ -30,7 +30,7 @@ export class FilterInstruction {
     }
 
     public createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
-        const result = new FilterOrErrorMessage();
+        const result = new FilterOrErrorMessage(line);
 
         if (line === this._instruction) {
             result.filterFunction = this._filter;
diff --git a/src/Query/Filter/FilterInstructions.ts b/src/Query/Filter/FilterInstructions.ts
index 0bb82c8b..c7bfceca 100644
--- a/src/Query/Filter/FilterInstructions.ts
+++ b/src/Query/Filter/FilterInstructions.ts
@@ -35,7 +35,7 @@ export class FilterInstructions {
             }
         }
 
-        const result = new FilterOrErrorMessage();
+        const result = new FilterOrErrorMessage(line);
         result.error = `do not understand filter: ${line}`;
         return result;
     }
diff --git a/src/Query/Filter/HappensDateField.ts b/src/Query/Filter/HappensDateField.ts
index 2e889c3e..0329853a 100644
--- a/src/Query/Filter/HappensDateField.ts
+++ b/src/Query/Filter/HappensDateField.ts
@@ -25,16 +25,16 @@ export class HappensDateField extends Field {
     }
 
     public createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
-        const result = new FilterOrErrorMessage();
+        const result = new FilterOrErrorMessage(line);
 
         if (line === HappensDateField.instructionForFieldPresence) {
-            const result = new FilterOrErrorMessage();
+            const result = new FilterOrErrorMessage(line);
             result.filterFunction = (task: Task) => this.dates(task).some((date) => date !== null);
             return result;
         }
 
         if (line === HappensDateField.instructionForFieldAbsence) {
-            const result = new FilterOrErrorMessage();
+            const result = new FilterOrErrorMessage(line);
             result.filterFunction = (task: Task) => !this.dates(task).some((date) => date !== null);
             return result;
         }
diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts
index b2965eda..a32ed320 100644
--- a/src/Query/Filter/PriorityField.ts
+++ b/src/Query/Filter/PriorityField.ts
@@ -6,7 +6,7 @@ export class PriorityField extends Field {
     private static readonly priorityRegexp = /^priority (is )?(above|below)? ?(low|none|medium|high)/;
 
     createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
-        const result = new FilterOrErrorMessage();
+        const result = new FilterOrErrorMessage(line);
         const priorityMatch = Field.getMatch(this.filterRegExp(), line);
         if (priorityMatch !== null) {
             const filterPriorityString = priorityMatch[3];
diff --git a/src/Query/Filter/TextField.ts b/src/Query/Filter/TextField.ts
index b46535ae..202eb62e 100644
--- a/src/Query/Filter/TextField.ts
+++ b/src/Query/Filter/TextField.ts
@@ -17,7 +17,7 @@ export abstract class TextField extends Field {
         if (match === null) {
             // If Field.canCreateFilterForLine() has been checked, we should never get
             // in to this block.
-            return FilterOrErrorMessage.fromError(`do not understand query filter (${this.fieldName()})`);
+            return FilterOrErrorMessage.fromError(line, `do not understand query filter (${this.fieldName()})`);
         }
 
         // Construct an IStringMatcher for this filter, or return
@@ -30,6 +30,7 @@ export abstract class TextField extends Field {
             matcher = RegexMatcher.validateAndConstruct(filterValue);
             if (matcher === null) {
                 return FilterOrErrorMessage.fromError(
+                    line,
                     `cannot parse regex (${this.fieldName()}); check your leading and trailing slashes for your query`,
                 );
             }
@@ -38,14 +39,14 @@ export abstract class TextField extends Field {
         if (matcher === null) {
             // It's likely this can now never be reached.
             // Retained for safety, for now.
-            return FilterOrErrorMessage.fromError(`do not understand query filter (${this.fieldName()})`);
+            return FilterOrErrorMessage.fromError(line, `do not understand query filter (${this.fieldName()})`);
         }
 
         // Finally, we can create the Filter, that takes a task
         // and tests if it matches the string filtering rule
         // represented by this object.
         const negate = filterOperator.match(/not/) !== null;
-        return FilterOrErrorMessage.fromFilter(this.getFilter(matcher, negate));
+        return FilterOrErrorMessage.fromFilter(line, this.getFilter(matcher, negate));
     }
 
     public static stringIncludesCaseInsensitive(haystack: string, needle: string): boolean {


Test patch:
diff --git a/tests/Query/Filter/Filter.test.ts b/tests/Query/Filter/Filter.test.ts
index 953eb5dd..71855532 100644
--- a/tests/Query/Filter/Filter.test.ts
+++ b/tests/Query/Filter/Filter.test.ts
@@ -12,7 +12,10 @@ describe('Filter', () => {
         const filterFunction: FilterFunction = (task: Task) => {
             return task.description.length > 20;
         };
-        const filter = new Filter(filterFunction);
+        const line = 'some sample instruction';
+        const filter = new Filter(line, filterFunction);
+
+        expect(filter.instruction).toEqual(line);
         expect(filter.filterFunction).not.toBeUndefined();
     });
 });