Repository: obsidian-tasks-group/obsidian-tasks
Pull request #3221 (2024-12-05 09:58:38)

feat: 'explain' shows effect of Line Continuations & Placeholders
# Types of changes

Changes visible to users:

- [x] **New feature** (prefix: `feat` - non-breaking change which adds functionality)
  - Issue/discussion: <https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2349>

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)

## Description

The explanations of `sort by` and `group by` now reveal any **placeholders** and **continuation lines**, making it easier to understand the underlying query.

Note that there is one change in behaviour, revealed by a probable typo in an old test...

This:

```
sort by priority()
```

Used to be explained as:

```
sort by priority
```

And is now explained as:

```
sort by priority()
```


I was not immediately able to see the cause of the change. But decided to accept it, as it makes the `explain` output better match the underlying text - and also if any users share explanations in GitHub or Discord, the extra characters will at least be visible.


## Motivation and Context

Fix <https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2349>.

## How has this been tested?

- Automated tests
- Exploratory testing

## Screenshots (if appropriate)

<img width="899" alt="image" src="https://github.com/user-attachments/assets/823c803e-74d5-48ef-b72e-7b13f174c1ba">


## Checklist

- [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://publish.obsidian.md/tasks-contributing/Documentation/About+Documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

- [x] My contribution follows 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/Explain/Explainer.ts b/src/Query/Explain/Explainer.ts
index 2c4d3e8f..e8e3cf66 100644
--- a/src/Query/Explain/Explainer.ts
+++ b/src/Query/Explain/Explainer.ts
@@ -61,7 +61,7 @@ export class Explainer {
             return this.indent('No grouping instructions supplied.\n');
         }
 
-        return query.grouping.map((group) => this.indentation + group.instruction).join('\n') + '\n';
+        return query.grouping.map((group) => group.statement.explainStatement(this.indentation)).join('\n\n') + '\n';
     }
 
     public explainSorters(query: Query) {
@@ -69,7 +69,7 @@ export class Explainer {
             return this.indent('No sorting instructions supplied.\n');
         }
 
-        return query.sorting.map((group) => this.indentation + group.instruction).join('\n') + '\n';
+        return query.sorting.map((sort) => sort.statement.explainStatement(this.indentation)).join('\n\n') + '\n';
     }
 
     public explainQueryLimits(query: Query) {
diff --git a/src/Query/Group/Grouper.ts b/src/Query/Group/Grouper.ts
index 89f60479..5793845e 100644
--- a/src/Query/Group/Grouper.ts
+++ b/src/Query/Group/Grouper.ts
@@ -1,5 +1,6 @@
 import type { Task } from '../../Task/Task';
 import type { SearchInfo } from '../SearchInfo';
+import { Statement } from '../Statement';
 
 /**
  * A group-naming function, that takes a Task object and returns zero or more
@@ -25,7 +26,8 @@ export type GrouperFunction = (task: Task, searchInfo: SearchInfo) => string[];
  * @see {@link TaskGroups} for how to use {@link Grouper} objects to group tasks together.
  */
 export class Grouper {
-    public instruction: string;
+    /** _statement may be updated later with {@link setStatement} */
+    private _statement: Statement;
 
     /**
      * The type of grouper, for example 'tags' or 'due'.
@@ -46,9 +48,27 @@ export class Grouper {
     public readonly reverse: boolean;
 
     constructor(instruction: string, property: string, grouper: GrouperFunction, reverse: boolean) {
-        this.instruction = instruction;
+        this._statement = new Statement(instruction, instruction);
         this.property = property;
         this.grouper = grouper;
         this.reverse = reverse;
     }
+
+    /**
+     * Optionally record more detail about the source statement.
+     *
+     * In tests, we only care about the actual instruction being parsed and executed.
+     * However, in {@link Query}, we want the ability to show user more information.
+     */
+    public setStatement(statement: Statement) {
+        this._statement = statement;
+    }
+
+    public get statement(): Statement {
+        return this._statement;
+    }
+
+    public get instruction(): string {
+        return this._statement.anyPlaceholdersExpanded;
+    }
 }
diff --git a/src/Query/Query.ts b/src/Query/Query.ts
index 996b6842..2507573a 100644
--- a/src/Query/Query.ts
+++ b/src/Query/Query.ts
@@ -105,9 +105,9 @@ export class Query implements IQuery {
             case this.limitRegexp.test(line):
                 this.parseLimit(line);
                 break;
-            case this.parseSortBy(line):
+            case this.parseSortBy(line, statement):
                 break;
-            case this.parseGroupBy(line):
+            case this.parseGroupBy(line, statement):
                 break;
             case this.hideOptionsRegexp.test(line):
                 this.parseHideOptions(line);
@@ -352,9 +352,10 @@ ${statement.explainStatement('    ')}
         }
     }
 
