Skip to content

Commit

Permalink
Merge pull request #2719 from obsidian-tasks-group/fix-explain-filter…
Browse files Browse the repository at this point in the history
…s-in-booleans

fix: 'explain' omitted the instruction line in some Boolean expressions
  • Loading branch information
claremacrae committed Mar 19, 2024
2 parents 0328f49 + 896ec97 commit 4a1a461
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 14 deletions.
3 changes: 2 additions & 1 deletion docs/Queries/Explaining Queries.md
Expand Up @@ -114,7 +114,8 @@ Explanation of this Tasks code block query:
(due before tomorrow) AND (is recurring) =>
AND (All of):
due date is before 2022-10-22 (Saturday 22nd October 2022)
due before tomorrow =>
due date is before 2022-10-22 (Saturday 22nd October 2022)
is recurring
No grouping instructions supplied.
Expand Down
10 changes: 9 additions & 1 deletion src/Query/Filter/BooleanField.ts
Expand Up @@ -200,7 +200,15 @@ export class BooleanField extends Field {
throw Error('null token value'); // This should not happen
}
const filter = this.subFields[token.value.trim()];
explanationStack.push(filter.explanation);
const explanation = this.simulateExplainFilter(filter);
explanationStack.push(explanation);
}

private simulateExplainFilter(filter: Filter) {
// In our caller, the explanationStack keeps the explanations but not the instruction lines.
// So to replicate the logic in Filter.explainFilterIndented(), we may need to add the
// instruction to the explanation.
return filter.simulateExplainFilter();
}

private explainOperator(token: Token, explanationStack: Explanation[]) {
Expand Down
11 changes: 10 additions & 1 deletion src/Query/Filter/Filter.ts
@@ -1,5 +1,5 @@
import type { Task } from '../../Task/Task';
import type { Explanation } from '../Explain/Explanation';
import { Explanation } from '../Explain/Explanation';
import type { SearchInfo } from '../SearchInfo';
import { Statement } from '../Statement';

Expand Down Expand Up @@ -64,4 +64,13 @@ export class Filter {
return `${explainedStatement} =>\n${explanation.asString(indent + ' ')}\n`;
}
}

public simulateExplainFilter() {
const needToShowInstruction = this.instruction !== this.explanation.asString();
if (needToShowInstruction) {
return new Explanation(this.instruction + ' =>', [this.explanation]);
} else {
return this.explanation;
}
}
}
Expand Up @@ -4,7 +4,8 @@ Explanation of this Tasks code block query:

(due before tomorrow) AND (is recurring) =>
AND (All of):
due date is before 2022-10-22 (Saturday 22nd October 2022)
due before tomorrow =>
due date is before 2022-10-22 (Saturday 22nd October 2022)
is recurring

No grouping instructions supplied.
Expand Down
Expand Up @@ -219,7 +219,8 @@ Result:
(description includes d1) AND (priority medium) =>
AND (All of):
description includes d1
priority is medium
priority medium =>
priority is medium


--------------------------------------------------------
Expand Down Expand Up @@ -247,7 +248,8 @@ Result:
AND (All of):
description includes d1
NOT:
priority is medium
priority medium =>
priority is medium


--------------------------------------------------------
Expand All @@ -274,7 +276,8 @@ Result:
OR (At least one of):
description includes d1
description includes d2
priority is medium
priority medium =>
priority is medium


--------------------------------------------------------
Expand All @@ -287,7 +290,8 @@ Result:
(description includes d1) OR (priority medium) =>
OR (At least one of):
description includes d1
priority is medium
priority medium =>
priority is medium


--------------------------------------------------------
Expand Down Expand Up @@ -315,7 +319,8 @@ Result:
OR (At least one of):
description includes d1
NOT:
priority is medium
priority medium =>
priority is medium


--------------------------------------------------------
Expand All @@ -341,7 +346,8 @@ Result:
(description includes d1) XOR (priority medium) =>
XOR (Exactly one of):
description includes d1
priority is medium
priority medium =>
priority is medium


--------------------------------------------------------
Expand Down
7 changes: 3 additions & 4 deletions tests/Query/Filter/BooleanField.test.ts
Expand Up @@ -220,7 +220,7 @@ describe('boolean query - explain', () => {
return new Explainer(indentation).explainFilters(query1);
}

it.failing('should make multi-line explanations consistent in and out of Boolean filter - issue #2707', () => {
it('should make multi-line explanations consistent in and out of Boolean filter - issue #2707', () => {
// See https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2707
const filter = 'description regex matches /buy/i';

Expand All @@ -231,7 +231,6 @@ describe('boolean query - explain', () => {
const filter2Explanation = explainFilters(0, filter2);

// Ensure that the full standalone explanation of filter1 is present in its explanation in the Boolean query:
// TODO: Make this test pass:
expect(filter2Explanation).toContain(filter1Explanation);

// These inline snapshots are provided for ease of visualising the behaviour:
Expand All @@ -241,12 +240,12 @@ describe('boolean query - explain', () => {
"
`);

// TODO: Include the original regex source line in this string:
expect(filter2Explanation).toMatchInlineSnapshot(`
"( description regex matches /buy/i ) AND ( path includes {{query.file.path}} ) =>
( description regex matches /buy/i ) AND ( path includes some/sample/note.md ) =>
AND (All of):
using regex: 'buy' with flag 'i'
description regex matches /buy/i =>
using regex: 'buy' with flag 'i'
path includes some/sample/note.md
"
`);
Expand Down

0 comments on commit 4a1a461

Please sign in to comment.