Merge pull request #3391 from obsidian-tasks-group/refactor-description-render

refactor: Remove need to construct temporary Task objects

Author: claremacrae

resources/sample_vaults/Tasks-Demo/Test Data/internal_heading_links.md

@@ -41,3 +41,13 @@ This file contains test cases for internal heading links in tasks.
 ## Escaped Links
 
 - [ ] #task Task with \[\[#Escaped Links\]\] escaped link
+
+## Search
+
+Hover over each heading in these tasks, and to make the Page Preview plugin show that the links exist.
+
+```tasks
+path includes {{query.file.path}}
+group by heading
+hide backlinks
+```

src/Obsidian/InlineRenderer.ts

@@ -133,7 +133,11 @@ export class InlineRenderer {
             }
             const dataLine: string = renderedElement.getAttr('data-line') ?? '0';
             const taskIndex: number = Number.parseInt(dataLine, 10);
-            const taskElement = await taskLineRenderer.renderTaskLine(task, taskIndex);
+            const taskElement = await taskLineRenderer.renderTaskLine({
+                task: task,
+                taskIndex: taskIndex,
+                isTaskInQueryFile: true,
+            });
 
             // If the rendered element contains a sub-list or sub-div (e.g. the
             // folding arrow), we need to keep it.

src/Renderer/QueryResultsRenderer.ts

@@ -372,9 +372,12 @@ export class QueryResultsRenderer {
         queryRendererParameters: QueryRendererParameters,
     ) {
         const isFilenameUnique = this.isFilenameUnique({ task }, queryRendererParameters.allMarkdownFiles);
-        const processedTask = this.processTaskLinks(task);
-
-        const listItem = await taskLineRenderer.renderTaskLine(processedTask, taskIndex, isFilenameUnique);
+        const listItem = await taskLineRenderer.renderTaskLine({
+            task: task,
+            taskIndex: taskIndex,
+            isTaskInQueryFile: this.filePath === task.path,
+            isFilenameUnique: isFilenameUnique,
+        });
 
         // Remove all footnotes. They don't re-appear in another document.
         const footnotes = listItem.querySelectorAll('[data-footnote-id]');
@@ -500,45 +503,6 @@ export class QueryResultsRenderer {
             backLink.append(')');
         }
     }
