Merge pull request #3690 from ilandikov/refactor/reuse-query-result

refactor: reuse `QueryResult`

Author: claremacrae

src/Renderer/HtmlQueryResultsRenderer.ts

@@ -102,23 +102,23 @@ export class HtmlQueryResultsRenderer extends QueryResultsRendererBase {
         explanationsBlock.textContent = explanation;
     }
 
-    private addToolbar(_queryResult: QueryResult) {
+    private addToolbar(queryResult: QueryResult) {
         if (this.getters.query().queryLayoutOptions.hideToolbar) {
             return;
         }
 
         const toolbar = createAndAppendElement('div', this.content);
         toolbar.classList.add('plugin-tasks-toolbar');
-        this.addCopyButton(toolbar);
+        this.addCopyButton(toolbar, queryResult);
     }
 
-    private addCopyButton(toolbar: HTMLDivElement): void {
+    private addCopyButton(toolbar: HTMLDivElement, queryResult: QueryResult): void {
         const copyButton = createAndAppendElement('button', toolbar);
         setIcon(copyButton, 'lucide-copy');
         setTooltip(copyButton, 'Copy results');
         copyButton.addEventListener('click', async () => {
             // TODO reimplement this using QueryResult.asMarkdown() when it supports trees and list items.
-            await this.markdownRenderer.renderQuery(State.Warm, this.queryRendererParameters.allTasks());
+            await this.markdownRenderer.renderQuery(State.Warm, queryResult);
             await navigator.clipboard.writeText(this.markdownRenderer.markdown);
             new Notice('Results copied to clipboard');
         });

src/Renderer/QueryResultsRenderer.ts

@@ -1,6 +1,7 @@
 import type { App, Component, TFile } from 'obsidian';
 import { GlobalQuery } from '../Config/GlobalQuery';
 import type { IQuery } from '../IQuery';
+import { PerformanceTracker } from '../lib/PerformanceTracker';
 import type { State } from '../Obsidian/Cache';
 import { getQueryForQueryRenderer } from '../Query/QueryRendererHelper';
 import type { TasksFile } from '../Scripting/TasksFile';
@@ -138,8 +139,16 @@ export class QueryResultsRenderer {
         return this.tasksFile.path;
     }
 
-    public async render(state: State | State.Warm, tasks: Task[], content: HTMLDivElement) {
+    public async render(state: State, tasks: Task[], content: HTMLDivElement) {
+        const measureSearch = new PerformanceTracker(`Search: ${this.query.queryId} - ${this.filePath}`);
+        measureSearch.start();
+        const queryResult = this.query.applyQueryToTasks(tasks);
+        measureSearch.finish();
+
+        const measureRender = new PerformanceTracker(`Render: ${this.query.queryId} - ${this.filePath}`);
+        measureRender.start();
         this.htmlRenderer.content = content;
-        await this.htmlRenderer.renderQuery(state, tasks);
+        await this.htmlRenderer.renderQuery(state, queryResult);
+        measureRender.finish();
     }
 }

src/Renderer/QueryResultsRendererBase.ts

@@ -1,15 +1,14 @@
-import type { TasksFile } from '../Scripting/TasksFile';
+import { GlobalFilter } from '../Config/GlobalFilter';
+import { GlobalQuery } from '../Config/GlobalQuery';
 import type { IQuery } from '../IQuery';
 import { State } from '../Obsidian/Cache';
-import { Task } from '../Task/Task';
-import type { QueryResult } from '../Query/QueryResult';
-import type { TaskGroups } from '../Query/Group/TaskGroups';
-import type { ListItem } from '../Task/ListItem';
 import type { GroupDisplayHeading } from '../Query/Group/GroupDisplayHeading';
-import { PerformanceTracker } from '../lib/PerformanceTracker';
+import type { TaskGroups } from '../Query/Group/TaskGroups';
 import { explainResults } from '../Query/QueryRendererHelper';
-import { GlobalFilter } from '../Config/GlobalFilter';
-import { GlobalQuery } from '../Config/GlobalQuery';
+import type { QueryResult } from '../Query/QueryResult';
+import type { TasksFile } from '../Scripting/TasksFile';
+import type { ListItem } from '../Task/ListItem';
+import { Task } from '../Task/Task';
 
 /**
  * Because properties in QueryResultsRenderer may be modified during the lifetime of this class,
@@ -34,7 +33,7 @@ export abstract class QueryResultsRendererBase {
         return this.getters.tasksFile().path;
     }
 
-    public async renderQuery(state: State | State.Warm, tasks: Task[]) {
+    public async renderQuery(state: State, queryResult: QueryResult) {
         this.beginRender();
 
         // Don't log anything here, for any state, as it generates huge amounts of
@@ -43,8 +42,7 @@ export abstract class QueryResultsRendererBase {
         const query = this.getters.query();
         const error = query.error;
         if (state === State.Warm && error === undefined) {
-            query.debug(`[render] Render called: plugin state: ${state}; searching ${tasks.length} tasks`);
-            await this.renderQuerySearchResults(tasks);
+            await this.renderQuerySearchResults(queryResult);
         } else if (error) {
             this.renderErrorMessage(error);
         } else {
@@ -59,8 +57,8 @@ export abstract class QueryResultsRendererBase {
      */
     protected abstract beginRender(): void;
 
-    private async renderQuerySearchResults(tasks: Task[]) {
-        const queryResult = this.explainAndPerformSearch(tasks);
+    private async renderQuerySearchResults(queryResult: QueryResult) {
+        this.explainQuery();
 
         if (queryResult.searchErrorMessage !== undefined) {
             // There was an error in the search, for example due to a problem custom function.
@@ -71,10 +69,7 @@ export abstract class QueryResultsRendererBase {
         await this.renderSearchResults(queryResult);
     }
 
-    private explainAndPerformSearch(tasks: Task[]) {
-        const measureSearch = new PerformanceTracker(`Search: ${this.getters.query().queryId} - ${this.filePath}`);
-        measureSearch.start();
-
+    private explainQuery() {
         if (this.getters.query().queryLayoutOptions.explainQuery) {
             const explanation = explainResults(
                 this.getters.source(),
@@ -84,17 +79,9 @@ export abstract class QueryResultsRendererBase {
             );
             this.renderExplanation(explanation);
         }
-
-        const queryResult = this.getters.query().applyQueryToTasks(tasks);
-
-        measureSearch.finish();
-        return queryResult;
     }
 
     private async renderSearchResults(queryResult: QueryResult) {
-        const measureRender = new PerformanceTracker(`Render: ${this.getters.query().queryId} - ${this.filePath}`);
-        measureRender.start();
-
         this.renderSearchResultsHeader(queryResult);
 
         await this.addAllTaskGroups(queryResult.taskGroups);
@@ -103,8 +90,6 @@ export abstract class QueryResultsRendererBase {
         this.getters.query().debug(`[render] ${totalTasksCount} tasks displayed`);
 
         this.renderSearchResultsFooter(queryResult);
-
-        measureRender.finish();
     }
 
     protected abstract renderSearchResultsHeader(queryResult: QueryResult): void;

tests/Renderer/MarkdownQueryResultsRenderer.test.ts

@@ -15,16 +15,17 @@ window.moment = moment;
 function createMarkdownRenderer(source: string) {
     const tasksFile = new TasksFile('query.md');
     const query = new Query(source, tasksFile);
-    return new MarkdownQueryResultsRenderer({
+    const renderer = new MarkdownQueryResultsRenderer({
         query: () => query,
         tasksFile: () => tasksFile,
         source: () => source,
     });
+    return { renderer, query };
 }
 
 async function renderMarkdown(source: string, tasks: Task[]) {
-    const renderer = createMarkdownRenderer(source);
-    await renderer.renderQuery(State.Warm, tasks);
+    const { renderer, query } = createMarkdownRenderer(source);
+    await renderer.renderQuery(State.Warm, query.applyQueryToTasks(tasks));
     return '\n' + renderer.markdown;
 }
 
@@ -51,14 +52,14 @@ describe('MarkdownQueryResultsRenderer tests', () => {
 
     it('should render single task twice', async () => {
         const source = 'hide tree';
-        const renderer = createMarkdownRenderer(source);
+        const { renderer, query } = createMarkdownRenderer(source);
 
-        const task = new TaskBuilder().description('hello').priority(Priority.Medium).build();
+        const task = [new TaskBuilder().description('hello').priority(Priority.Medium).build()];
 
-        await renderer.renderQuery(State.Warm, [task]);
+        await renderer.renderQuery(State.Warm, query.applyQueryToTasks(task));
         const r1 = renderer.markdown;
 
-        await renderer.renderQuery(State.Warm, [task]);
+        await renderer.renderQuery(State.Warm, query.applyQueryToTasks(task));
         const r2 = renderer.markdown;
 
         expect(r1).toEqual(r2);