-    private parseSortBy(line: string): boolean {
+    private parseSortBy(line: string, statement: Statement): boolean {
         const sortingMaybe = FilterParser.parseSorter(line);
         if (sortingMaybe) {
+            sortingMaybe.setStatement(statement);
             this._sorting.push(sortingMaybe);
             return true;
         }
@@ -366,11 +367,13 @@ ${statement.explainStatement('    ')}
      * classes.
      *
      * @param line
+     * @param statement
      * @private
      */
-    private parseGroupBy(line: string): boolean {
+    private parseGroupBy(line: string, statement: Statement): boolean {
         const groupingMaybe = FilterParser.parseGrouper(line);
         if (groupingMaybe) {
+            groupingMaybe.setStatement(statement);
             this._grouping.push(groupingMaybe);
             return true;
         }
diff --git a/src/Query/Sort/Sorter.ts b/src/Query/Sort/Sorter.ts
index b6a98fea..2797e899 100644
--- a/src/Query/Sort/Sorter.ts
+++ b/src/Query/Sort/Sorter.ts
@@ -1,5 +1,6 @@
 import type { Task } from '../../Task/Task';
 import type { SearchInfo } from '../SearchInfo';
+import { Statement } from '../Statement';
 
 /**
  * A sorting function, that takes two Task objects and returns
@@ -20,7 +21,8 @@ export type Comparator = (a: Task, b: Task, searchInfo: SearchInfo) => number;
  * It stores the comparison function as a {@link Comparator}.
  */
 export class Sorter {
-    public readonly instruction: string;
+    /** _statement may be updated later with {@link setStatement} */
+    private _statement: Statement;
     public readonly property: string;
     public readonly comparator: Comparator;
 
@@ -34,11 +36,29 @@ export class Sorter {
      * @param reverse - whether the sort order should be reversed.
      */
     constructor(instruction: string, property: string, comparator: Comparator, reverse: boolean) {
-        this.instruction = instruction;
+        this._statement = new Statement(instruction, instruction);
         this.property = property;
         this.comparator = Sorter.maybeReverse(reverse, comparator);
     }
 
+    /**
+     * Optionally record more detail about the source statement.
+     *
+     * In tests, we only care about the actual instruction being parsed and executed.
+     * However, in {@link Query}, we want the ability to show user more information.
+     */
+    public setStatement(statement: Statement) {
+        this._statement = statement;
+    }
+
+    public get statement(): Statement {
+        return this._statement;
+    }
+
+    public get instruction(): string {
+        return this._statement.anyPlaceholdersExpanded;
+    }
+
     private static maybeReverse(reverse: boolean, comparator: Comparator) {
         return reverse ? Sorter.makeReversedComparator(comparator) : comparator;
     }


Test patch:
diff --git a/tests/Query/Explain/Explainer.test.ts b/tests/Query/Explain/Explainer.test.ts
index 954e4d21..5e5acf3f 100644
--- a/tests/Query/Explain/Explainer.test.ts
+++ b/tests/Query/Explain/Explainer.test.ts
@@ -46,12 +46,18 @@ describe('explain errors', () => {
 
 describe('explain everything', () => {
     const sampleOfAllInstructionTypes = `
+filter by function \\
+     task.path === '{{query.file.path}}'
 not done
 (has start date) AND (description includes some)
 
+group by function \\
+    task.path.includes('{{query.file.path}}')
 group by priority reverse
 group by happens
 
+sort by function \\
+    task.path.includes('{{query.file.path}}')
 sort by description reverse
 sort by path
 
@@ -65,19 +71,39 @@ limit groups 3
         // Disable sort instructions
         updateSettings({ debugSettings: new DebugSettings(true) });
 
-        const query = new Query(sampleOfAllInstructionTypes);
+        const query = new Query(sampleOfAllInstructionTypes, new TasksFile('sample.md'));
         expect(explainer.explainQuery(query)).toMatchInlineSnapshot(`
-            "not done
+            "filter by function \\
+                 task.path === '{{query.file.path}}'
+             =>
+            filter by function task.path === '{{query.file.path}}' =>
+            filter by function task.path === 'sample.md'
+
+            not done
 
             (has start date) AND (description includes some) =>
               AND (All of):
                 has start date
                 description includes some
 
+            group by function \\
+                task.path.includes('{{query.file.path}}')
+             =>
+            group by function task.path.includes('{{query.file.path}}') =>
+            group by function task.path.includes('sample.md')
+
             group by priority reverse
+
             group by happens
 
+            sort by function \\
+                task.path.includes('{{query.file.path}}')
+             =>
+            sort by function task.path.includes('{{query.file.path}}') =>
+            sort by function task.path.includes('sample.md')
+
             sort by description reverse
+
             sort by path
 
             At most 50 tasks.
@@ -93,20 +119,40 @@ limit groups 3
         // Disable sort instructions
         updateSettings({ debugSettings: new DebugSettings(true) });
 
-        const query = new Query(sampleOfAllInstructionTypes);
+        const query = new Query(sampleOfAllInstructionTypes, new TasksFile('sample.md'));
         const indentedExplainer = new Explainer('  ');
         expect(indentedExplainer.explainQuery(query)).toMatchInlineSnapshot(`
-            "  not done
+            "  filter by function \\
+                   task.path === '{{query.file.path}}'
+               =>
+              filter by function task.path === '{{query.file.path}}' =>
+              filter by function task.path === 'sample.md'
+
+              not done
 
               (has start date) AND (description includes some) =>
                 AND (All of):
                   has start date
                   description includes some
 
+              group by function \\
+                  task.path.includes('{{query.file.path}}')
+               =>
+              group by function task.path.includes('{{query.file.path}}') =>
+              group by function task.path.includes('sample.md')
+
               group by priority reverse
+
               group by happens
 
+              sort by function \\
+                  task.path.includes('{{query.file.path}}')
+               =>
+              sort by function task.path.includes('{{query.file.path}}') =>
+              sort by function task.path.includes('sample.md')
+
               sort by description reverse
+
               sort by path
 
               At most 50 tasks.
@@ -178,7 +224,9 @@ describe('explain groupers', () => {
         const query = new Query(source);
         expect(explainer.explainGroups(query)).toMatchInlineSnapshot(`
             "group by due
+
             group by status.name reverse
+
             group by function task.description.toUpperCase()
             "
         `);
@@ -199,11 +247,25 @@ describe('explain groupers', () => {
         ];
         const query = makeQueryFromContinuationLines(lines);
 
-        // TODO Make this also show the original instruction, including continuations. See #2349.
         expect(explainer.explainGroups(query)).toMatchInlineSnapshot(`
-            "group by function const date = task.due; if (!date.moment) { return "Undated"; } if (date.moment.day() === 0) {  return date.format("[%%][8][%%]dddd"); } return date.format("[%%]d[%%]dddd");
+            "group by function                                   \\
+                const date = task.due;                          \\
+                if (!date.moment) {                             \\
+                    return "Undated";                           \\
+                }                                               \\
+                if (date.moment.day() === 0) {                  \\
+                    {{! Put the Sunday group last: }}           \\
+                    return date.format("[%%][8][%%]dddd");      \\
+                }                                               \\
+                return date.format("[%%]d[%%]dddd");
+             =>
+            group by function const date = task.due; if (!date.moment) { return "Undated"; } if (date.moment.day() === 0) { {{! Put the Sunday group last: }} return date.format("[%%][8][%%]dddd"); } return date.format("[%%]d[%%]dddd"); =>
+            group by function const date = task.due; if (!date.moment) { return "Undated"; } if (date.moment.day() === 0) {  return date.format("[%%][8][%%]dddd"); } return date.format("[%%]d[%%]dddd");
             "
         `);
+        expect(query.grouping[0].instruction).toEqual(
+            'group by function const date = task.due; if (!date.moment) { return "Undated"; } if (date.moment.day() === 0) {  return date.format("[%%][8][%%]dddd"); } return date.format("[%%]d[%%]dddd");',
+        );
     });
 });
 
@@ -211,9 +273,13 @@ describe('explain sorters', () => {
     it('should explain "sort by" options', () => {
         const source = 'sort by due\nsort by priority()';
         const query = new Query(source);
+        // This shows the accidental presence of stray () characters after 'sort by priority'.
+        // They are not *required* in the explanation, but are retained here to help in user support
+        // when I ask users to supply an explanation of their query.
         expect(explainer.explainSorters(query)).toMatchInlineSnapshot(`
             "sort by due
-            sort by priority
+
+            sort by priority()
             "
         `);
     });
@@ -229,11 +295,20 @@ describe('explain sorters', () => {
         ];
         const query = makeQueryFromContinuationLines(lines);
 
-        // TODO Make this also show the original instruction, including continuations. See #2349.
         expect(explainer.explainSorters(query)).toMatchInlineSnapshot(`
-            "sort by function const priorities = [..."🟥🟧🟨🟩🟦"]; for (let i = 0; i < priorities.length; i++) { if (task.description.includes(priorities[i])) return i; } return 999;
+            "sort by function                                                   \\
+                const priorities = [..."🟥🟧🟨🟩🟦"];                        \\
+                for (let i = 0; i < priorities.length; i++) {                  \\
+                    if (task.description.includes(priorities[i])) return i;    \\
+                }                                                              \\
+                return 999;
+             =>
+            sort by function const priorities = [..."🟥🟧🟨🟩🟦"]; for (let i = 0; i < priorities.length; i++) { if (task.description.includes(priorities[i])) return i; } return 999;
             "
         `);
+        expect(query.sorting[0].instruction).toEqual(
+            'sort by function const priorities = [..."🟥🟧🟨🟩🟦"]; for (let i = 0; i < priorities.length; i++) { if (task.description.includes(priorities[i])) return i; } return 999;',
+        );
     });
 });
 
diff --git a/tests/Query/Group/Grouper.test.ts b/tests/Query/Group/Grouper.test.ts
new file mode 100644
index 00000000..aa06f0c5
--- /dev/null
+++ b/tests/Query/Group/Grouper.test.ts
@@ -0,0 +1,27 @@
+import { Grouper, type GrouperFunction } from '../../../src/Query/Group/Grouper';
+import type { Task } from '../../../src/Task/Task';
+import type { SearchInfo } from '../../../src/Query/SearchInfo';
+import { Statement } from '../../../src/Query/Statement';
+
+describe('Grouper', () => {
+    const grouperFunction: GrouperFunction = (task: Task, _searchInfo: SearchInfo) => {
+        return [task.lineNumber.toString()];
+    };
+
+    it('should supply the original instruction', () => {
+        const grouper = new Grouper('group by lineNumber', 'lineNumber', grouperFunction, false);
+
+        expect(grouper.instruction).toBe('group by lineNumber');
+        expect(grouper.statement.rawInstruction).toBe('group by lineNumber');
+    });
+
+    it('should store a Statement object', () => {
+        const instruction = 'group by lineNumber';
+        const statement = new Statement(instruction, instruction);
+        const grouper = new Grouper('group by lineNumber', 'lineNumber', grouperFunction, false);
+
+        grouper.setStatement(statement);
+
+        expect(grouper.statement.rawInstruction).toBe('group by lineNumber');
+    });
+});
diff --git a/tests/Query/Sort/Sorter.test.ts b/tests/Query/Sort/Sorter.test.ts
new file mode 100644
index 00000000..0925998f
--- /dev/null
+++ b/tests/Query/Sort/Sorter.test.ts
@@ -0,0 +1,27 @@
+import { type Comparator, Sorter } from '../../../src/Query/Sort/Sorter';
+import type { Task } from '../../../src/Task/Task';
+import type { SearchInfo } from '../../../src/Query/SearchInfo';
+import { Statement } from '../../../src/Query/Statement';
+
+describe('Sorter', () => {
+    const comparator: Comparator = (a: Task, b: Task, _searchInfo: SearchInfo) => {
+        return a.lineNumber - b.lineNumber;
+    };
+
+    it('should supply the original instruction', () => {
+        const sorter = new Sorter('sort by lineNumber', 'lineNumber', comparator, false);
+
+        expect(sorter.instruction).toBe('sort by lineNumber');
+        expect(sorter.statement.rawInstruction).toBe('sort by lineNumber');
+    });
+
+    it('should store a Statement object', () => {
+        const instruction = 'sort by lineNumber';
+        const statement = new Statement(instruction, instruction);
+        const sorter = new Sorter('sort by lineNumber', 'lineNumber', comparator, false);
+
+        sorter.setStatement(statement);
+
+        expect(sorter.statement.rawInstruction).toBe('sort by lineNumber');
+    });
+});