Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2537 (2023-12-26 17:01:02)

fix: Tidy up blank lines in 'explain' output
<!--- Provide a general summary of your changes in the Title above -->

# Description

Tidy up the blank lines in `explain` output:

- Remove some duplicate blank lines
- Add some missing blank lines

See differences in the test output.

## Motivation and Context

Make the code easier to maintain, before adding some more explanations.

## How has this been tested?

- Automated tests
- Exploratory testing

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)
- [x] **Documentation** (prefix: `docs` - improvements to any documentation content **for users**)

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)

## Checklist

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [x] My change requires a change to the documentation.
- [x] 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

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://publish.obsidian.md/tasks-contributing) and
[Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md).
Put an x in the boxes to confirm you agree.
-->

- [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/docs/Queries/Explaining Queries.md b/docs/Queries/Explaining Queries.md
index 4008407a..b5177777 100644
--- a/docs/Queries/Explaining Queries.md
+++ b/docs/Queries/Explaining Queries.md
@@ -194,7 +194,6 @@ heading includes tasks
 
 No grouping instructions supplied.
 
-
 At most 50 tasks.
 
 Explanation of this Tasks code block query:
diff --git a/src/Query/Explain/Explainer.ts b/src/Query/Explain/Explainer.ts
index cd87c2cd..7c71d49f 100644
--- a/src/Query/Explain/Explainer.ts
+++ b/src/Query/Explain/Explainer.ts
@@ -13,13 +13,21 @@ export class Explainer {
             return this.explainError(query);
         }
 
-        let result = '';
-        result += this.explainFilters(query);
-        result += this.explainGroups(query);
-        result += this.explainQueryLimits(query);
-        result += this.explainDebugSettings();
+        /**
+         * Each function result should:
+         * - either:
+         *     - be empty, if there is no information,
+         * - or:
+         *     - begin with a non-newline,
+         *     - end with a single newline.
+         */
+        const results: string[] = [];
+        results.push(this.explainFilters(query));
+        results.push(this.explainGroups(query));
+        results.push(this.explainQueryLimits(query));
+        results.push(this.explainDebugSettings());
 
-        return result;
+        return results.filter((explanation) => explanation !== '').join('\n');
     }
 
     private explainError(query: Query) {
@@ -30,54 +38,53 @@ export class Explainer {
     }
 
     public explainFilters(query: Query) {
-        let result = '';
         const numberOfFilters = query.filters.length;
         if (numberOfFilters === 0) {
-            result += 'No filters supplied. All tasks will match the query.';
-        } else {
-            for (let i = 0; i < numberOfFilters; i++) {
-                if (i > 0) result += '\n';
-                result += query.filters[i].explainFilterIndented('');
-            }
+            return 'No filters supplied. All tasks will match the query.\n';
         }
-        return result;
+
+        return query.filters
+            .map((filter) => {
+                return filter.explainFilterIndented('');
+            })
+            .join('\n');
     }
 
     public explainGroups(query: Query) {
-        let result = '\n';
         const numberOfGroups = query.grouping.length;
         if (numberOfGroups === 0) {
-            result += 'No grouping instructions supplied.\n';
-        } else {
-            for (let i = 0; i < numberOfGroups; i++) {
-                if (i > 0) result += '\n';
-                result += query.grouping[i].instruction;
-            }
+            return 'No grouping instructions supplied.\n';
+        }
+
+        let result = '';
+        for (let i = 0; i < numberOfGroups; i++) {
+            result += query.grouping[i].instruction + '\n';
         }
         return result;
     }
 
     public explainQueryLimits(query: Query) {
-        let result = '';
-
         function getPluralisedText(limit: number) {
-            let text = `\n\nAt most ${limit} task`;
+            let text = `At most ${limit} task`;
             if (limit !== 1) {
                 text += 's';
             }
             return text;
         }
 
+        const results: string[] = [];
+
         if (query.limit !== undefined) {
-            result += getPluralisedText(query.limit);
-            result += '.\n';
+            const result = getPluralisedText(query.limit) + '.\n';
+            results.push(result);
         }
 
         if (query.taskGroupLimit !== undefined) {
-            result += getPluralisedText(query.taskGroupLimit);
-            result += ' per group (if any "group by" options are supplied).\n';
+            const result =
+                getPluralisedText(query.taskGroupLimit) + ' per group (if any "group by" options are supplied).\n';
+            results.push(result);
         }
-        return result;
+        return results.join('\n');
     }
 
     private explainDebugSettings() {
@@ -85,7 +92,7 @@ export class Explainer {
         const { debugSettings } = getSettings();
         if (debugSettings.ignoreSortInstructions) {
             result +=
-                "\n\nNOTE: All sort instructions, including default sort order, are disabled, due to 'ignoreSortInstructions' setting.";
+                "NOTE: All sort instructions, including default sort order, are disabled, due to 'ignoreSortInstructions' setting.\n";
         }
         return result;
     }


Test patch:
diff --git a/tests/Config/DebugSettings.test.ts b/tests/Config/DebugSettings.test.ts
index 5f71ba9d..ab44086f 100644
--- a/tests/Config/DebugSettings.test.ts
+++ b/tests/Config/DebugSettings.test.ts
@@ -36,10 +36,11 @@ describe('DebugSettings', () => {
 
         expect(query.explainQuery()).toMatchInlineSnapshot(`
             "No filters supplied. All tasks will match the query.
-            No grouping instructions supplied.
 
+            No grouping instructions supplied.
 
-            NOTE: All sort instructions, including default sort order, are disabled, due to 'ignoreSortInstructions' setting."
+            NOTE: All sort instructions, including default sort order, are disabled, due to 'ignoreSortInstructions' setting.
+            "
         `);
     });
 });
