Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2759 (2024-04-09 09:58:09)

fix: allow filters with ( ) and " delimiters in Boolean filters
# Description

Major fix of Boolean filters, to allow use of `(`, `)` and `"` in filters within Boolean expressions.

*Note: more improvements are coming so I am not updating the documentation yet.*

### Caveat

The one current caveat with this implementation is that it still does not work for filters the **end** with any of the characters `()"`, as they are swallowed up and treated as delimiters.

So for example, this filter:

```
(description includes "maybe")
```

is interpreted as this simplified line - note the stray `"`:

```
(f1")
```

where `f1` is this - note the missing `"`:

```
description includes "maybe
```


### Workarounds

`filter by function` instructions are very likely to end with `)`, due to the expression often ending with a function call.

Available workarounds:

- add a trailing `;` at the end of any `filter by function` that ends in `)` or `"`.
- convert Boolean combinations of multiple `filter by function` to be a single `filter by function` that uses JavaScript `&&`, `||` and `!`.

## Motivation and Context

- Fixes https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1308
- Fixes https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1500

## How has this been tested?

- Automated tests
- Exploratory testing

## Screenshots (if appropriate)

Examples of searches that now work, followed by their `explain` output:

### Example 1 - containing a regular expression with ()

`( description regex matches /(buy|order|voucher|lakeland|purchase|\spresent)/i ) OR ( path includes Home/Shopping )`

`explain` output:

```
  ( description regex matches /(buy|order|voucher|lakeland|purchase|\spresent)/i ) OR ( path includes Home/Shopping ) =>
    OR (At least one of):
      description regex matches /(buy|order|voucher|lakeland|purchase|\spresent)/i =>
        using regex:            '(buy|order|voucher|lakeland|purchase|\spresent)' with flag 'i'
      path includes Home/Shopping
```

### Example 2

`(path includes (some example) OR (path includes )some example()`

`explain` output:

```
  (path includes (some example) OR (path includes )some example() =>
    OR (At least one of):
      path includes (some example
      path includes )some example(
```

### Example 3 - complex combination of custom filters

**Important:** Note that the following only works because every filter expression that ends with `)` is followed by a `;`, to prevent the `)` being wrongly treated as part of the Boolean logic.

`( filter by function ! 'NON_TASK,CANCELLED'.includes(task.status.type); ) OR ( filter by function const date = task.due.moment; return date ? !date.isValid() : false; ) OR ( filter by function task.due.moment?.isSameOrBefore(moment(), 'day') || false; ) OR ( filter by function task.urgency.toFixed(2) === 1.95.toFixed(2); ) OR ( filter by function (!task.isRecurring) && task.originalMarkdown.includes('🔁'); ) OR ( filter by function task.file.path.toLocaleLowerCase() === 'TASKS RELEASES/4.1.0 RELEASE.MD'.toLocaleLowerCase(); ) OR ( filter by function const taskDate = task.due.moment; const now = moment(); return taskDate?.isSame(now, 'day') || ( !taskDate && task.heading?.includes(now.format('YYYY-MM-DD')) ) || false; ) OR ( filter by function const wanted = '#context/home'; return task.heading?.includes(wanted) || task.tags.find( (tag) => tag === wanted ) && true || false; )`

`explain` output:

```
  ( filter by function ! 'NON_TASK,CANCELLED'.includes(task.status.type); ) OR ( filter by function const date = task.due.moment; return date ? !date.isValid() : false; ) OR ( filter by function task.due.moment?.isSameOrBefore(moment(), 'day') || false; ) OR ( filter by function task.urgency.toFixed(2) === 1.95.toFixed(2); ) OR ( filter by function (!task.isRecurring) && task.originalMarkdown.includes('🔁'); ) OR ( filter by function task.file.path.toLocaleLowerCase() === 'TASKS RELEASES/4.1.0 RELEASE.MD'.toLocaleLowerCase(); ) OR ( filter by function const taskDate = task.due.moment; const now = moment(); return taskDate?.isSame(now, 'day') || ( !taskDate && task.heading?.includes(now.format('YYYY-MM-DD')) ) || false; ) OR ( filter by function const wanted = '#context/home'; return task.heading?.includes(wanted) || task.tags.find( (tag) => tag === wanted ) && true || false; ) =>
    OR (At least one of):
      filter by function ! 'NON_TASK,CANCELLED'.includes(task.status.type);
      filter by function const date = task.due.moment; return date ? !date.isValid() : false;
      filter by function task.due.moment?.isSameOrBefore(moment(), 'day') || false;
      filter by function task.urgency.toFixed(2) === 1.95.toFixed(2);
      filter by function (!task.isRecurring) && task.originalMarkdown.includes('🔁');
      filter by function task.file.path.toLocaleLowerCase() === 'TASKS RELEASES/4.1.0 RELEASE.MD'.toLocaleLowerCase();
      filter by function const taskDate = task.due.moment; const now = moment(); return taskDate?.isSame(now, 'day') || ( !taskDate && task.heading?.includes(now.format('YYYY-MM-DD')) ) || false;
      filter by function const wanted = '#context/home'; return task.heading?.includes(wanted) || task.tags.find( (tag) => tag === wanted ) && true || false;
```

