Skip to content

Commit

Permalink
Merge pull request #3221 from obsidian-tasks-group/explain-grouping-a…
Browse files Browse the repository at this point in the history
…nd-sorting

feat: 'explain' shows effect of Line Continuations & Placeholders
  • Loading branch information
claremacrae authored Dec 5, 2024
2 parents 7ff004e + 3ce58c8 commit 3700d09
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 19 deletions.
4 changes: 2 additions & 2 deletions src/Query/Explain/Explainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ 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) {
if (query.sorting.length === 0) {
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) {
Expand Down
24 changes: 22 additions & 2 deletions src/Query/Group/Grouper.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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'.
Expand All @@ -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;
}
}
11 changes: 7 additions & 4 deletions src/Query/Query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
24 changes: 22 additions & 2 deletions src/Query/Sort/Sorter.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;

Expand All @@ -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;
}
Expand Down
93 changes: 84 additions & 9 deletions tests/Query/Explain/Explainer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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()
"
`);
Expand All @@ -199,21 +247,39 @@ 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");',
);
});
});

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()
"
`);
});
Expand All @@ -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;',
);
});
});

Expand Down
27 changes: 27 additions & 0 deletions tests/Query/Group/Grouper.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
27 changes: 27 additions & 0 deletions tests/Query/Sort/Sorter.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});

0 comments on commit 3700d09

Please sign in to comment.