Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d462c6b
refactor: . Add getter for Grouper.instruction
claremacrae Dec 4, 2024
fbb58b4
test: - Add first test of Grouper
claremacrae Dec 4, 2024
155724c
refactor: - Add Statement field to Grouper
claremacrae Dec 4, 2024
9b93deb
feat: - Explanations of 'group by' instructions now show continuation…
claremacrae Dec 4, 2024
469188d
test: - Add a non-trivial test for Grouper.instruction
claremacrae Dec 4, 2024
533d983
refactor: - Remove now-redundant Grouper._instruction field
claremacrae Dec 4, 2024
6171be5
refactor: . Make Grouper.setStatement() the only way to modify the st…
claremacrae Dec 4, 2024
889a5bf
refactor: . Reorder statement getter to after functions that initiali…
claremacrae Dec 4, 2024
7b8a5de
refactor: . Add getter for Sorter.instruction
claremacrae Dec 4, 2024
593600e
test: - Add first test of Sorter
claremacrae Dec 4, 2024
62caa2d
refactor: - Add Statement field to Sorter
claremacrae Dec 4, 2024
08d250a
refactor: . Add getter for Sorter.statement
claremacrae Dec 4, 2024
31ef4a5
jsdoc: Add missing parameter to Query.parseGroupBy() docs
claremacrae Dec 4, 2024
73cc522
feat: - Explanations of 'sort by' instructions now show continuation …
claremacrae Dec 4, 2024
8f9a253
refactor: . Fix misleading variable name in Explainer.explainSorters()
claremacrae Dec 4, 2024
6c36030
test: - Add a non-trivial test for Sorter.instruction
claremacrae Dec 4, 2024
bd20841
refactor: - Remove now-redundant Sorter._instruction field
claremacrae Dec 4, 2024
290b02b
refactor: . Reorder statement getter to after functions that initiali…
claremacrae Dec 4, 2024
f68ce86
refactor: - Make Sorter.instruction consistent with Grouper
claremacrae Dec 4, 2024
9f25b94
test: - Test group.statement
claremacrae Dec 4, 2024
615fe16
refactor: - Make white-space between sort and group explanations same…
claremacrae Dec 5, 2024
34ed20c
test: - Add multi-line examples to the 'explain everything' tests
claremacrae Dec 5, 2024
3ce58c8
comment: Note that stray characters after sort-by instructions are no…
claremacrae Dec 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
});
});
Loading