## Types of changes

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)
- [ ] **Documentation** (prefix: `docs` - improvements to any documentation content **for users**)

Internal changes:

- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## Checklist

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [x] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://publish.obsidian.md/tasks-contributing/Documentation/About+Documentation) accordingly.
    - *Will be done later*
- [x] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

- [x] My contribution follow this project's [contributing guide](https://publish.obsidian.md/tasks-contributing)
- [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 1cb3c7f4..b648ca7a 100644
--- a/src/Query/Filter/BooleanField.ts
+++ b/src/Query/Filter/BooleanField.ts
@@ -10,6 +10,7 @@ import { Field } from './Field';
 import { FilterOrErrorMessage } from './FilterOrErrorMessage';
 import { Filter } from './Filter';
 import { BooleanDelimiters } from './BooleanDelimiters';
+import { BooleanPreprocessor } from './BooleanPreprocessor';
 
 /**
  * BooleanField is a 'container' field type that parses a high-level filtering query of
@@ -82,15 +83,19 @@ export class BooleanField extends Field {
      * @private
      */
     private parseLineV1(line: string) {
-        const preprocessed = BooleanField.preprocessExpressionV1(line);
+        const parseResult = BooleanPreprocessor.preprocessExpression(line);
+        const simplifiedLine = parseResult.simplifiedLine;
+        const filters = parseResult.filters;
         try {
             // Convert the (preprocessed) line into a postfix logical expression
-            const postfixExpression = boonParse(preprocessed);
+            const postfixExpression: Token[] = boonParse(simplifiedLine);
             // Construct sub-field map, i.e. have subFields include a filter function for every
             // final token in the expression
             for (const token of postfixExpression) {
                 if (token.name === 'IDENTIFIER' && token.value) {
-                    const filter = token.value.trim();
+                    const placeholder = token.value.trim();
+                    const filter = filters[placeholder];
+                    token.value = filter;
                     if (!(filter in this.subFields)) {
                         const parsedField = parseFilter(filter);
                         if (parsedField === null) {


Test patch:
diff --git a/tests/Query/Filter/BooleanField.test.boolean_query_-_exhaustive_tests_explain.approved.txt b/tests/Query/Filter/BooleanField.test.boolean_query_-_exhaustive_tests_explain.approved.txt
index 8db9676b..d6c46bfa 100644
--- a/tests/Query/Filter/BooleanField.test.boolean_query_-_exhaustive_tests_explain.approved.txt
+++ b/tests/Query/Filter/BooleanField.test.boolean_query_-_exhaustive_tests_explain.approved.txt
@@ -415,7 +415,12 @@ Input:
 '( description regex matches /(buy|order|voucher|lakeland|purchase|\spresent)/i ) OR ( path includes Home/Shopping )'
 =>
 Result:
-malformed boolean query -- Invalid token (check the documentation for guidelines)
+  ( description regex matches /(buy|order|voucher|lakeland|purchase|\spresent)/i ) OR ( path includes Home/Shopping ) =>
+    OR (At least one of):
+      description regex matches /(buy|order|voucher|lakeland|purchase|\spresent)/i =>
+        using regex:            '(buy|order|voucher|lakeland|purchase|\spresent)' with flag 'i'
+      path includes Home/Shopping
+
 
 --------------------------------------------------------
 
@@ -447,7 +452,17 @@ Input:
 '( filter by function ! 'NON_TASK,CANCELLED'.includes(task.status.type); ) OR ( filter by function const date = task.due.moment; return date ? !date.isValid() : false; ) OR ( filter by function task.due.moment?.isSameOrBefore(moment(), 'day') || false; ) OR ( filter by function task.urgency.toFixed(2) === 1.95.toFixed(2); ) OR ( filter by function (!task.isRecurring) && task.originalMarkdown.includes('🔁'); ) OR ( filter by function task.file.path.toLocaleLowerCase() === 'TASKS RELEASES/4.1.0 RELEASE.MD'.toLocaleLowerCase(); ) OR ( filter by function const taskDate = task.due.moment; const now = moment(); return taskDate?.isSame(now, 'day') || ( !taskDate && task.heading?.includes(now.format('YYYY-MM-DD')) ) || false; ) OR ( filter by function const wanted = '#context/home'; return task.heading?.includes(wanted) || task.tags.find( (tag) => tag === wanted ) && true || false; )'
 =>
 Result:
-malformed boolean query -- Invalid token (check the documentation for guidelines)
+  ( filter by function ! 'NON_TASK,CANCELLED'.includes(task.status.type); ) OR ( filter by function const date = task.due.moment; return date ? !date.isValid() : false; ) OR ( filter by function task.due.moment?.isSameOrBefore(moment(), 'day') || false; ) OR ( filter by function task.urgency.toFixed(2) === 1.95.toFixed(2); ) OR ( filter by function (!task.isRecurring) && task.originalMarkdown.includes('🔁'); ) OR ( filter by function task.file.path.toLocaleLowerCase() === 'TASKS RELEASES/4.1.0 RELEASE.MD'.toLocaleLowerCase(); ) OR ( filter by function const taskDate = task.due.moment; const now = moment(); return taskDate?.isSame(now, 'day') || ( !taskDate && task.heading?.includes(now.format('YYYY-MM-DD')) ) || false; ) OR ( filter by function const wanted = '#context/home'; return task.heading?.includes(wanted) || task.tags.find( (tag) => tag === wanted ) && true || false; ) =>
+    OR (At least one of):
+      filter by function ! 'NON_TASK,CANCELLED'.includes(task.status.type);
+      filter by function const date = task.due.moment; return date ? !date.isValid() : false;
+      filter by function task.due.moment?.isSameOrBefore(moment(), 'day') || false;
+      filter by function task.urgency.toFixed(2) === 1.95.toFixed(2);
+      filter by function (!task.isRecurring) && task.originalMarkdown.includes('🔁');
+      filter by function task.file.path.toLocaleLowerCase() === 'TASKS RELEASES/4.1.0 RELEASE.MD'.toLocaleLowerCase();
+      filter by function const taskDate = task.due.moment; const now = moment(); return taskDate?.isSame(now, 'day') || ( !taskDate && task.heading?.includes(now.format('YYYY-MM-DD')) ) || false;
+      filter by function const wanted = '#context/home'; return task.heading?.includes(wanted) || task.tags.find( (tag) => tag === wanted ) && true || false;
+
 
 --------------------------------------------------------
 
@@ -832,7 +847,7 @@ Input:
 '(description includes "hello world") OR (description includes "42")'
 =>
 Result:
-malformed boolean query -- Unexpected character: h Expected ) character or separator (check the documentation for guidelines)
+malformed boolean query -- Unexpected character: " (check the documentation for guidelines)
 
 --------------------------------------------------------
 
@@ -2175,7 +2190,11 @@ Input:
 '(path includes (some example) OR (path includes )some example()'
 =>
 Result:
-malformed boolean query -- Invalid token (check the documentation for guidelines)
+  (path includes (some example) OR (path includes )some example() =>
+    OR (At least one of):
+      path includes (some example
+      path includes )some example(
+
 
 --------------------------------------------------------
 
@@ -2193,7 +2212,7 @@ Input:
 '(path includes )some example() OR (path includes (some example))'
 =>
 Result:
-malformed boolean query -- Unexpected character: s. A closing parenthesis should be followed by another closing parenthesis or whitespace (check the documentation for guidelines)
+malformed boolean query -- Invalid token (check the documentation for guidelines)
 
 --------------------------------------------------------
 
diff --git a/tests/Query/Filter/BooleanField.test.ts b/tests/Query/Filter/BooleanField.test.ts
index 2b3a258d..d74d4c42 100644
--- a/tests/Query/Filter/BooleanField.test.ts
+++ b/tests/Query/Filter/BooleanField.test.ts
@@ -163,7 +163,7 @@ describe('boolean query - filter', () => {
                 const filter = createValidFilter('(description includes "hello world") OR (description includes "42")');
                 // TODO Fix this:
                 expect(explanationOrError(filter)).toMatchInlineSnapshot(
-                    '"malformed boolean query -- Unexpected character: h Expected ) character or separator (check the documentation for guidelines)"',
+                    '"malformed boolean query -- Unexpected character: " (check the documentation for guidelines)"',
                 );
             });
         });