-
-    private processTaskLinks(task: Task): Task {
-        // Skip if task is from the same file as the query
-        if (this.filePath === task.path) {
-            return task;
-        }
-
-        const linkCache = task.file.cachedMetadata.links;
-        if (!linkCache) return task;
-
-        // Find links in the task description
-        const taskLinks = linkCache.filter((link) => {
-            return (
-                link.position.start.line === task.taskLocation.lineNumber &&
-                task.description.includes(link.original) &&
-                link.link.startsWith('#')
-            );
-        });
-        if (taskLinks.length === 0) return task;
-
-        let description = task.description;
-
-        // a task can only be from one file, so we can replace all the internal links
-        //in the description with the new file path
-        for (const link of taskLinks) {
-            const fullLink = `[[${task.path}${link.link}|${link.displayText}]]`;
-            // Replace the first instance of this link:
-            description = description.replace(link.original, fullLink);
-        }
-
-        // Return a new Task with the updated description
-        return new Task({
-            ...task,
-            description,
-            // Preserve the original task's location information
-            taskLocation: task.taskLocation,
-        });
-    }
-
     private addPostponeButton(listItem: HTMLElement, task: Task, shortMode: boolean) {
         const amount = 1;
         const timeUnit = 'day';

src/Renderer/TaskLineRenderer.ts

@@ -127,18 +127,28 @@ export class TaskLineRenderer {
      * @note Output is based on the {@link DefaultTaskSerializer}'s format, with default (emoji) symbols
      * @param task The task to be rendered.
      * @param taskIndex Task's index in the list. This affects `data-line` data attributes of the list item.
+     * @param isTaskInQueryFile
      * @param isFilenameUnique Whether the name of the file that contains the task is unique in the vault.
      *                         If it is undefined, the outcome will be the same as with a unique file name:
      *                         the file name only. If set to `true`, the full path will be returned.
      */
-    public async renderTaskLine(task: Task, taskIndex: number, isFilenameUnique?: boolean): Promise<HTMLLIElement> {
+    public async renderTaskLine({
+        task,
+        taskIndex,
+        isTaskInQueryFile,
+        isFilenameUnique,
+    }: {
+        task: Task;
+        taskIndex: number;
+        isTaskInQueryFile: boolean;
+        isFilenameUnique?: boolean;
+    }): Promise<HTMLLIElement> {
         const li = createAndAppendElement('li', this.parentUlElement);
-
         li.classList.add('task-list-item', 'plugin-tasks-list-item');
 
         const textSpan = createAndAppendElement('span', li);
         textSpan.classList.add('tasks-list-text');
-        await this.taskToHtml(task, textSpan, li);
+        await this.taskToHtml(task, textSpan, li, isTaskInQueryFile);
 
         // NOTE: this area is mentioned in `CONTRIBUTING.md` under "How does Tasks handle status changes". When
         // moving the code, remember to update that reference too.
@@ -193,7 +203,12 @@ export class TaskLineRenderer {
         return li;
     }
 
-    private async taskToHtml(task: Task, parentElement: HTMLElement, li: HTMLLIElement): Promise<void> {
+    private async taskToHtml(
+        task: Task,
+        parentElement: HTMLElement,
+        li: HTMLLIElement,
+        isTaskInQueryFile: boolean,
+    ): Promise<void> {
         const fieldRenderer = new TaskFieldRenderer();
         const emojiSerializer = TASK_FORMATS.tasksPluginEmoji.taskSerializer;
         // Render and build classes for all the task's visible components
@@ -212,7 +227,7 @@ export class TaskLineRenderer {
                 // to differentiate between the container of the text and the text itself, so it will be possible
                 // to do things like surrounding only the text (rather than its whole placeholder) with a highlight
                 const internalSpan = createAndAppendElement('span', span);
-                await this.renderComponentText(internalSpan, componentString, component, task);
+                await this.renderComponentText(internalSpan, componentString, component, task, isTaskInQueryFile);
                 this.addInternalClasses(component, internalSpan);
 
                 // Add the component's CSS class describing what this component is (priority, due date etc.)
@@ -269,15 +284,17 @@ export class TaskLineRenderer {
         componentString: string,
         component: TaskLayoutComponent,
         task: Task,
+        isTaskInQueryFile: boolean,
     ) {
         if (component === TaskLayoutComponent.Description) {
-            return await this.renderDescription(task, span);
+            return await this.renderDescription(task, span, isTaskInQueryFile);
         }
         span.innerHTML = componentString;
     }
 
-    private async renderDescription(task: Task, span: HTMLSpanElement) {
-        let description = GlobalFilter.getInstance().removeAsWordFromDependingOnSettings(task.description);
+    private async renderDescription(task: Task, span: HTMLSpanElement, isTaskInQueryFile: boolean) {
+        let description = this.adjustRelativeLinksInDescription(task, isTaskInQueryFile);
+        description = GlobalFilter.getInstance().removeAsWordFromDependingOnSettings(description);
 
         const { debugSettings } = getSettings();
         if (debugSettings.showTaskHiddenData) {
@@ -314,6 +331,40 @@ export class TaskLineRenderer {
         });
     }
 
+    private adjustRelativeLinksInDescription(task: Task, isTaskInQueryFile: boolean) {
+        // Skip if task is from the same file as the query
+        if (isTaskInQueryFile) {
+            return task.description;
+        }
+
+        // Skip if the task is in a file with no links
+        const linkCache = task.file.cachedMetadata.links;
+        if (!linkCache) {
+            return task.description;
+        }
+
+        // Find links in the task description
+        const taskLinks = linkCache.filter((link) => {
+            return (
+                link.position.start.line === task.taskLocation.lineNumber &&
+                task.description.includes(link.original) &&
+                link.link.startsWith('#')
+            );
+        });
+
+        let description = task.description;
+        if (taskLinks.length !== 0) {
+            // a task can only be from one file, so we can replace all the internal links
+            //in the description with the new file path
+            for (const link of taskLinks) {
+                const fullLink = `[[${task.path}${link.link}|${link.displayText}]]`;
+                // Replace the first instance of this link:
+                description = description.replace(link.original, fullLink);
+            }
+        }
+        return description;
+    }
+
     /*
      * Adds internal classes for various components (right now just tags actually), meaning that we modify the existing
      * rendered element to add classes inside it.

tests/Obsidian/__test_data__/internal_heading_links.json

@@ -192,6 +192,22 @@
             "offset": 902
           }
         }
+      },
+      {
+        "heading": "Search",
+        "level": 2,
+        "position": {
+          "end": {
+            "col": 9,
+            "line": 44,
+            "offset": 988
+          },
+          "start": {
+            "col": 0,
+            "line": 44,
+            "offset": 979
+          }
+        }
       }
     ],
     "links": [
@@ -859,6 +875,51 @@
           }
         },
         "type": "list"
+      },
+      {
+        "position": {
+          "end": {
+            "col": 9,
+            "line": 44,
+            "offset": 988
+          },
+          "start": {
+            "col": 0,
+            "line": 44,
+            "offset": 979
+          }
+        },
+        "type": "heading"
+      },
+      {
+        "position": {
+          "end": {
+            "col": 102,
+            "line": 46,
+            "offset": 1092
+          },
+          "start": {
+            "col": 0,
+            "line": 46,
+            "offset": 990
+          }
+        },
+        "type": "paragraph"
+      },
+      {
+        "position": {
+          "end": {
+            "col": 3,
+            "line": 52,
+            "offset": 1172
+          },
+          "start": {
+            "col": 0,
+            "line": 48,
+            "offset": 1094
+          }
+        },
+        "type": "code"
       }
     ],
     "tags": [
@@ -1014,7 +1075,7 @@
       }
     ]
   },
-  "fileContents": "# Internal Heading Links Test\n\nThis file contains test cases for internal heading links in tasks.\n\n## Simple Headers\n\n## Another Header\n\n## Basic Internal Links\n\n- [ ] #task Task with<br>[[#Basic Internal Links]]\n\n## Multiple Links In One Task\n\n- [ ] #task Task with<br>[[#Multiple Links In One Task]] and<br>[[#Simple Headers]]\n\n## External File Links\n\n- [ ] #task Task with<br>[[Other File]]\n\n## Mixed Link Types\n\n- [ ] #task Task with<br>[[Other File]] and<br>[[#Mixed Link Types]]\n\n## Header Links With File Reference\n\n- [ ] #task<br>[[#Header Links With File Reference]] then<br>[[Other File#Some Header]] and<br>[[#Another Header]]\n\n## Headers-With_Special Characters\n\n- [ ] #task Task with<br>[[#Headers-With_Special Characters]]\n\n## Aliased Links\n\n- [ ] #task Task with<br>[[#Aliased Links|I am an alias]]\n\n## Links In Code Blocks\n\n- [ ] #task Task with `[[#Links In Code Blocks]]` code block\n\n## Escaped Links\n\n- [ ] #task Task with \\[\\[#Escaped Links\\]\\] escaped link\n",
+  "fileContents": "# Internal Heading Links Test\n\nThis file contains test cases for internal heading links in tasks.\n\n## Simple Headers\n\n## Another Header\n\n## Basic Internal Links\n\n- [ ] #task Task with<br>[[#Basic Internal Links]]\n\n## Multiple Links In One Task\n\n- [ ] #task Task with<br>[[#Multiple Links In One Task]] and<br>[[#Simple Headers]]\n\n## External File Links\n\n- [ ] #task Task with<br>[[Other File]]\n\n## Mixed Link Types\n\n- [ ] #task Task with<br>[[Other File]] and<br>[[#Mixed Link Types]]\n\n## Header Links With File Reference\n\n- [ ] #task<br>[[#Header Links With File Reference]] then<br>[[Other File#Some Header]] and<br>[[#Another Header]]\n\n## Headers-With_Special Characters\n\n- [ ] #task Task with<br>[[#Headers-With_Special Characters]]\n\n## Aliased Links\n\n- [ ] #task Task with<br>[[#Aliased Links|I am an alias]]\n\n## Links In Code Blocks\n\n- [ ] #task Task with `[[#Links In Code Blocks]]` code block\n\n## Escaped Links\n\n- [ ] #task Task with \\[\\[#Escaped Links\\]\\] escaped link\n\n## Search\n\nHover over each heading in these tasks, and to make the Page Preview plugin show that the links exist.\n\n```tasks\npath includes {{query.file.path}}\ngroup by heading\nhide backlinks\n```\n",
   "filePath": "Test Data/internal_heading_links.md",
   "getAllTags": [
     "#task",

tests/Renderer/TaskLineRenderer.test.ts

@@ -46,7 +46,7 @@ async function renderListItem(
         taskLayoutOptions: taskLayoutOptions ?? new TaskLayoutOptions(),
         queryLayoutOptions: queryLayoutOptions ?? new QueryLayoutOptions(),
     });
-    return await taskLineRenderer.renderTaskLine(task, 0);
+    return await taskLineRenderer.renderTaskLine({ task: task, taskIndex: 0, isTaskInQueryFile: true });
 }
 
 function getTextSpan(listItem: HTMLElement) {
@@ -89,7 +89,11 @@ describe('task line rendering - HTML', () => {
             taskLayoutOptions: new TaskLayoutOptions(),
             queryLayoutOptions: new QueryLayoutOptions(),
         });
-        const listItem = await taskLineRenderer.renderTaskLine(new TaskBuilder().build(), 0);
+        const listItem = await taskLineRenderer.renderTaskLine({
+            task: new TaskBuilder().build(),
+            taskIndex: 0,
+            isTaskInQueryFile: true,
+        });
 
         // Just one element
         expect(ulElement.children.length).toEqual(1);