diff --git a/tests/Query/Explain/DocsSamplesForExplain.test.explain_explains_task_block_with_global_query_active.approved.explanation.text b/tests/Query/Explain/DocsSamplesForExplain.test.explain_explains_task_block_with_global_query_active.approved.explanation.text
index 73fadf55..7ce44753 100644
--- a/tests/Query/Explain/DocsSamplesForExplain.test.explain_explains_task_block_with_global_query_active.approved.explanation.text
+++ b/tests/Query/Explain/DocsSamplesForExplain.test.explain_explains_task_block_with_global_query_active.approved.explanation.text
@@ -4,7 +4,6 @@ heading includes tasks
 
 No grouping instructions supplied.
 
-
 At most 50 tasks.
 
 Explanation of this Tasks code block query:
diff --git a/tests/Query/Explain/Explainer.test.ts b/tests/Query/Explain/Explainer.test.ts
index 253780c7..1c0dd179 100644
--- a/tests/Query/Explain/Explainer.test.ts
+++ b/tests/Query/Explain/Explainer.test.ts
@@ -7,11 +7,14 @@ import moment from 'moment';
 import { GlobalFilter } from '../../../src/Config/GlobalFilter';
 import { Query } from '../../../src/Query/Query';
 import { Explainer } from '../../../src/Query/Explain/Explainer';
+import { resetSettings, updateSettings } from '../../../src/Config/Settings';
+import { DebugSettings } from '../../../src/Config/DebugSettings';
 
 window.moment = moment;
 
 afterEach(() => {
     GlobalFilter.getInstance().reset();
+    resetSettings();
 });
 
 const explainer = new Explainer();
@@ -29,13 +32,56 @@ describe('explain errors', () => {
     });
 });
 
+describe('explain everything', () => {
+    it('all types of instruction', () => {
+        // Disable sort instructions
+        updateSettings({ debugSettings: new DebugSettings(true) });
+
+        const source = `
+not done
+(has start date) AND (description includes some)
+
+group by priority reverse
+group by happens
+
+sort by description reverse
+sort by path
+
+show urgency
+short mode
+limit 50
+limit groups 3
+`;
+        const query = new Query(source);
+        expect(explainer.explainQuery(query)).toMatchInlineSnapshot(`
+            "not done
+
+            (has start date) AND (description includes some) =>
+              AND (All of):
+                has start date
+                description includes some
+
+            group by priority reverse
+            group by happens
+
+            At most 50 tasks.
+
+            At most 3 tasks per group (if any "group by" options are supplied).
+
+            NOTE: All sort instructions, including default sort order, are disabled, due to 'ignoreSortInstructions' setting.
+            "
+        `);
+    });
+});
+
 describe('explain filters', () => {
     it('should explain 0 filters', () => {
         const source = '';
         const query = new Query(source);
-        expect(explainer.explainFilters(query)).toMatchInlineSnapshot(
-            '"No filters supplied. All tasks will match the query."',
-        );
+        expect(explainer.explainFilters(query)).toMatchInlineSnapshot(`
+            "No filters supplied. All tasks will match the query.
+            "
+        `);
     });
 
     it('should explain 1 filter', () => {
@@ -65,10 +111,10 @@ describe('explain groupers', () => {
         const source = 'group by due\ngroup by status.name reverse\ngroup by function task.description.toUpperCase()';
         const query = new Query(source);
         expect(explainer.explainGroups(query)).toMatchInlineSnapshot(`
-            "
-            group by due
+            "group by due
             group by status.name reverse
-            group by function task.description.toUpperCase()"
+            group by function task.description.toUpperCase()
+            "
         `);
     });
 });
@@ -77,23 +123,17 @@ describe('explain limits', () => {
     it('should explain limit 5', () => {
         const source = 'limit 5';
         const query = new Query(source);
-        expect(explainer.explainQuery(query)).toMatchInlineSnapshot(`
-                "No filters supplied. All tasks will match the query.
-                No grouping instructions supplied.
-
-
-                At most 5 tasks.
-                "
-            `);
+        expect(explainer.explainQueryLimits(query)).toMatchInlineSnapshot(`
+            "At most 5 tasks.
+            "
+        `);
     });
 
     it('should explain limit 1', () => {
         const source = 'limit 1';
         const query = new Query(source);
         expect(explainer.explainQueryLimits(query)).toMatchInlineSnapshot(`
-            "
-
-            At most 1 task.
+            "At most 1 task.
             "
         `);
     });
@@ -102,9 +142,7 @@ describe('explain limits', () => {
         const source = 'limit 0';
         const query = new Query(source);
         expect(explainer.explainQueryLimits(query)).toMatchInlineSnapshot(`
-            "
-
-            At most 0 tasks.
+            "At most 0 tasks.
             "
         `);
     });
@@ -113,9 +151,7 @@ describe('explain limits', () => {
         const source = 'limit groups 4';
         const query = new Query(source);
         expect(explainer.explainQueryLimits(query)).toMatchInlineSnapshot(`
-            "
-
-            At most 4 tasks per group (if any "group by" options are supplied).
+            "At most 4 tasks per group (if any "group by" options are supplied).
             "
         `);
     });
@@ -124,10 +160,7 @@ describe('explain limits', () => {
         const source = 'limit 127\nlimit groups to 8 tasks';
         const query = new Query(source);
         expect(explainer.explainQueryLimits(query)).toMatchInlineSnapshot(`
-            "
-
-            At most 127 tasks.
-
+            "At most 127 tasks.
 
             At most 8 tasks per group (if any "group by" options are supplied).
             "
diff --git a/tests/lib/QueryRendererHelper.test.ts b/tests/lib/QueryRendererHelper.test.ts
index 44813a3d..f588faea 100644
--- a/tests/lib/QueryRendererHelper.test.ts
+++ b/tests/lib/QueryRendererHelper.test.ts
@@ -17,6 +17,7 @@ describe('explain', () => {
             "Explanation of this Tasks code block query:
 
             No filters supplied. All tasks will match the query.
+
             No grouping instructions supplied.
             "
         `);
@@ -34,6 +35,7 @@ describe('explain', () => {
             Explanation of this Tasks code block query:
 
             No filters supplied. All tasks will match the query.
+
             No grouping instructions supplied.
             "
         `);
@@ -54,6 +56,7 @@ describe('explain', () => {
             Explanation of this Tasks code block query:
 
             No filters supplied. All tasks will match the query.
+
             No grouping instructions supplied.
             "
         `);
@@ -78,6 +81,7 @@ describe('explain', () => {
             Explanation of this Tasks code block query:
 
             No filters supplied. All tasks will match the query.
+
             No grouping instructions supplied.
             "
         `);
@@ -92,6 +96,7 @@ describe('explain', () => {
             "Explanation of this Tasks code block query:
 
             No filters supplied. All tasks will match the query.
+
             No grouping instructions supplied.
             "
         `);